[#304] Throw PersistitClosedException when a commit races Persistit.close()#305
Merged
vharseko merged 1 commit intoJul 24, 2026
Conversation
…it races Persistit.close() A transaction commit racing Persistit.close() could reach JournalManager.waitForDurability after close() cleared the JOURNAL_FLUSHER reference and fail with a raw IllegalStateException, bypassing callers' closed-state handling (seen as a TransactionTest2.transactionsConcurrentWithPersistitClose flake). A commit that instead captured the flusher reference just before it was cleared would poll forever against the stopped thread. Throw PersistitClosedException in both cases: when the flusher reference is already null, and inside the JournalFlusher wait loop once the flusher is stopped and its final durability marks cannot cover the caller's timestamp. The final marks are published before the flusher stops, so a single re-read after observing the stop distinguishes a durable commit from one that can never be confirmed. New JournalManagerTest cases cover both windows; without the fix the first fails with the IllegalStateException and the second hangs past its join deadline. Fixes OpenIdentityPlatform#304
maximthomas
approved these changes
Jul 24, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #304
Problem
A transaction commit racing
Persistit.close()has two failure windows inJournalManager.waitForDurability:IllegalStateException:close()nulls the_flusherreference before the committing thread reads it, sowaitForDurabilitythrowsIllegalStateException("JOURNAL_FLUSHER is not running"). That is not aPersistitException, so callers' closed-state handling is bypassed — observed as theTransactionTest2.transactionsConcurrentWithPersistitCloseflake on master run 30078532797, where the worker landed in the test'scatch (Throwable)branch and the stranded-thread assert failed.JournalFlusher.waitForDurabilitypolling loop; once the flusher stops, its durability marks never advance, and aflushedTimestampallocated after the flusher's final cycle can never be covered — the loop kicks a stopped thread forever. Confirmed real: the new test hangs past a 10-second join deadline without the fix.Fix
Throw
PersistitClosedExceptionin both windows:JournalManager.waitForDurabilitythrowsPersistitClosedExceptioninstead ofIllegalStateExceptionwhen the flusher reference is already null, so a commit racingclose()unwinds through the same documented path as every other closed-state operation.JournalFlusher.waitForDurabilitypolling loop checksisStopped()after the success check. The flusher publishes its final_startTimestamp/_endTimestampbefore stopping, so a single re-read after observing the stop is exact: either the final flush cycle covered the caller's timestamp (return success) or no future cycle ever will (throwPersistitClosedException).TransactionTest2needs no changes:PersistitClosedExceptionis already one of its expected shutdown outcomes.Testing
JournalManagerTestcases, one per window:waitForDurabilityAfterCloseThrowsPersistitClosedException(direct call afterclose()) andwaitForDurabilityOnStoppedFlusherThrowsInsteadOfSpinning(viacrash(), which stops the flusher without clearing the reference). Both verified to fail without the fix — the first with the rawIllegalStateException, the second by hanging past its join deadline.persistit/coresuite on JDK 11: 592 tests, 0 failures, 0 errors, 5 skipped.