Repository navigation
fix: every lock removal happens under a removal lock, so two stale breaks cannot both take the lock - #344
Merged
Conversation
…eaks cannot both take the lock acquireLock broke a stale lock with an unconditional rm after a stat. Two waiters that both saw the stale file both removed it, and the second rm landed after the first waiter had re-created the file: both held the lock (#342, found by a Broad-Side defect run on this repository, confirmed by verify). Removals — stale breaks and releases — now run under `<lock>.break` and re-check what they remove there; since a lock file can only be created while the path is free, what a remover verified is what it removes. An abandoned removal lock is cleared after BREAK_LOCK_STALE_MS; a release that cannot take it in time falls back to the token-checked removal alone. Closes #342 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2 tasks done
This was referenced Sep 14, 2026
Merged
TheAmericanMaker
added a commit
that referenced
this pull request
Sep 16, 2026
One shared lock file, broken by removing and re-creating its path, is a race between any two waiters that saw it stale (#344 closed one level of it and left the next), and a fixed 60 s age broke live holders — a publish across a full reindex reopened the version race #240 fixed. Each waiter now owns a ticket beside the lock, refreshed on a timer while it waits and holds; the holder is the first ticket whose owner is alive and refreshing; a dead owner's ticket is removed at once and a hung one's after STALE_LOCK_MS, each by its own unique name. Lamport's doorway keeps a waiter from being overtaken between taking its number and writing it. A pre-existing plain lock file is honoured while fresh. Closes #355 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
TheAmericanMaker
added a commit
that referenced
this pull request
Sep 16, 2026
One shared lock file, broken by removing and re-creating its path, is a race between any two waiters that saw it stale (#344 closed one level of it and left the next), and a fixed 60 s age broke live holders — a publish across a full reindex reopened the version race #240 fixed. Each waiter now owns a ticket beside the lock, refreshed on a timer while it waits and holds; the holder is the first ticket whose owner is alive and refreshing; a dead owner's ticket is removed at once and a hung one's after STALE_LOCK_MS, each by its own unique name. Lamport's doorway keeps a waiter from being overtaken between taking its number and writing it. A pre-existing plain lock file is honoured while fresh. Closes #355 Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
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.
Summary
Closes #342.
acquireLockbroke a lock older thanSTALE_LOCK_MSwithstat→rm. Two waiters that both saw the stale file both removed it; the secondrmlanded after the first waiter had re-created the file, so both held the lock. Every waiter polls on the same 125 ms cadence, so "two waiters see it stale in the same tick" is the normal case, not a corner.Design (the issue's rename-then-rm idea does not close it — a rename takes whatever inode is at the path at that moment, a fresh lock included):
<lock>.break(O_EXCL, held across one stat/read and one rm).wx), so nothing can be created between a remover's check and its rm, and only removal-lock holders remove: what was verified is what is removed. This also closes the narrower race where the original holder's late release and a stale break overlapped.BREAK_LOCK_STALE_MS(=LOCK_TIMEOUT_MS, 5 s; it is held for milliseconds). A release that cannot take the removal lock withinLOCK_TIMEOUT_MSfalls back to the token-checked removal alone — the guarantee before this change, never less.acquireLock's API and the lock file format are unchanged;BREAK_LOCK_STALE_MSis exported beside the other constants.Test plan
tests/state-store.test.mjs, four new tests, all failing onmain: a waiter that finds a stale lock being broken by someone else does not remove it (and does not remove the fresh lock that replaces it either); a release waits for a break in progress; an abandoned removal lock is cleared; eight waiters on one stale lock — exactly one breaks it, never two holders at once, no files left behind.npm run build+npm test— 911 pass.🤖 Generated with Claude Code