docs/attachments/LUCENE-5436/LUCENE-5436.patch

Index: lucene/CHANGES.txt
===================================================================
--- lucene/CHANGES.txt	(revision 1565314)
+++ lucene/CHANGES.txt	(working copy)
@@ -235,6 +235,12 @@
   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
Index: lucene/core/src/java/org/apache/lucene/index/ReaderManager.java
===================================================================
--- lucene/core/src/java/org/apache/lucene/index/ReaderManager.java	(revision 1565314)
+++ lucene/core/src/java/org/apache/lucene/index/ReaderManager.java	(working copy)
@@ -82,4 +82,9 @@
     return reference.tryIncRef();
   }
 
+  @Override
+  protected int getRefCount(DirectoryReader reference) {
+    return reference.getRefCount();
+  }
+
 }
Index: lucene/core/src/java/org/apache/lucene/search/ReferenceManager.java
===================================================================
--- lucene/core/src/java/org/apache/lucene/search/ReferenceManager.java	(revision 1565314)
+++ lucene/core/src/java/org/apache/lucene/search/ReferenceManager.java	(working copy)
@@ -92,12 +92,28 @@
    */
   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 @@
   }
 
   /**
+   * 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} 
    * */
Index: lucene/core/src/java/org/apache/lucene/search/SearcherManager.java
===================================================================
--- lucene/core/src/java/org/apache/lucene/search/SearcherManager.java	(revision 1565314)
+++ lucene/core/src/java/org/apache/lucene/search/SearcherManager.java	(working copy)
@@ -128,6 +128,11 @@
     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>.
Index: lucene/core/src/test/org/apache/lucene/search/TestSearcherManager.java
===================================================================
--- lucene/core/src/test/org/apache/lucene/search/TestSearcherManager.java	(revision 1565314)
+++ lucene/core/src/test/org/apache/lucene/search/TestSearcherManager.java	(working copy)
@@ -303,6 +303,37 @@
     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();
Index: lucene/facet/src/java/org/apache/lucene/facet/taxonomy/SearcherTaxonomyManager.java
===================================================================
--- lucene/facet/src/java/org/apache/lucene/facet/taxonomy/SearcherTaxonomyManager.java	(revision 1565314)
+++ lucene/facet/src/java/org/apache/lucene/facet/taxonomy/SearcherTaxonomyManager.java	(working copy)
@@ -141,4 +141,9 @@
       return new SearcherAndTaxonomy(SearcherManager.getSearcher(searcherFactory, newReader), tr);
     }
   }
+
+  @Override
+  protected int getRefCount(SearcherAndTaxonomy reference) {
+    return reference.searcher.getIndexReader().getRefCount();
+  }
 }