LUCENE-9408: Ensure OneMerge#mergeFinished is only called once (#1590)

in the case of an exception it's possible that some OneMerge instances
will be closed multiple times. This commit ensures that mergeFinished is
really just called once instead of multiple times.

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

@@ -29,6 +29,7 @@ import java.util.LinkedList;
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Queue;
 import java.util.Set;
 import java.util.concurrent.ConcurrentLinkedQueue;
@@ -4286,25 +4287,30 @@ public class IndexWriter implements Closeable, TwoPhaseCommit, Accountable,
 
   @SuppressWarnings("try")
   private synchronized void closeMergeReaders(MergePolicy.OneMerge merge, boolean suppressExceptions) throws IOException {
-    final boolean drop = suppressExceptions == false;
-    try (Closeable finalizer = () -> merge.mergeFinished(suppressExceptions == false)) {
-      IOUtils.applyToAll(merge.readers, sr -> {
-        final ReadersAndUpdates rld = getPooledInstance(sr.getOriginalSegmentInfo(), false);
-        // We still hold a ref so it should not have been removed:
-        assert rld != null;
-        if (drop) {
-          rld.dropChanges();
-        } else {
-          rld.dropMergingUpdates();
-        }
-        rld.release(sr);
-        release(rld);
-        if (drop) {
-          readerPool.drop(rld.info);
-        }
-      });
-    } finally {
-      Collections.fill(merge.readers, null);
+    if (merge.hasFinished() == false) {
+      final boolean drop = suppressExceptions == false;
+      try (Closeable finalizer = () -> merge.mergeFinished(suppressExceptions == false)) {
+        IOUtils.applyToAll(merge.readers, sr -> {
+          final ReadersAndUpdates rld = getPooledInstance(sr.getOriginalSegmentInfo(), false);
+          // We still hold a ref so it should not have been removed:
+          assert rld != null;
+          if (drop) {
+            rld.dropChanges();
+          } else {
+            rld.dropMergingUpdates();
+          }
+          rld.release(sr);
+          release(rld);
+          if (drop) {
+            readerPool.drop(rld.info);
+          }
+        });
+      } finally {
+        Collections.fill(merge.readers, null);
+      }
+    } else {
+      assert merge.readers.stream().filter(Objects::nonNull).count() == 0 : "we are done but still have readers: " + merge.readers;
+      assert suppressExceptions : "can't be done and not suppressing exceptions";
     }
   }
 

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

@@ -256,11 +256,9 @@ public abstract class MergePolicy {
     /** Called by {@link IndexWriter} after the merge is done and all readers have been closed.
      * @param success true iff the merge finished successfully ie. was committed */
     public void mergeFinished(boolean success) throws IOException {
-      mergeCompleted.complete(success);
-      // https://issues.apache.org/jira/browse/LUCENE-9408
-      // if (mergeCompleted.complete(success) == false) {
-      //   throw new IllegalStateException("merge has already finished");
-      // }
+      if (mergeCompleted.complete(success) == false) {
+        throw new IllegalStateException("merge has already finished");
+      }
     }
 
     /** Wrap the reader in order to add/remove information to the merged segment. */
@@ -390,7 +388,7 @@ public abstract class MergePolicy {
      * Returns true if the merge has finished or false if it's still running or
      * has not been started. This method will not block.
      */
-    boolean isDone() {
+    boolean hasFinished() {
       return mergeCompleted.isDone();
     }
 

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

@@ -110,7 +110,6 @@ public class TestMergePolicy extends LuceneTestCase {
     }
   }
 
-  @AwaitsFix(bugUrl = "https://issues.apache.org/jira/browse/LUCENE-9408")
   public void testFinishTwice() throws IOException {
     try (Directory dir = newDirectory()) {
       MergePolicy.MergeSpecification spec = createRandomMergeSpecification(dir, 1);