AnalyzingInfixSuggester: one failed add() after build() makes every later refresh() throw AlreadyClosedException

By default (closeIndexWriterOnBuild=true) AnalyzingInfixSuggester.build() closes its IndexWriter at the end but keeps the SearcherManager it opened on that writer, and refresh() is then a deliberate no-op because the writer field is null (AnalyzingInfixSuggester.java:493-497 at 645d4fe14a20). The first add()/update() after build() runs ensureOpen(), which assigns the new writer to the field (:423/:427) and only then opens and installs a SearcherManager on it (:431). If that constructor throws, nothing undoes the assignment: the retried add() succeeds, and every refresh() now refreshes build()'s old manager and fails with AlreadyClosedException: this IndexWriter is closed; new suggestions stay invisible until build() runs again or the suggester is reopened. build() handles the same failure with catch (Throwable), rollback() and writer = null (:362-368); ensureOpen() has no such handling. The same publish-before-install order lets a refresh() on another thread fail, transiently, during the first add() after every default build(). Present since 6.4.0 (LUCENE-7564, #8615); not fixed upstream. A one-method test reproduces it on one thread; a 17-line change to ensureOpen() fixes that, and adding a lock to refresh() also closes the concurrent windows.

1. After a default build() (closeIndexWriterOnBuild=true, AnalyzingInfixSuggester.java:163) the state is: writer = null, searcherMgr = manager M1, opened on build()'s writer W1, which build() closed (:361, :370-373).
2. refresh() (:489-498) takes no lock. It calls searcherMgr.maybeRefreshBlocking() only if (writer != null) (:493-494); otherwise it is a no-op, with the comment "writer was committed and closed after the index was built ... so refresh is unnecessary" (:496-497).
3. The first add() after build() calls ensureOpen() (:418-434). Under writerLock it assigns the new writer W2 to the field (:423, or :427 for an empty directory) and only then calls setAndCloseOldSearcherManager(new SearcherManager(writer, null)) (:431), which installs M2 and closes M1.
4. Between those two lines writer is W2 but searcherMgr is still M1. A refresh() there sees writer != null, refreshes M1, and M1 asks the closed W1 for changes (IndexWriter.nrtIsCurrent -> ensureOpen): AlreadyClosedException: this IndexWriter is closed.
5. If new SearcherManager(writer, null) at :431 throws (it opens an NRT reader, which flushes and reads files), nothing resets writer. The add() throws, but writer = W2 stays next to M1. A retried add() skips the open (writer != null, :420) and writes into W2; every refresh() fails as in step 4. build() repairs it: it closes W2 and installs a new manager (:329-373), and, as always, recreates the index from its own input (OpenMode.CREATE, :338-340). build() itself handles the same failure (its own SearcherManager failing at :361) correctly: catch (Throwable t) { writer.rollback(); writer = null; throw t; } (:362-368).

Without the I/O error, step 2 installs M2 and closes the gap. A refresh() on another thread that runs while step 2 is opening M2 fails the same way once; a later refresh() works. build() has the same order (writer at :338, manager at :361), so a refresh() during a concurrent rebuild also fails (a contested trigger, see below).

BlendedInfixSuggester extends the class and overrides none of build(), ensureOpen(), add(), update() or refresh() (read; not executed).

• Default settings. closeIndexWriterOnBuild=true is the default of every constructor that does not take it (AnalyzingInfixSuggester.java:163, :244). The repro has one suggestion, so size does not matter. The class is @lucene.experimental (:95).
• Effect. One I/O error in the first add() after a build() leaves the suggester stuck: the retried add() succeeds, every refresh() fails, and additions stay invisible until a rebuild or a reopen. An application that retries a failed add(), the natural reaction to an IOException, gets an exception that names a writer the suggester closed itself.
• Concurrent form. A background thread that calls refresh() while another thread is inside the first add()/update() after build() (a window of one NRT reader open) gets the same exception once; a later refresh() works. A refresher thread that does not catch it stops, and suggestions go stale. A hook-free refresher loop hit it in 36 to 40 of 40 rounds in the independent reproducer's runs (33 and 37 of 40 in the adjudication's re-run of that test); a 60-round stress run counted 20,759 such exceptions.
• What does not happen. No wrong or extra suggestion is returned: lookups serve the last published view. Nothing hangs or grows, and the index is not damaged: the addition sits in the open new writer, which commit() still reaches (:382-391); after commit() and close() a reopened suggester finds it. A later build() replaces the index with its own input, as every build() does.
• Related forms (same root cause, executed by the fact check on main and 10.5.2). Without the retry, the refresh() right after the failed add() already throws. On a suggester opened over an existing index (no build() in that instance) the same one failure is silent: refresh() returns normally, but the addition stays invisible until commit(). Both fixes cover both forms.
• Reach. Of the suggester's mutating API, Solr's suggest component calls only build(), never add(), update() or refresh(); no use of the class was found in Elasticsearch or OpenSearch (read-only checks). Exposed are applications that call the near-real-time API directly. The recorded searches found no public report, although the code dates from 6.4.0 (2017), so it is rated rare and low priority.

