Add concurrent close-during-read regression tests (#279) - #294
Closed
bernardladenthin wants to merge 1 commit into
Closed
Conversation
Cover the intermittent close-during-read SIGSEGV (lmdbjava#253/lmdbjava#279) end-to-end against native LMDB with safe close enabled. The existing RefCounter tests stress the counter in isolation (mock env) and the EnvTest safeClose tests are single-threaded, so neither exercises Env.close() racing in-flight reads on other threads through the real Env/Txn/Cursor wiring. Two tests hammer txnRead()/Dbi.get (and a cursor variant) from many threads while another thread races Env.close(); a resource held on the test thread makes the fail-fast EnvInUseException deterministic, and the assertions prove close() never unmaps while readers are live, readers only ever see AlreadyClosedException, and the env closes cleanly once readers stop. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T9DLq5CoaZA9eB6fbgrW4d
This was referenced Aug 10, 2026
Closed
Collaborator
|
Thanks @bernardladenthin. I have manually added your tests in to save dealing with the conflicts |
Author
|
@at055612 Ty! Perfect! |
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
2 participants
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.
Targets your
gh-279-env-close-protectionbranch (#293) — additive, tests only, no production changes.While comparing our two approaches I noticed a coverage gap worth filling regardless of which implementation lands: nothing currently exercises
Env.close()racing in-flight reads on other threads through the realEnv/Txn/Cursorwiring.RefCounterTeststresses the counter thoroughly but against a mockObjectenv (no native LMDB), and theEnvTestsafeClose tests (closeWithOpenReadTxn, etc.) are single-threaded. So a wiring regression — e.g. acquiring the ref counter aftermdb_txn_begininstead of before — would leave every existing test green while reintroducing themdb_txn_renew0SIGSEGV from #253/#279.This adds two tests to
EnvTest, written to your fail-fast semantics:closeDuringConcurrentReads_isRejectedWhileReadersLiveAndSurvives— 16 threads hammertxnRead()/Dbi.getwhile another racesclose().closeDuringConcurrentCursorReads_isRejectedWhileCursorsLiveAndSurvives— same, with aCursorper txn, covering the new cursor tracking.Each holds one resource on the test thread so the
EnvInUseExceptionis deterministic, then assertsclose()never unmaps while readers are live, readers only ever observeAlreadyClosedException, and the env closes cleanly once readers stop. Onmasterthese crash the JVM; on your branch they pass. Ported from my #289 stress test.Note: I left
RefCounterTest.javauntouched even though it currently tripsfmt:checkon your branch (unrelated to this change) — didn't want to add noise here.🤖 Generated with Claude Code