A failing keepFullyDeletedSegment check during an NRT open leaks the segment's reader

When IndexWriter opens an NRT reader, StandardDirectoryReader.open(IndexWriter, ...) obtains a read-only SegmentReader for each segment and, for a segment with no live documents, asks the merge policy whether to keep it: reader.numDocs() > 0 || mergePolicy.keepFullyDeletedSegment(() -> reader). Only after that is the reader added to the readers list, and the method's failure handler releases only that list. keepFullyDeletedSegment is public and declared throws IOException, and the stock SoftDeletesRetentionMergePolicy runs the application's retention query against the segment there. If the check throws, the open fails as it should, but the reader that was just obtained is neither listed nor returned, so nothing releases it: the segment's core and its files stay open after IndexWriter.close() and rollback. The cleanest trigger is the first NRT open of a freshly opened writer over a commit that holds a retained, fully soft-deleted segment; on a writer that already pools the segment, a persistent failure is thrown earlier, in IndexWriter.writeReaderPool, and leaks nothing. No wrong results and no index damage. The gap came with LUCENE-8233 (ecc17f90233, 2018) and is in every release from 7.4.0 through 10.6.0 and in main; releasing the reader when the keep decision throws fixes it.

• Read-only clone: the SegmentReader that IndexWriter.getReader hands to the NRT reader, rld.getReadOnlyClone(IOContext.DEFAULT) (IndexWriter.java:554). It holds a reference on the segment's core, which owns the open files; the writer releases only its pool handle (:561).
• Keep check: MergePolicy.keepFullyDeletedSegment(IOSupplier<CodecReader>), public, non-final and declared throws IOException; the default returns false (MergePolicy.java:760-768).

1. IndexWriter.getReader calls writeReaderPool (IndexWriter.java:608), then StandardDirectoryReader.open(this, readerFactory, segmentInfos, ...) (:613-615).
2. The loop in StandardDirectoryReader.open(IndexWriter, ...) (StandardDirectoryReader.java:129-146):

