You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@jackrabbit.apache.org by ju...@apache.org on 2011/09/27 18:52:10 UTC

svn commit: r1176465 - in /jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core: ItemManager.java state/SessionItemStateManager.java state/SharedItemStateManager.java

Author: jukka
Date: Tue Sep 27 16:52:09 2011
New Revision: 1176465

URL: http://svn.apache.org/viewvc?rev=1176465&view=rev
Log:
JCR-3063: NullPointerException in ItemManager

Restore the JCR-2171 fix to avoid deadlocks.

Introduce extra synchronization and checks to prevent
the CMEs and replace the NPEs with InvalidItemStateExceptions.

Modified:
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/ItemManager.java
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/SessionItemStateManager.java
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/SharedItemStateManager.java

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/ItemManager.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/ItemManager.java?rev=1176465&r1=1176464&r2=1176465&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/ItemManager.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/ItemManager.java Tue Sep 27 16:52:09 2011
@@ -159,7 +159,13 @@ public class ItemManager implements Item
         if (parentId == null) {
             // removed state has parentId set to null
             // get from overlayed state
-            parentId = state.getOverlayedState().getParentId();
+            ItemState overlaid = state.getOverlayedState();
+            if (overlaid != null) {
+                parentId = overlaid.getParentId();
+            } else {
+                throw new InvalidItemStateException(
+                        "Could not find parent of node " + state.getNodeId());
+            }
         }
         NodeState parentState = null;
         try {
@@ -197,6 +203,12 @@ public class ItemManager implements Item
 
         // get child node entry
         ChildNodeEntry cne = parentState.getChildNodeEntry(state.getNodeId());
+        if (cne == null) {
+            throw new InvalidItemStateException(
+                    "Could not find child " + state.getNodeId()
+                    + " of node " + parentState.getNodeId());
+        }
+
         NodeTypeRegistry ntReg = sessionContext.getNodeTypeRegistry();
         try {
             EffectiveNodeType ent = ntReg.getEffectiveNodeType(

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/SessionItemStateManager.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/SessionItemStateManager.java?rev=1176465&r1=1176464&r2=1176465&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/SessionItemStateManager.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/SessionItemStateManager.java Tue Sep 27 16:52:09 2011
@@ -65,13 +65,13 @@ public class SessionItemStateManager
      * map of those states that have been removed transiently
      */
     private final Map<ItemId, ItemState> atticStore =
-        new HashMap<ItemId, ItemState>();
+            Collections.synchronizedMap(new HashMap<ItemId, ItemState>());
 
     /**
      * map of new or modified transient states
      */
     private final Map<ItemId, ItemState> transientStore =
-        new HashMap<ItemId, ItemState>();
+            Collections.synchronizedMap(new HashMap<ItemId, ItemState>());
 
     /**
      * ItemStateManager view of the states in the attic; lazily instantiated
@@ -415,7 +415,8 @@ public class SessionItemStateManager
             // Group the descendants by reverse relative depth
             SortedMap<Integer, Collection<ItemState>> statesByReverseDepth =
                 new TreeMap<Integer, Collection<ItemState>>();
-            for (ItemState state : store.values()) {
+            ItemState[] states = store.values().toArray(new ItemState[0]);
+            for (ItemState state : states) {
                 // determine relative depth: > 0 means it's a descendant
                 int depth = hierarchyManager.getShareRelativeDepth(
                         (NodeId) id, state.getId());
@@ -732,11 +733,12 @@ public class SessionItemStateManager
     public void disposeAllTransientItemStates() {
         // dispose item states in transient map & attic
         // (use temp collection to avoid ConcurrentModificationException)
-        Collection<ItemState> tmp = new ArrayList<ItemState>(transientStore.values());
+        ItemState[] tmp;
+        tmp = transientStore.values().toArray(new ItemState[0]);
         for (ItemState state : tmp) {
             disposeTransientItemState(state);
         }
-        tmp = new ArrayList<ItemState>(atticStore.values());
+        tmp = atticStore.values().toArray(new ItemState[0]);
         for (ItemState state : tmp) {
             disposeTransientItemStateInAttic(state);
         }
@@ -841,9 +843,8 @@ public class SessionItemStateManager
                 visibleState = transientState;
             } else {
                 // check attic
-                transientState = atticStore.get(destroyed.getId());
+                transientState = atticStore.remove(destroyed.getId());
                 if (transientState != null) {
-                    atticStore.remove(destroyed.getId());
                     transientState.onDisposed();
                 }
             }

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/SharedItemStateManager.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/SharedItemStateManager.java?rev=1176465&r1=1176464&r2=1176465&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/SharedItemStateManager.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/SharedItemStateManager.java Tue Sep 27 16:52:09 2011
@@ -771,13 +771,14 @@ public class SharedItemStateManager
 
             ISMLocking.ReadLock readLock = null;
             try {
-                // Let the shared item listeners know about the change
-                shared.persisted();
-
                 // downgrade to read lock
                 readLock = writeLock.downgrade();
                 writeLock = null;
 
+                // Let the shared item listeners know about the change
+                // JCR-2171: This must happen after downgrading the lock!
+                shared.persisted();
+
                 /* notify virtual providers about node references */
                 for (int i = 0; i < virtualNodeReferences.length; i++) {
                     ChangeLog virtualRefs = virtualNodeReferences[i];