svn commit: r1038201 - /jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/SharedItemStateManager.java

[ju...@apache.org]

Author: jukka
Date: Tue Nov 23 16:55:10 2010
New Revision: 1038201

URL: http://svn.apache.org/viewvc?rev=1038201&view=rev
Log:
JCR-2813: "overwriting cached entry" warnings

Use a currentlyLoading set instead of full synchronization to prevent two threads from concurrently loading the same item.

Modified:
    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/state/SharedItemStateManager.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/SharedItemStateManager.java?rev=1038201&r1=1038200&r2=1038201&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 Nov 23 16:55:10 2010
@@ -17,6 +17,7 @@
 package org.apache.jackrabbit.core.state;
 
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Set;
@@ -1693,33 +1694,64 @@ public class SharedItemStateManager
     }
 
     /**
+     * Identifiers of the item states that are currently being loaded from
+     * the underlying persistence manager. Used exclusively by the
+     * {@link #getNonVirtualItemState(ItemId)} method to prevent two threads
+     * from concurrently loading the same items.
+     */
+    private final Set<ItemId> currentlyLoading = new HashSet<ItemId>();
+
+    /**
      * Returns the item state for the given id without considering virtual
      * item state providers.
      */
     private ItemState getNonVirtualItemState(ItemId id)
             throws NoSuchItemStateException, ItemStateException {
+        // First check if the item state is already in the cache
         ItemState state = cache.retrieve(id);
-        if (state == null) {
-            // not found in cache, load from persistent storage
+        if (state != null) {
+            return state;
+        }
+
+        // Wait if another thread is already loading this item state
+        synchronized (this) {
+            while (currentlyLoading.contains(id)) {
+                try {
+                    wait();
+                } catch (InterruptedException e) {
+                    throw new ItemStateException(
+                            "Interrupted while waiting for " + id, e);
+                }
+            }
+
+            state = cache.retrieve(id);
+            if (state != null) {
+                return state;
+            }
+
+            // No other thread has loaded the item state, so we'll do it
+            currentlyLoading.add(id);
+        }
+
+        try {
+            // Load the item state from persistent storage
+            // NOTE: This needs to happen outside a synchronized block!
             state = loadItemState(id);
             state.setStatus(ItemState.STATUS_EXISTING);
+            state.setContainer(this);
+
+            // put it in cache
+            cache.cache(state);
+
+            return state;
+        } finally {
+            // Notify other concurrent threads that we're done with this item
+            // NOTE: This needs to happen within the finally block!
             synchronized (this) {
-                // Use a double check to ensure that the cache entry is
-                // not created twice. We don't synchronize the entire
-                // method to allow the first cache retrieval to proceed
-                // even when another thread is loading a new item state.
-                ItemState cachedState = cache.retrieve(id);
-                if (cachedState == null) {
-                    // put it in cache
-                    cache.cache(state);
-                    // set parent container
-                    state.setContainer(this);
-                } else {
-                    state = cachedState;
-                }
+                currentlyLoading.remove(id);
+                notifyAll();
             }
         }
-        return state;
     }
 
     /**