You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-commits@jackrabbit.apache.org by th...@apache.org on 2015/08/14 16:01:27 UTC

svn commit: r1695917 - in /jackrabbit/oak/trunk/oak-core/src: main/java/org/apache/jackrabbit/oak/cache/CacheLIRS.java test/java/org/apache/jackrabbit/oak/cache/ConcurrentTest.java

Author: thomasm
Date: Fri Aug 14 14:01:27 2015
New Revision: 1695917

URL: http://svn.apache.org/r1695917
Log:
OAK-3234 LIRS cache: possible deadlock while loading an entry

Modified:
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/cache/CacheLIRS.java
    jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/cache/ConcurrentTest.java

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/cache/CacheLIRS.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/cache/CacheLIRS.java?rev=1695917&r1=1695916&r2=1695917&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/cache/CacheLIRS.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/cache/CacheLIRS.java Fri Aug 14 14:01:27 2015
@@ -75,6 +75,7 @@ public class CacheLIRS<K, V> implements
 
     private static final Logger LOG = LoggerFactory.getLogger(CacheLIRS.class);
     private static final AtomicInteger NEXT_CACHE_ID = new AtomicInteger();
+    private static final ThreadLocal<Integer> CURRENTLY_LOADING = new ThreadLocal<Integer>();
 
     /**
      * Listener for items that are evicted from the cache. The listener
@@ -940,6 +941,15 @@ public class CacheLIRS<K, V> implements
                 if (value != null) {
                     return value;
                 }
+                // if we are within a loader, and are currently loading
+                // an entry, then we need to avoid a possible deadlock
+                // (we ensure that while loading an entry, we only load
+                // entries with a higher hash code, so there is a clear order)
+                Integer outer = CURRENTLY_LOADING.get();
+                if (outer != null && hash <= outer) {
+                    // to prevent a deadlock, we also load the value ourselves
+                    return load(key, hash, valueLoader);
+                }
                 ConcurrentHashMap<K, Object> loading = cache.loadingInProgress;
                 // the object we have to wait for in case another thread loads
                 // this value
@@ -955,11 +965,13 @@ public class CacheLIRS<K, V> implements
                     if (alreadyLoading == null) {
                         // we are loading ourselves
                         try {
+                            CURRENTLY_LOADING.set(hash);
                             return load(key, hash, valueLoader);
                         } finally {
                             loading.remove(key);
                             // notify other threads
                             loadNow.notifyAll();
+                            CURRENTLY_LOADING.remove();
                         }
                     }
                 }

Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/cache/ConcurrentTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/cache/ConcurrentTest.java?rev=1695917&r1=1695916&r2=1695917&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/cache/ConcurrentTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/cache/ConcurrentTest.java Fri Aug 14 14:01:27 2015
@@ -40,7 +40,8 @@ public class ConcurrentTest {
     public void testLoaderBlock() throws Exception {
         // access to the same segment should not be blocked while loading an entry
         // only access to this entry is blocked
-        final CacheLIRS<Integer, Integer> cache = new CacheLIRS.Builder().
+        final CacheLIRS<Integer, Integer> cache = 
+                new CacheLIRS.Builder<Integer, Integer>().
                 maximumWeight(100).averageWeight(10).build();
         final Exception[] ex = new Exception[1];
         int threadCount = 10;
@@ -104,10 +105,11 @@ public class ConcurrentTest {
     @Test
     public void testCacheAccessInLoaderDeadlock() throws Exception {
         final Random r = new Random(1);
-        final CacheLIRS<Integer, Integer> cache = new CacheLIRS.Builder().
+        final CacheLIRS<Integer, Integer> cache = 
+                new CacheLIRS.Builder<Integer, Integer>().
                 maximumWeight(100).averageWeight(10).build();
         final Exception[] ex = new Exception[1];
-        final int entryCount = 100;
+        final int entryCount = 10;
         int size = 3;
         Thread[] threads = new Thread[size];
         final AtomicBoolean stop = new AtomicBoolean();
@@ -118,7 +120,11 @@ public class ConcurrentTest {
                     Callable<Integer> callable = new Callable<Integer>() {
                         @Override
                         public Integer call() throws ExecutionException {
-                            cache.get(r.nextInt(entryCount));
+                            if (r.nextBoolean()) {
+                                cache.get(r.nextInt(entryCount), this);
+                            } else {
+                                cache.get(r.nextInt(entryCount));
+                            }
                             return 1;
                         }
                     };
@@ -156,7 +162,8 @@ public class ConcurrentTest {
     @Test
     public void testRandomOperations() throws Exception {
         Random r = new Random(1);
-        final CacheLIRS<Integer, Integer> cache = new CacheLIRS.Builder().
+        final CacheLIRS<Integer, Integer> cache = 
+                new CacheLIRS.Builder<Integer, Integer>().
                 maximumWeight(100).averageWeight(10).build();
         final Exception[] ex = new Exception[1];
         int size = 3;