SearcherTaxonomyManager refresh throws "Self-suppression not permitted" and loses the taxonomy error when closing the new reader also fails

SearcherTaxonomyManager refreshes an IndexSearcher and its facet TaxonomyReader together: it reopens the index reader, then the taxonomy reader. If the taxonomy reopen throws, it closes the index reader it just opened and rethrows the taxonomy error, a cleanup added in 7.4.0 by LUCENE-8244 to stop a file-handle leak. When that close also throws, the cleanup calls t2.addSuppressed(t2) (SearcherTaxonomyManager.java:209) where t1.addSuppressed(t2) was meant. Java rejects self-suppression with IllegalArgumentException("Self-suppression not permitted"), so maybeRefresh() and maybeRefreshBlocking() throw that unchecked exception, caused by the close failure, instead of the documented IOException, and the taxonomy failure is lost from the whole chain. Nothing leaks, the index is not damaged and, once the faults are gone, the next refresh works; but code that catches only IOException misses it, and a scheduled refresh task that does so is cancelled, leaving its searcher stale. 7.3.1 passes, 7.4.0 fails and 7.4.0 with the one-token fix passes; every release executed (7.4.0, 7.6.0, 7.7.3, 8.0.0, 8.11.4, 9.0.0, 9.12.3, 10.0.0, 10.5.2) fails, and 10.6.0 and main have the same line. Low priority: it needs two failures in one refresh.

// SearcherTaxonomyManager.refreshIfNeeded, condensed
IndexReader newReader = DirectoryReader.openIfChanged(dr, refreshCommit);   // :195  index first
...
try {
  tr = TaxonomyReader.openIfChanged(ref.taxonomyReader);                     // :204  taxonomy: throws t1
} catch (Throwable t1) {
  try {
    IOUtils.close(newReader);                                                // :207  close: throws t2
  } catch (Throwable t2) {
    t2.addSuppressed(t2);                                                    // :209  always throws
  }
  throw t1;                                                                  // :211  skipped
}

1. ReferenceManager.doMaybeRefresh calls refreshIfNeeded (ReferenceManager.java:167). Line 195 reopens the index reader; line 204 reopens the taxonomy reader, which throws t1.
2. Line 207 closes the new index reader. When that throws t2, line 209 calls t2.addSuppressed(t2).
3. Throwable.addSuppressed starts with if (exception == this) throw new IllegalArgumentException(SELF_SUPPRESSION_MESSAGE, exception); (JDK Throwable.java:1099-1101; javadoc: "a throwable cannot suppress itself", since Java 7). So line 209 always throws, with t2 as the cause.
4. That exception leaves the inner catch, so throw t1 is skipped and t1 is dropped. It is unchecked, so it passes refreshIfNeeded's throws IOException and comes out of maybeRefresh / maybeRefreshBlocking.
5. doMaybeRefresh's finally blocks release the current reference, notify listeners and unlock (ReferenceManager.java:180-187): the manager is unchanged and usable.

What the caller gets on main, with assertions on and off:

java.lang.IllegalArgumentException: Self-suppression not permitted
    at java.base/java.lang.Throwable.addSuppressed(Throwable.java:1101)
    at org.apache.lucene.facet.taxonomy.SearcherTaxonomyManager.refreshIfNeeded(SearcherTaxonomyManager.java:209)
    ...
Caused by: java.io.IOException: simulated close failure        <- the close failure (t2)
    ...
                                                                <- IndexNotFoundException (t1): nowhere

(The arrows are ours.) A search of Lucene's Java sources for the same-identifier pattern x.addSuppressed(x) finds no other place.

