A refresh after ControlledRealTimeReopenThread.close() undoes the closed state, so waitForGeneration can block forever

ControlledRealTimeReopenThread registers a refresh listener on the ReferenceManager in its constructor and never removes it. close() stops the reopen thread and sets searchingGen = Long.MAX_VALUE ("Max it out so any waiting search threads will return"), so a wait issued after close() returns at once. But the manager is a separate object and stays open: one more refresh of it runs the leftover listener, which sets searchingGen to the writer's current generation with no check. An untimed waitForGeneration(gen) on the closed instance for a newer change then waits for a reopen that the stopped thread will never do. Since 8.6.0 a later refresh that covers gen releases it; if refreshes stop (for example the manager is closed next) only an interrupt or a second close() does, and on 4.4.0-8.5.2 no refresh releases it at all. The timed overload returns false at its bound, nothing is corrupted, no result is wrong and no lock is held. Present in every release from 4.4.0 to 10.6.0 and on main; the class it replaced, NRTManager, guarded this value.

manager.addListener(new HandleRefresh());                     // :85, constructor; never removed

public void beforeRefresh() {
  refreshStartGen = writer.getMaxCompletedSequenceNumber();   // :96
}
public void afterRefresh(boolean didRefresh) {                // :100, runs on EVERY refresh of the manager
  synchronized (waitingRoom) {
    searchingGen = refreshStartGen;                            // :102, no check, also after close()
    waitingRoom.notifyAll();
  }
}

public void close() {                                         // :110
  finish = true; reloadRequests.release(); join();            // :111-117, stops the reopen thread
  // Max it out so any waiting search threads will return:
  searchingGen = Long.MAX_VALUE;                               // :122-123, outside waitingRoom
  synchronized (waitingRoom) { waitingRoom.notifyAll(); }      // :126-128
}

// waitForGeneration(targetGen, maxMS), :159-187
while (targetGen > searchingGen) { if (maxMS < 0) waitingRoom.wait(); ... }   // :171-173

• Nothing removes the listener. The class has no removeListener call. ReferenceManager calls every registered listener's beforeRefresh and afterRefresh on every refresh, from maybeRefresh() and maybeRefreshBlocking(), whoever calls them (ReferenceManager.java:154-188, listeners at :166 and :182).
• After close() no reopen thread is left to end a wait. The reopen thread, whose loop is the class's only refresh call (:213), has exited. ReferenceManager.close() calls no listener (ReferenceManager.java:134-142), and after it every refresh throws AlreadyClosedException, so once the manager is closed only an interrupt or another close() call can release the waiter (close() has no closed-state guard: join() on the finished thread returns at once, and it writes Long.MAX_VALUE and notifies again; executed on main and 8.5.2).
• 4.4.0-8.5.2 are worse. There the start generation was captured by the reopen thread's own loop, not in beforeRefresh (moved by the LUCENE-9304 commit 18af6325ed9, first in 8.6.0). After close() every refresh writes back the same stale value (3 on 8.5.2), so no refresh releases the waiter.
• A refresh that starts before close() returns and finishes after it overwrites the value too (executed on 4.4.0, 8.5.2, 8.6.0, 9.12.3, 10.5.2 and main, with the waiter started afterwards). The form where a waiter that close() already woke is put back to sleep was shown only by TLA+ model checking, never executed.

Without step 2 the wait in step 3 returns at once (every control run, 4.4.0 to main). With the recommended fix getSearchingGen() stays Long.MAX_VALUE in step 2 and step 3 returns at once.

• Who. The class is opt-in and @lucene.experimental (ControlledRealTimeReopenThread.java:36). It takes an application that refreshes the still-open manager after crt.close() (a final refresh at shutdown, or a new reopen thread on the same manager) and a thread that still holds the closed instance and calls the untimed waitForGeneration(gen) for a newer change.
• Effect since 8.6.0 (all 9.x, 10.x, main). The thread waits until the next refresh whose start generation covers gen. With a replacement reopen thread that is extra latency (its refreshes released 16 of 16 old waiters in a probe). If refreshes stop, for example because the manager is closed next, it waits until it is interrupted or close() is called again, and forever if neither happens.
• Effect on 4.4.0-8.5.2. No later refresh releases it; only an interrupt or a second close() does.
• Bounded parts. The timed waitForGeneration(gen, maxMS) returns false at its bound, which its @return allows (300 ms in the probe). The parked thread holds no Lucene lock and uses no CPU; search, indexing and commit continue (13-19 ms while 16 waiters were pinned). The bug only delays a wait; it never reports a generation as visible early, so no stale or deleted data is served because of it.
• Leak. Every closed instance stays registered on the manager and runs on every later refresh: about 400 bytes and 65 ns per refresh each (5,000 closed instances: median no-change refresh 2.8 µs -> 330 µs; approximate, one host).
• Scale. Any index size; nothing depends on the codec or the IndexWriterConfig settings; the repro indexes one empty document.
• Likelihood: contrived. Every executed trigger waits on an already-closed instance after an extra refresh between close() and the wait. Upstream tests never refresh the manager after closing the reopen thread (TestControlledRealTimeReopenThread.java:327-330, :408-411, :573-574, :592). A read-only code search found no use of the class in the default branches of Elasticsearch, OpenSearch or Solr, and nobody has reported it since 4.4.0 (2013).

