LUCENE-5574: when closing an NRT reader, do not delete files if the original writer has since been closed

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

lucene/CHANGES.txt

@@ -236,6 +236,13 @@ Bug fixes
 
 * LUCENE-5568: Benchmark module's "default.codec" option didn't work. (David Smiley)
 
+* LUCENE-5574: Closing a near-real-time reader no longer attempts to
+  delete unreferenced files if the original writer has been closed;
+  this could cause index corruption in certain cases where index files
+  were directly changed (deleted, overwritten, etc.) in the index
+  directory outside of Lucene.  (Simon Willnauer, Shai Erera, Robert
+  Muir, Mike McCandless)
+
 Test Framework
 
 * LUCENE-5567: When a suite fails with zombie threads failure marker and count 

lucene/core/src/java/org/apache/lucene/index/IndexFileDeleter.java

@@ -29,6 +29,7 @@ import java.util.List;
 import java.util.Map;
 import java.util.regex.Matcher;
 
+import org.apache.lucene.store.AlreadyClosedException;
 import org.apache.lucene.store.Directory;
 import org.apache.lucene.store.NoSuchDirectoryException;
 import org.apache.lucene.util.CollectionUtil;
@@ -262,6 +263,14 @@ final class IndexFileDeleter implements Closeable {
     deleteCommits();
   }
 
+  private void ensureOpen() throws AlreadyClosedException {
+    if (writer == null) {
+      throw new AlreadyClosedException("this IndexWriter is closed");
+    } else {
+      writer.ensureOpen(false);
+    }
+  }
+
   public SegmentInfos getLastSegmentInfos() {
     return lastSegmentInfos;
   }
@@ -578,6 +587,7 @@ final class IndexFileDeleter implements Closeable {
   void deleteFile(String fileName)
        throws IOException {
     assert locked();
+    ensureOpen();
     try {
       if (infoStream.isEnabled("IFD")) {
         infoStream.message("IFD", "delete \"" + fileName + "\"");

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

@@ -4566,8 +4566,7 @@ public class IndexWriter implements Closeable, TwoPhaseCommit{
     deleter.revisitPolicy();
   }
 
-  // Called by DirectoryReader.doClose
-  synchronized void deletePendingFiles() throws IOException {
+  private synchronized void deletePendingFiles() throws IOException {
     deleter.deletePendingFiles();
   }
   
@@ -4665,10 +4664,12 @@ public class IndexWriter implements Closeable, TwoPhaseCommit{
   }
   
   synchronized void incRefDeleter(SegmentInfos segmentInfos) throws IOException {
+    ensureOpen();
     deleter.incRef(segmentInfos, false);
   }
   
   synchronized void decRefDeleter(SegmentInfos segmentInfos) throws IOException {
+    ensureOpen();
     deleter.decRef(segmentInfos);
   }
   

lucene/core/src/java/org/apache/lucene/index/StandardDirectoryReader.java

@@ -25,6 +25,7 @@ import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 
+import org.apache.lucene.store.AlreadyClosedException;
 import org.apache.lucene.store.Directory;
 import org.apache.lucene.store.IOContext;
 import org.apache.lucene.util.IOUtils;
@@ -365,11 +366,15 @@ final class StandardDirectoryReader extends DirectoryReader {
     }
 
     if (writer != null) {
-      writer.decRefDeleter(segmentInfos);
-      
-      // Since we just closed, writer may now be able to
-      // delete unused files:
-      writer.deletePendingFiles();
+      try {
+        writer.decRefDeleter(segmentInfos);
+      } catch (AlreadyClosedException ex) {
+        // This is OK, it just means our original writer was
+        // closed before we were, and this may leave some
+        // un-referenced files in the index, which is
+        // harmless.  The next time IW is opened on the
+        // index, it will delete them.
+      }
     }
 
     // throw the first exception

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

@@ -2371,4 +2371,42 @@ public class TestIndexWriter extends LuceneTestCase {
     r.close();
     dir.close();
   }
+
+  // LUCENE-5574
+  public void testClosingNRTReaderDoesNotCorruptYourIndex() throws IOException {
+    MockDirectoryWrapper dir = newMockDirectory();
+
+    // Allow deletion of still open files:
+    dir.setNoDeleteOpenFile(false);
+
+    // Allow writing to same file more than once:
+    dir.setPreventDoubleWrite(false);
+
+    IndexWriterConfig iwc = newIndexWriterConfig(TEST_VERSION_CURRENT, new MockAnalyzer(random()));
+    LogMergePolicy lmp = new LogDocMergePolicy();
+    lmp.setMergeFactor(2);
+    iwc.setMergePolicy(lmp);
+
+    RandomIndexWriter w = new RandomIndexWriter(random(), dir, iwc);
+    Document doc = new Document();
+    doc.add(new TextField("a", "foo", Field.Store.NO));
+    w.addDocument(doc);
+    w.commit();
+    w.addDocument(doc);
+
+    // Get a new reader, but this also sets off a merge:
+    IndexReader r = w.getReader();
+    w.close();
+
+    // Blow away index and make a new writer:
+    for(String fileName : dir.listAll()) {
+      dir.deleteFile(fileName);
+    }
+
+    w = new RandomIndexWriter(random(), dir);
+    w.addDocument(doc);
+    w.close();
+    r.close();
+    dir.close();
+  }
 }

lucene/core/src/test/org/apache/lucene/util/junitcompat/TestFailIfUnreferencedFiles.java

@@ -22,15 +22,14 @@ import java.util.Collections;
 import org.apache.lucene.document.Document;
 import org.apache.lucene.index.IndexWriter;
 import org.apache.lucene.index.IndexWriterConfig;
-import org.apache.lucene.store.Directory;
 import org.apache.lucene.store.IOContext;
 import org.apache.lucene.store.IndexOutput;
+import org.apache.lucene.store.MockDirectoryWrapper;
 import org.junit.Assert;
 import org.junit.Test;
 import org.junit.runner.JUnitCore;
 import org.junit.runner.Result;
 import org.junit.runner.notification.Failure;
-
 import com.carrotsearch.randomizedtesting.RandomizedTest;
 
 // LUCENE-4456: Test that we fail if there are unreferenced files
@@ -41,7 +40,8 @@ public class TestFailIfUnreferencedFiles extends WithNestedTests {
   
   public static class Nested1 extends WithNestedTests.AbstractNestedTest {
     public void testDummy() throws Exception {
-      Directory dir = newMockDirectory();
+      MockDirectoryWrapper dir = newMockDirectory();
+      dir.setAssertNoUnrefencedFilesOnClose(true);
       IndexWriter iw = new IndexWriter(dir, new IndexWriterConfig(TEST_VERSION_CURRENT, null));
       iw.addDocument(new Document());
       iw.close();

lucene/test-framework/src/java/org/apache/lucene/store/MockDirectoryWrapper.java

@@ -622,7 +622,8 @@ public class MockDirectoryWrapper extends BaseDirectoryWrapper {
     return size;
   }
 
-  private boolean assertNoUnreferencedFilesOnClose = true;
+  // NOTE: This is off by default; see LUCENE-5574
+  private boolean assertNoUnreferencedFilesOnClose;
 
   public void setAssertNoUnrefencedFilesOnClose(boolean v) {
     assertNoUnreferencedFilesOnClose = v;