Index: CHANGES.txt =================================================================== --- CHANGES.txt (revision 745972) +++ CHANGES.txt (working copy) @@ -73,6 +73,9 @@ 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 Index: src/test/org/apache/lucene/index/TestIndexWriter.java =================================================================== --- src/test/org/apache/lucene/index/TestIndexWriter.java (revision 745972) +++ src/test/org/apache/lucene/index/TestIndexWriter.java (working copy) @@ -4283,4 +4283,38 @@ _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(); + } } Index: src/java/org/apache/lucene/index/IndexWriter.java =================================================================== --- src/java/org/apache/lucene/index/IndexWriter.java (revision 745972) +++ src/java/org/apache/lucene/index/IndexWriter.java (working copy) @@ -367,8 +367,10 @@ // 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 @@ 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 @@ * 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 @@ } 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 @@ // 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 {