You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by si...@apache.org on 2014/02/06 21:14:08 UTC
svn commit: r1565430 - in /lucene/dev/branches/branch_4x: ./ lucene/
lucene/core/ lucene/core/src/java/org/apache/lucene/index/
lucene/core/src/java/org/apache/lucene/search/
lucene/core/src/test/org/apache/lucene/search/ lucene/facet/
lucene/facet/src...
Author: simonw
Date: Thu Feb 6 20:14:07 2014
New Revision: 1565430
URL: http://svn.apache.org/r1565430
Log:
LUCENE-5436: RefrenceManager#accquire can result in infinite loop if manager resource is abused outside of the manager
Modified:
lucene/dev/branches/branch_4x/ (props changed)
lucene/dev/branches/branch_4x/lucene/ (props changed)
lucene/dev/branches/branch_4x/lucene/CHANGES.txt (contents, props changed)
lucene/dev/branches/branch_4x/lucene/core/ (props changed)
lucene/dev/branches/branch_4x/lucene/core/src/java/org/apache/lucene/index/ReaderManager.java
lucene/dev/branches/branch_4x/lucene/core/src/java/org/apache/lucene/search/ReferenceManager.java
lucene/dev/branches/branch_4x/lucene/core/src/java/org/apache/lucene/search/SearcherManager.java
lucene/dev/branches/branch_4x/lucene/core/src/test/org/apache/lucene/search/TestSearcherManager.java
lucene/dev/branches/branch_4x/lucene/facet/ (props changed)
lucene/dev/branches/branch_4x/lucene/facet/src/java/org/apache/lucene/facet/taxonomy/SearcherTaxonomyManager.java
Modified: lucene/dev/branches/branch_4x/lucene/CHANGES.txt
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_4x/lucene/CHANGES.txt?rev=1565430&r1=1565429&r2=1565430&view=diff
==============================================================================
--- lucene/dev/branches/branch_4x/lucene/CHANGES.txt (original)
+++ lucene/dev/branches/branch_4x/lucene/CHANGES.txt Thu Feb 6 20:14:07 2014
@@ -181,6 +181,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/branches/branch_4x/lucene/core/src/java/org/apache/lucene/index/ReaderManager.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_4x/lucene/core/src/java/org/apache/lucene/index/ReaderManager.java?rev=1565430&r1=1565429&r2=1565430&view=diff
==============================================================================
--- lucene/dev/branches/branch_4x/lucene/core/src/java/org/apache/lucene/index/ReaderManager.java (original)
+++ lucene/dev/branches/branch_4x/lucene/core/src/java/org/apache/lucene/index/ReaderManager.java Thu Feb 6 20:14:07 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/branches/branch_4x/lucene/core/src/java/org/apache/lucene/search/ReferenceManager.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_4x/lucene/core/src/java/org/apache/lucene/search/ReferenceManager.java?rev=1565430&r1=1565429&r2=1565430&view=diff
==============================================================================
--- lucene/dev/branches/branch_4x/lucene/core/src/java/org/apache/lucene/search/ReferenceManager.java (original)
+++ lucene/dev/branches/branch_4x/lucene/core/src/java/org/apache/lucene/search/ReferenceManager.java Thu Feb 6 20:14:07 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/branches/branch_4x/lucene/core/src/java/org/apache/lucene/search/SearcherManager.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_4x/lucene/core/src/java/org/apache/lucene/search/SearcherManager.java?rev=1565430&r1=1565429&r2=1565430&view=diff
==============================================================================
--- lucene/dev/branches/branch_4x/lucene/core/src/java/org/apache/lucene/search/SearcherManager.java (original)
+++ lucene/dev/branches/branch_4x/lucene/core/src/java/org/apache/lucene/search/SearcherManager.java Thu Feb 6 20:14:07 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/branches/branch_4x/lucene/core/src/test/org/apache/lucene/search/TestSearcherManager.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_4x/lucene/core/src/test/org/apache/lucene/search/TestSearcherManager.java?rev=1565430&r1=1565429&r2=1565430&view=diff
==============================================================================
--- lucene/dev/branches/branch_4x/lucene/core/src/test/org/apache/lucene/search/TestSearcherManager.java (original)
+++ lucene/dev/branches/branch_4x/lucene/core/src/test/org/apache/lucene/search/TestSearcherManager.java Thu Feb 6 20:14:07 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/branches/branch_4x/lucene/facet/src/java/org/apache/lucene/facet/taxonomy/SearcherTaxonomyManager.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_4x/lucene/facet/src/java/org/apache/lucene/facet/taxonomy/SearcherTaxonomyManager.java?rev=1565430&r1=1565429&r2=1565430&view=diff
==============================================================================
--- lucene/dev/branches/branch_4x/lucene/facet/src/java/org/apache/lucene/facet/taxonomy/SearcherTaxonomyManager.java (original)
+++ lucene/dev/branches/branch_4x/lucene/facet/src/java/org/apache/lucene/facet/taxonomy/SearcherTaxonomyManager.java Thu Feb 6 20:14:07 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();
+ }
}