A failed IndexRearranger.execute() closes its input reader under tasks that are still running

IndexRearranger.execute() opens the output IndexWriter and the input reader in one try-with-resources and runs one task per segment selector in a pool of min(processors, selectors) threads; each task calls its DocumentSelector on the input's segments and then writer.addIndexes(...). It waits with for (future : futures) future.get() in list order, so the first failed task makes it leave the block at once, without waiting for or stopping the tasks listed after it, and leaving the block closes the reader and then the writer while those tasks still use them. The delete phase (10.0.0+) has the same loop. In the one-test repro the reader is closed under a running selector in 20 of 20 runs; under natural timing a selector scanning a 200,000-document segment saw the reader closed 4.6 ms into its scan and failed with AlreadyClosedException, and since 9.4.0 an in-flight addIndexes merge also reports an uncaught MergeException on a merge thread. A one-thread pool is no protection: its thread can start the next queued task while the caller closes the reader (9 of 10 check runs). execute() still throws the same first exception as before, the input is only read and successful runs are unaffected, so the impact is minor. Waiting for every task before rethrowing fixes it; it is as old as the class (8.9.0) and not fixed upstream.

• Segment selector (IndexRearranger.DocumentSelector): user code, BitSet getFilteredDocs(CodecReader reader) throws IOException, called once per input segment; the selected documents form one output segment.
• Task: one per selector, submitted to the pool; it wraps each input segment in a filtered reader (whose constructor calls the selector) and passes them to writer.addIndexes(...).
• try-with-resources: Java closes the resources in reverse order when the block ends, also when an exception leaves it; here the reader first, then the writer.

