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 ch...@apache.org on 2017/02/15 04:40:41 UTC

svn commit: r1783063 - in /jackrabbit/oak/branches/1.6: ./ oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexNode.java oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexNodeTest.java

Author: chetanm
Date: Wed Feb 15 04:40:41 2017
New Revision: 1783063

URL: http://svn.apache.org/viewvc?rev=1783063&view=rev
Log:
OAK-5649 - Error in RefreshPolicy can lead to IndexNode lock leak

Merging 1783061,1782945

Modified:
    jackrabbit/oak/branches/1.6/   (props changed)
    jackrabbit/oak/branches/1.6/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexNode.java
    jackrabbit/oak/branches/1.6/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexNodeTest.java

Propchange: jackrabbit/oak/branches/1.6/
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Wed Feb 15 04:40:41 2017
@@ -1,3 +1,3 @@
 /jackrabbit/oak/branches/1.0:1665962
-/jackrabbit/oak/trunk:1781068,1781075,1781248,1781386,1781846,1781907,1782000,1782029,1782196,1782447,1782770,1782990
+/jackrabbit/oak/trunk:1781068,1781075,1781248,1781386,1781846,1781907,1782000,1782029,1782196,1782447,1782770,1782945,1782990,1783061
 /jackrabbit/trunk:1345480

Modified: jackrabbit/oak/branches/1.6/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexNode.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.6/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexNode.java?rev=1783063&r1=1783062&r2=1783063&view=diff
==============================================================================
--- jackrabbit/oak/branches/1.6/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexNode.java (original)
+++ jackrabbit/oak/branches/1.6/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexNode.java Wed Feb 15 04:40:41 2017
@@ -146,8 +146,16 @@ public class IndexNode {
             lock.readLock().unlock();
             return false;
         } else {
-            refreshPolicy.refreshOnReadIfRequired(refreshCallback);
-            return true;
+            boolean success = false;
+            try {
+                refreshPolicy.refreshOnReadIfRequired(refreshCallback);
+                success = true;
+                return true;
+            } finally {
+                if (!success) {
+                    lock.readLock().unlock();
+                }
+            }
         }
     }
 

Modified: jackrabbit/oak/branches/1.6/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexNodeTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.6/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexNodeTest.java?rev=1783063&r1=1783062&r2=1783063&view=diff
==============================================================================
--- jackrabbit/oak/branches/1.6/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexNodeTest.java (original)
+++ jackrabbit/oak/branches/1.6/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexNodeTest.java Wed Feb 15 04:40:41 2017
@@ -21,9 +21,13 @@ package org.apache.jackrabbit.oak.plugin
 
 import java.io.File;
 import java.io.IOException;
+import java.util.Collections;
 
+import org.apache.jackrabbit.oak.plugins.index.lucene.hybrid.NRTIndex;
 import org.apache.jackrabbit.oak.plugins.index.lucene.hybrid.NRTIndexFactory;
+import org.apache.jackrabbit.oak.plugins.index.lucene.hybrid.ReaderRefreshPolicy;
 import org.apache.jackrabbit.oak.plugins.index.lucene.reader.DefaultIndexReaderFactory;
+import org.apache.jackrabbit.oak.plugins.index.lucene.reader.LuceneIndexReader;
 import org.apache.jackrabbit.oak.plugins.index.lucene.reader.LuceneIndexReaderFactory;
 import org.apache.jackrabbit.oak.plugins.index.lucene.util.IndexDefinitionBuilder;
 import org.apache.jackrabbit.oak.spi.mount.Mounts;
@@ -43,7 +47,12 @@ import static com.google.common.util.con
 import static org.apache.jackrabbit.oak.plugins.index.lucene.FieldNames.PATH;
 import static org.apache.jackrabbit.oak.plugins.index.lucene.TestUtil.newDoc;
 import static org.apache.jackrabbit.oak.plugins.nodetype.write.InitialContent.INITIAL_CONTENT;
-import static org.junit.Assert.*;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.fail;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.spy;
 
 public class IndexNodeTest {
     @Rule
@@ -98,10 +107,42 @@ public class IndexNodeTest {
         assertNull(IndexNode.open("/foo", rootBuilder.getNodeState(), builder.getNodeState(), readerFactory, nrtFactory));
     }
 
+    @Test
+    public void lockAndRefreshPolicy() throws Exception {
+        NodeState state = createNRTIndex();
+        IndexDefinition definition = new IndexDefinition(root, state, "/foo");
+        NRTIndex nrtIndex = nrtFactory.createIndex(definition);
+        NRTIndex mock = spy(nrtIndex);
+        doReturn(new FailingPolicy()).when(mock).getRefreshPolicy();
+        IndexNode node = new IndexNode("/foo", definition, Collections.<LuceneIndexReader>emptyList(), mock);
+
+        try {
+            node.acquire();
+            fail();
+        } catch (Exception ignore) {
+
+        }
+
+        node.close();
+    }
+
     private static NodeState createNRTIndex(){
         IndexDefinitionBuilder idx = new IndexDefinitionBuilder();
         idx.indexRule("nt:base").property("foo").propertyIndex();
         idx.async("async", "sync");
         return idx.build();
     }
+
+    private static class FailingPolicy implements ReaderRefreshPolicy {
+
+        @Override
+        public void refreshOnReadIfRequired(Runnable refreshCallback) {
+            throw new IllegalStateException();
+        }
+
+        @Override
+        public void refreshOnWriteIfRequired(Runnable refreshCallback) {
+            throw new IllegalStateException();
+        }
+    }
 }
\ No newline at end of file