IndexReader#decRef javadoc says the refCount is unchanged on exception, but a failing final decRef leaves the reader closed at refCount 0

The javadoc of IndexReader#decRef says "If the refCount drops to 0, then this reader is closed. If an exception is hit, the refCount is unchanged." The method decrements first; at 0 it marks the reader closed and runs doClose() with the closed-listener notification in a try-with-resources, and nothing puts the count back if either throws. So after a final decRef() that throws, getRefCount() is 0, the reader is closed, and a retried decRef() throws AlreadyClosedException.

The code is the intended side. The sentence was written in 2009 (LUCENE-2156) together with a "Put reference back on failure" block; LUCENE-5553 (#6615, first released in 4.7.1) removed that block on purpose so that a failing close really closes the reader and still notifies its closed listeners, and left the sentence. The fix is javadoc-only. It affects the documentation in every release from 4.7.1 to 10.6.0 and on main.

1. The javadoc. IndexReader.java:207-213: "Expert: decreases the refCount of this IndexReader instance. If the refCount drops to 0, then this reader is closed. If an exception is hit, the refCount is unchanged." and "@throws IOException in case an IOException occurs in doClose()".
2. The code (IndexReader.java:215-232): :218-220 throws AlreadyClosedException if the count is already <= 0 (here the count really is unchanged); :222 refCount.decrementAndGet(); :223-227 at 0: closed = true; then try (Closeable _ = this::notifyReaderClosedListeners) { doClose(); }; :228-231 below 0: IllegalStateException ("too many decRef calls"). No line restores the count, so an exception from doClose() or from a listener leaves 0.
3. Every listener is still called. StandardDirectoryReader.java:668-672 and SegmentReader.java:318-322 notify the listeners through IOUtils.applyToAll, so one that throws does not skip the others; the try-with-resources runs the notification also when doClose() throws (if both throw, the listener's exception is attached to the doClose() one as suppressed). The base notifyReaderClosedListeners() is a no-op (:153-155); only these two readers override it.
4. Afterwards the reader is closed for good. ensureOpen() throws AlreadyClosedException when the count is <= 0 (:238-241); a retried decRef() hits :218-220; close() is a no-op because closed is already true (:351-356). So most reader operations (for example terms()) fail. A few accessors skip ensureOpen() on purpose and still answer: getRefCount(), "so that callers can see refCount is 0 (reader is closed)" (:159-160), and numDocs() / maxDoc() (BaseCompositeReader.java:141-166, SegmentReader.java:234-243).
5. Both triggers are legal. ClosedListener.onClose(CacheKey key) throws IOException (:141-146, @lucene.experimental); doClose() is a protected hook declared to throw IOException (:358-359); Lucene's own TestIndexReaderClose uses both a throwing doClose() and throwing listeners.

An empty commit, DirectoryReader.open (count 1), one closed listener that throws IOException. decRef(): the count goes to 0, closed is set, doClose() succeeds, then the listener throws and decRef() rethrows its IOException. getRefCount() returns 0; the javadoc promises 1.

• Who. Code that drives reader reference counting itself through the Expert incRef/decRef API (or close(), which calls decRef()) and, after the final release throws, follows the javadoc: it retries decRef(), or keeps the reader (for example in its own pool) believing it still holds a reference.
• When. Only when closing a reader throws: a closed listener that throws, a reader subclass whose doClose() throws, or an I/O error while index files are released. All uncommon.
• What goes wrong. A retried decRef() throws AlreadyClosedException at once, and so do most later reader operations; a few metadata accessors (numDocs(), maxDoc()) skip that check on purpose. No wrong result, hang or extra resource use was found; whatever a failing doClose() did not release stays unreleased whichever way the javadoc reads. The @throws clause names only doClose(), so the javadoc also hides that a listener can be the source.
• Scope of the repro. It covers one StandardDirectoryReader; composite readers close or decRef() their children in their own doClose() with their own exception handling, which this page does not examine.
• Default vs opt-in. No configuration changes the behaviour; the text is on a public final method of the core reader class, read by anyone writing reader-lifecycle code.
• Scale and age. Every release from 4.7.1 (2014) to 10.6.0 and main (a few untagged 8.x releases inferred); the recorded searches found no report of the mismatch.

• Deterministic. The test code supplies no random input (ByteBuffersDirectory, new IndexWriterConfig()), though LuceneTestCase still randomizes its environment (locale, time zone); seeds DEADBEEF and 5EED1234 give the same failure, with assertions on and off.
• It transcribes the current javadoc. Under the recommended javadoc fix it does not pass; it becomes obsolete, because the documented contract becomes "stays closed at refCount 0".
• As a regression test for the javadoc fix (optional for a javadoc-only change): this same test expecting 0 instead of 1, with the comment "decRef javadoc (fixed): the reader stays closed (refCount 0) even if closing it throws" and the method name testReaderStaysClosedAfterFailedDecRef; the body is otherwise the same. It passes with the javadoc fix (re-run with its final wording), with assertions on (together with TestIndexReaderClose and TestReaderClosed, 5 tests) and off (1 test). It pins the behaviour, so a change that restored the count would be caught.
• Releases. The same file fails unchanged on the 10.6.0, 10.5.2 and 9.12.3 release jars. Older releases lack the org.apache.lucene.tests framework; there the study used public-API main() probes (Version history).

cp $REPRO/TestDecRefExceptionRefCount.java $LUCENE/lucene/core/src/test/org/apache/lucene/index/
cd $LUCENE
./gradlew :lucene:core:test --tests org.apache.lucene.index.TestDecRefExceptionRefCount -Ptests.seed=DEADBEEF -Ptests.asserts=true
# assertions off: -Ptests.asserts=false (same failure)

# gradle, main 645d4fe14a20, seed DEADBEEF, -Ptests.asserts=true
TestDecRefExceptionRefCount > testRefCountUnchangedAfterFailedDecRef FAILED
    java.lang.AssertionError: refCount after the failed decRef expected:<1> but was:<0>
        at __randomizedtesting.SeedInfo.seed([DEADBEEF:EBBFFB376FC65C1D]:0)
        at org.junit.Assert.fail(Assert.java:89)
        at org.junit.Assert.failNotEquals(Assert.java:835)
        at org.junit.Assert.assertEquals(Assert.java:647)
        at org.apache.lucene.index.TestDecRefExceptionRefCount.testRefCountUnchangedAfterFailedDecRef(TestDecRefExceptionRefCount.java:38)
:lucene:core:test (FAILURE): 1 test, 1  failure
# -Ptests.asserts=false: the same failure
# stock main, TestIndexReaderClose + TestReaderClosed: 4 tests, BUILD SUCCESSFUL
# javadoc fix (option A), a comment-only change: this test still fails the same way;
#   the same test expecting 0 passes (with the two suites: 5 tests, BUILD SUCCESSFUL)

• Written for the removed code. LUCENE-2156 (dc3f92e6e58c, Michael McCandless, 2009-12-16, first in 3.1) added the sentence in the same hunk as boolean success = false; try { ... doClose(); success = true; } finally { if (!success) { // Put reference back on failure refCount.incrementAndGet(); } }.
• The change that made it false was deliberate. LUCENE-5553 / #6615, "IndexReader#ReaderClosedListener is not always called on IndexReader#close()" (Simon Willnauer, 2014-03-25), replaced the put-back with closed = true and unconditional listener notification. In the issue Simon Willnauer wrote that they "dropped putting the reference back since this should really just close everything and hand the exception back", and Michael McCandless gave "+1 for the patch and to change to 'close really closes and throws first exc' behavior". Nobody in the issue mentions the javadoc.
• The new behaviour is tested. TestIndexReaderClose.testCloseUnderException, added by LUCENE-5553, expects IllegalStateException from close() when doClose() or a listener throws (TestIndexReaderClose.java:89), may then call close() again, which must not throw (:99-101), and checks that every counting listener was notified despite the failure (:102): "close really closes".
• Later edits kept the sentence. LUCENE-8215 (2018), the 2020 reformat (which only re-wrapped it), #14302 (2025, which renamed the try-with-resources variables) and #15002 (2025) all touched decRef.

Strongest case for "not a defect" (the study's intended-behaviour devil's advocate and the credibility audit): (1) the sentence can be read as covering only the already-closed pre-check (IndexReader.java:218-220), where the count really is unchanged; (2) it is a cosmetic nit in an Expert API: the behaviour is intended, a caller who trusts the sentence and retries gets AlreadyClosedException at once, and no complaint since 2014 was found.

Why it fails. (1) History rules out the narrow reading: the sentence was written in the same hunk as the put-back block it described, and the @throws line under it names doClose(), the path that block covered. (2) It is a concrete, false statement about the reader's state after an exception, on the method whose callers must write the exception handling, and it is read exactly then; the fix is two javadoc lines with no behaviour change. Upstream may still call it a cosmetic javadoc cleanup.

Javadoc only; the behaviour is right and must not change. Both javadoc options were applied in a scratch worktree of main 645d4fe14a20 and checked with gradle.

Say that the reader stays closed (refCount 0) even if closing it throws, and name closed listeners in @throws. It makes no promise about closed listeners, which not every reader keeps or notifies itself.

   * Expert: decreases the refCount of this IndexReader instance. If the refCount drops to 0, then
-  * this reader is closed. If an exception is hit, the refCount is unchanged.
+  * this reader is closed. It stays closed (refCount 0) even if closing it throws an exception.
   *
-  * @throws IOException in case an IOException occurs in doClose()
+  * @throws IOException in case an IOException occurs in doClose() or in a closed listener

Trade-offs

• No behaviour change. It tells callers what they need after a failed final release (the reader is gone: do not retry, do not reuse).
• No listener promise on purpose: the base notifyReaderClosedListeners() is a no-op, only StandardDirectoryReader and SegmentReader override it, and readers such as MultiReader return a null cache helper or delegate to a sub-reader. An earlier draft said "its closed listeners are still notified"; it was dropped after review.

What the checks validated

• Gradle, scratch worktree of 645d4fe14a20, seed DEADBEEF, this exact wording: :lucene:core:checkGoogleJavaFormat and :lucene:core:renderJavadoc pass, and the rendered IndexReader.html shows the new text.
• The minimal test in its corrected form (expecting 0) together with TestIndexReaderClose (3 tests) and TestReaderClosed (1 test): 5 tests pass with assertions on; the corrected test also passes with assertions off.
• The minimal test as written was re-run with the fix and still fails (expected:<1> but was:<0>), as it must for a comment-only change.
• Not run: gradlew check or the whole core test suite (the change is inside a javadoc comment).

Drop "If an exception is hit, the refCount is unchanged." and leave the rest.

-  * this reader is closed. If an exception is hit, the refCount is unchanged.
+  * this reader is closed.

Trade-offs

• The smallest change that removes the false promise; it leaves @throws incomplete and does not tell callers what state the reader is in after a failure.

What the checks validated

• Same worktree: :lucene:core:checkGoogleJavaFormat passes. Nothing else was run (a comment-only change).

Putting the reference back on failure would make the sentence true, but it would bring back the pre-4.7.1 state that LUCENE-5553 removed on purpose: a partly closed reader that still looks open, so a retried decRef() would run doClose() again (read from the 4.7.0 code). Related: facet TaxonomyReader.decRef still puts the reference back on failure, and its javadoc makes no promise either way; a different class, not part of this finding.

Trade-offs

• Reverts a deliberate design decision; not proposed.

What the checks validated

• Not run.

• Lead. Code reading of IndexReader.decRef's exception edge in a hunt unit (reader and shared-arena reference counting), while modelling it in TLA+. The model produced no counterexample for this; the finding is the code reading plus the executed test.
• Witness. A public-API JUnit test on the stock jars that asserts the javadoc sentence, failing on main with assertions on and off and on 10.5.2 and 9.12.3; later trimmed to the one-method test shown here.
• Verification. An independent reproducer (3 tests: listener path, doClose() path, control), a cross-model reviewer (OpenAI GPT-5.6, reading only), an intended-behaviour devil's advocate, an adjudication, version-history runs with public-API probes from 4.6.1 to main, and a credibility audit.