DirectoryTaxonomyWriter's constructor leaves the taxonomy write-locked when a file read fails after it opened its IndexWriter

DirectoryTaxonomyWriter's constructor opens its internal IndexWriter first (DirectoryTaxonomyWriter.java:145), which takes the taxonomy directory's write.lock, and only then reads the latest commit of an existing taxonomy (:158) or, for a new one, adds the root category through a near-real-time reader (:185). Nothing after :145 is inside a try, so when one of those reads throws an IOException the constructor throws with the writer still open. The caller gets no object, so neither close() nor rollback(), the only releases, can ever run, and NativeFSLockFactory keeps the lock path in a static JVM-wide set until the lock is closed. Every later new DirectoryTaxonomyWriter(dir) in that JVM then fails with LockObtainFailedException ("Lock held by this virtual machine"), other processes are locked out only until that JVM's garbage collector closes the leaked lock channel, and with SimpleFSLockFactory the stale write.lock survives even a normal exit. No data was lost or corrupted in any tested scenario, and readers keep working. IndexWriter's own constructor releases its lock on any failure; this one never did. A history probe reproduced it on 20 releases from 3.4.0 to 10.5.2 and on main; 10.6.0 and upstream main have the same file. Low priority: it needs an I/O error inside a short window and an in-process retry.

// DirectoryTaxonomyWriter(Directory, OpenMode, TaxonomyWriterCache), condensed
indexWriter = openIndexWriter(dir, config);                // :145  new IndexWriter: takes write.lock
...
if (DirectoryReader.indexExists(directory) == false) {     // :154
  indexEpoch = 1;
} else {
  SegmentInfos infos = SegmentInfos.readLatestCommit(dir); // :158  path A: IOException -> writer leaks
  ...
}
...
if (nextID.get() == 0) {
  addCategory(new FacetLabel());                           // :185  path B: NRT reader open -> writer leaks
}
// no try/catch anywhere after :145

