lucene-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sim...@apache.org
Subject svn commit: r1565429 - in /lucene/dev/trunk/lucene: ./ core/src/java/org/apache/lucene/index/ core/src/java/org/apache/lucene/search/ core/src/test/org/apache/lucene/search/ facet/src/java/org/apache/lucene/facet/taxonomy/
Date Thu, 06 Feb 2014 20:09:37 GMT
Author: simonw
Date: Thu Feb  6 20:09:36 2014
New Revision: 1565429

URL: http://svn.apache.org/r1565429
Log:
LUCENE-5436: RefrenceManager#accquire can result in infinite loop if manager resource is abused
outside of the manager

Modified:
    lucene/dev/trunk/lucene/CHANGES.txt
    lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/index/ReaderManager.java
    lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/search/ReferenceManager.java
    lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/search/SearcherManager.java
    lucene/dev/trunk/lucene/core/src/test/org/apache/lucene/search/TestSearcherManager.java
    lucene/dev/trunk/lucene/facet/src/java/org/apache/lucene/facet/taxonomy/SearcherTaxonomyManager.java

Modified: lucene/dev/trunk/lucene/CHANGES.txt
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/CHANGES.txt?rev=1565429&r1=1565428&r2=1565429&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/CHANGES.txt (original)
+++ lucene/dev/trunk/lucene/CHANGES.txt Thu Feb  6 20:09:36 2014
@@ -246,6 +246,12 @@ Bug fixes
   to byte, before calling Similarity.decodeNormValue.  (Peng Cheng via
   Mike McCandless)
 
+* LUCENE-5436: RefrenceManager#accquire can result in infinite loop if
+  managed resource is abused outside of the RefrenceManager. Decrementing
+  the reference without a corresponding incRef() call can cause an infinite
+  loop. ReferenceManager now throws IllegalStateException if currently managed
+  resources ref count is 0. (Simon Willnauer)
+
 API Changes
 
 * LUCENE-5339: The facet module was simplified/reworked to make the

Modified: lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/index/ReaderManager.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/index/ReaderManager.java?rev=1565429&r1=1565428&r2=1565429&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/index/ReaderManager.java (original)
+++ lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/index/ReaderManager.java Thu Feb
 6 20:09:36 2014
@@ -82,4 +82,9 @@ public final class ReaderManager extends
     return reference.tryIncRef();
   }
 
+  @Override
+  protected int getRefCount(DirectoryReader reference) {
+    return reference.getRefCount();
+  }
+
 }

Modified: lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/search/ReferenceManager.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/search/ReferenceManager.java?rev=1565429&r1=1565428&r2=1565429&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/search/ReferenceManager.java (original)
+++ lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/search/ReferenceManager.java Thu
Feb  6 20:09:36 2014
@@ -92,12 +92,28 @@ public abstract class ReferenceManager<G
    */
   public final G acquire() throws IOException {
     G ref;
+
     do {
       if ((ref = current) == null) {
         throw new AlreadyClosedException(REFERENCE_MANAGER_IS_CLOSED_MSG);
       }
-    } while (!tryIncRef(ref));
-    return ref;
+      if (tryIncRef(ref)) {
+        return ref;
+      }
+      if (getRefCount(ref) == 0 && current == ref) {
+        assert ref != null;
+        /* if we can't increment the reader but we are
+           still the current reference the RM is in a
+           illegal states since we can't make any progress
+           anymore. The reference is closed but the RM still
+           holds on to it as the actual instance.
+           This can only happen if somebody outside of the RM
+           decrements the refcount without a corresponding increment
+           since the RM assigns the new reference before counting down
+           the reference. */
+        throw new IllegalStateException("The managed reference has already closed - this
is likely a bug when the reference count is modified outside of the ReferenceManager");
+      }
+    } while (true);
   }
   
   /**
@@ -133,6 +149,11 @@ public abstract class ReferenceManager<G
   }
 
   /**
+   * Returns the current reference count of the given reference.
+   */
+  protected abstract int getRefCount(G reference);
+
+  /**
    *  Called after close(), so subclass can free any resources.
    *  @throws IOException if the after close operation in a sub-class throws an {@link IOException}

    * */

