LUCENE-1544: fix deadlock in IW.addIndexes(IndexReader[])

git-svn-id: https://svn.apache.org/repos/asf/lucene/java/trunk@746661 13f79535-47bb-0310-9956-ffa450edef68

CHANGES.txt

@@ -73,6 +73,9 @@ Bug fixes
    happen if you use IndexReader.open with a File or String argument.
    (Mark Miller via Mike McCandless)
 
+5. LUCENE-1544: Fix deadlock in IndexWriter.addIndexes(IndexReader[]).
+   (Mike McCandless via Doug Sale)
+
 New features
 
  1. LUCENE-1411: Added expert API to open an IndexWriter on a prior

src/java/org/apache/lucene/index/IndexWriter.java

@@ -367,8 +367,10 @@ public class IndexWriter {
   // TODO: use ReadWriteLock once we are on 5.0
   private int readCount;                          // count of how many threads are holding read lock
   private Thread writeThread;                     // non-null if any thread holds write lock
-    
+  private int upgradeCount;
+
   synchronized void acquireWrite() {
+    assert writeThread != Thread.currentThread();
     while(writeThread != null || readCount > 0)
       doWait();
 
@@ -392,11 +394,25 @@ public class IndexWriter {
     readCount++;
   }
 
+  // Allows one readLock to upgrade to a writeLock even if
+  // there are other readLocks as long as all other
+  // readLocks are also blocked in this method:
+  synchronized void upgradeReadToWrite() {
+    assert readCount > 0;
+    upgradeCount++;
+    while(readCount > upgradeCount || writeThread != null) {
+      doWait();
+    }
+    
+    writeThread = Thread.currentThread();
+    readCount--;
+    upgradeCount--;
+  }
+
   synchronized void releaseRead() {
     readCount--;
     assert readCount >= 0;
-    if (0 == readCount)
-      notifyAll();
+    notifyAll();
   }
 
   /**
@@ -2680,7 +2696,7 @@ public class IndexWriter {
    * within the transactions, so they must be flushed before the
    * transaction is started.
    */
-  private synchronized void startTransaction(boolean haveWriteLock) throws IOException {
+  private synchronized void startTransaction(boolean haveReadLock) throws IOException {
 
     boolean success = false;
     try {
@@ -2705,12 +2721,15 @@ public class IndexWriter {
     } finally {
       // Release the write lock if our caller held it, on
       // hitting an exception
-      if (!success && haveWriteLock)
-        releaseWrite();
+      if (!success && haveReadLock)
+        releaseRead();
     }
 
-    if (!haveWriteLock)
+    if (haveReadLock) {
+      upgradeReadToWrite();
+    } else {
       acquireWrite();
+    }
 
     success = false;
     try {
@@ -3351,34 +3370,37 @@ public class IndexWriter {
     // Do not allow add docs or deletes while we are running:
     docWriter.pauseAllThreads();
 
-    // We must pre-acquire the write lock here (and not in
-    // startTransaction below) so that no other addIndexes
-    // is allowed to start up after we have flushed &
-    // optimized but before we then start our transaction.
-    // This is because the merging below requires that only
-    // one segment is present in the index:
-    acquireWrite();
+    // We must pre-acquire a read lock here (and upgrade to
+    // write lock in startTransaction below) so that no
+    // other addIndexes is allowed to start up after we have
+    // flushed & optimized but before we then start our
+    // transaction.  This is because the merging below
+    // requires that only one segment is present in the
+    // index:
+    acquireRead();
 
     try {
 
-      boolean success = false;
       SegmentInfo info = null;
       String mergedName = null;
       SegmentMerger merger = null;
 
+      boolean success = false;
+
       try {
         flush(true, false, true);
         optimize();					  // start with zero or 1 seg
         success = true;
       } finally {
-        // Take care to release the write lock if we hit an
+        // Take care to release the read lock if we hit an
         // exception before starting the transaction
         if (!success)
-          releaseWrite();
+          releaseRead();
       }
 
-      // true means we already have write lock; if this call
-      // hits an exception it will release the write lock:
+      // true means we already have a read lock; if this
+      // call hits an exception it will release the write
+      // lock:
       startTransaction(true);
 
       try {

src/test/org/apache/lucene/index/TestIndexWriter.java

@@ -4283,4 +4283,38 @@ public class TestIndexWriter extends LuceneTestCase
       _TestUtil.rmDir(indexDir);
     }
   }
+
+  public void testDeadlock() throws Exception {
+    MockRAMDirectory dir = new MockRAMDirectory();
+    IndexWriter writer = new IndexWriter(dir, true, new WhitespaceAnalyzer());
+    writer.setMaxBufferedDocs(2);
+    Document doc = new Document();
+    doc.add(new Field("content", "aaa bbb ccc ddd eee fff ggg hhh iii", Field.Store.YES,
+                      Field.Index.ANALYZED, Field.TermVector.WITH_POSITIONS_OFFSETS));
+    writer.addDocument(doc);
+    writer.addDocument(doc);
+    writer.addDocument(doc);
+    writer.commit();
+    // index has 2 segments
+
+    MockRAMDirectory dir2 = new MockRAMDirectory();
+    IndexWriter writer2 = new IndexWriter(dir2, new WhitespaceAnalyzer(), IndexWriter.MaxFieldLength.LIMITED);
+    writer2.addDocument(doc);
+    writer2.close();
+
+    IndexReader r1 = IndexReader.open(dir2);
+    IndexReader r2 = (IndexReader) r1.clone();
+    writer.addIndexes(new IndexReader[] {r1, r2});
+    writer.close();
+
+    IndexReader r3 = IndexReader.open(dir);
+    assertEquals(5, r3.numDocs());
+    r3.close();
+
+    r1.close();
+    r2.close();
+
+    dir2.close();
+    dir.close();
+  }
 }