Monitor.deleteById and Monitor.clear can leave a concurrently registered query partly indexed

The Monitor indexes a registered query as one document per decomposed part, and register adds those documents one by one while holding commitLock. Monitor.deleteById and Monitor.clear send their IndexWriter delete before they take that lock, so a delete or clear that arrives while register is between two addDocument calls removes only the parts already added. The other parts are committed: the query is still reported as registered, but it silently misses every document that only the removed parts match, and for deleteById that was shown to survive close, reopen and cache purges until the id is registered again. Neither order of the two calls can produce this state. It has been in every release with lucene-monitor, from 8.2.0 to 10.6.0 (17 of them run, the rest by code identity, 10.6.0 by reading), and is on main.

• Monitor (lucene-monitor): stores queries under ids and matches documents against them (reverse search). Internally it keeps the queries in a small Lucene index, the query index.
• Decomposition: the default QueryDecomposer splits a disjunction into its clauses. Each part becomes one query-index document with the query's id in the field _query_id and a cache id q_0, q_1, ... (QueryCacheEntry.java:51-60, WritableQueryIndex.java:185-203). Matching searches the query index, so a part without a live document can never match.
• commitLock: the lock in WritableQueryIndex that serializes changes to the query index (WritableQueryIndex.java:64).

1. register calls commitWithoutNotify (WritableQueryIndex.java:107-133). Inside synchronized (commitLock) (:109) it deletes the id's old documents (:117-119), then calls writer.addDocument once per part (:120-125), then commits and refreshes (:127-128).
2. deleteQueries (behind Monitor.deleteById) calls writer.deleteDocuments(new Term(FIELDS.query_id, id)) (:339-341) before commitWithoutNotify(Collections.emptyList()) takes the lock (:342). clear does the same with writer.deleteAll() (:348).
3. IndexWriter orders every change by a sequence number, and a term delete applies only to documents indexed before it (IndexWriter.java:117-119, DocumentsWriterPerThread.java:361-383). A delete that arrives while q_1 is being added deletes q_0 and keeps q_1 and every later part.
4. register then commits the survivors. getQueryIds (an index scan, Monitor.java:276-280) still lists q, getDisjunctCount (numDocs, Monitor.java:257-262) counts the survivors, and match (WritableQueryIndex.java:205-246) can only reach parts that have a document.
5. On reopen the cache is rebuilt from the stored query, all three parts, but the index still has only q_1 and q_2 (WritableQueryIndex.java:137-181); purgeCache rebuilds the cache from the index (WritableQueryIndex.java:249-258), so neither repairs it (both executed for deleteById). Registering q again (delete all, add all) or deleting it does.
6. clear() uses deleteAll(), which is not tied to an id, so it can cut short the register of any multi-part query. Its IndexWriter side differs: deleteAll waits for the part being added and then drops every buffered document (IndexWriter.java:2765, DocumentsWriter.java:266-325). In the pinned timing only q_2 survives: getDisjunctCount=1, only c matches. A single-part query cannot end partial: its one add is either before or after the delete.

• Pinned test (q = a OR b OR c, the delete arrives while the second part is indexed): getQueryIds=[q], getDisjunctCount=2, a document with a gets 0 matches. An independent reproducer's test of the same timing also shows b and c matching once each, and a persistent Monitor the same before and after close and reopen, with getQuery("q").getQueryString() = a b c (the query string it passed at register; without a serializer getQuery throws). With clear() in the same timing: getDisjunctCount=1, only c matches.
• Hook-free stress test (64-clause disjunction, stock presearcher, random 0-3 ms delay before the delete, 400 races per method): 15-37 of 400 deleteById races and 20-53 of 400 clear races ended partial in the recorded runs on main, 10.5.2, 9.12.3 and 8.2.0, for example getDisjunctCount=59 of 64. These are forced-race rates, not field rates.

