A failed SpellChecker.indexDictionary leaves the spell index write-locked until the JVM restarts

SpellChecker.indexDictionary (lucene-suggest) opens an IndexWriter on the spell index and closes it only when the build succeeds. When the build throws, from an I/O error while the dictionary is read, a word list that is not valid UTF-8, or a line over 32766 UTF-8 bytes, the writer is neither closed nor rolled back. It keeps the index's write.lock, the caller cannot reach it, and SpellChecker.close() does not close it. With the default FSDirectory, every later indexDictionary, clearIndex() or new IndexWriter on that path in the same JVM throws LockObtainFailedException: Lock held by this virtual machine until the JVM restarts. Suggestions keep coming from the last good build; nothing is corrupted. Every release from 2.4.0 (2008) to 10.6.0 and main is affected; rolling the writer back on failure fixes it.

// SpellChecker.indexDictionary, condensed
final IndexWriter writer = new IndexWriter(dir, config);       // :496  takes write.lock
IndexSearcher indexSearcher = obtainSearcher();                 // :497
...                                                             // :498-510
try {                                                           // :512
  BytesRefIterator iter = dict.getEntryIterator();              // :513  may throw IOException
  while ((currentTerm = iter.next()) != null) {                 // :517  may throw IOException
    ...
    writer.addDocument(doc);                                    // :535  may throw IllegalArgumentException
  }
} finally {
  releaseSearcher(indexSearcher);                               // :537-539  the only cleanup
}
if (fullMerge) writer.forceMerge(1);                            // :540-542
writer.close();                                                 // :544  success path only