1. openIndexWriter (:215-218) returns new IndexWriter(directory, config), which obtains write.lock (IndexWriter.java:954). If IndexWriter's own constructor fails, it releases the lock itself (IndexWriter.java:1202-1208, "init: hit exception on init; releasing write lock").
2. Path A, existing taxonomy: SegmentInfos.readLatestCommit(dir) (:158) reads segments_N again, under the lock, for the index.epoch entry. An IOException there propagates.
3. Path B, new taxonomy: addCategory(new FacetLabel()) (:185) adds the root document (:468), then getTaxoArrays() (:475, :665-683) calls initReaderManager(), which opens a near-real-time reader (new ReaderManager(indexWriter, false, false), :252). An IOException while that reader opens a segment file propagates; a reader manager already built leaks too.
4. The exception leaves the constructor. The only releases, close() (:288-294) and rollback() (:861-865), are instance methods of an object the caller never got.
5. NativeFSLockFactory adds the lock path to its static LOCK_HELD set when it obtains the lock (NativeFSLockFactory.java:68, :108) and removes it only when the lock is closed (:145-150); a later attempt throws "Lock held by this virtual machine" (:119). Garbage collection does not help in that JVM (an independent reproducer's probe: still held after five System.gc() rounds); it only closes the leaked file channel, which lets other processes in (see Who is affected). There is no unlock API (removed in 5.0, LUCENE-6060).

Kept out of the claim (by reading only, not executed): the indexExists edge (:154); a NumberFormatException at :165, which needs a malformed index.epoch entry (crafted files or misuse); the -ea-only merge-policy assert at :148. Other steps after :145 could throw too (getDocStats() at :174 calls ensureOpen(); any Error); only the two paths above were executed, and the fix's catch (Throwable) covers them all.

• Who. Applications that use taxonomy facets (the opt-in facet module) with DirectoryTaxonomyWriter: its default constructor, IndexWriterConfig, codec and FSDirectory suffice. In-tree, the facet demo examples and the benchmark's taxonomy tasks construct it.
• When. A non-aborting I/O error in the short window after the constructor opened its writer (the commit read of an existing taxonomy, or the reader open for the root category of a new one), and a retry in the same process. Likelihood is rated rare: the searches found no public report of it since 2011.
• What goes wrong (executed probe on main with assertions on and off, 10.5.2 and 9.12.3): every later writer open on that taxonomy in the failed JVM fails fast with LockObtainFailedException until that JVM exits; 200 retries all failed. Another process is locked out only until that JVM's garbage collector closes the leaked lock channel: NativeFSLockFactory opens the lock with FileChannel.open (NativeFSLockFactory.java:132), and OpenJDK 25 registers a Cleaner that closes an unreachable channel's descriptor. A cross-process probe showed it: another JVM was refused ("Lock held by another program"), then after one System.gc() in the failed JVM it opened a writer while the failed JVM was still alive and that JVM's own retry still failed. When that collection happens is up to the JVM. With the non-default SimpleFSLockFactory, write.lock survives even a normal exit and must be deleted by hand. The message says the lock is held, not why, so it looks like a second writer the application does not have.
• Not a trigger: a disk-full error while the root category is flushed. IndexWriter treats that as tragic and closes itself, so nothing leaks (executed).
• What it does not do (executed, in the tested fault scenarios): nothing was lost or corrupted. Taxonomy readers return the exact committed label-to-ordinal map, the committed files stay byte-identical, other taxonomies and the main index are unaffected, and after a restart (default lock factory) every ordinal is preserved. The leak is bounded: only the first failure on a taxonomy leaks, one unreachable IndexWriter and its lock descriptor, until garbage collection reclaims them; retries fail before they acquire anything.

One test class, one method, single thread, deterministic. The fault fires once, only for a segments_N open with no IndexWriter frame on the stack, which is the constructor's own readLatestCommit (DirectoryTaxonomyWriter.java:158) after the writer opened; a failure inside IndexWriter's constructor would release the lock by itself. expectThrows checks that the constructor throws an IOException; on its own it would also accept a LockObtainFailedException, so a probe repeating the same steps confirmed that the first exception is the injected one, fired once, from :158.

ByteBuffersDirectory uses SingleInstanceLockFactory, so the message is "lock instance already obtained"; on FSDirectory with the default NativeFSLockFactory the same retry fails with "Lock held by this virtual machine" (the independent checks' tests).

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 lines cut)
TestDirectoryTaxonomyWriterCtorFailure > testWriterOpensAfterConstructorFailedReadingCommit FAILED
    java.lang.AssertionError: the failed constructor left the taxonomy write-locked: org.apache.lucene.store.LockObtainFailedException: lock instance already obtained: (dir=ByteBuffersDirectory@3225a843 lockFactory=org.apache.lucene.store.SingleInstanceLockFactory@31547513, lockName=write.lock)
        at __randomizedtesting.SeedInfo.seed([DEADBEEF:ED5CA87A0D7F0AAE]:0)
        at org.junit.Assert.fail(Assert.java:89)
        at org.apache.lucene.facet.taxonomy.directory.TestDirectoryTaxonomyWriterCtorFailure.testWriterOpensAfterConstructorFailedReadingCommit(TestDirectoryTaxonomyWriterCtorFailure.java:55)

1 test completed, 1 failed

# the same failure and message with -Ptests.asserts=false
# FSDirectory with the default NativeFSLockFactory (an independent check's test on main, path shortened):
#   org.apache.lucene.store.LockObtainFailedException: Lock held by this virtual machine: $TMP/taxo-fs-001/write.lock
#   java.lang.RuntimeException: file handle leaks: [FileChannel($TMP/taxo-fs-001/write.lock)]

• The contract. Constructor javadoc: "@throws LockObtainFailedException if the taxonomy is locked by another writer" (DirectoryTaxonomyWriter.java:137). The openIndexWriter javadoc makes the taxonomy writer the owner of the IndexWriter: "the instance this method returns will be closed upon calling to close()" (:206); a failed constructor leaves no object to call close() on.
• The codebase convention. IndexWriter's constructor catches any Throwable and releases its lock (IndexWriter.java:1202-1208), added after the same symptom in IndexWriter (LUCENE-2476, #3550). In the same module, LUCENE-8244 added a failure-path close to SearcherTaxonomyManager.
• The principle, stated for this class (as Doron Cohen pointed out on LUCENE-3596, #4670, 2011): a merge-policy check done "after the index was already opened ... must be closed prior to throwing that exception". That check became the :148 assert; the later I/O steps never got the cleanup.
• History. LUCENE-4532 (#5598, 2012) moved the commit read after openIndexWriter so that it runs under the lock; that explains the order, not why the lock should outlive a failed read, and the issue does not discuss a failing read. No commit ever added or removed cleanup in this constructor (126 release tags checked).
• Tests. No upstream taxonomy test injects I/O failures at all, so this path was never exercised.

The strongest "works as intended" case (built by a cross-model reviewer, OpenAI GPT-5.6, and by a devil's-advocate verifier): the class is @lucene.experimental (:76); @throws IOException if another error occurred promises no exception safety; the openIndexWriter javadoc promises closing only through close(); LUCENE-4532 put the read under the lock on purpose; every repro injects its error; and after an I/O error one should restart.

Why it fails: @lucene.experimental is about API stability, not about leaking an unreachable writer. The caller of a failed constructor has no object to close and no unlock API, so only the constructor can release the lock; IndexWriter's constructor shows the expected contract, and LUCENE-3596 states it for this class. An IOException from Directory.openInput is an ordinary declared failure, reached in one check through a plain FSDirectory throwing "Too many open files". Restarting is a workaround, and with SimpleFSLockFactory it does not even work. Both the reviewer and the verifier rated the intended-behaviour argument weak; a later read-only fact check by the same model family rated it moderate, because a subclass may supply its own writer through openIndexWriter and every trigger was injected (see Suggested fix).

Option A was run through gradle in a scratch worktree of 645d4fe14a20; B is the shortcut a reviewer may suggest, and why it is worse. Moving the commit read back before openIndexWriter is not a fix: LUCENE-4532 put it under the lock on purpose, and the new-taxonomy path needs the open writer anyway.

In the constructor (DirectoryTaxonomyWriter.java:145-195): the existing body moves, unchanged and re-indented, into the try; ignoring whitespace the change is +16/-1, the IOUtils import included.

    indexWriter = openIndexWriter(dir, config);
    try {
      // ... the existing constructor body, unchanged ...
    } catch (Throwable t) {
      IOUtils.closeWhileSuppressingExceptions(
          t,
          () -> {
            if (initializedReaderManager) {
              readerManager.close();
            }
          },
          indexWriter::rollback);
      throw t;
    }

Trade-offs

• Roll back, not close: IndexWriter.close() commits by default (IndexWriter.java:1363-1369), so on the new-taxonomy path it would commit the root category the failed constructor had buffered. rollback() also works on a writer that already closed itself after a tragic error (IndexWriter.java:2576-2582).
• No virtual call: it does not call closeResources(), a documented subclass hook, on a partially constructed object. Both points came from the cross-model review of the first fix draft.
• The original exception is rethrown with cleanup exceptions suppressed into it; only if a cleanup step throws an Error while the original is not one is that Error thrown instead, with the original suppressed into it (IOUtils.java:116-136), as in IndexWriter's own constructor (IndexWriter.java:1206). The cache is not closed: the default LruTaxonomyWriterCache is heap-only (its close() only clears a map), and a cache the caller passed in stays with the caller.
• A subclass that overrides openIndexWriter to supply its own IndexWriter gets that writer rolled back when the constructor fails. The openIndexWriter javadoc already makes the taxonomy writer its owner (:206), but a reviewer may want that stated.
• The helper exists from 10.4.0 on; a 9.x backport would use IOUtils.closeWhileHandlingException in the same place (not run).

What the checks validated

• Stock 645d4fe14a20: the repro fails with assertions on and off.
• With the fix: the repro and TestDirectoryTaxonomyWriter pass with assertions on and off (16 tests, 0 failures, 1 skipped: testHugeLabel is @Nightly); the whole :lucene:facet:test passes with assertions on (40 suites, 307 tests, 0 failures, 7 skipped).
• The independent reproducer's test, unchanged, passes 4 of 4: the existing taxonomy on ByteBuffersDirectory and on FSDirectory, the new taxonomy whose root-category reader open fails, and the control; no leaked-file report. Its 3 bug tests fail on stock code.
• ecjLintMain, ecjLintTest, forbiddenApisMain and forbiddenApisTest pass; checkGoogleJavaFormat passes for the fixed class and, once ./gradlew tidy rewraps two lines of the repro test (line breaks only), for the test too.

Shorter, but close() commits first (DirectoryTaxonomyWriter.java:288-294), and when they complete normally both close() and rollback() (:861-865) end in the overridable closeResources() (:296-299, :308), which runs subclass code on a partially built object.

Trade-offs

• Reads naturally, but can commit a half-built taxonomy and runs subclass code too early.

Method: semi-formal static analysis (a custom bytecode abstract interpretation), from a sweep over Lucene's failure paths.

• The analysis interprets the bytecode of the core, codecs, facet, suggest, misc and replicator jars (3,388 classes, 24,094 methods) with explicit exception edges, a live set of acquired resources and interprocedural summaries. Its vacuity check flagged all 7 planted leaks and none of 8 clean idioms.
• It reported that the IndexWriter from openIndexWriter (:145) is still held on the exception edges of DirectoryReader.indexExists (:154), SegmentInfos.readLatestCommit (:158) and addCategory (:185).
• A single-fault sweep with the test framework's MockDirectoryWrapper (2,917 fault events on an existing taxonomy, 5,390 on an empty one) showed lock leaks in both cases; a JUnit test on the stock jars confirmed it.