The class sits in org.apache.lucene.search next to TestControlledRealTimeReopenThread; it uses no package-private access. Smallest input: ByteBuffersDirectory, a default IndexWriterConfig, a SearcherManager, one empty document. The test framework still picks its per-seed defaults (codec, locale, time zone); the trigger does not depend on them, and every seed run fails. The second thread exists only because the symptom is that the call blocks; the trigger itself is sequential and deterministic. The test releases its own waiter with one more refresh before asserting (on 8.6.0+ a covering refresh frees it), so a failure is reported as the one assertion, not as a leaked thread. The test is meant for 8.6.0 and later: on 4.4.0-8.5.2 no refresh releases the waiter, so its final waiter.join() would not return there (by reading; the old-release runs used the study's ports).

On 10.x releases before #15830 the constructor takes seconds as double: use 5.0, 0.0 instead of the Duration arguments (that was the only change for the 10.6.0 run).

Regression test: this same class (or its one method added to TestControlledRealTimeReopenThread); with the recommended fix it passes in under a second.

cp $REPRO/TestWaitForGenerationAfterClose.java $LUCENE/lucene/core/src/test/org/apache/lucene/search/
cd $LUCENE && ./gradlew :lucene:core:test --tests org.apache.lucene.search.TestWaitForGenerationAfterClose -Ptests.seed=DEADBEEF
# takes about 5 s to fail; assertions off fails the same way: -Ptests.asserts=false (the recorded runs added --offline --no-daemon)

# gradle, stock 645d4fe14a20, -Ptests.seed=DEADBEEF (stack trimmed)
TestWaitForGenerationAfterClose > testWaitForGenerationReturnsAfterCloseAndRefresh FAILED
    java.lang.AssertionError: waitForGeneration(2) on the closed ControlledRealTimeReopenThread still blocked after 5 s (getSearchingGen()=1 after one refresh following close())
        at __randomizedtesting.SeedInfo.seed([DEADBEEF:69CC3397A5C0D1E7]:0)
        at org.apache.lucene.search.TestWaitForGenerationAfterClose.testWaitForGenerationReturnsAfterCloseAndRefresh(TestWaitForGenerationAfterClose.java:57)
:lucene:core:test (FAILURE): 1 test, 1  failure

# standalone runner: main 645d4fe14a20 fails with -ea and -da (and 3 of 3 with seed 5EED1234, -da);
# release 10.6.0 (double-seconds constructor) fails with the same message

• The code says close() releases waiters: "Max it out so any waiting search threads will return" (ControlledRealTimeReopenThread.java:122, in every release since 4.4.0). On main, the @return of waitForGeneration(long, int) (added by #15830, 2026): "true if the targetGen is now available, or false if maxMS wait time was exceeded or this instance was closed. True might also be returned if this call returned due to a close()" (:155-157).
• Sticky by design: a wait issued after close() with no refresh in between returns at once in every version run, 4.4.0 to main. Only the leftover listener undoes it.
• The one-argument javadoc says the call blocks "until the searcher is reopened by another thread calling ReferenceManager#maybeRefresh, or until the ReferenceManager is closed" (:132-135). The shutdown sequence (one refresh, then mgr.close()) breaks even that literal text. On its own this is weak support, because closing the manager has no wake-up mechanism in any version.
• History: NRTManager.close() set its generation to Long.MAX_VALUE from LUCENE-3488 (2011) on; LUCENE-3528 (#4602, 2011) named that closed value MAX_SEARCHER_GEN and added a guard for it in the reopen path, and LUCENE-3776 (#4849, 2012) kept the guard as if (searchingGen != MAX_SEARCHER_GEN) before writing a new generation; on LUCENE-3776, Michael McCandless called it trappy that refreshing that class's manager by another path "can unexpectedly cause threads to block forever" (a different trigger in the older class, fixed then). LUCENE-4967 (#6031, 2013) moved the tracking into the new listener without the guard, and its comments do not discuss close semantics or listener removal. Caveat: NRTManager owned its managers, so a refresh after close was also rejected there.
• Same package: LiveFieldValues adds itself as a listener in its constructor and removes itself in close() (LiveFieldValues.java:46, :50-52). The Lucene.NET port has guarded the same write since 2017 (finish ? long.MaxValue : refreshStartGen), without describing the Java behaviour.
• Legal trigger: nothing says the manager must not be refreshed after the reopen thread is closed; maybeRefresh: "You must call this ... periodically" (ReferenceManager.java:190-192).

The strongest case for "intended" (the second cross-model review, the devil's-advocate verifier, the adjudication and the audit): the class is an experimental Thread whose close() is documented as "Stop this thread. Blocks until it terminates." (:108). Waiting on a closed instance while someone else keeps refreshing the manager is mixed ownership; the one-argument javadoc literally says the call blocks until another thread refreshes, which a further refresh does satisfy on 8.6.0+; and in the shutdown form the waiting thread's next acquire() would fail anyway. The audit's view: a real but minor robustness defect, not a production hang.

Why it fails: nothing documents that waits or refreshes after close() are invalid; the close() comment and, on main, the @return expect close() to release waiters; the closed state is deliberately sticky until a listener that close() never removes undoes it; the shutdown form does not honour even the one-argument javadoc's "or until the ReferenceManager is closed"; and on 4.4.0-8.5.2 no refresh releases the waiter at all. Hence the narrow claim: only the untimed overload, and "can block forever" when refreshes stop.

One fix was validated through gradle in a scratch worktree of 645d4fe14a20; a smaller variant without the listener removal is the hunt's fix and passed every earlier check; removing the listener alone is not enough. A combined change with open PR #16210 (an interrupted close()) is natural.

Keep the listener in a field, remove it in close() after join(), write Long.MAX_VALUE inside synchronized (waitingRoom), and make the listener's write monotone.

public void afterRefresh(boolean didRefresh) {
  synchronized (waitingRoom) {
    searchingGen = Math.max(searchingGen, refreshStartGen);
    waitingRoom.notifyAll();
  }
}
...
  manager.removeListener(refreshListener);   // in close(), after join()
  synchronized (waitingRoom) {
    searchingGen = Long.MAX_VALUE;
    waitingRoom.notifyAll();
  }

Trade-offs

• After close() no later refresh calls the listener, and a refresh whose after-refresh loop had already taken its snapshot of the listener list can no longer lower the value. Each closed instance also stops costing memory and refresh time.
• Math.max changes behaviour only when a refresh's start generation is lower than the current value; outside the post-close case that was not analysed beyond the upstream suite passing.
• Removing the listener alone would not cover a refresh already in progress during close(), which is why the monotone write stays.

What the checks validated

• Gradle, scratch worktree of 645d4fe14a20, OpenJDK 25.0.3 (2026-10-31): stock, the minimal test fails. With the fix: the minimal test passes (under a second) with assertions on and off; TestControlledRealTimeReopenThread 5 passed, 1 skipped (testCRTReopen, @AwaitsFix) with seed DEADBEEF, and 15 passed, 3 skipped with seed 5EEDCAFE and 3 iterations; 0 failures. It is the only upstream test of the class. Formatting was not checked.

The same change without the listener removal.

Trade-offs

Each closed instance stays registered on the manager and runs on every later refresh (about 400 bytes and 65 ns per refresh each).

What the checks validated

Passes the finder's test and upstream TestControlledRealTimeReopenThread 5 of 5 on ea57360912ed, an independent verifier's 3-test class, and the minimal repro on 645d4fe14a20 with assertions on and off. TLA+ model checking of this fix is clean for the sequential and the concurrent forms.

Call manager.removeListener(...) in close().

Trade-offs

Fixes the sequential form by reading, but a refresh whose after-refresh loop took its snapshot of the listener list before the removal still calls the removed listener and can still lower the value. Not run.

searchingGen = finish ? Long.MAX_VALUE : refreshStartGen in afterRefresh.

Trade-offs

Still needs the close() write under waitingRoom, may publish the closed value before the thread has stopped (the @return tolerates that), and leaves the listener registered. Not run.

Method: formal (TLA+ model checking, the tool's own counterexample, then a public-API JUnit).

• Model. CRT.tla models the reopen thread with its Semaphore signal (#15830), waitForGeneration's wait/notify, close(), the HandleRefresh listener and an application thread that refreshes the same ReferenceManager.
• Counterexamples. With the application refresh allowed after close(), TLC reported a deadlock (19-state traces). With the refresh started before close() returns, it reported a violation of the invariant NoReblockAfterClose (16-state trace): a waiter woken by close() is put back to sleep. The fix models are clean (12,451 and 83,460 distinct states).
• Confirmation. The sequential trace became a JUnit that fails on the stock main jar with assertions on and off. An independent reproducer, a devil's-advocate verifier, an adjudication and a credibility audit re-ran it or their own tests; a follow-up ran the version history, a prior-report search and a security probe. The concurrent re-block form was never executed.