Save the file as lucene/suggest/src/test/org/apache/lucene/search/suggest/analyzing/TestInfixSuggesterEnsureOpenFailure.java and run the command from the checkout root. The failure hook is installed after build() and fires once, inside the first add()'s ensureOpen(), when the call stack contains the SearcherManager constructor. MockDirectoryWrapper.failOn is the usual Lucene way to inject an I/O error; no specific production cause was observed. The final assertion states what refresh() should achieve; it passes with either fix below.

./gradlew :lucene:suggest:test --tests TestInfixSuggesterEnsureOpenFailure -Ptests.seed=DEADBEEF -Ptests.asserts=true
# assertions off: -Ptests.asserts=false (fails the same way)

# gradle, main 645d4fe14a20, seed DEADBEEF, -Ptests.asserts=true (the same with -Ptests.asserts=false)
# (the module-path prefixes gradle prints before each class name are removed)
TestInfixSuggesterEnsureOpenFailure > testRefreshAfterFailedFirstAdd FAILED
    org.apache.lucene.store.AlreadyClosedException: this IndexWriter is closed
        at org.apache.lucene.index.IndexWriter.ensureOpen(IndexWriter.java:915)
        at org.apache.lucene.index.IndexWriter.ensureOpen(IndexWriter.java:928)
        at org.apache.lucene.index.IndexWriter.nrtIsCurrent(IndexWriter.java:6141)
        at org.apache.lucene.index.StandardDirectoryReader.doOpenFromWriter(StandardDirectoryReader.java:462)
        at org.apache.lucene.index.StandardDirectoryReader.doOpenIfChanged(StandardDirectoryReader.java:413)
        at org.apache.lucene.index.DirectoryReader.openIfChanged(DirectoryReader.java:284)
        at org.apache.lucene.search.SearcherManager.refreshIfNeeded(SearcherManager.java:200)
        at org.apache.lucene.search.SearcherManager.refreshIfNeeded(SearcherManager.java:53)
        at org.apache.lucene.search.ReferenceManager.doMaybeRefresh(ReferenceManager.java:167)
        at org.apache.lucene.search.ReferenceManager.maybeRefreshBlocking(ReferenceManager.java:240)
        at org.apache.lucene.search.suggest.analyzing.AnalyzingInfixSuggester.refresh(AnalyzingInfixSuggester.java:494)
        at org.apache.lucene.search.suggest.analyzing.TestInfixSuggesterEnsureOpenFailure.testRefreshAfterFailedFirstAdd(TestInfixSuggesterEnsureOpenFailure.java:54)
1 test completed, 1 failed

# with the recommended fix: the test passes, assertions on and off;
# :lucene:suggest:test -Ptests.asserts=true with the fix: 32 classes, 237 tests, 7 skipped, 0 failures