1. execute() (:113-137) creates Executors.newFixedThreadPool(Math.min(availableProcessors, segmentSelectors.size()), ...) (:114-117) and opens try (IndexWriter writer = new IndexWriter(output, ...); IndexReader reader = DirectoryReader.open(input)) (:126-129).
2. createRearrangedIndex (:180-200) submits one task per selector (:188-195). Each runs addOneSegment (:202-210): a DocSelectorFilteredCodecReader per leaf, whose constructor calls selector.getFilteredDocs(in) (:256), then writer.addIndexes(readers) (:209).
3. It waits with for (Future<Void> future : futures) { future.get(); } (:197-199), in list order, so every task listed before the failed one has already finished. The first failed get() throws ExecutionException (the first failure in list order, not necessarily the first in time); the later futures are neither waited for nor cancelled. A check of both orders on main: [failing, running] closed the reader under the running selector in 5 of 5 runs, [running, failing] in 0 of 5.
4. The exception leaves the try-with-resources, which closes the reader (its segment readers' closed listeners fire) and then the writer while the later tasks are still inside their selector or addIndexes. Only then does execute() throw.
5. A running selector's next read fails with AlreadyClosedException: this IndexReader is closed; since 9.4.0 an addIndexes merge reading the input fails on its merge thread with MergePolicy$MergeException: AlreadyClosedException: Already closed: MemorySegmentIndexInput(.../_0.cfs) [slice=_0.fdx].
6. In the selector phase, a task that starts only after the close fails at once in reader.leaves() (:204-205; CompositeReader.getContext() calls ensureOpen()), in a future nobody reads, so it never reaches its selector. The delete phase differs (code reading, not run): each task gets its segment reader before it is submitted (:221-227) and calls the deletes selector directly (:237), so a delete task that starts late does run the user's deletes selector.
7. applyDeletes (:212-233, since 10.0.0) has the same loop (:230-232) inside the second try-with-resources (:132-135), whose reader is an NRT reader of that writer. It is a separate failure point, reached only when the selector phase succeeded, with one task per output segment (normally one per selector).

The one-test repro needs a pool of two threads, so that its signals can run both selectors at once. The bug does not: with one thread the tasks run one after the other, but FutureTask records the failure and wakes the caller before the pool thread takes the next queued task, which can then start while the caller is closing the reader. A check with the JVM limited to one processor (on a 32-processor host): the reader was closed under the next selector's scan in 9 of 10 runs on main, in 0 of 10 with the fix; a machine with a single core was not tried. Separately, executor.shutdown() (:136) runs only when both phases succeed, so a failed run also leaves its pool threads idle; that needs its own change.

• The one-test repro (one document, two selectors): the reader is closed under the running second selector in 20 of 20 runs on main with assertions off, and fails on the 10.5.2 and 10.6.0 release jars too; with the fix it passes 20 of 20.
• A natural-timing probe (verification only, no ordering signals): one selector throws at once, the other scans a NumericDocValues field of a 200,000-document segment 100 times (57-93 ms when both succeed, 21 runs). On stock main the reader was closed 4.6 ms into the scan and the scan failed with AlreadyClosedException: Already closed: MemorySegmentIndexInput(.../_0.cfs) [slice=_0_Lucene90_0.dvd]. Over 10 iterations all 10 failure runs went wrong: 4 hit that exception mid-scan, and in 6 the scan never started within the probe's 60 s wait (from the code, the task met the already closed reader first). With the fix 20 of 20 pass, and the scan always finished 33-97 ms before the reader was closed.
• A one-thread-pool check (the same selectors, -XX:ActiveProcessorCount=1, assertions off, 10 iterations): on main the reader was closed under the running scan in 9 of 10 runs (in the 10th the scan never started); with the fix in 0 of 10.

• Who is hit. Callers of IndexRearranger (@lucene.experimental, IndexRearranger.java:66) with two or more selectors whose run fails while a task listed after the failed one is still running (the await loop waits in list order). Two or more processors, the normal shape, make that the usual case; with a one-thread pool it is a race, which still happened in 9 of 10 check runs.
• Triggers are ordinary. DocumentSelector.getFilteredDocs is declared throws IOException; addIndexes can fail on I/O. The shipped BinaryDocValueSelector.getFilteredDocs dereferences reader.getBinaryDocValues(field) without a null check, so a segment without that field throws NullPointerException (code reading). #1019 (2022) fixed a race after its author hit a rare TestIndexRearranger task failure, and a Jenkins build in November 2022 shows another task failure.
• Default vs opt-in. The tool itself is opt-in (a misc module). Inside it no setting is needed: every multi-selector run on a multi-core machine waits with the early-leaving loop.
• Per failed run. The tasks listed after the failed one that are running when the reader closes (at most one per pool thread) can each fail with AlreadyClosedException on their next read of the closed input, while execute() unwinds or after it has already thrown; since 9.4.0 each addIndexes merge still running also reports one uncaught MergePolicy.MergeException on a merge thread. In the selector phase, tasks that start after the close fail in futures nobody reads. The user's selector code can still run after execute() has thrown.
• What does not happen. The input index is not changed (it is only read); execute() throws the same first exception as before (the first failure in list order); successful runs are unaffected; assertions on or off make no difference. The output of a failed run should not be relied on (not part of this claim).
• Reach. Its main known consumer is benchmark tooling (luceneutil discussed using it for its nightly benchmark). No public report since 8.9.0 (2021) was found; likelihood rare, priority low.

# Lucene main 645d4fe14a20, assertions on (-ea), seed DEADBEEF: 1 test(s) run, 1 failed
    => java.lang.AssertionError: reader closed under the running second selector
       org.junit.Assert.fail(Assert.java:89)
       org.junit.Assert.assertTrue(Assert.java:42)
       org.junit.Assert.assertFalse(Assert.java:65)
       org.apache.lucene.misc.index.TestIndexRearrangerFailedExecute.testFailedExecuteClosesReaderUnderRunningSelector(TestIndexRearrangerFailedExecute.java:86)

• Reader contract. IndexReader.close(): "Closes files associated with this index. Also saves any new deletions to disk. No other methods should be called after this has been called." (IndexReader.java:345-346). addIndexes(CodecReader...): "The provided IndexReaders are not closed." (IndexWriter.java:3257), so the caller keeps them open for the call. IndexRearranger opens the reader itself and hands it to its own tasks, so it is that caller.
• Precedent. Issue #12278 (opened by javanna) asked: "Is it a reasonable expectation that once IndexSearcher#search(Query, CollectorManager) returns, that specific search has completed and has left no threads behind still working on it? I would expect this to be true". 9.9.0 then shipped "GITHUB#12523: TaskExecutor waits for all tasks to complete before returning when Exceptions are thrown during concurrent operations" and "GITHUB#12689: TaskExecutor to cancel all tasks on exception to avoid needless computation." (lucene/CHANGES.txt:3168, :3198); the code says "We rather want to make sure that invokeAll does not leave any running tasks behind when it returns." (TaskExecutor.java:186). StandardDirectoryReader keeps processing the remaining futures after one fails (:208-216), and CheckIndex shuts down and awaits its pool in a finally (:599-614).
• Not documented. execute() has no javadoc; the class javadoc says nothing about failures; the CHANGES entries are features; TestIndexRearranger has 3 tests, all on successful runs, with no expectThrows.
• Legal trigger. DocumentSelector.getFilteredDocs is declared throws IOException (IndexRearranger.java:283), and addIndexes(CodecReader...) declares IOException.

The strongest case for "works as designed" (the intended-behaviour verifier, the cross-model review and the adjudication): IndexRearranger is an @lucene.experimental offline tool whose javadoc promises nothing on failure, fail-fast Future.get() loops are a common idiom, and IndexWriter is built to tolerate close() racing addIndexes: LUCENE-1335's TestAddIndexes.testAddIndexesWithClose, testAddIndexesWithCloseNoWait and testAddIndexesWithRollback close or roll back the writer while other threads add indexes, and since LUCENE-10216 (9.4.0) addIndexes(CodecReader...) merges are aborted on close (IndexWriter.java:3298-3302). So the IndexWriter.close() NOTE, "You must ensure no other threads are still making changes at the same time that this method is invoked." (:1359-1360), is weak support for the writer half.

Why it fails: it does not cover the reader half, where both observed failures come from (the selector's read, and the merge's read of the input _0.cfs, because the reader is closed before the writer). IndexRearranger owns the reader and the tasks and closes the reader while its own tasks still call it, and Lucene rejected fail-fast for its own concurrent code (#12278, #12523, #12689). @lucene.experimental is about API stability, not about leaving work running. The argument limits priority (low), not validity; this is an inference from the code, its contracts and precedent, not a maintainer ruling. Conceded: IndexReader.close() is a usage rule, not a concurrency or lifetime protocol, and a maintainer could reasonably see this as low-value clean-up of a run that has already failed.

Option A was run through gradle in a scratch worktree of 645d4fe14a20 and is recommended, ideally together with shutting the pool down in a finally (the separate pool defect). B and C are alternatives a maintainer may prefer; neither was run as written. No open PR changes the await loops.

Both await loops (createRearrangedIndex and applyDeletes) call a new awaitAll: get() on every future, keep the first ExecutionException, add the later causes as suppressed, and throw the first one after the loop. The reader and the writer then close only after every task has finished.

--- a/lucene/misc/src/java/org/apache/lucene/misc/index/IndexRearranger.java
+++ b/lucene/misc/src/java/org/apache/lucene/misc/index/IndexRearranger.java
@@ -194,8 +194,29 @@
       futures.add(executor.submit(addSegment));
     }
 
+    awaitAll(futures);
+  }
+
+  /**
+   * Waits for every task before rethrowing the first failure, so that no task is still using the
+   * writer or the reader when the caller closes them.
+   */
+  private static void awaitAll(List<Future<Void>> futures)
+      throws ExecutionException, InterruptedException {
+    ExecutionException first = null;
     for (Future<Void> future : futures) {
-      future.get();
+      try {
+        future.get();
+      } catch (ExecutionException e) {
+        if (first == null) {
+          first = e;
+        } else {
+          first.addSuppressed(e.getCause());
+        }
+      }
+    }
+    if (first != null) {
+      throw first;
     }
   }
 
