Return-Path: Delivered-To: apmail-incubator-jackrabbit-commits-archive@www.apache.org Received: (qmail 43103 invoked from network); 1 Apr 2005 10:25:01 -0000 Received: from hermes.apache.org (HELO mail.apache.org) (209.237.227.199) by minotaur.apache.org with SMTP; 1 Apr 2005 10:25:01 -0000 Received: (qmail 92714 invoked by uid 500); 1 Apr 2005 10:24:38 -0000 Mailing-List: contact jackrabbit-commits-help@incubator.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: jackrabbit-dev@incubator.apache.org Delivered-To: mailing list jackrabbit-commits@incubator.apache.org Received: (qmail 92679 invoked by uid 500); 1 Apr 2005 10:24:37 -0000 Delivered-To: apmail-incubator-jackrabbit-cvs@incubator.apache.org Received: (qmail 92666 invoked by uid 99); 1 Apr 2005 10:24:37 -0000 X-ASF-Spam-Status: No, hits=-9.8 required=10.0 tests=ALL_TRUSTED,NO_REAL_NAME X-Spam-Check-By: apache.org Received: from minotaur.apache.org (HELO minotaur.apache.org) (209.237.227.194) by apache.org (qpsmtpd/0.28) with SMTP; Fri, 01 Apr 2005 02:24:34 -0800 Received: (qmail 42973 invoked by uid 65534); 1 Apr 2005 10:24:32 -0000 Message-ID: <20050401102432.42972.qmail@minotaur.apache.org> Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable X-Mailer: svnmailer-1.0.0-dev Date: Fri, 01 Apr 2005 10:24:32 -0000 Subject: svn commit: r159676 - in incubator/jackrabbit/trunk/src/java/org/apache/jackrabbit/core/state: ItemStateCache.java LocalItemStateManager.java SharedItemStateManager.java TransientItemStateManager.java To: jackrabbit-cvs@incubator.apache.org From: dpfister@apache.org X-Virus-Checked: Checked X-Spam-Rating: minotaur.apache.org 1.6.2 0/1000/N Author: dpfister Date: Fri Apr 1 02:24:31 2005 New Revision: 159676 URL: http://svn.apache.org/viewcvs?view=3Drev&rev=3D159676 Log: Fixed deadlock issue. - introduced separate monitor for item state cache operations - removed synchronized() where not needed and added comment Modified: incubator/jackrabbit/trunk/src/java/org/apache/jackrabbit/core/state/It= emStateCache.java incubator/jackrabbit/trunk/src/java/org/apache/jackrabbit/core/state/Lo= calItemStateManager.java incubator/jackrabbit/trunk/src/java/org/apache/jackrabbit/core/state/Sh= aredItemStateManager.java incubator/jackrabbit/trunk/src/java/org/apache/jackrabbit/core/state/Tr= ansientItemStateManager.java Modified: incubator/jackrabbit/trunk/src/java/org/apache/jackrabbit/core/st= ate/ItemStateCache.java URL: http://svn.apache.org/viewcvs/incubator/jackrabbit/trunk/src/java/org/= apache/jackrabbit/core/state/ItemStateCache.java?view=3Ddiff&r1=3D159675&r2= =3D159676 =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D --- incubator/jackrabbit/trunk/src/java/org/apache/jackrabbit/core/state/It= emStateCache.java (original) +++ incubator/jackrabbit/trunk/src/java/org/apache/jackrabbit/core/state/It= emStateCache.java Fri Apr 1 02:24:31 2005 @@ -39,6 +39,13 @@ private final Map cache; =20 /** + * Monitor for cache object. Derived classes should synchronize on + * this monitor when the identity of cached objects is critical, i.e. + * when object should not be cached more than once. + */ + protected final Object cacheMonitor =3D new Object(); + + /** * Creates a new ItemStateCache that will use instance * hard references to keys and soft references to values. */ Modified: incubator/jackrabbit/trunk/src/java/org/apache/jackrabbit/core/st= ate/LocalItemStateManager.java URL: http://svn.apache.org/viewcvs/incubator/jackrabbit/trunk/src/java/org/= apache/jackrabbit/core/state/LocalItemStateManager.java?view=3Ddiff&r1=3D15= 9675&r2=3D159676 =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D --- incubator/jackrabbit/trunk/src/java/org/apache/jackrabbit/core/state/Lo= calItemStateManager.java (original) +++ incubator/jackrabbit/trunk/src/java/org/apache/jackrabbit/core/state/Lo= calItemStateManager.java Fri Apr 1 02:24:31 2005 @@ -121,11 +121,6 @@ protected PropertyState getPropertyState(PropertyId id) throws NoSuchItemStateException, ItemStateException { =20 - // check cache - if (isCached(id)) { - return (PropertyState) retrieve(id); - } - // load from parent manager and wrap PropertyState state =3D (PropertyState) sharedStateMgr.getItemStat= e(id); state =3D new PropertyState(state, state.getStatus(), false); @@ -154,7 +149,7 @@ /** * {@inheritDoc} */ - public synchronized ItemState getItemState(ItemId id) + public ItemState getItemState(ItemId id) throws NoSuchItemStateException, ItemStateException { =20 // check change log @@ -163,16 +158,18 @@ return state; } =20 - // check cache - if (isCached(id)) { - return retrieve(id); - } + // check cache. synchronized to ensure an entry is not created twi= ce. + synchronized (cacheMonitor) { + if (isCached(id)) { + return retrieve(id); + } =20 - // regular behaviour - if (id.denotesNode()) { - return getNodeState((NodeId) id); - } else { - return getPropertyState((PropertyId) id); + // regular behaviour + if (id.denotesNode()) { + return getNodeState((NodeId) id); + } else { + return getPropertyState((PropertyId) id); + } } } =20 @@ -203,7 +200,7 @@ /** * {@inheritDoc} */ - public synchronized NodeReferences getNodeReferences(NodeReferencesId = id) + public NodeReferences getNodeReferences(NodeReferencesId id) throws NoSuchItemStateException, ItemStateException { =20 // check change log @@ -352,6 +349,7 @@ */ public void stateDestroyed(ItemState destroyed) { destroyed.removeListener(this); + evict(destroyed.getId()); } =20 @@ -360,6 +358,7 @@ */ public void stateDiscarded(ItemState discarded) { discarded.removeListener(this); + evict(discarded.getId()); } } Modified: incubator/jackrabbit/trunk/src/java/org/apache/jackrabbit/core/st= ate/SharedItemStateManager.java URL: http://svn.apache.org/viewcvs/incubator/jackrabbit/trunk/src/java/org/= apache/jackrabbit/core/state/SharedItemStateManager.java?view=3Ddiff&r1=3D1= 59675&r2=3D159676 =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D --- incubator/jackrabbit/trunk/src/java/org/apache/jackrabbit/core/state/Sh= aredItemStateManager.java (original) +++ incubator/jackrabbit/trunk/src/java/org/apache/jackrabbit/core/state/Sh= aredItemStateManager.java Fri Apr 1 02:24:31 2005 @@ -98,17 +98,27 @@ } =20 /** - * Adds a new virtual item state provider - * + * Adds a new virtual item state provider.