1. Line 496 opens the writer. With FSDirectory, NativeFSLockFactory puts the lock path into its static LOCK_HELD set (NativeFSLockFactory.java:68, :108) and holds an OS file lock.
2. An exception from the dictionary at line 513 or 517, or the immense-term IllegalArgumentException at line 535, runs the finally, which releases the searcher only, and leaves the method: writer.close() at line 544 is skipped, and nothing rolls the writer back. IndexWriter does not treat these exceptions as tragic, so it does not roll itself back either. The writer is a local variable, so it becomes unreachable with the lock still held.
3. SpellChecker.close() (SpellChecker.java:639-647) closes only the searcher's reader; there is no API to reach the writer.
4. The next new IndexWriter on that path (line 496 in a retry, line 456 in clearIndex(), or the application's own) finds the path in LOCK_HELD and throws LockObtainFailedException("Lock held by this virtual machine: ...") (NativeFSLockFactory.java:119). Once the lock is held, only closing it removes the path (NativeFSLock.close(), :212 -> :145-146), and nothing closes it; GC does not remove the path either.

• A word list that is not valid UTF-8. PlainTextDictionary(Path) decodes strictly as UTF-8 ("NOTE: content is treated as UTF-8", PlainTextDictionary.java:47-50): alpha, bravo, café, delta saved as ISO-8859-1 throws CharacterCodingException at café.
• An I/O error while the dictionary is read. Dictionary.getEntryIterator() and BytesRefIterator.next() both declare IOException; the minimal repro uses this one.
• A line over IndexWriter.MAX_TERM_LENGTH = 32766 UTF-8 bytes (IndexWriter.java:258). addDocument throws IllegalArgumentException "Document contains at least one immense term", as documented at IndexWriter.java:1480-1481 (from 4.8.0 on).

After any of them, a retry with good input, clearIndex(), and a plain new IndexWriter on a freshly opened FSDirectory after SpellChecker.close(), Directory.close() and five System.gc() calls all throw LockObtainFailedException (executed on main with assertions on and off, 10.5.2 and 9.12.3).

Not every failure leaks. When the writer itself hits an unrecoverable error, IndexWriter treats it as tragic and rolls itself back, so the retry works (executed in the fact check): an I/O error on the spell index while addDocument writes a segment (DocumentsWriterPerThread.java:585-603, IndexWriter.java:1629-1641, 6090-6104; the immediate retry succeeds), and a failed merge inside forceMerge (IndexWriter.java:5108-5113; a retry 3 s later succeeds, an immediate one can still race the merge thread's rollback). The leak needs an exception the writer does not treat as tragic, as in the three triggers above.

• Who. Users of the classic n-gram SpellChecker of lucene-suggest (not DirectSpellChecker, which has no spell index) that rebuild or clear the spell index without restarting the JVM. Defaults suffice: new SpellChecker(dir), new IndexWriterConfig(), FSDirectory.open with its default NativeFSLockFactory.
• When. One build fails once: an I/O error while the dictionary is read, a word list that is not valid UTF-8, a line over 32766 UTF-8 bytes, or a custom Dictionary that throws. Index-based dictionaries (LuceneDictionary, HighFrequencyDictionary) over text the index accepted cannot produce the over-long word (executed with 32766-byte terms).
• What goes wrong. After the input is fixed, the spell index cannot be rebuilt or cleared again in that JVM: every write throws LockObtainFailedException until restart. If the application called clearIndex() just before the failed build, that spellchecker returns no suggestions until the restart (by reading).
• What it does not do (executed on main with assertions on and off, 10.5.2 and 9.12.3): no corruption and no data loss; the last good build keeps being served; none of the 20 words the failed build added is visible, even to a fresh DirectoryReader (numDocs=3, the last commit); another spell index in the same JVM works; the source index is never written; the leak is bounded in the probe (5 to 10 file descriptors after the failed build, 0 more over 200 retries; the heap the leaked writer keeps was not measured).
• How likely. Rare: it needs a failed build followed by an in-process rebuild, and no diagnosed field report exists since 2.4.0 (2008). One undiagnosed 2012 solr-user mail shows the same symptom from clearIndex (see Prior reports).

One test class, one method, single thread, nothing random that matters. The failing Dictionary lands in the same try (SpellChecker.java:512-539) as a failing word-list reader. Each call gets a new IndexWriterConfig, as IndexWriter requires.

It uses ByteBuffersDirectory, whose SingleInstanceLockFactory reports "lock instance already obtained"; with an on-disk FSDirectory the test framework would also report the leaked write.lock handle as a second, suite-level failure. With FSDirectory.open(path) the retry's message is "Lock held by this virtual machine: <path>/write.lock", and the lock survives SpellChecker.close(), Directory.close(), System.gc() and a reopened FSDirectory (executed by the independent checks; not a second repro here).

This same test is the suggested regression test: it fails on stock main and passes with the fix.

# gradle, scratch worktree of 645d4fe14a20, seed DEADBEEF, -Ptests.asserts=true (module prefixes and environment notes cut)
TestSpellCheckerFailedIndexDictionary > testRetryAfterFailedIndexDictionary FAILED
    java.lang.AssertionError: expected the retry to succeed, but the failed call's writer still holds the lock
        at __randomizedtesting.SeedInfo.seed([DEADBEEF:BA04F15DCE3F9F8B]:0)
        at org.apache.lucene.search.spell.TestSpellCheckerFailedIndexDictionary.testRetryAfterFailedIndexDictionary(TestSpellCheckerFailedIndexDictionary.java:43)

        Caused by:
        org.apache.lucene.store.LockObtainFailedException: lock instance already obtained: (dir=ByteBuffersDirectory@49739f33 lockFactory=org.apache.lucene.store.SingleInstanceLockFactory@74a4c501, lockName=write.lock)
            at org.apache.lucene.store.SingleInstanceLockFactory.obtainLock(SingleInstanceLockFactory.java:39)
            at org.apache.lucene.store.BaseDirectory.obtainLock(BaseDirectory.java:44)
            at org.apache.lucene.index.IndexWriter.<init>(IndexWriter.java:954)
            at org.apache.lucene.search.spell.SpellChecker.indexDictionary(SpellChecker.java:496)
            at org.apache.lucene.search.spell.TestSpellCheckerFailedIndexDictionary.testRetryAfterFailedIndexDictionary(TestSpellCheckerFailedIndexDictionary.java:40)

1 test completed, 1 failed

# -Ptests.asserts=false: the same failure and message, 1 test, 1 failure
# with the fix: 1 test, 0 failures (the retry builds and exist("hello") is true)

• The contract. indexDictionary: "@throws IOException If there is a low-level I/O error" (SpellChecker.java:489). Nothing documents that the spell index stays locked afterwards, and the writer is a local variable, so only the method can release it.
• The module's own idiom. AnalyzingInfixSuggester.build: catch (Throwable t) { if (writer != null) { writer.rollback(); writer = null; } throw t; } (AnalyzingInfixSuggester.java:362-368); PlainTextDictionary.FileIterator.next closes its reader on any Throwable (PlainTextDictionary.java:92-95).
• History. Up to 2.3.2 every indexDictionary force-unlocked the index first, so a retry after a failed build worked (executed: 1.9.1 to 2.3.2 pass). LUCENE-1138 (#2215) objected that SpellChecker "shouldn't be calling unlock unless it has reason to think there is a 'stale lock'" (hossman), and LUCENE-1152 (#2229) removed the unlock in a clean-up; neither thread discusses a failed build. The later try/finally (LUCENE-2391, #3466, 2011) covered only the searcher.
• Upstream precedent. LUCENE-2476 (#3550, "Constructor of IndexWriter let's runtime exceptions pop up, while keeping the writeLock obtained") was a type:bug, Blocker, fixed in 2.9.3, 3.0.2, 3.1 and 4.0.
• Tests. TestSpellChecker never makes a build fail; no test asserts the current behaviour.

The strongest "intended or not worth it" case (the devil's-advocate verifier and the intended-lens review): the javadoc promises only @throws IOException and says nothing about the state afterwards; PlainTextDictionary says "content is treated as UTF-8", so a Latin-1 file is misuse, and a 32766-byte "word" is bad input; SpellChecker is an old class with DirectSpellChecker as the newer alternative; a restart fixes it.

Why it fails: the caller has no handle on the writer, so no correct usage can release the lock on that path (the application can only move the SpellChecker to another directory with setSpellIndex, or restart); even where the input is misuse, the documented result is the exception, not a lock that outlives the fixed input; the I/O-error trigger involves no misuse at all; the class is not deprecated, experimental or internal; and the same module rolls back in the identical situation. The cross-model lenses rated the intended argument weak.

Option A was run through gradle in a scratch worktree of 645d4fe14a20. B was not run; C is rejected. All of them change only the failure path: a successful build behaves as before.

In SpellChecker.indexDictionary (SpellChecker.java:496-544), plus import org.apache.lucene.util.IOUtils;: 41 lines added and 34 removed, almost all re-indentation; ignoring whitespace, the change is the try { line, the five-line catch and the import.

       final IndexWriter writer = new IndexWriter(dir, config);
+      try {
         IndexSearcher indexSearcher = obtainSearcher();
         ... (body unchanged, indented two more spaces) ...
         if (fullMerge) {
           writer.forceMerge(1);
         }
+      } catch (Throwable t) {
+        // the build failed: roll back so the write lock is released and the last good build stays
+        IOUtils.closeWhileSuppressingExceptions(t, writer::rollback);
+        throw t;
+      }
       // close writer
       writer.close();

Trade-offs

• IndexWriter.rollback() closes the writer without committing (IndexWriter.java:2566-2576): the lock is released and the spell index stays at its last commit, which is what readers already see today, so nothing partial becomes visible.
• The original exception is rethrown, with a rollback failure attached as suppressed (IOUtils.java:104-137). By that helper's design, if the build threw something other than an Error and the rollback throws an Error, that Error is thrown instead, with the original attached as suppressed. AnalyzingInfixSuggester.build calls writer.rollback() directly, which would lose the original exception if the rollback also threw.
• The try starts right after the writer opens, so it also covers obtainSearcher(), the terms() setup and forceMerge (by reading). If the terms() setup throws, the writer is rolled back but, as today, the searcher is not released, because the inner finally starts only at line 512; moving that setup inside the inner try would cover it too. writer.close() stays outside: when close() fails, IndexWriter already rolls back itself (with the default commitOnClose through shutdown(), IndexWriter.java:1307-1338; with commitOnClose=false close() just calls rollback(), :1363-1369).
• Backports: IOUtils.closeWhileSuppressingExceptions(Throwable, ...) exists from 10.4.0 on; on older branches use IOUtils.closeWhileHandlingException(writer::rollback) or the AnalyzingInfixSuggester shape.

What the checks validated

• Stock 645d4fe14a20: the repro fails with assertions on and off.
• With A: the repro and TestSpellChecker pass with assertions on and off (2 suites, 7 tests, 0 failures); the whole :lucene:suggest:test passes with assertions on (32 suites, 237 tests, 0 failures, 7 skipped).
• checkGoogleJavaFormat, ecjLintMain, ecjLintTest, forbiddenApisMain and forbiddenApisTest of :lucene:suggest pass, with the repro file as it is.
• Earlier, the finder's equivalent fix passed the finder's, the independent reproducer's, the intended-lens review's and the devil's-advocate verifier's tests on main ea57360912ed, and the minimal repro on 645d4fe14a20.

Not run. Releases the lock too.

Trade-offs

• close() commits by default, so the words added before the failure would become visible as a half-built spell index, unlike today, where readers keep the last good build.

The JVM-wide lock record and the OS file lock belong to the leaked writer, and there is no public API to drop them; force-unlocking is also what LUCENE-1138 objected to (SpellChecker "shouldn't be calling unlock unless it has reason to think there is a 'stale lock'"), and by our reading it could break a lock that another writer legitimately holds.

Method: a custom bytecode abstract interpretation (semi-formal in this hunt's tiers). The tool tracks a live set of acquired resources along normal and exception edges, with interprocedural summaries. It ran over the stock main jars of core, codecs, facet, suggest, misc and replicator (3,388 classes, 24,094 methods). Vacuity checks: all 7 planted leaks flagged, none of 8 clean idioms flagged.

• It reported the IndexWriter acquired at SpellChecker.java:496 as leaked on the exception edges of Dictionary.getEntryIterator (:513), BytesRefIterator.next (:517) and releaseSearcher (:538).
• The same unit's fault-injection sweep of the directory (3,753 single-fault runs) could not reach it: here the dictionary fails, not the directory.
• The finder triaged the finding by reading and confirmed it with a JUnit test on the stock jar; independent verifiers then reproduced it without the finder's code and added the non-UTF-8 trigger.