final SegmentReader reader = readerFunction.apply(info);                       // :136 obtained
if (reader.numDocs() > 0
    || writer.getConfig().mergePolicy.keepFullyDeletedSegment(() -> reader)) { // :137-138 may throw
  // Steal the ref:
  readers.add(reader);                                                         // :140 listed

1. The failure handler releases only the list: catch (Throwable t) { ... IOUtils.applyToAll(readers, SegmentReader::decRef); ... throw t; } (:158-165). A throw at :138 skips :140.
2. Nothing else can release the reader. getReader's own catch block closes r, still null because the open never returned, and onGetReaderMergeResources (IndexWriter.java:711-721), which is set only after a successful open (:649-666). getReader also records each clone in a local map, openedReadOnlyClones (:556-558, when maxFullFlushMergeWaitMillis > 0, the default), but reads it only after a successful open (:684-694). close() and rollback() release the pool's readers, not this clone.
3. writer.incRefDeleter(segmentInfos) (StandardDirectoryReader.java:148) has not run yet, so file-deletion bookkeeping stays consistent: the leak is open handles and memory only.
4. SoftDeletesRetentionMergePolicy.keepFullyDeletedSegment calls retentionQuerySupplier.get() (SoftDeletesRetentionMergePolicy.java:86) and runs the query on the segment (:84-91), so a throwing supplier or a read error there is enough.

• Cold writer (the clean case). writeReaderPool first runs the same check, but only for segments already in the reader pool (IndexWriter.java:4128-4140, readerPool.get(info, false) at :4131). On the first NRT open of a new writer the segment is not pooled yet, so StandardDirectoryReader.open runs the only check and any failure leaks.
• Warm writer. A persistent failure is thrown in writeReaderPool, before any reader is obtained, and leaks nothing; only a transient failure that hits the second check leaks, re-pinning the existing core with no new handles.
• Merge-on-refresh. IndexWriter.maybeReopenMergedNRTReader uses the same method and says it "will take care of closing" the readers it is handed (IndexWriter.java:766-771); same window, not executed.

In the failing cases the writer stays open, has no tragic exception, and a later NRT open works (verifiers' tests on a9779ef3c193).

• Who is hit. Applications using soft deletes with SoftDeletesRetentionMergePolicy, or any merge policy whose keepFullyDeletedSegment can throw, that open NRT readers from their writer (DirectoryReader.open(writer), openIfChanged, SearcherManager). The default MergePolicy.keepFullyDeletedSegment returns false with no I/O; in core only SoftDeletesRetentionMergePolicy does real work there (NoMergePolicy and FilterMergePolicy delegate). Downstream, Elasticsearch's engine passes a sequence-number retention query to the stock SoftDeletesRetentionMergePolicy (code read only, not run).
• What it takes. A fully soft-deleted segment that the retention query keeps, and an exception inside the keep check when StandardDirectoryReader.open runs it: an I/O error while the retention query reads that segment, or a throwing retention query supplier. Cleanest on the first NRT open of a freshly opened writer. Exceptions inside this keep check have been reported on another call site (#13353 and elastic/elasticsearch #105122, #105474, #99075; no leak reported there).
• What happens. One SegmentReader per failure stays open (in the repro the compound file and three doc-values update files); IndexWriter.close() and rollback do not release it. No wrong results: the failed open returns no reader, and under a SearcherManager 20 of 20 failed refreshes kept serving the previous reader with the soft-deleted document hidden.
• Scale. 10 cold-writer reopens while the check kept failing left 40 inputs open, and under MMapDirectory 40 mappings still listed in /proc/self/maps at the end of the loop (48 after a later successful open and close); 0 with the fix. Warm-writer repeats re-pin the same core and add no handles. Not tested: Windows (open files cannot be deleted), disk space of deleted-but-open files, heap of a large core.
• Rating. Minor, likelihood rare, priority low: an exceptional event at one specific step, silent outside a leak-checking test framework.

The leak is detected by MockDirectoryWrapper.close(), the standard Lucene open-files check that TestIndexWriterReader#testNRTOpenExceptions also uses. The package is org.apache.lucene.index by convention only; the test uses public API. NoMergePolicy inside the retention policy keeps the segments stable; a verifier's variant with default merge settings (TieredMergePolicy inside) fails the same way. The test framework randomizes the codec; it made no difference in the runs: on main, seeds DEADBEEF and 5EED1234 (codec Asserting(Lucene104)) and 5EED0626 (codec CheapBastard) leave the same 4 files open, and on 10.6.0 a seed that drew the Asserting doc-values format leaves the 4 corresponding files open. Only Linux aarch64 with OpenJDK 25 was run.

As the regression test: the same test, for example as a new method in TestSoftDeletesRetentionMergePolicy or next to testNRTOpenExceptions in TestIndexWriterReader.

cp TestNrtOpenKeepCheckFailureLeaksReader.java $LUCENE/lucene/core/src/test/org/apache/lucene/index/
cd $LUCENE && ./gradlew :lucene:core:test --tests org.apache.lucene.index.TestNrtOpenKeepCheckFailureLeaksReader -Ptests.seed=DEADBEEF -Ptests.asserts=true
# assertions off: -Ptests.asserts=false (same result)

# gradle, main 645d4fe14a20, seed DEADBEEF, -Ptests.asserts=true (standalone runs give the same with -ea and -da)
TestNrtOpenKeepCheckFailureLeaksReader > testFailedKeepCheckOnFirstNrtOpen FAILED
    java.lang.RuntimeException: MockDirectoryWrapper: cannot close: there are still 4 open files: {_0.cfs=1, _0_1_Lucene90_0.dvp=1, _0_1_Lucene90_0.dvs=1, _0_1_Lucene90_0.dvd=1}
        at __randomizedtesting.SeedInfo.seed([DEADBEEF:9CABF7EEC9364E88]:0)
        at org.apache.lucene.tests.store.MockDirectoryWrapper.close(MockDirectoryWrapper.java:884)
        at org.apache.lucene.index.TestNrtOpenKeepCheckFailureLeaksReader.testFailedKeepCheckOnFirstNrtOpen(TestNrtOpenKeepCheckFailureLeaksReader.java:62)

        Caused by:
        java.lang.RuntimeException: unclosed IndexInput: _0.cfs
            at org.apache.lucene.tests.store.MockDirectoryWrapper.addFileHandle(MockDirectoryWrapper.java:782)
            at org.apache.lucene.tests.store.MockDirectoryWrapper.openInput(MockDirectoryWrapper.java:843)
            at org.apache.lucene.codecs.lucene90.Lucene90CompoundReader.<init>(Lucene90CompoundReader.java:87)
            ...
            at org.apache.lucene.index.SegmentCoreReaders.<init>(SegmentCoreReaders.java:83)
            at org.apache.lucene.index.SegmentReader.<init>(SegmentReader.java:95)
            at org.apache.lucene.index.ReadersAndUpdates.getReader(ReadersAndUpdates.java:191)
            at org.apache.lucene.index.ReadersAndUpdates.getReadOnlyClone(ReadersAndUpdates.java:233)
            at org.apache.lucene.index.IndexWriter.lambda$getReader$0(IndexWriter.java:554)
            at org.apache.lucene.index.StandardDirectoryReader.open(StandardDirectoryReader.java:136)
            at org.apache.lucene.index.IndexWriter.getReader(IndexWriter.java:614)
            at org.apache.lucene.index.DirectoryReader.open(DirectoryReader.java:144)
            ...
  2> NOTE: test environment is: codec=Asserting(Lucene104): {}, ...

# 10.6.0 release jars, same test: there are still 4 open files: {_0.cfs=1, _0_1_Asserting_0.dvd=1, _0_1_Asserting_0.dvs=1, _0_1_Asserting_0.dvp=1}
# with the fix: 1 test, 0 failures (assertions on and off)

• The catch block's purpose. StandardDirectoryReader.java:158-165 decRefs every reader the loop took. It goes back to LUCENE-5262 (#6326, 2013) and was rewritten by LUCENE-8215 (2e35ef2b3da, 2018-03-19), two weeks before the keep hook arrived.
• Lucene's own test. TestIndexWriterReader#testNRTOpenExceptions (TestIndexWriterReader.java:1108-1110): "LUCENE-5262: test that several failed attempts to obtain an NRT reader don't leak file handles." It ends with the same MockDirectoryWrapper open-files check as the repro (dir.close(), :1152).
• A declared exception. keepFullyDeletedSegment is public, non-final and declared throws IOException (MergePolicy.java:765-766); SoftDeletesRetentionMergePolicy does query I/O inside it (SoftDeletesRetentionMergePolicy.java:80-94).
• Ownership is handed over. IndexWriter.java:769-771: "as soon as we remove the reader and return it the StandardDirectoryReader#open will take care of closing it."
• The other keep-check site that holds readers releases them in a finally block (IndexWriter.closeSegmentStates, IndexWriter.java:6743-6762).
• An oversight in history. Before LUCENE-8233 (ecc17f90233, 2018-04-04) the condition used writer.getKeepFullyDeletedSegments(), a package-private boolean getter that cannot throw; that commit put the public throwing hook in the same spot. No comment on #9280, no CHANGES or MIGRATE entry, test or issue sanctions the leak, and no upstream test throws from the hook.

The strongest case for "intended" (the intended-behaviour devil's advocate and the cross-model reviewer's intended lens, which rated it moderate): MergePolicy, SoftDeletesRetentionMergePolicy and DirectoryReader.open(IndexWriter) are @lucene.experimental, no javadoc promises cleanup after a merge-policy callback throws, the finder's trigger was a synthetic call-stack-filtered fault, and on a warm writer a persistent failure is caught earlier with no leak.

Why it does not hold up: @lucene.experimental is about API stability, not resource safety; the hook is declared to throw IOException; the release-on-failure catch block and testNRTOpenExceptions show the intent; the minimal repro uses only stock classes, with no subclass and no call-stack filter; and the warm-writer pre-check narrows how often it happens, not whether it is a leak: on a freshly opened writer any failure leaks.

One option was run through gradle in a scratch worktree of 645d4fe14a20 and is recommended; the alternative was not run.

Compute the keep decision inside try; on any Throwable, decRef() the reader that is not in readers yet (adding a secondary failure as suppressed) and rethrow, the same pattern as the method's outer catch block. The original exception still propagates. It covers both callers of the method, getReader and the merge-on-refresh reopen (IndexWriter.java:766); the second was not executed. The snippet is a hunk of the full diff, which applies with git apply at 645d4fe14a20.

         final SegmentReader reader = readerFunction.apply(info);
-        if (reader.numDocs() > 0
-            || writer.getConfig().mergePolicy.keepFullyDeletedSegment(() -> reader)) {
+        final boolean keep;
+        try {
+          keep =
+              reader.numDocs() > 0
+                  || writer.getConfig().mergePolicy.keepFullyDeletedSegment(() -> reader);
+        } catch (Throwable t) {
+          // the reader is not in readers yet, so the catch block below would not release it
+          try {
+            reader.decRef();
+          } catch (Throwable t1) {
+            t.addSuppressed(t1);
+          }
+          throw t;
+        }
+        if (keep) {
           // Steal the ref:
           readers.add(reader);

Trade-offs

• On the success path nothing extra runs beyond entering the try block (not benchmarked); the default merge policy never throws here.
• It does not change when the check runs or what it returns, and touches one method.

What the checks validated

• Gradle in a scratch worktree of 645d4fe14a20 (2026-11-01): stock, the minimal test fails; with the fix it passes with assertions on and off.
• With the fix: the closest suites (TestSoftDeletesRetentionMergePolicy, TestSoftDeletesDirectoryReaderWrapper, TestIndexWriterReader with testNRTOpenExceptions, TestDirectoryReaderReopen, TestIndexWriterMergePolicy and TestIndexWriterExceptions) plus the repro: 7 classes, 135 tests, 0 failures, 7 skipped (5 @Nightly, 2 @AwaitsFix), seed DEADBEEF, assertions on. The rest of the core module was not run.
• Format and lint: checkGoogleJavaFormat, ecjLint and forbiddenApis for :lucene:core pass with the fix and the test in place.
• The same change shadowed into the stock main jars, run standalone: the minimal test passes with -ea and -da. Earlier, the finder's identical change made every verifier's repro and the security probe pass (every leak count 0), and a cross-model reviewer ran TestSoftDeletesRetentionMergePolicy with it (14 tests, 0 failures).
• Not run: the full Lucene test suite.

Add the reader to readers right after readerFunction.apply(info), so the existing catch block owns it, and when the segment is not kept, remove it from the list again before the decRef() and segmentInfos.remove(infosUpto).

Trade-offs

No new try, but it restructures the loop and needs care that a reader removed from the list is decRef'd exactly once.

Method: non-formal baseline (randomized stress with injected faults, then executed tests). An earlier hunt unit's stress harness saw the open-files failure in 6 rounds and recorded the root cause by code reading as a lead. A later unit completed that harness: it saw MockDirectoryWrapper report open files after IndexWriter.close() in 40 of 54,400 unfiltered rounds, all with the injected fault fired at StandardDirectoryReader.open:138, and none with this signature in 48,000 rounds with the fix shadowed in. It then reduced the case to a deterministic single-thread JUnit. Verifiers replaced the finder's call-stack-filtered fault with a stock-classes trigger, which became the minimal repro.