+ * NOTE: This method is not synchronized, because it is called right a= fter + * creation only by the same thread and therefore concurrency issues + * do not occur. Should this ever change, the synchronization status + * has to be re-examined. * @param prov */ - public synchronized void addVirtualItemStateProvider(VirtualItemStateP= rovider prov) { + public void addVirtualItemStateProvider(VirtualItemStateProvider prov)= { VirtualItemStateProvider[] provs =3D new VirtualItemStateProvider[= virtualProviders.length + 1]; System.arraycopy(virtualProviders, 0, provs, 0, virtualProviders.l= ength); provs[virtualProviders.length] =3D prov; virtualProviders =3D provs; } =20 + /** + * Create root node state + * @param rootNodeUUID root node UUID + * @param ntReg node type registry + * @return root node state + * @throws ItemStateException if an error occurs + */ private NodeState createRootNodeState(String rootNodeUUID, NodeTypeRegistry ntReg) throws ItemStateException { @@ -178,11 +188,6 @@ private NodeState getNodeState(NodeId id) throws NoSuchItemStateException, ItemStateException { =20 - // check cache - if (isCached(id)) { - return (NodeState) retrieve(id); - } - // load from persisted state NodeState state =3D persistMgr.load(id); state.setStatus(ItemState.STATUS_EXISTING); @@ -204,11 +209,6 @@ private PropertyState getPropertyState(PropertyId id) throws NoSuchItemStateException, ItemStateException { =20 - // check cache - if (isCached(id)) { - return (PropertyState) retrieve(id); - } - // load from persisted state PropertyState state =3D persistMgr.load(id); state.setStatus(ItemState.STATUS_EXISTING); @@ -225,7 +225,7 @@ /** * {@inheritDoc} */ - public synchronized ItemState getItemState(ItemId id) + public ItemState getItemState(ItemId id) throws NoSuchItemStateException, ItemStateException { // check the virtual root ids (needed for overlay) for (int i =3D 0; i < virtualProviders.length; i++) { @@ -252,17 +252,24 @@ */ private ItemState getNonVirtualItemState(ItemId id) throws NoSuchItemStateException, ItemStateException { - if (id.denotesNode()) { - return getNodeState((NodeId) id); - } else { - return getPropertyState((PropertyId) id); + + // check cache. synchronized to ensure an entry is not created twi= ce. + synchronized (cacheMonitor) { + if (isCached(id)) { + return retrieve(id); + } + if (id.denotesNode()) { + return getNodeState((NodeId) id); + } else { + return getPropertyState((PropertyId) id); + } } } =20 /** * {@inheritDoc} */ - public synchronized boolean hasItemState(ItemId id) { + public boolean hasItemState(ItemId id) { if (isCached(id)) { return true; } @@ -308,11 +315,9 @@ /** * {@inheritDoc} */ - public synchronized NodeReferences getNodeReferences(NodeReferencesId = id) + public NodeReferences getNodeReferences(NodeReferencesId id) throws NoSuchItemStateException, ItemStateException { =20 - // todo: add caching - // check persistence manager try { return persistMgr.load(id); @@ -390,14 +395,20 @@ * item state manager but rather must be reconnected to items provided * by this state manager.

* After successfully storing the states the observation manager is in= formed - * about the changes, if an observation manager is passed to this meth= od. + * about the changes, if an observation manager is passed to this meth= od.

+ * NOTE: This method is not synchronized, because all methods it invok= es + * on instance members (such as {@link PersistenceManager#store} are + * considered to be thread-safe. Should this ever change, the + * synchronization status has to be re-examined. * * @param local change log containing local items * @param obsMgr the observation manager to inform, or null if * no observation manager should be informed. * @throws ItemStateException if an error occurs */ - public synchronized void store(ChangeLog local, ObservationManagerImpl= obsMgr) throws ItemStateException { + public void store(ChangeLog local, ObservationManagerImpl obsMgr) + throws ItemStateException { + ChangeLog shared =3D new ChangeLog(); =20 // set of virtual node references @@ -428,7 +439,8 @@ throw new NoSuchItemStateException(); } } catch (NoSuchItemStateException e) { - String msg =3D "Target node " + id + " of REFERENC= E property does not exist"; + String msg =3D "Target node " + id + + " of REFERENCE property does not exist"; throw new ItemStateException(msg); } } Modified: incubator/jackrabbit/trunk/src/java/org/apache/jackrabbit/core/st= ate/TransientItemStateManager.java URL: http://svn.apache.org/viewcvs/incubator/jackrabbit/trunk/src/java/org/= apache/jackrabbit/core/state/TransientItemStateManager.java?view=3Ddiff&r1= =3D159675&r2=3D159676 =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D --- incubator/jackrabbit/trunk/src/java/org/apache/jackrabbit/core/state/Tr= ansientItemStateManager.java (original) +++ incubator/jackrabbit/trunk/src/java/org/apache/jackrabbit/core/state/Tr= ansientItemStateManager.java Fri Apr 1 02:24:31 2005 @@ -66,6 +66,7 @@ */ public ItemState getItemState(ItemId id) throws NoSuchItemStateException, ItemStateException { + ItemState state =3D retrieve(id); if (state !=3D null) { return state; @@ -143,20 +144,26 @@ * @return * @throws ItemStateException */ - NodeState createNodeState(String uuid, QName nodeTypeName, String pare= ntUUID, int initialStatus) + NodeState createNodeState(String uuid, QName nodeTypeName, + String parentUUID, int initialStatus) throws ItemStateException { + NodeId id =3D new NodeId(uuid); - // check cache - if (isCached(id)) { - String msg =3D "there's already a node state instance with id = " + id; - log.debug(msg); - throw new ItemStateException(msg); - } =20 - NodeState state =3D new NodeState(uuid, nodeTypeName, parentUUID, = initialStatus, true); - // put it in cache - cache(state); - return state; + // check cache. synchronized to ensure an entry is not created twi= ce. + synchronized (cacheMonitor) { + if (isCached(id)) { + String msg =3D "there's already a node state instance with= id " + id; + log.debug(msg); + throw new ItemStateException(msg); + } + + NodeState state =3D new NodeState(uuid, nodeTypeName, parentUU= ID, + initialStatus, true); + // put it in cache + cache(state); + return state; + } } =20 /** @@ -167,18 +174,22 @@ */ NodeState createNodeState(NodeState overlayedState, int initialStatus) throws ItemStateException { + ItemId id =3D overlayedState.getId(); - // check cache - if (isCached(id)) { - String msg =3D "there's already a node state instance with id = " + id; - log.debug(msg); - throw new ItemStateException(msg); - } =20 - NodeState state =3D new NodeState(overlayedState, initialStatus, t= rue); - // put it in cache - cache(state); - return state; + // check cache. synchronized to ensure an entry is not created twi= ce. + synchronized (cacheMonitor) { + if (isCached(id)) { + String msg =3D "there's already a node state instance with= id " + id; + log.debug(msg); + throw new ItemStateException(msg); + } + + NodeState state =3D new NodeState(overlayedState, initialStatu= s, true); + // put it in cache + cache(state); + return state; + } } =20 /** @@ -190,18 +201,22 @@ */ PropertyState createPropertyState(String parentUUID, QName propName, i= nt initialStatus) throws ItemStateException { + PropertyId id =3D new PropertyId(parentUUID, propName); - // check cache - if (isCached(id)) { - String msg =3D "there's already a property state instance with= id " + id; - log.debug(msg); - throw new ItemStateException(msg); - } =20 - PropertyState state =3D new PropertyState(propName, parentUUID, in= itialStatus, true); - // put it in cache - cache(state); - return state; + // check cache. synchronized to ensure an entry is not created twi= ce. + synchronized (cacheMonitor) { + if (isCached(id)) { + String msg =3D "there's already a property state instance = with id " + id; + log.debug(msg); + throw new ItemStateException(msg); + } + + PropertyState state =3D new PropertyState(propName, parentUUID= , initialStatus, true); + // put it in cache + cache(state); + return state; + } } =20 /** @@ -212,18 +227,23 @@ */ PropertyState createPropertyState(PropertyState overlayedState, int in= itialStatus) throws ItemStateException { - PropertyId id =3D new PropertyId(overlayedState.getParentUUID(), o= verlayedState.getName()); - // check cache - if (isCached(id)) { - String msg =3D "there's already a property state instance with= id " + id; - log.debug(msg); - throw new ItemStateException(msg); - } =20 - PropertyState state =3D new PropertyState(overlayedState, initialS= tatus, true); - // put it in cache - cache(state); - return state; + PropertyId id =3D new PropertyId(overlayedState.getParentUUID(), + overlayedState.getName()); + + // check cache. synchronized to ensure an entry is not created twi= ce. + synchronized (cacheMonitor) { + if (isCached(id)) { + String msg =3D "there's already a property state instance = with id " + id; + log.debug(msg); + throw new ItemStateException(msg); + } + + PropertyState state =3D new PropertyState(overlayedState, init= ialStatus, true); + // put it in cache + cache(state); + return state; + } } =20 /**