jackrabbit-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From dpfis...@apache.org
Subject svn commit: r159676 - in incubator/jackrabbit/trunk/src/java/org/apache/jackrabbit/core/state: ItemStateCache.java LocalItemStateManager.java SharedItemStateManager.java TransientItemStateManager.java
Date Fri, 01 Apr 2005 10:24:32 GMT
Author: dpfister
Date: Fri Apr  1 02:24:31 2005
New Revision: 159676

URL: http://svn.apache.org/viewcvs?view=rev&rev=159676
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/ItemStateCache.java
    incubator/jackrabbit/trunk/src/java/org/apache/jackrabbit/core/state/LocalItemStateManager.java
    incubator/jackrabbit/trunk/src/java/org/apache/jackrabbit/core/state/SharedItemStateManager.java
    incubator/jackrabbit/trunk/src/java/org/apache/jackrabbit/core/state/TransientItemStateManager.java

Modified: incubator/jackrabbit/trunk/src/java/org/apache/jackrabbit/core/state/ItemStateCache.java
URL: http://svn.apache.org/viewcvs/incubator/jackrabbit/trunk/src/java/org/apache/jackrabbit/core/state/ItemStateCache.java?view=diff&r1=159675&r2=159676
==============================================================================
--- incubator/jackrabbit/trunk/src/java/org/apache/jackrabbit/core/state/ItemStateCache.java
(original)
+++ incubator/jackrabbit/trunk/src/java/org/apache/jackrabbit/core/state/ItemStateCache.java
Fri Apr  1 02:24:31 2005
@@ -39,6 +39,13 @@
     private final Map cache;
 
     /**
+     * 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 = new Object();
+
+    /**
      * Creates a new <code>ItemStateCache</code> that will use instance
      * hard references to keys and soft references to values.
      */

Modified: incubator/jackrabbit/trunk/src/java/org/apache/jackrabbit/core/state/LocalItemStateManager.java
URL: http://svn.apache.org/viewcvs/incubator/jackrabbit/trunk/src/java/org/apache/jackrabbit/core/state/LocalItemStateManager.java?view=diff&r1=159675&r2=159676
==============================================================================
--- incubator/jackrabbit/trunk/src/java/org/apache/jackrabbit/core/state/LocalItemStateManager.java
(original)
+++ incubator/jackrabbit/trunk/src/java/org/apache/jackrabbit/core/state/LocalItemStateManager.java
Fri Apr  1 02:24:31 2005
@@ -121,11 +121,6 @@
     protected PropertyState getPropertyState(PropertyId id)
             throws NoSuchItemStateException, ItemStateException {
 
-        // check cache
-        if (isCached(id)) {
-            return (PropertyState) retrieve(id);
-        }
-
         // load from parent manager and wrap
         PropertyState state = (PropertyState) sharedStateMgr.getItemState(id);
         state = 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 {
 
         // check change log
@@ -163,16 +158,18 @@
             return state;
         }
 
-        // check cache
-        if (isCached(id)) {
-            return retrieve(id);
-        }
+        // check cache. synchronized to ensure an entry is not created twice.
+        synchronized (cacheMonitor) {
+            if (isCached(id)) {
+                return retrieve(id);
+            }
 
-        // 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);
+            }
         }
     }
 
@@ -203,7 +200,7 @@
     /**
      * {@inheritDoc}
      */