• Who. Embedders of lucene-monitor (alerting, saved searches, routing, classification, content screening) that call register and deleteById (same id) or clear() (any id) from different threads. The documents passed to match() cannot trigger it.
• Default or opt-in. Default within the module: new Monitor(analyzer) with the default MonitorConfiguration, TermFilteredPresearcher and QueryDecomposer, in memory or persistent. The module itself is optional.
• Window. The time register needs to index the query's remaining parts, so it grows with the number of parts. A three-clause disjunction suffices.
• Damage. The query is reported as registered but matches only a subset of what it should. No exception and no log in the executed runs. For deleteById it survives commit, close, reopen and purgeCache until the id is registered again or deleted. deleteById leaves other queries alone, clear() removes them as it should, and nothing over-matches.
• How likely. Rare: it takes concurrent mutation inside a short window, and no report of it was found since 8.2.0 (2019). An application that applies all query changes from one thread never hits it.

# Lucene main 645d4fe14a20, assertions on (-ea), seed DEADBEEF: 1 test(s) run, 1 failed
    => java.lang.AssertionError: matches of docA, getQueryIds=[q] expected:<1> but was:<0>
       org.junit.Assert.fail(Assert.java:89)
       org.junit.Assert.failNotEquals(Assert.java:835)
       org.junit.Assert.assertEquals(Assert.java:647)
       org.apache.lucene.monitor.TestMonitorDeleteByIdDuringRegister.testDeleteByIdDuringRegisterLeavesQueryPartlyIndexed(TestMonitorDeleteByIdDuringRegister.java:101)

