Hunspell: a Dictionary load that fails while reading the .dic leaves its offline-sort temp output open (regression in 9.9.0)

The Dictionary(Directory tempDir, ...) constructors and HunspellStemFilterFactory sort the .dic entries on disk: Dictionary's constructor calls sortingStrategy.start(), which opens a temp IndexOutput in tempDir, and then mergeDictionaries(...), which reads the .dic and writes every entry to it, with no failure handling. Only finishAndSort() closes the output. So when reading the .dic throws (an IOException the constructor documents), or a write to tempDir fails, the constructor throws and the output stays open; the caller has no object to close, and FSDirectory.close() does not close it. Each failed load keeps one file descriptor open until garbage collection and, in a caller-supplied temp directory, leaves the file on disk; through the factory, which deletes its private temp directory, the file is unlinked but its descriptor stays open. Before #12834 (9.8.0) the entries were written inside a try-with-resources and the output was closed on failure; every release from 9.9.0 to 10.6.0 and main has the leak. The load itself fails loudly, and there are no wrong results. A catch (Throwable t) { ...; throw t; } cleanup at each step of the offline load fixes it.

• Offline sorting (SortingStrategy.offline(tempDir, prefix)): what the Dictionary(Directory, ...) constructors (Dictionary.java:213) and HunspellStemFilterFactory use. Its start() opens the temp IndexOutput; finishAndSort() writes the footer, closes it, sorts, and returns a supplier that reads the sorted file and deletes it on close().
• Temp output: the IndexOutput from tempDir.createTempOutput(prefix, "dat", ...), a file such as dict_dat_0.tmp with an open file handle.

1. The constructor runs EntryAccumulator acc = sortingStrategy.start(); (Dictionary.java:267), which opens the temp output and its writer (SortingStrategy.java:71-72).
2. Then mergeDictionaries(dictionaries, decoder, acc); (:268) reads the .dic (:1016, :1019) and writes each entry (:1034, SortingStrategy.java:77-80). It is outside any try: only the supplier returned by finishAndSort() is in a try-with-resources (:269).
3. If it throws (a .dic read error, a write error in tempDir, or an undefined AM alias, LFV-1012), finishAndSort() never runs, and it is the only code that closes the output (SortingStrategy.java:83-88).
4. The constructor throws, so the caller has no Dictionary and no handle; FSDirectory.close() only marks the directory closed and retries pending deletes; it closes no open outputs (FSDirectory.java:277-280). The descriptor stays until the output's channel is garbage-collected; the file stays in tempDir.
5. HunspellStemFilterFactory.inform loads into a fresh directory under java.io.tmpdir and calls IOUtils.rm(tempPath) in finally (HunspellStemFilterFactory.java:109-113): the open file is unlinked, and its descriptor stays open until GC.
6. The same gap exists at later steps (fault injection, below): a failing footer write or writer close, a failing delete of the unsorted input after the sort, a failing open of the sorted file, and a failing reader close in the supplier's close(). Failures inside the sort itself and while reading the sorted entries are cleaned up.

Minimal test (empty .aff, .dic stream throws on its first read, MockDirectoryWrapper): cannot close: there are still 1 open files: {dict_dat_0.tmp=1}, caused by unclosed IndexOutput: dict_dat_0.tmp from SortingStrategy$1.start(SortingStrategy.java:71) and Dictionary.<init>(Dictionary.java:267). Plain FSDirectory, 5 failed loads: 5 files and 5 open descriptors after Directory.close(); descriptors 0 after System.gc(); files still there after a later successful load. Factory, 10 failed inform() calls: 10 descriptors on .../HunspellNNN/hunspell_dat_0.tmp (deleted). Fault injection (a fault-free baseline and 227 injected failures, 225 of them I/O errors): 80 of the 228 checks leak on main, 0 with the recommended fix.

