MultiPassIndexSplitter leaves the output write-locked when a split fails inside addIndexes

MultiPassIndexSplitter.split(IndexReader, Directory[], boolean), the misc module's index splitter, opens an IndexWriter on each output directory, copies that part's documents in with addIndexes and calls close() only when addIndexes returns normally. If addIndexes throws, for example because the source index was created by the previous major version, a source file is corrupted or the disk fills up, the writer stays open. No caller can reach it, so the output's write lock is never released: every later IndexWriter on that directory, including a retried split, fails with LockObtainFailedException, and with the default FSDirectory lock factory this lasts until the JVM exits, even after the Directory is closed and garbage is collected. The code dates from the class's first commit (LUCENE-1959, 2009), so every release from 3.0.0 to 10.6.0 that was run or read, and main, have it; the sibling PKIndexSplitter has closed its writer on failure since 2011. The command-line main() is unaffected because its JVM exits on the exception, and nothing is corrupted. A few lines fix it: roll the writer back when addIndexes throws, or wrap it in try-with-resources.

All lines are at 645d4fe14a20; MultiPassIndexSplitter.java unless named.

1. For each part, split() opens new IndexWriter(outputs[i], new IndexWriterConfig(null).setOpenMode(OpenMode.CREATE)) (:108-109), prints Writing part i ... (:110), calls w.addIndexes(...) (:113) and then w.close() (:114). No try/finally, no rollback(), no try-with-resources; w never leaves the method.
2. Opening the writer takes the lock: "Opening an IndexWriter creates a lock file for the directory in use. Trying to open another IndexWriter on the same directory will lead to a LockObtainFailedException" (IndexWriter.java:137-139); close() "Closes all open resources and releases the write lock" (:1342).
3. addIndexes(CodecReader...) catches only Error and makes it a tragic event (IndexWriter.java:3390-3391); an IOException, CorruptIndexException or IllegalArgumentException propagates with the writer still open. Its javadoc lists CorruptIndexException and IOException (:3270-3271) and points to addIndexes(Directory...), which names "disk full" as an expected exception (:3084-3086).
4. The deterministic trigger: split() always builds its outputs with new IndexWriterConfig(null) (:109), the current major version, and validateMergeReader (IndexWriter.java:3228-3236, called first at :3290) rejects a source leaf with another created-version major. No merge thread starts.
5. With FSDirectory, NativeFSLockFactory keeps held lock paths in private static final Set<String> LOCK_HELD (NativeFSLockFactory.java:68) and throws "Lock held by this virtual machine" for a path already in it (:119); only NativeFSLock.close() removes the path (:145, :212). The leaked writer's lock is never closed, so the path stays refused until the JVM exits; garbage collection can drop the file handle but not the record. With the handle gone, another JVM could then lock the path (code reading, not run); the refusal is inside the JVM that ran the split.
6. main() calls split() (:182) and declares throws Exception, so for the command-line tool the exception ends the JVM and the OS releases the lock. A program that calls main() and survives its exception is hit like any other caller (code reading).

• Corrupted source: one flipped byte in the source's .fdt file makes split() throw CorruptIndexException: checksum failed from the merge; the output, which never had an I/O problem, then refuses a new writer with "Lock held by this virtual machine".
• Full disk (simulated): an output whose createOutput throws IOException leaves it locked the same way, and a retried split into the same outputs fails with the same message after the fault is removed.
• With both, the merge runs on a ConcurrentMergeScheduler thread, which also prints an uncaught MergeException to stderr. That is normal behaviour and not part of this bug; split() itself throws the original exception.

Previous-major note: that setIndexCreatedVersionMajor(LATEST.major - 1) behaves like an index really written by the previous release is code reading (both carry created-version major N-1, the only value this check reads; IndexWriterConfig.java:173-180 documents the setter for exactly this addIndexes restriction). That split() cannot split a previous-major index at all is a separate limitation and not part of this bug.

• Who is hit. Java callers of MultiPassIndexSplitter.split(...) in a JVM that keeps running after a failed split: a service, a maintenance tool, a test suite. Not the command-line main().
• Default vs opt-in. The class is in the optional lucene-misc module; inside it nothing else is needed. The default FSDirectory and NativeFSLockFactory give the JVM-lifetime lock; assertions make no difference.
• Effect. One leaked open IndexWriter per failed split; that output path refuses new writers until the JVM restarts, other paths keep working (a split into fresh directories succeeds in the same JVM).
• No damage. No hang (the slowest failed split threw in 168 ms), no wrong results, no corruption: the failed output gets no commit, an index already there keeps its previous commit, the source index is unchanged. This was run for the previous-major trigger; for the merge-time triggers it follows from the code: nobody can commit or close the leaked writer.
• Scale. None needed: 2 documents reproduce it. In the hunt's fault-injection sweep, a one-shot IOException at a single file-system event left the output locked in 18,654 of 18,866 split runs.