• No explicit promise. The Monitor class javadoc and the register, deleteById and clear javadocs say nothing about thread safety or atomicity (Monitor.java:38-41, :147-205). The contract is implied by the design, not documented.
• A query is stored as all its parts. getDisjunctCount is "the number of queries (after decomposition) stored in this Monitor" (Monitor.java:257-259), and register replaces all parts of an id under one lock, so two register calls on the same id are already atomic (WritableQueryIndex.java:109-125).
• One lock for every document change. commitLock (WritableQueryIndex.java:64) guards commitWithoutNotify (:109) and both cache swaps in purgeCache (:294, :305). The two deletes are the only document changes outside it (close() also commits outside it, :321-325). deleteQueries and clear wait for it right after their delete (:342, :349), so the placement buys almost no concurrency.
• Concurrent use is designed in, at least in part. A background purge thread runs by default (WritableQueryIndex.java:86-98; "This is normally called from a background thread", Monitor.java:131), and ConcurrentQueryLoader is a "Utility class for concurrently loading queries into a Monitor" (ConcurrentQueryLoader.java:35), documented for loading "before starting matches" (:37-38). These show purge-versus-change and register-versus-register concurrency, not register racing delete or clear. Upstream fixed earlier Monitor concurrency bugs (LUCENE-9791 / #10830; #12801, whose description has Monitor.match "called concurrently with Monitor.register/Monitor.deleteById and QueryIndex.purgeCache").
• Precedent. In Luwak, the predecessor project, flaxsearch/luwak#68 (2015, another contributor's report) showed update() issuing its delete outside the synchronized commit; the maintainer answered "I think you've identified genuine issues" and commit 18fc644 "Move queryindex deletions into synchronized block" fixed update() only. deleteById and clear kept their unlocked deletes and were imported into Lucene in 2019.
• Tests. No upstream test runs deleteById or clear concurrently with register: in TestCachePurging.doConcurrentPurgesAndUpdatesTest the deletes (TestCachePurging.java:108-110) finish before the second thread starts (:114).

The strongest "works as designed" case (independent reviewers, a devil's advocate and a cross-model fact check): Monitor never says it is thread-safe, and Java classes with mutable state are not normally presumed thread-safe unless documented; ConcurrentQueryLoader documents only concurrent loading; register(Iterable) already commits big calls in chunks; IndexWriter's add, delete, add order is legal; an application that races register(q) against deleteById(q) cannot know whether q survives anyway; and clear(), a global reset, is something callers may be expected to run while no other writes are in flight. The cross-model reviewer rated this case moderate overall, somewhat stronger for clear().

Why it fails: either serial outcome would be fine, but this is a third outcome, a query reported as registered that silently matches only some of its clauses, persistently. The chunking is per query, so all parts of one query are always written in one locked call: the single query is exactly the unit the design protects. clear() hits a register of any id, so "the application races on the same id" does not hold there; the "quiesce before a reset" expectation is undocumented, but it is the strongest point against the clear() half. What survives is a severity point (a rare trigger), not a validity point. Verdict: a defect.

Option A was built and validated in a scratch worktree at ea57360912ed; the others are untested alternatives.

+11/-5 in WritableQueryIndex.java. Java monitors are reentrant, so the nested commitWithoutNotify call takes the same lock again; the listeners still run after the lock is released. With the fix the race resolves to a serial order: in the pinned test, register then delete (q gone).

public void deleteQueries(List<String> ids) throws IOException {
  synchronized (commitLock) {
    for (String id : ids) {
      writer.deleteDocuments(new Term(FIELDS.query_id, id));
    }
    commitWithoutNotify(Collections.emptyList());
  }
  listeners.forEach(l -> l.afterDelete(ids));
}
// clear(): the same, with writer.deleteAll() inside synchronized (commitLock)

Trade-offs

• Little concurrency is lost: both methods already wait for commitLock right after the delete; only the delete call itself moves inside, so a concurrent register or purge swap now waits for it. A term delete is cheap; deleteAll() waits for in-flight indexing, aborts buffered documents and merges and waits for pending flushes (IndexWriter.java:2750-2815).
• The Monitor-level lock order stays commitLock then purgeLock, as in commitWithoutNotify and purgeCache. match() takes only purgeLock's read lock briefly (WritableQueryIndex.java:211-217), so searches do not wait. The IndexWriter locks deleteAll() now takes under commitLock were not analysed beyond the module's tests.
• It does not serialize close(), which still closes and commits the IndexWriter outside commitLock (WritableQueryIndex.java:321-325).
• Does not touch the other Monitor bugs (a duplicate id in one register call, a failed register not rolled back), which have other fix sites.

What the checks validated

• Stock: the pinned test fails with assertions on and off; the stress test fails 2 of 2 (30/400, 42/400).
• Fixed: the pinned test passes with assertions on and off and with 20 iterations; the stress test passes with assertions on and off (0/400 and 0/400, printed with -Ptests.verbose=true in the fact check's re-run); TestCachePurging, TestMonitor, TestMonitorPersistence and TestConcurrentQueryLoader pass with 5 iterations; the whole :lucene:monitor:test passes with assertions on and off (34 classes, 184 tests, 1 skipped, 0 failures); :lucene:monitor:check -x test passes.
• Earlier checks: the same fix shadowed into the stock jars passes the pinned test with fresh seeds and the stress test (0 of 400), and all upstream monitor tests pass with it.

Give commitWithoutNotify the ids to delete and a "delete all" flag, so that all IndexWriter writes of the Monitor happen in one place under commitLock.

Trade-offs

Same behaviour as the recommended option and harder to break later, but a bigger diff.

In commitWithoutNotify, replace "delete the id, then one addDocument per part" with one writer.updateDocuments(new Term(FIELDS.query_id, id), docsOfId) per id, which "Atomically deletes documents matching the provided delTerm and adds a block of documents" (IndexWriter.java:1594-1596).

Trade-offs

By reading DocumentsWriterPerThread.java:265-301, 361-383, a concurrent term delete then hits all of the block or none of it, which would fix deleteById; but clear()'s deleteAll() would still run outside the lock, it changes how the query index is written, and it was not run.

Javadoc only.

Trade-offs

Leaves the silent partial state for anyone who misses the note, while the fix is a few lines.

Method: TLA+ model checking (TLC, a tla2tools development build of 2026-10-20, rev d2e1472), a hunt unit (Monitor lifecycle and persistence, 2026-10-26).

• The finder modelled WritableQueryIndex's write protocol with query decomposition, concurrent deleteById and clear, non-aborting add failures and close plus reopen, with a model-to-Java line map. The invariant AtomicQuery (in every idle state each id's indexed documents are none or all parts of one version) is violated for register concurrent with deleteById (counterexample after 359 distinct states) and with clear (after 454). A fixed variant holds (13,440 distinct states); it models a combined fix (the deletes under commitLock plus one atomic updateDocuments per query and cache puts after the add), not the recommended fix alone, which the Java tests validated.
• Caveat: the model was written while reading the code, and the kept TLC logs are dated after the candidate was first recorded, so the hypothesis may have come from the code reading ("semi-formal" in the hunt's terms).
• The same unit's exhaustive single-threaded model-based tests (every script of up to 4 operations checked after every step, every 5-operation script at its final state) found 0 mismatches: the bug needs two threads.