• Who is hit. Applications that load hunspell dictionaries through Dictionary(Directory, String, InputStream, InputStream), Dictionary(Directory, String, InputStream, List<InputStream>, boolean) or HunspellStemFilterFactory (SPI name hunspellStem, for example in CustomAnalyzer). Users of the SortingStrategy.inMemory() constructor are not affected.
• What it takes. A load that fails after the temp file was opened: an I/O error while reading the .dic stream, a full or failing temp disk, or a .dic entry with an undefined AM morphological alias (LFV-1012). These are exceptional events, so it is rated rare. Failures while parsing the .aff happen before the file is opened and leak nothing.
• What happens. The load fails loudly with its original exception; the leak is silent. One file descriptor per failed load until GC. With a caller-supplied persistent temp directory, one small temp file per failure stays on disk (not removed by Directory.close() or a later successful load). Through the factory the file is unlinked at once and only the descriptor remains. An application that retries a failing load accumulates one descriptor per attempt until a GC runs.
• Not claimed. Windows was not tested (an open file cannot be deleted there, so the factory's IOUtils.rm may behave differently).

random() only sets MockDirectoryWrapper's internal knobs (random I/O failures are off by default); the test failed with every seed tried (DEADBEEF, 5EED1234, and 20 of 20 iterations from 5EED1006). It pins the 9.9.0 regression, the open output (its directory is in memory, so it shows an unclosed handle, not an OS descriptor); it does not assert that the closed file is gone, because that part is older and would make 9.8.0 fail too.

As the regression test: the same test, for example as a new method in TestDictionary. Note that at this commit gradle derives the default of tests.asserts from the seed, and DEADBEEF picks assertions off; the commands below set it explicitly.

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

# gradle, main 645d4fe14a20, seed DEADBEEF, -Ptests.asserts=true, verbatim (the same with -Ptests.asserts=false)
TestDictionaryFailedLoadClosesTempOutput > testDictionaryReadErrorClosesTempOutput FAILED
    java.lang.RuntimeException: MockDirectoryWrapper: cannot close: there are still 1 open files: {dict_dat_0.tmp=1}
        at __randomizedtesting.SeedInfo.seed([DEADBEEF:2E5D5E3A4F318B44]:0)
        at org.apache.lucene.test_framework@11.0.0-SNAPSHOT/org.apache.lucene.tests.store.MockDirectoryWrapper.close(MockDirectoryWrapper.java:884)
        at org.apache.lucene.analysis.hunspell.TestDictionaryFailedLoadClosesTempOutput.testDictionaryReadErrorClosesTempOutput(TestDictionaryFailedLoadClosesTempOutput.java:40)

        Caused by:
        java.lang.RuntimeException: unclosed IndexOutput: dict_dat_0.tmp
            at org.apache.lucene.test_framework@11.0.0-SNAPSHOT/org.apache.lucene.tests.store.MockDirectoryWrapper.addFileHandle(MockDirectoryWrapper.java:782)
            at org.apache.lucene.test_framework@11.0.0-SNAPSHOT/org.apache.lucene.tests.store.MockDirectoryWrapper.createTempOutput(MockDirectoryWrapper.java:761)
            at org.apache.lucene.analysis.hunspell.SortingStrategy$1.start(SortingStrategy.java:71)
            at org.apache.lucene.analysis.hunspell.Dictionary.<init>(Dictionary.java:267)
            at org.apache.lucene.analysis.hunspell.Dictionary.<init>(Dictionary.java:213)
            at org.apache.lucene.analysis.hunspell.Dictionary.<init>(Dictionary.java:192)
            at org.apache.lucene.analysis.hunspell.TestDictionaryFailedLoadClosesTempOutput.lambda$testDictionaryReadErrorClosesTempOutput$0(TestDictionaryFailedLoadClosesTempOutput.java:38)
            at org.apache.lucene.test_framework@11.0.0-SNAPSHOT/org.apache.lucene.tests.util.LuceneTestCaseParent._expectThrows(LuceneTestCaseParent.java:1587)
            at org.apache.lucene.test_framework@11.0.0-SNAPSHOT/org.apache.lucene.tests.util.LuceneTestCaseParent.expectThrows(LuceneTestCaseParent.java:1427)
            at org.apache.lucene.test_framework@11.0.0-SNAPSHOT/org.apache.lucene.tests.util.LuceneTestCaseParent.expectThrows(LuceneTestCaseParent.java:1418)
            at org.apache.lucene.analysis.hunspell.TestDictionaryFailedLoadClosesTempOutput.testDictionaryReadErrorClosesTempOutput(TestDictionaryFailedLoadClosesTempOutput.java:38)

• It worked before. 9.8.0 wrote the entries inside try (ByteSequencesWriter writer = new ByteSequencesWriter(output)) (9.8.0 Dictionary.java:992). #12834 moved the writer into SortingStrategy.offline().start() and its close into finishAndSort(); its review threads (Dawid Weiss and Peter Gromov, 2023-11-22) discuss only the sorted file's cleanup. On 9.9.0 jars with only the parent commit's Dictionary.java swapped in, the open output and the factory descriptor leak disappear.
• The neighbouring steps release on failure. sortWordsOffline deletes its input when the sort throws (SortingStrategy.java:116-121, the catch-and-rethrow of #14633); the sorted file is closed and deleted by the try-with-resources at Dictionary.java:269; OfflineSorter wraps its directory "So we can remove any partially written temp files on exception" (OfflineSorter.java:295).
• Maintainer intent. In LUCENE-6829 (#7887, 2015-10-13) Michael McCandless wanted the factory "to recursively remove the temp dir, and any files under it, because the temp dir is likely not empty on exception", and Dawid Weiss answered "we always attempt to remove the temp. stuff ... Otherwise you'd have temp files piling up without any visible reason?". That cleanup is still in the factory; since 9.9.0 the still-open output defeats it.
• Contract. @throws IOException Can be thrown while reading from the InputStreams (Dictionary.java:186, :203, :223): a read error is an expected outcome, not misuse.
• Test oracle. MockDirectoryWrapper.close() refuses to close a directory with open files. No hunspell test makes the .dic phase fail (TestDictionary.testInvalidData fails in the .aff, before the temp file is opened).

The strongest case for "intended" (an intended-behaviour checker, and read-only reviewers from another model family, OpenAI GPT-5.6): no javadoc promises cleanup after a failed load; the caller owns tempDir and can delete it, as the factory does; HunspellStemFilterFactory is @lucene.experimental; and the load fails loudly while GC reclaims the descriptor.

Why it fails: it speaks to severity, not intent. A caller can delete files but cannot close an output it never received, and FSDirectory.close() does not close it; the same code closed it before 9.9.0; every neighbouring step releases on failure; and the 2015 maintainers explicitly did not want temp files piling up. Hence a minor leak on a failure path, a bug of low priority.

Two options were run through gradle in a scratch worktree of 645d4fe14a20 and through the fault-injection harness; the full one is recommended.

Dictionary.java:267-268 wraps mergeDictionaries and calls a new EntryAccumulator.abort(Throwable) (a no-op default; the offline accumulator closes the writer and deletes the temp file, adding its own exceptions to t as suppressed; as in IOUtils, an Error thrown while releasing propagates instead, with t attached). In SortingStrategy.offline(): finishAndSort aborts when the footer write or the writer close fails, deletes the sorted file when deleting the unsorted input or opening the sorted file fails, and the supplier's close() becomes IOUtils.close(reader, () -> tempDir.deleteFile(sortedFile)), the form Dawid Weiss suggested in the #12834 review. +39/-8 lines in two files. The constructor hunk:

       EntryAccumulator acc = sortingStrategy.start();
-      mergeDictionaries(dictionaries, decoder, acc);
+      try {
+        mergeDictionaries(dictionaries, decoder, acc);
+      } catch (Throwable t) {
+        acc.abort(t);
+        throw t;
+      }
       try (EntrySupplier sorted = acc.finishAndSort()) {

Trade-offs

• Same style as the existing sortWordsOffline cleanup (#14633) and the later success-flag removals (#15134); no public API change (EntryAccumulator is package-private).
• Cleanup exceptions are added to the original exception as suppressed, so the caller still sees the real cause; only an Error during cleanup is thrown in its place, with the original attached (the convention of IOUtils.closeWhileSuppressingExceptions and deleteFilesSuppressingExceptions, IOUtils.java:114-136, :247-267).
• Nothing changes on the success path or for SortingStrategy.inMemory().

What the checks validated

• Gradle, scratch worktree of 645d4fe14a20, seed DEADBEEF: without the fix the repro fails with -Ptests.asserts=true and false; with it, it passes with both (assertions on: in the package run below). Standalone runs agree with -ea and -da.
• Closest suites: every test class of the hunspell package (TestDictionary, TestHunspellStemFilterFactory, TestSpellChecking, TestStemmer, ...), assertions on: 47 classes with the repro, 159 tests, 0 failures, 13 skipped (classes that need external dictionaries or corpora, or are @Ignored); the same with the seed's default (assertions off).
• ./gradlew :lucene:analysis:common:check -x test with the fix and the test in place passes (google-java-format, ecj lint, forbidden APIs, checksum checks). The full test suite was not run.
• Fault injection on the stock jars with the fix's two classes swapped in: 228 checks (a fault-free baseline and 227 injected failures), 0 leak (stock: 80).

The constructor hunk and abort(Throwable) from A, without the changes to finishAndSort, sortWordsOffline and the supplier's close().

Trade-offs

Smallest change and enough for the minimal test's case, a failure while the .dic is read (all 60 reading-phase leaks of the fault run go away), but 20 of the 228 checks still leak at the later steps (footer write, writer close, input delete, sorted-file open, reader close); a full disk that the 8 KiB output buffer reports only at the footer write or the writer close is one of them.

What the checks validated

The repro passes in gradle (the seed's default, assertions off, alone and in the hunspell package: 159 tests, 0 failures) and standalone with -ea and -da. Fault injection: 20 of 228 checks leak.

The shape of the original fix sketch.

Trade-offs

Equivalent behaviour, but upstream has been replacing this pattern with catch-and-rethrow (#14633, #15134); a cleanup failure is swallowed instead of being attached to the original exception. Validated earlier by the hunt (the 4-test JUnit 4/4, 76 upstream hunspell tests, 0 of 228 checks leak).

Method: non-formal first signal, then formal characterisation. On 2026-10-28 an exception-safety probe (HunspellTempLeakProbe) made the .dic stream fail at byte 2, 8 or 14 and the temp directory fail after 0, 5 or 20 bytes; every case left hunspell_dat_0.tmp open and in the temp directory (13 of 20 checks failed, also on the 10.6.0 release candidate). The hunt unit for analysis loaders then wrote a TLA+ model of the offline load with an exception allowed at every throwing step (TLC: 168 distinct states, 56 traces violating the release invariant on 6 edges; the fixed variant satisfies it; 5 vacuity probes) and replayed it on the real jar with model-based bounded-exhaustive fault injection: one IOException at each of 142 directory and file operations and at every byte of the .dic (38) and .aff (45) streams, plus two content-driven exceptions and a fault-free baseline: 228 checks, 80 leaking, 0 mismatches with the model. A public-API JUnit test followed; independent checkers reproduced it with their own tests, and it was cut to the one-test repro.