@@ -227,9 +248,7 @@
       futures.add(executor.submit(applyDeletesToSegment));
     }
 
-    for (Future<Void> future : futures) {
-      future.get();
-    }
+    awaitAll(futures);
   }
 
   private static void applyDeletesToOneSegment(

Trade-offs

• A failing execute() returns only after every submitted task has finished, including queued ones, which still do their full addIndexes work for a run that is reported as failed (bounded: one task per selector). B avoids that.
• Only ExecutionException is collected. An InterruptedException in get() still leaves the loop at once, so an interrupt of the calling thread can still close the reader under running tasks; a fuller fix keeps waiting and restores the interrupt flag, or uses C. (A CancellationException cannot occur: nothing cancels these futures.)
• A task that hangs now also keeps a failing execute() waiting, indefinitely, as it already does for a successful one.
• It does not shut the pool down on failure (:136); the study's combined change adds a finally that does (+35/-13). With both, the regression test needs no thread-leak filter (executed).
• Future.cancel(false) alone would not do: a running FutureTask keeps running after cancel, and get() then throws CancellationException at once, which is why TaskExecutor overrides cancel.
• Error path only; no API or format change.

What the checks validated

Gradle with JDK 25 in a scratch worktree of 645d4fe14a20 (removed afterwards):

• the repro fails on the stock build and passes with the fix, assertions on and off; its committable copy (unused lambda parameter renamed to _) too;
• with the fix, TestIndexRearranger, TestBinaryDocValueSelector and the repro with 5 iterations and -XX:ActiveProcessorCount=4: 30 tests, 0 failures;
• with the fix, the whole :lucene:misc:test (default settings): 24 classes, 317 tests, 0 failures, 10 skipped;
• :lucene:misc:check -x test with the fix and the committable test: BUILD SUCCESSFUL;
• a filter-free copy of the test (a check only): with A alone the suite fails on 2 leaked idle rearranger threads; with A plus the pool shutdown in a finally it passes (1 of 1 with assertions on, 5 of 5 off).

Earlier, by the study: the finder's two-test class and an independent three-leg class pass with the fix.

As TaskExecutor does since #12689: wrap each task so that it records when it starts; on the first failure cancel every task that has not started and wait for the started ones before rethrowing.

Trade-offs

Saves the wasted work of A on a failing run. More code: plain Future.cancel(false) returns true for a queued and for a running task alike, so it cannot tell them apart; it needs an atomic started flag as in TaskExecutor. Not tried.

Use a pool per phase and declare it as the last resource of each try-with-resources, so that its close() runs first: ExecutorService.close() (JDK 19+) shuts down and waits for all tasks, and on interrupt calls shutdownNow() and keeps waiting for the running ones. This also fixes the pool leak.

Trade-offs

Queued tasks still run unless also cancelled. Interrupts are cooperative, so a selector that ignores them keeps close() waiting. Lucene 10.x and main (Java 21+) have close(); a 9.x backport (Java 11) needs an explicit shutdown() and awaitTermination() in a finally. Not tried.

Method: semi-formal (the hunt's unit on failed-run cleanup in IndexRearranger and PKIndexSplitter, 2026-10-29, main 645d4fe14a20). The hunting AI tool built a failure-point model of execute() with obligation I4: no task of the call outlives execute(), and the writer and reader are not closed while a task uses them. Reading the modelled resource lifecycle against the reader and writer contracts, it saw that the await loop leaves on the first failure inside the try-with-resources.

• The model's bounded-exhaustive single-fault model-based test (4,147 IndexRearranger runs, each at 1 and at 4 processors; vacuity mutants killed) never fired I4 with its 6-document inputs, so the model did not find it by itself.
• Two deterministic, latch-ordered public-API JUnit tests on the stock jars showed it (a selector still running after execute() threw, and an addIndexes merge dying on a merge thread).
• On 2026-10-29 the repro was cut to one test with no sleeps or timeouts; on 2026-11-01 a current-main re-verification and a tie-break re-ran it and added a natural-timing probe.