state.db WAL lock guard follows the handle lifecycle: reopen, checkpoint-vs-close, fd reuse (review of #110544) - #110872
Merged
Merged
Conversation
Three gaps in the #110544 guard, all reported in its review and reproduced: - A writer reopened by _reopen_after_close_locked (teardown/worker race, #94736) came back with no guard: the next stray close + foreign close deleted its WAL again. - _try_wal_checkpoint refreshed the guard outside self._lock; landing after close() it pinned an OFD lock with no connection behind it, so a foreign `PRAGMA journal_mode=DELETE` saw `database is locked` forever. - Refcounts keyed on (fd, inode) treated a recycled fd number as a surviving lock: A+B live, close A, C reuses A's fd, close B left C recorded as guarded while a foreign EXCLUSIVE succeeded. The guard now counts handles per inode, re-locks every matching descriptor on each hold (OFD re-lock is idempotent), and unlocks on the last handle only; the reopen path holds it; the checkpoint refresh runs under self._lock and skips a closed handle. The macOS holder scan folds case so a case-only alias of the sidecar path on APFS still matches.
alt-glitch
added
type/bug
This was referenced Sep 14, 2026
Closed
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.
The WAL lock guard from #110544 now follows the lifecycle of the handle it protects: a reopened writer is guarded again, a checkpoint refresh can no longer pin a lock after
close(), and a recycled descriptor number is never mistaken for a surviving lock.All three gaps were raised in the #110544 review by @andrexibiza and @ehz0ah before it merged and were not addressed there; this PR fixes them forward, with each one reproduced first.
Changes
hermes_state_lockguard.py: ownership is counted per inode, not per(fd, inode). Everyhold()re-locks every matching descriptor (OFD re-lock on an already-locked description is idempotent), so a new descriptor that reused an old number gets its own lock;release()drops this handle's claim and unlocks only when it was the last handle on that inode.Heldis now{inode: range}.hermes_state.py::_reopen_after_close_locked: the reopened writer takes the guard like a first open.hermes_state.py::_try_wal_checkpoint: the guard refresh runs underself._lockand is skipped when_conn is None, so a refresh landing afterclose()no longer leaves an OFD lock with no connection behind it.hermes_state_dbfile.py::_iter_darwin_sidecar_holders: sidecar paths are compared case-folded;os.path.normcaseis the identity on darwin while APFS/HFS+ are case-insensitive, so a case-only alias of the path was missed.Root cause
The first cut modeled the guard as a property of a descriptor number and refreshed it without the connection lock; the guard is a property of the handle's lifetime.
Validation
close()→append_message()reopen: guard present, foreign EXCLUSIVE refused{}, EXCLUSIVE acquired_HANDLESemptytests/hermes_state/test_wal_lock_guard_lifecycle.py(2 invariants)tests/hermes_state/,tests/test_hermes_state.py, ledger/delegation fd-leak suitesTestFTS5Search::test_search_projection_skips_context_enrichment_queries, fails identically on unpatched main)The checkpoint-vs-close race is covered by construction (refresh under
self._lock,_conn is Nonecheck) rather than a timing test; the macOS case-fold change needs a darwin host to exercise and is not covered by a new test here.Refs #100896 #103339. Follow-up to #110544.
Infographic