• The code says refresh() is a no-op here. LUCENE-7564 added the writer != null gate and the comment "else no-op: writer was committed and closed after the index was built and before searchMgr was constructed, so refresh is unnecessary" (AnalyzingInfixSuggester.java:493-497). The exception comes from the publication order, not from a policy.
• Upstream tests assert both halves. testRefreshAfterBuild (TestAnalyzingInfixSuggester.java:1586-1592) calls refresh() after build() for every option combination and expects no exception. testCloseIndexWriterOnBuild (:1501-1555) asserts that maybeRefreshBlocking() on the post-build SearcherManager throws AlreadyClosedException (:1553-1554): exactly the object refresh() reaches here.
• The documented flow fails. add(): "After adding or updating a batch of new suggestions, you must call refresh in the end in order to see the suggestions in lookup" (AnalyzingInfixSuggester.java:437-439; the same for update(), :450-451). In the repro the batch is one successful add() and one refresh().
• The class handles the same failure elsewhere. build() rolls back and nulls writer on any Throwable inside its try block, including its own SearcherManager failing (:361, :362-368); ensureOpen() leaves writer set without its manager, so no later add() repairs it (:420). Nothing documents that the suggester becomes unusable after an IOException from add(), and the writer underneath survives it: IndexWriter.addDocument says that after an exception "the index will be consistent, but this document may not have been added" (IndexWriter.java:1465-1466), and getReader() treats only an Error as tragic (IndexWriter.java:711-719), which is why the retried add() succeeds. No API promises that every IOException from add() can be retried.
• For the concurrent form. The field javadocs say writerLock and searcherMgrLock are "Used to manage concurrent access" (:138, :147), and SearcherManager recommends "a separate background thread, that periodically calls maybeRefresh" (SearcherManager.java:45-48), advice about SearcherManager itself, which refresh() wraps: supporting context, not a contract of this class. No javadoc forbids refresh() while another thread adds. This contract is implied, not explicit.