-    public synchronized NodeReferences getNodeReferences(NodeReferencesId id)
+    public NodeReferences getNodeReferences(NodeReferencesId id)
             throws NoSuchItemStateException, ItemStateException {
 
         // check change log
@@ -352,6 +349,7 @@
      */
     public void stateDestroyed(ItemState destroyed) {
         destroyed.removeListener(this);
+
         evict(destroyed.getId());
     }
 
@@ -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/state/SharedItemStateManager.java
URL: http://svn.apache.org/viewcvs/incubator/jackrabbit/trunk/src/java/org/apache/jackrabbit/core/state/SharedItemStateManager.java?view=diff&r1=159675&r2=159676
==============================================================================
--- incubator/jackrabbit/trunk/src/java/org/apache/jackrabbit/core/state/SharedItemStateManager.java
(original)
+++ incubator/jackrabbit/trunk/src/java/org/apache/jackrabbit/core/state/SharedItemStateManager.java
Fri Apr  1 02:24:31 2005
@@ -98,17 +98,27 @@
     }
 
     /**
-     * Adds a new virtual item state provider
-     *
+     * Adds a new virtual item state provider.<p/>
+     * NOTE: This method is not synchronized, because it is called right after
+     * 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(VirtualItemStateProvider prov) {
+    public void addVirtualItemStateProvider(VirtualItemStateProvider prov) {
         VirtualItemStateProvider[] provs = new VirtualItemStateProvider[virtualProviders.length
+ 1];
         System.arraycopy(virtualProviders, 0, provs, 0, virtualProviders.length);
         provs[virtualProviders.length] = prov;
         virtualProviders = provs;
     }
 
+    /**
+     * 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 {
 
-        // check cache
-        if (isCached(id)) {
-            return (NodeState) retrieve(id);
-        }
-
         // load from persisted state
         NodeState state = persistMgr.load(id);
         state.setStatus(ItemState.STATUS_EXISTING);
@@ -204,11 +209,6 @@
     private PropertyState getPropertyState(PropertyId id)
             throws NoSuchItemStateException, ItemStateException {
 
-        // check cache
-        if (isCached(id)) {
-            return (PropertyState) retrieve(id);
-        }
-
         // load from persisted state
         PropertyState state = 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 = 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 twice.
+        synchronized (cacheMonitor) {
+            if (isCached(id)) {
+                return retrieve(id);
+            }
+            if (id.denotesNode()) {
+                return getNodeState((NodeId) id);
+            } else {
+                return getPropertyState((PropertyId) id);
+            }
         }
     }
 
     /**
      * {@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 {
 
-        // 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.<p/>
      * After successfully storing the states the observation manager is informed
-     * about the changes, if an observation manager is passed to this method.
+     * about the changes, if an observation manager is passed to this method.<p/>
+     * NOTE: This method is not synchronized, because all methods it invokes
+     * 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 <code>null</code>
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 = new ChangeLog();
 
         // set of virtual node references
@@ -428,7 +439,8 @@
                             throw new NoSuchItemStateException();
                         }
                     } catch (NoSuchItemStateException e) {
-                        String msg = "Target node " + id + " of REFERENCE property does not
exist";
+                        String msg = "Target node " + id +
+                                " of REFERENCE property does not exist";
                         throw new ItemStateException(msg);
                     }
                 }

Modified: incubator/jackrabbit/trunk/src/java/org/apache/jackrabbit/core/state/TransientItemStateManager.java
URL: http://svn.apache.org/viewcvs/incubator/jackrabbit/trunk/src/java/org/apache/jackrabbit/core/state/TransientItemStateManager.java?view=diff&r1=159675&r2=159676
==============================================================================
--- incubator/jackrabbit/trunk/src/java/org/apache/jackrabbit/core/state/TransientItemStateManager.java
(original)
+++ incubator/jackrabbit/trunk/src/java/org/apache/jackrabbit/core/state/TransientItemStateManager.java
Fri Apr  1 02:24:31 2005
@@ -66,6 +66,7 @@
      */
     public ItemState getItemState(ItemId id)
             throws NoSuchItemStateException, ItemStateException {
+
         ItemState state = retrieve(id);
         if (state != null) {
             return state;
@@ -143,20 +144,26 @@
      * @return
      * @throws ItemStateException
      */
-    NodeState createNodeState(String uuid, QName nodeTypeName, String parentUUID, int initialStatus)
+    NodeState createNodeState(String uuid, QName nodeTypeName,
+                              String parentUUID, int initialStatus)
             throws ItemStateException {
+
         NodeId id = new NodeId(uuid);
-        // check cache
-        if (isCached(id)) {
-            String msg = "there's already a node state instance with id " + id;
-            log.debug(msg);
-            throw new ItemStateException(msg);
-        }
 
-        NodeState state = 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 twice.
+        synchronized (cacheMonitor) {
+            if (isCached(id)) {
+                String msg = "there's already a node state instance with id " + id;
+                log.debug(msg);
+                throw new ItemStateException(msg);
+            }
+
+            NodeState state = new NodeState(uuid, nodeTypeName, parentUUID,
+                    initialStatus, true);
+            // put it in cache
+            cache(state);
+            return state;
+        }
     }
 
     /**
@@ -167,18 +174,22 @@
      */
     NodeState createNodeState(NodeState overlayedState, int initialStatus)
             throws ItemStateException {
+
         ItemId id = overlayedState.getId();
-        // check cache
-        if (isCached(id)) {
-            String msg = "there's already a node state instance with id " + id;
-            log.debug(msg);
-            throw new ItemStateException(msg);
-        }
 
-        NodeState state = new NodeState(overlayedState, initialStatus, true);
-        // put it in cache
-        cache(state);
-        return state;
+        // check cache. synchronized to ensure an entry is not created twice.
+        synchronized (cacheMonitor) {
+            if (isCached(id)) {
+                String msg = "there's already a node state instance with id " + id;
+                log.debug(msg);
+                throw new ItemStateException(msg);
+            }
+
+            NodeState state = new NodeState(overlayedState, initialStatus, true);
+            // put it in cache
+            cache(state);
+            return state;
+        }
     }
 
     /**
@@ -190,18 +201,22 @@
      */
     PropertyState createPropertyState(String parentUUID, QName propName, int initialStatus)
             throws ItemStateException {
+
         PropertyId id = new PropertyId(parentUUID, propName);
-        // check cache
-        if (isCached(id)) {
-            String msg = "there's already a property state instance with id " + id;
-            log.debug(msg);
-            throw new ItemStateException(msg);
-        }
 
-        PropertyState state = new PropertyState(propName, parentUUID, initialStatus, true);
-        // put it in cache
-        cache(state);
-        return state;
+        // check cache. synchronized to ensure an entry is not created twice.
+        synchronized (cacheMonitor) {
+            if (isCached(id)) {
+                String msg = "there's already a property state instance with id " + id;
+                log.debug(msg);
+                throw new ItemStateException(msg);
+            }
+
+            PropertyState state = new PropertyState(propName, parentUUID, initialStatus,
true);
+            // put it in cache
+            cache(state);
+            return state;
+        }
     }
 
     /**
@@ -212,18 +227,23 @@
      */
     PropertyState createPropertyState(PropertyState overlayedState, int initialStatus)
             throws ItemStateException {
-        PropertyId id = new PropertyId(overlayedState.getParentUUID(), overlayedState.getName());
-        // check cache
-        if (isCached(id)) {
-            String msg = "there's already a property state instance with id " + id;
-            log.debug(msg);
-            throw new ItemStateException(msg);
-        }
 
-        PropertyState state = new PropertyState(overlayedState, initialStatus, true);
-        // put it in cache
-        cache(state);
-        return state;
+        PropertyId id = new PropertyId(overlayedState.getParentUUID(),
+                overlayedState.getName());
+
+        // check cache. synchronized to ensure an entry is not created twice.
+        synchronized (cacheMonitor) {
+            if (isCached(id)) {
+                String msg = "there's already a property state instance with id " + id;
+                log.debug(msg);
+                throw new ItemStateException(msg);
+            }
+
+            PropertyState state = new PropertyState(overlayedState, initialStatus, true);
+            // put it in cache
+            cache(state);
+            return state;
+        }
     }
 
     /**



Mime
View raw message