Modified: lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/search/SearcherManager.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/search/SearcherManager.java?rev=1565429&r1=1565428&r2=1565429&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/search/SearcherManager.java (original)
+++ lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/search/SearcherManager.java Thu
Feb  6 20:09:36 2014
@@ -128,6 +128,11 @@ public final class SearcherManager exten
     return reference.getIndexReader().tryIncRef();
   }
 
+  @Override
+  protected int getRefCount(IndexSearcher reference) {
+    return reference.getIndexReader().getRefCount();
+  }
+
   /**
    * Returns <code>true</code> if no changes have occured since this searcher
    * ie. reader was opened, otherwise <code>false</code>.

Modified: lucene/dev/trunk/lucene/core/src/test/org/apache/lucene/search/TestSearcherManager.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/core/src/test/org/apache/lucene/search/TestSearcherManager.java?rev=1565429&r1=1565428&r2=1565429&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/core/src/test/org/apache/lucene/search/TestSearcherManager.java
(original)
+++ lucene/dev/trunk/lucene/core/src/test/org/apache/lucene/search/TestSearcherManager.java
Thu Feb  6 20:09:36 2014
@@ -303,6 +303,37 @@ public class TestSearcherManager extends
     dir.close();
   }
 
+  public void testReferenceDecrementIllegally() throws Exception {
+    Directory dir = newDirectory();
+    IndexWriter writer = new IndexWriter(dir, newIndexWriterConfig(
+        TEST_VERSION_CURRENT, new MockAnalyzer(random())).setMergeScheduler(new ConcurrentMergeScheduler()));
+    SearcherManager sm = new SearcherManager(writer, false, new SearcherFactory());
+    writer.addDocument(new Document());
+    writer.commit();
+    sm.maybeRefreshBlocking();
+
+    IndexSearcher acquire = sm.acquire();
+    IndexSearcher acquire2 = sm.acquire();
+    sm.release(acquire);
+    sm.release(acquire2);
+
+
+    acquire = sm.acquire();
+    acquire.getIndexReader().decRef();
+    sm.release(acquire);
+    try {
+      sm.acquire();
+      fail("acquire should have thrown an IllegalStateException since we modified the refCount
outside of the manager");
+    } catch (IllegalStateException ex) {
+      //
+    }
+
+    // sm.close(); -- already closed
+    writer.close();
+    dir.close();
+  }
+
+
   public void testEnsureOpen() throws Exception {
     Directory dir = newDirectory();
     new IndexWriter(dir, new IndexWriterConfig(TEST_VERSION_CURRENT, null)).close();

Modified: lucene/dev/trunk/lucene/facet/src/java/org/apache/lucene/facet/taxonomy/SearcherTaxonomyManager.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/facet/src/java/org/apache/lucene/facet/taxonomy/SearcherTaxonomyManager.java?rev=1565429&r1=1565428&r2=1565429&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/facet/src/java/org/apache/lucene/facet/taxonomy/SearcherTaxonomyManager.java
(original)
+++ lucene/dev/trunk/lucene/facet/src/java/org/apache/lucene/facet/taxonomy/SearcherTaxonomyManager.java
Thu Feb  6 20:09:36 2014
@@ -141,4 +141,9 @@ public class SearcherTaxonomyManager ext
       return new SearcherAndTaxonomy(SearcherManager.getSearcher(searcherFactory, newReader),
tr);
     }
   }
+
+  @Override
+  protected int getRefCount(SearcherAndTaxonomy reference) {
+    return reference.searcher.getIndexReader().getRefCount();
+  }
 }



Mime
View raw message