The strongest case for "intended": Lookup and this class document no thread safety; add()/update() say to call refresh() "in the end" of a batch; LUCENE-10292 (#11328, 9.2.0) promised only lookup() and getCount() during a concurrent build() (lucene/CHANGES.txt:4289-4291), and committers wrote there and on dev@ (2022-04-27) that Lookup.build() has no general concurrency guarantee; upstream testNRTWithParallelAdds joins its adders before it calls refresh() (TestAnalyzingInfixSuggester.java:982-1011). So refresh() overlapping add() or build() would be unsupported use. Read-only cross-model reviews (OpenAI GPT-5.6) argued this for the concurrent triggers; one wrote that its verdict flips to real if a background refresher is part of the near-real-time contract.

Why it fails: it covers only the concurrent triggers, and neither review saw the single-threaded one. The repro uses one thread and only the documented calls (build, add, retry the failed add, refresh); its failure is an exception-safety defect in ensureOpen(). The remaining objection there: no API promises that an add() can be retried after an IOException, and a maintainer may say that recovery means a rebuild or a reopen; the defect is then the inconsistent state the failure leaves (a live writer next to a manager on a closed writer) and the misleading exception, not a broken retry promise. For the background refresher during the first add() the argument is credible but weak, since it rests on silence that the class's locks and comments contradict. For refresh() during a rebuild it is strong, so that trigger is secondary and marked contested. Verdict: not intended, a defect (adjudication: confirmed, medium confidence).

Two fixes were built and validated through gradle on main 645d4fe14a20 in a scratch worktree; the recommended one is an independent verifier's patch (+29/-10). The smaller one is its ensureOpen() half (+17/-3).

ensureOpen() builds the IndexWriter and the SearcherManager into locals; if the manager cannot be opened it rolls the new writer back (suppressing a second failure) and rethrows, so writer stays null and the next add() retries cleanly; otherwise it assigns writer and installs the manager. refresh() runs its unchanged body under writerLock, so it can no longer run between the two field writes of ensureOpen() or build(). +29/-10 in AnalyzingInfixSuggester.java.

private void ensureOpen() throws IOException {
  synchronized (writerLock) {
    if (writer == null) {
      final IndexWriter w = new IndexWriter(dir, ...); // APPEND or CREATE, as before
      final SearcherManager sm;
      try {
        sm = new SearcherManager(w, null);
      } catch (Throwable t) {
        try { w.rollback(); } catch (Throwable t2) { t.addSuppressed(t2); }
        throw t; // writer stays null, so the next add() retries cleanly
      }
      writer = w;                       // published only with its manager
      setAndCloseOldSearcherManager(sm);
    }
  }
}

public void refresh() throws IOException {
  synchronized (writerLock) {          // new: no refresh between the two field writes
    ... unchanged body ...
  }
}

Trade-offs

• refresh() now waits for a whole concurrent build() (with it, the finder's race test took about 33 s, because refresh() waited until the test's bounded waits expired). Before 9.2.0 the same call threw IllegalStateException; waiting is arguably the better contract, but an application that calls refresh() on a request thread could stall for the length of a rebuild.
• Every add()/update() enters writerLock in ensureOpen(), so an add() now also waits while a refresh() reopens the NRT reader.

What the checks validated

• Gradle on main 645d4fe14a20, scratch worktrees: the repro fails on stock (assertions on and off) and passes with the fix (both). ./gradlew :lucene:suggest:test -Ptests.seed=DEADBEEF -Ptests.asserts=true with the fix and the new test: 32 classes, 237 tests, 7 skipped, 0 failures (TestAnalyzingInfixSuggester 23 of 23, TestBlendedInfixSuggester 9 of 9). ./gradlew :lucene:suggest:check -x test (google-java-format, ecj lint, forbidden APIs) passes once the repro's long comment line is wrapped; unwrapped, only that line is flagged. The assertion mode is recorded in the 2026-10-30 re-runs, whose logs start with their exact command.
• Through the repro harness: the repro passes with the fix with -ea and -da on 645d4fe14a20. Earlier, on main ea57360912ed: the finder's race test (add path and rebuild path) and the single-threaded probe pass with it; the version-portable probe passes all three triggers; TestAnalyzingInfixSuggester and TestBlendedInfixSuggester 32 of 32.

The same ensureOpen() change without the refresh() lock (+17/-3).

Trade-offs

Fixes the single-threaded trigger and narrows the add-path window from one NRT reader open to the two field writes in program order (writer = w, then the manager swap). Neither field is volatile and refresh() takes no lock, so there is no happens-before edge and that bound is not guaranteed: the window is narrowed, not formally closed. refresh() during a concurrent rebuild still fails, because build() must set writer before its loop (its own add() calls use the field).

What the checks validated

Gradle on main 645d4fe14a20: the repro passes with assertions on and off; :lucene:suggest:test -Ptests.seed=DEADBEEF -Ptests.asserts=true: 32 classes, 237 tests, 7 skipped, 0 failures. Repro harness: the finder's 3-test race class passes its add-path test and its control and still fails its refresh-during-rebuild test.

The finder's first patch: refresh() under writerLock, ensureOpen() unchanged.

Trade-offs

Closes the race windows but not the single-threaded trigger: after the failed add(), writer is still set next to the old manager, and the single-threaded probe still fails with it.

Make refresh() a no-op while a build() runs (a flag set under writerLock) instead of waiting for it; or read both fields under writerLock and call maybeRefreshBlocking() outside it, so add() does not wait for a refresh.

Trade-offs

Neither was built or run. In the second, a manager swapped out by a concurrent rebuild can be closed under the caller (this ReferenceManager is closed).

Method: TLA+ model checking (TLC), in the hunt's suggester unit (main ea57360912ed).

• The finder, an AI tool, wrote a TLA+ model of the build/add/refresh/lookup protocol from the code (InfixNRT.tla), with an invariant NoError: no call raises an exception. In configuration MC_conc (an adder thread and a refresher-plus-lookup thread, 2 documents, closeIndexWriterOnBuild=true) TLC reported a NoError violation in a 7-state trace (38 states generated, 27 distinct, about 1 s): the adder publishes the new writer, the refresher sees writer != null and refreshes the manager on the closed build writer. The sequential configuration, closeIndexWriterOnBuild=false and the variant with a locked refresh() are clean; the model starts after build() and has no rebuild step and no injected I/O error, so those clean runs say nothing about the rebuild and I/O-error forms.
• The finder bridged the trace to a JUnit race test on the stock jar (an InfoStream hook holds the first add() inside the manager construction; 2 of 3 tests fail, 40 of 40 failing-test runs over 20 iterations), found the rebuild path by reading, and ran a hook-free stress run (20,759 exceptions in 60 rounds, no other invariant violation).
• Verifier, an AI tool asked to make the strongest "intended" case, read ensureOpen() next to build()'s catch (Throwable) and found the single-threaded trigger: one injected I/O error in the SearcherManager construction. It showed that the finder's lock-only fix does not cover it and wrote the complete fix. The one-method repro on this page is that trigger, reduced.