• Who. Applications that use taxonomy facets through SearcherTaxonomyManager, with any of its constructors, on 7.4.0 through 10.6.0 and main.
• When. Only when one refresh suffers two failures at once: the taxonomy reopen throws (an I/O error, an unreadable taxonomy commit, or any other Throwable) and closing the index segment files opened moments earlier also throws. Each is an ordinary environmental fault, but the second is uncommon on its own, so the pair is rare; the recorded searches found no report of it.
• What goes wrong. An unchecked IllegalArgumentException instead of the documented IOException, and the root cause, the taxonomy failure, is lost from logs and diagnostics; the message points at the JDK, not at the taxonomy.
• Code that catches IOException misses it (executed on main with assertions on and off, 10.5.2 and 9.12.3): a ScheduledExecutorService task that calls maybeRefresh() every 5 ms and catches only IOException is cancelled by the JDK after the double-failure refresh (1 tick); in that setup, where nothing inspects the task's ScheduledFuture, nothing is logged, and a document deleted in the new commit is still returned until the application refreshes by hand. With only the taxonomy failure, the task catches one IOException and keeps running (93 ticks); with the fix it keeps running too (77 ticks).
• Lucene's own ControlledRealTimeReopenThread ends on any refresh IOException (it wraps it in a RuntimeException), so for it only the exception type and message change.
• By reading only: if the taxonomy failure is an Error, such as an OutOfMemoryError, and the close also fails, the caller gets the IllegalArgumentException instead of the Error.
• What it does not do: no leak (the new reader's files are closed) and no index damage. The failed refresh keeps the previous searcher, as any failed refresh does, and once the faults are gone a later refresh succeeds (executed with transient faults). Results go stale only while nothing refreshes, for example after the application's refresh task died on the unchecked exception (above).

One test class, one method, single thread, nothing random that matters; the only subclass is the test framework's MockDirectoryWrapper.Failure, no reader or manager class. The close failure fires on every file close inside SegmentReader.doClose (two in the test run, which reach the manager as one close failure, the second suppressed in the first), which during maybeRefreshBlocking() runs only when the manager closes the index reader it just reopened after the taxonomy reopen failed. The failure is cleared before cleanup.

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

# gradle, scratch worktree of 645d4fe14a20, seed DEADBEEF, -Ptests.asserts=true (module prefixes and some frames cut)
TestSearcherTaxonomyManagerDoubleFailure > testTaxonomyFailureSurvivesReaderCloseFailure FAILED
    junit.framework.AssertionFailedError: Unexpected exception type, expected IndexNotFoundException but got java.lang.IllegalArgumentException: Self-suppression not permitted
        at __randomizedtesting.SeedInfo.seed([DEADBEEF:C54840C3490FFE44]:0)
        ...
        at org.apache.lucene.facet.taxonomy.TestSearcherTaxonomyManagerDoubleFailure.testTaxonomyFailureSurvivesReaderCloseFailure(TestSearcherTaxonomyManagerDoubleFailure.java:61)

        Caused by:
        java.lang.IllegalArgumentException: Self-suppression not permitted
            at java.base/java.lang.Throwable.addSuppressed(Throwable.java:1101)
            at org.apache.lucene.facet.taxonomy.SearcherTaxonomyManager.refreshIfNeeded(SearcherTaxonomyManager.java:209)
            at org.apache.lucene.facet.taxonomy.SearcherTaxonomyManager.refreshIfNeeded(SearcherTaxonomyManager.java:40)
            at org.apache.lucene.search.ReferenceManager.doMaybeRefresh(ReferenceManager.java:167)
            at org.apache.lucene.search.ReferenceManager.maybeRefreshBlocking(ReferenceManager.java:240)
            ...

            Caused by:
            java.io.IOException: simulated close failure
                at org.apache.lucene.facet.taxonomy.TestSearcherTaxonomyManagerDoubleFailure$1.eval(TestSearcherTaxonomyManagerDoubleFailure.java:53)
                ...
                at org.apache.lucene.index.SegmentReader.doClose(SegmentReader.java:217)
                ...
                at org.apache.lucene.facet.taxonomy.SearcherTaxonomyManager.refreshIfNeeded(SearcherTaxonomyManager.java:207)
                ... 7 more

1 test completed, 1 failed

• The code states its intent. The block closes the new reader and then throw t1 (SearcherTaxonomyManager.java:211). t2.addSuppressed(t2) can never succeed (the JDK rejects it by contract), so no design calls it; the evident intent is t1.addSuppressed(t2).
• The contract. maybeRefresh and maybeRefreshBlocking: "@throws IOException if refreshing the resource causes an IOException" (ReferenceManager.java:203, :231); refreshIfNeeded: "@throws IOException if the refresh operation failed" (:72). Nothing documents an IllegalArgumentException.
• The test that came with this code. TestSearcherTaxonomyManager.testExceptionDuringRefresh (:343-366), added by the same LUCENE-8244 commit, deletes the taxonomy's newest segments_N and asserts expectThrows(IndexNotFoundException.class, mgr::maybeRefreshBlocking). Its close succeeds, and no upstream test makes that close fail.
• The codebase idiom. DirectoryTaxonomyReader.doOpenIfChanged handles the same situation with catch (Throwable t) { IOUtils.closeWhileSuppressingExceptions(t, r2); throw t; } (DirectoryTaxonomyReader.java:221-223).
• History. The line came in the LUCENE-8244 patch ("Do not leak open file descriptors in SearcherTaxonomyManager's refresh on exception", 7.4.0 Bug Fixes); its issue (#9291) holds only the patch note and two commit notices (the JIRA record adds the 2022 migration notice), so nobody discussed this line there. Before that fix the taxonomy failure propagated (and the reader leaked).

The strongest "works as intended or not worth it" case (the refute and intended-lens reviews and the devil's-advocate verifier): it needs two simultaneous failures; nothing leaks and the IllegalArgumentException still carries the close failure as its cause; the classes involved are @lucene.experimental: ReferenceManager (ReferenceManager.java:35), whose maybeRefresh javadoc is the contract quoted above, TaxonomyReader (TaxonomyReader.java:57) and DirectoryTaxonomyReader (DirectoryTaxonomyReader.java:61), while SearcherTaxonomyManager itself has no such tag; Java throws clauses are not exhaustive and no exception priority is documented for two failures at once; and the repro deletes an index file, while Lucene's SECURITY.md treats index files as trusted.

Why it fails: @lucene.experimental says the API may change between releases, not that a call which can never succeed is designed behaviour; none of this makes x.addSuppressed(x) meaningful, the block itself says throw t1, and the test that came with it asserts the taxonomy's exception. The deleted segments_N is the upstream test's own way to make the taxonomy reopen fail, and there Lucene asserts a clean IndexNotFoundException; an independent reproducer gets the same result with a plain injected IOException during DirectoryTaxonomyReader.doOpenIfChanged and no file deleted. Rarity and the unharmed state argue for Low priority, which is how it is classed, not for intent. The cross-model lenses rated the intended-behaviour argument weak; a later cross-model fact check rated it moderate, citing the stale searcher left by a dead refresh task, the experimental tags and fix B's Error handling, all three covered in this doc.

Both options were run through gradle in a scratch worktree of 645d4fe14a20 and both fix the repro. B is recommended for main; A is the smallest change and the one for branches that lack the helper. Both change only the double-failure path: a single taxonomy failure still propagates unchanged (the upstream testExceptionDuringRefresh passes with both).

In SearcherTaxonomyManager.refreshIfNeeded (SearcherTaxonomyManager.java:205-212), -5/+1.

       } catch (Throwable t1) {
-        try {
-          IOUtils.close(newReader);
-        } catch (Throwable t2) {
-          t2.addSuppressed(t2);
-        }
+        IOUtils.closeWhileSuppressingExceptions(t1, newReader);
         throw t1;
       }

Trade-offs

• The same code as the sibling DirectoryTaxonomyReader.java:221-223; it removes the hand-written block that held the typo.
• By reading IOUtils.java:114-137: the close failure is added to t1 as suppressed and t1 is rethrown; if the close throws an Error and t1 is not one, that Error is thrown instead, with t1 suppressed.
• The helper exists from 10.4.0 on (added by #14633; absent at the 10.3.2 and 9.12.3 tags), so a backport to an older branch would use A.

What the checks validated

• Stock 645d4fe14a20: the repro fails with assertions on and off.
• With B: the repro and TestSearcherTaxonomyManager pass with assertions on and off (7 tests, 0 failures, 1 skipped: testNRT is @Nightly); the whole :lucene:facet:test passes with assertions on (40 suites, 307 tests, 0 failures, 7 skipped).
• checkGoogleJavaFormat, ecjLintMain, ecjLintTest, forbiddenApisMain and forbiddenApisTest of :lucene:facet pass once the repro's long comment on line 60 is wrapped onto two lines, which the formatter asks for (comment only; the fixed class was not flagged).

In SearcherTaxonomyManager.java:209, +1/-1.

         } catch (Throwable t2) {
-          t2.addSuppressed(t2);
+          t1.addSuppressed(t2);
         }

Trade-offs

• The evident intent and the smallest change; it applies unchanged to every affected branch back to 7.4.
• An Error thrown by the close is attached to t1 as suppressed instead of being thrown; the hand-written block stays.

What the checks validated

• With A: the repro and TestSearcherTaxonomyManager pass with assertions on and off (7 tests, 0 failures, 1 skipped).
• Earlier checks ran it on main ea57360912ed (the finder's, the independent reproducer's and the stock-classes tests, the periodic-refresh probe: all pass) and on 7.4.0 (passes).

Method: code reading (the study's non-formal tier). A hunting AI tool working on the near-real-time reference managers wrote a TLA+ model of ReferenceManager and SearcherManager with refresh and reader-close failures, and read the refresh exception paths of the subclasses while doing so. It noticed t2.addSuppressed(t2) in SearcherTaxonomyManager.refreshIfNeeded.

• The model covers ReferenceManager and SearcherManager, not SearcherTaxonomyManager, and no TLC trace produced this signal; so the find is credited to code reading, not to the model.
• The finder confirmed it with a JUnit test that uses two protected extension points (a DirectoryTaxonomyReader subclass whose doOpenIfChanged throws, and a FilterDirectoryReader whose reopened instances fail in doClose).
• Independent checks then reproduced it without subclassing any reader: one with plain injected I/O errors on the public API, one with the upstream testExceptionDuringRefresh scenario plus injected close errors; that scenario became the minimal repro.