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 2011/10/01 10:20:41 UTC

svn commit: r1177943 - in /lucene/dev/branches/branch_3x: ./ lucene/ lucene/backwards/src/test/ lucene/contrib/misc/src/java/org/apache/lucene/search/SearcherManager.java lucene/contrib/misc/src/test/org/apache/lucene/search/TestSearcherManager.java solr/

Author: simonw
Date: Sat Oct  1 08:20:40 2011
New Revision: 1177943

URL: http://svn.apache.org/viewvc?rev=1177943&view=rev
Log:
LUCENE-3476: SearcherManager misses to close IR if manager is closed during reopen

Modified:
    lucene/dev/branches/branch_3x/   (props changed)
    lucene/dev/branches/branch_3x/lucene/   (props changed)
    lucene/dev/branches/branch_3x/lucene/backwards/src/test/   (props changed)
    lucene/dev/branches/branch_3x/lucene/contrib/misc/src/java/org/apache/lucene/search/SearcherManager.java
    lucene/dev/branches/branch_3x/lucene/contrib/misc/src/test/org/apache/lucene/search/TestSearcherManager.java
    lucene/dev/branches/branch_3x/solr/   (props changed)

Modified: lucene/dev/branches/branch_3x/lucene/contrib/misc/src/java/org/apache/lucene/search/SearcherManager.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_3x/lucene/contrib/misc/src/java/org/apache/lucene/search/SearcherManager.java?rev=1177943&r1=1177942&r2=1177943&view=diff
==============================================================================
--- lucene/dev/branches/branch_3x/lucene/contrib/misc/src/java/org/apache/lucene/search/SearcherManager.java (original)
+++ lucene/dev/branches/branch_3x/lucene/contrib/misc/src/java/org/apache/lucene/search/SearcherManager.java Sat Oct  1 08:20:40 2011
@@ -153,7 +153,15 @@ public class SearcherManager implements 
               }
             }
           }
-          swapSearcher(newSearcher);
+          boolean success = false;
+          try {
+            swapSearcher(newSearcher);
+            success = true;
+          } finally {
+            if (!success) {
+              release(newSearcher);
+            }
+          }
           return true;
         } else {
           return false;
@@ -205,7 +213,12 @@ public class SearcherManager implements 
    *  affected, and they should still call {@link #release}
    *  after they are done. */
   // Not in Java 5: @Override
-  public void close() throws IOException {
-    swapSearcher(null);
+  public synchronized void close() throws IOException {
+    if (currentSearcher != null) {
+      // make sure we can call this more than once
+      // closeable javadoc says:
+      //   if this is already closed then invoking this method has no effect.
+      swapSearcher(null);
+    }
   }
 }

Modified: lucene/dev/branches/branch_3x/lucene/contrib/misc/src/test/org/apache/lucene/search/TestSearcherManager.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_3x/lucene/contrib/misc/src/test/org/apache/lucene/search/TestSearcherManager.java?rev=1177943&r1=1177942&r2=1177943&view=diff
==============================================================================
--- lucene/dev/branches/branch_3x/lucene/contrib/misc/src/test/org/apache/lucene/search/TestSearcherManager.java (original)
+++ lucene/dev/branches/branch_3x/lucene/contrib/misc/src/test/org/apache/lucene/search/TestSearcherManager.java Sat Oct  1 08:20:40 2011
@@ -18,10 +18,17 @@ package org.apache.lucene.search;
  */
 
 import java.io.IOException;
+import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.ExecutorService;
+import java.util.concurrent.atomic.AtomicBoolean;
 
+import org.apache.lucene.analysis.MockAnalyzer;
+import org.apache.lucene.document.Document;
+import org.apache.lucene.index.IndexWriter;
 import org.apache.lucene.index.Term;
 import org.apache.lucene.index.ThreadedIndexingAndSearchingTestCase;
+import org.apache.lucene.store.AlreadyClosedException;
+import org.apache.lucene.store.Directory;
 import org.apache.lucene.util._TestUtil;
 
 public class TestSearcherManager extends ThreadedIndexingAndSearchingTestCase {
@@ -110,4 +117,74 @@ public class TestSearcherManager extends
     }
     mgr.close();
   }
+  
+  public void testIntermediateClose() throws IOException, InterruptedException {
+    Directory dir = newDirectory();
+    IndexWriter writer = new IndexWriter(dir, newIndexWriterConfig(
+        TEST_VERSION_CURRENT, new MockAnalyzer(random)));
+    writer.addDocument(new Document());
+    writer.commit();
+    final CountDownLatch awaitEnterWarm = new CountDownLatch(1);
+    final CountDownLatch awaitClose = new CountDownLatch(1);
+
+    final SearcherManager searcherManager = new SearcherManager(dir,
+        new SearcherWarmer() {
+          //@Override - not on java 5
+          public void warm(IndexSearcher s) throws IOException {
+            try {
+              awaitEnterWarm.countDown();
+              awaitClose.await();
+            } catch (InterruptedException e) {
+              //
+            }
+          }
+        });
+    IndexSearcher searcher = searcherManager.acquire();
+    try {
+      assertEquals(1, searcher.getIndexReader().numDocs());
+    } finally {
+      searcherManager.release(searcher);
+    }
+    writer.addDocument(new Document());
+    writer.commit();
+    final AtomicBoolean success = new AtomicBoolean(false);
+    final AtomicBoolean triedReopen = new AtomicBoolean(false);
+    final Throwable[] exc = new Throwable[1];
+    Thread thread = new Thread(new Runnable() {
+      //@Override - not on java 5
+      public void run() {
+        try {
+          triedReopen.set(true);
+          searcherManager.maybeReopen();
+          success.set(true);
+        } catch (AlreadyClosedException e) {
+          // expected
+        } catch (Throwable e) {
+          exc[0] = e;
+          // use success as the barrier here to make sure we see the write
+          success.set(false);
+
+        }
+      }
+    });
+    thread.start();
+    awaitEnterWarm.await();
+    for (int i = 0; i < 2; i++) {
+      searcherManager.close();
+    }
+    awaitClose.countDown();
+    thread.join();
+    try {
+      searcherManager.acquire();
+      fail("already closed");
+    } catch (AlreadyClosedException ex) {
+      // expected
+    }
+    assertFalse(success.get());
+    assertTrue(triedReopen.get());
+    assertNull("" + exc[0], exc[0]);
+    writer.close();
+    dir.close();
+
+  }
 }