Two documents, because split() refuses fewer than 2. The previous-major input is the one trigger that fails before any merge thread starts; the full-disk and corrupted-input triggers fail inside the hardcoded ConcurrentMergeScheduler, whose merge thread reports an uncaught MergeException that a test would have to handle. With FSDirectory instead of ByteBuffersDirectory the message is "Lock held by this virtual machine: .../write.lock", but the test framework then adds a second, suite-level failure for the leaked write.lock file handle (the same leak), so the minimal test keeps ByteBuffersDirectory.

As the regression test: the same test, for example as a new method in TestMultiPassIndexSplitter; it fails on stock and passes with either fix.

# gradle, main 645d4fe14a20, seed DEADBEEF, assertions on (trimmed; the same with -Ptests.asserts=false)
> Task :lucene:misc:test FAILED

TestMultiPassIndexSplitterFailedSplit > testFailedSplitReleasesOutputLock FAILED
    java.lang.AssertionError: expected the failed split to release out1's write lock, but: lock instance already obtained: (dir=ByteBuffersDirectory@678bbc81 lockFactory=org.apache.lucene.store.SingleInstanceLockFactory@29488613, lockName=write.lock)
        at __randomizedtesting.SeedInfo.seed([DEADBEEF:C546DB47F853A75B]:0)
        at junit@4.13.2/org.junit.Assert.fail(Assert.java:89)
        at org.apache.lucene.misc.index.TestMultiPassIndexSplitterFailedSplit.testFailedSplitReleasesOutputLock(TestMultiPassIndexSplitterFailedSplit.java:50)

  2> Writing part 1 ...
1 test completed, 1 failed

# the exception split() threw (fix-comparison probe, same input, main 645d4fe14a20)
IllegalArgumentException: Cannot merge a segment that has been created with major version 10 into this index which has been created by major version 11

# the same test with FSDirectory instead of ByteBuffersDirectory (default NativeFSLockFactory)
java.lang.AssertionError: expected the failed split to release out1's write lock, but: Lock held by this virtual machine: $DIR/write.lock
java.lang.RuntimeException: file handle leaks: [FileChannel($DIR/write.lock)]   (the test framework's suite-level report of the same leak)

• Only split() can release the writer. It is a local variable (MultiPassIndexSplitter.java:108-109); the IndexWriter javadoc ties the lock to an open writer (IndexWriter.java:137-139), and addIndexes keeps the writer open after a non-Error exception (:3390-3391), so closing it is the opener's job.
• A public library API that expects failures. split() documents @throws IOException If there is a low-level I/O error (:68); the upstream TestMultiPassIndexSplitter calls split() directly (testSplitRR, testSplitSeq), not main().
• The sibling does it on purpose. PKIndexSplitter was added in LUCENE-2919 (bba6b01826d, 2011-06-17) and got failure-path closing the next day (40f5948b3df, Uwe Schindler, IOUtils.closeSafely(!success, w)); today try (IndexWriter w = new IndexWriter(target, config)) wraps the same addIndexes call (PKIndexSplitter.java:117-130, since #14913). IndexRearranger in the same package also uses try-with-resources.
• Nobody chose it. The LUCENE-1959 thread (#3034, 30 comments) never discusses exceptions, cleanup or the lock; none of the 16 later commits that touched the block added or rejected cleanup; a different leak in this class (the MultiReader) was fixed as a bug in 2012 (1f192290c5a).

The strongest case for "works as designed" (the devil's-advocate verifier and a reviewer from another model family, OpenAI GPT-5.6): the class is a command-line tool (@SuppressForbidden(reason = "System.out required: command line tool"), and main() never closes its readers either), so it relies on JVM exit for cleanup; split() promises only IOException and nothing about the outputs after a failure; a half-written output must be thrown away anyway; and the first trigger was synthetic fault injection.

Why it fails: split() is a separate public method with full javadoc that the upstream test calls directly; the caller cannot release the writer at all; the lock blocks the path for the JVM's whole life, even for an output that never had an I/O problem (the corrupted-input case); the previous-major trigger needs no fault injection at all, and the corrupted-source one no faulty directory (the test flips one byte of a source file); and the sibling class has handled the same step exception-safely since 2011. Every verifier rated this argument weak; the cross-model fact check of this doc rated it moderate and still called the behaviour a minor bug. It bounds the impact (minor), not the validity.

Both options were run through gradle in a scratch worktree of 645d4fe14a20, and both fix the repro and pass the closest suite. They differ in what a failed part leaves in its output directory: a comparison probe split a previous-major input into an output that already held a 3-document index. Stock and B leave the 3 documents; A replaces them with an empty index. B is recommended because it keeps that outcome; A is the smaller change and matches the sibling class.

In split() (MultiPassIndexSplitter.java:108-114): wrap the addIndexes step in try/catch (Throwable t), roll the writer back there and rethrow; keep w.close() after it for the success path. +11/-4, including one import (org.apache.lucene.util.IOUtils).

       IndexWriter w =
           new IndexWriter(outputs[i], new IndexWriterConfig(null).setOpenMode(OpenMode.CREATE));
-      System.err.println("Writing part " + (i + 1) + " ...");
-      ...
-      w.addIndexes(sr.toArray(CodecReader[]::new)); // TODO: maybe take List<IR> here?
+      try {
+        System.err.println("Writing part " + (i + 1) + " ...");
+        ...
+        w.addIndexes(sr.toArray(CodecReader[]::new)); // TODO: maybe take List<IR> here?
+      } catch (Throwable t) {
+        // release the output's write lock without committing anything into it
+        IOUtils.closeWhileSuppressingExceptions(t, w::rollback);
+        throw t;
+      }
       w.close();

Trade-offs

• Keeps today's outcome for the output of a failed part: nothing is committed (rollback() returns the directory to its last commit, IndexWriter.java:2566-2571), so an index already there keeps its documents (probe: 3 before, 3 after), and an empty output stays without an index (probe: no files), which marks the part as failed.
• An exception from rollback() is added to the original one as suppressed (an Error from rollback() is thrown instead, with the original suppressed into it, IOUtils.java:114-136); after an Error the tragic-event handling has already rolled the writer back (IndexWriter.java:6090-6106), and the second rollback() does nothing (:2576-2584).
• The same helper as NativeFSLockFactory.tryLock (NativeFSLockFactory.java:139-141); slightly longer than A.

What the checks validated

• Stock 645d4fe14a20: the repro fails with assertions on and off.
• With B: the repro passes with assertions on and off; TestMultiPassIndexSplitter passes together with it (2 classes, 3 tests, 0 failures); the comparison probe keeps the 3 documents; the whole :lucene:misc:test with the repro in place passes (24 classes, 317 tests, 0 failures, 9 skipped, all in other classes).
• The independent reproducer's public-API program on 645d4fe14a20 (plain FSDirectory; a simulated full disk, then a corrupted .fdt), with B: after each failed split a new writer opens, and after the simulated full disk the retried split succeeds, assertions on and off; stock fails.
• The module's checkGoogleJavaFormat, ecjLintMain, ecjLintTest, forbiddenApisMain and forbiddenApisTest pass with B and the repro in place once the repro's long comment is wrapped onto two lines, which the formatter asks for (comment only; the fixed class was not flagged).

Wrap the per-part writer in try (IndexWriter w = new IndexWriter(...)) { ... } and drop the explicit w.close() (MultiPassIndexSplitter.java:108-114), +7/-7. This is the hunt's fix.

-      IndexWriter w =
-          new IndexWriter(outputs[i], new IndexWriterConfig(null).setOpenMode(OpenMode.CREATE));
-      ...
-      w.addIndexes(sr.toArray(CodecReader[]::new)); // TODO: maybe take List<IR> here?
-      w.close();
+      try (IndexWriter w =
+          new IndexWriter(outputs[i], new IndexWriterConfig(null).setOpenMode(OpenMode.CREATE))) {
+        ...
+        w.addIndexes(sr.toArray(CodecReader[]::new)); // TODO: maybe take List<IR> here?
+      }

Trade-offs

• The smallest change and the sibling's idiom (PKIndexSplitter.java:117).
• On failure close() commits: commitOnClose defaults to true (IndexWriterConfig.java:102), close() commits when it is set (IndexWriter.java:1363-1368), and a CREATE-mode writer records a pending change when it opens (IndexWriter.java:1017-1019). So a failed part leaves a valid, empty index in its output (probe: segments_1) and replaces any index that was there (probe: 3 documents before, 0 after).
• If that commit fails too (for example the disk is still full), close() still closes the writer and releases the lock (IndexWriter.java:1351-1352); its exception is suppressed into the original one.

What the checks validated

• With A: the repro passes with assertions on and off; TestMultiPassIndexSplitter passes together with it (2 classes, 3 tests, 0 failures); the comparison probe shows the 3 documents replaced by an empty index.
• Earlier checks of the same edit on main ea57360912ed: the finder's, the reproducer's, the cross-model adversary's and the devil's-advocate verifier's tests all pass with it, and upstream TestMultiPassIndexSplitter passes 2 of 2.

Method: exception-edge resource scan (the study's semi-formal tier). A hunting AI tool wrote ResourceScan, a bytecode abstract interpreter that tracks acquired resources (directory locks, writers, readers, inputs) along every exception edge, with interprocedural summaries, and ran it over the stock main jars (core, codecs, facet, suggest, misc, replicator: 3,388 classes, 24,094 methods). Vacuity check: all 7 planted leaks were flagged and none of 8 clean idioms.

• It reported the IndexWriter acquired at MultiPassIndexSplitter.java:109 as live on the exception edge of IndexWriter.addIndexes (:113); the AI tool triaged the group by reading the source.
• A dynamic single-fault sweep then re-ran a split once per file-system event with a one-shot IOException at that event (seed 1531531531): the output lock leaked in 18,654 of 18,866 MultiPassIndexSplitter.split runs and in 0 of 19,020 PKIndexSplitter.split runs.
• A JUnit with an injected full disk confirmed it on the stock jars; an independent verifier added the corrupted-input trigger and the devil's-advocate verifier the previous-major trigger, which became the minimal repro.