Found during adversarial validation of #906 (Design v2, "Adjacent defects"), and deliberately kept out of that PR. Verified against main at e2e57a7c.
Bug
Stealing a stale lock is unlink-then-create, not compare-and-swap, in both lock classes:
src/lib/locks/checkout-lock.ts — acquire() calls unlinkSafe(lockPath) when staleness() reports stale, then falls through to writeAtomic.src/lib/locks/lock-manager.ts — same shape.
The create half is genuinely atomic (openSync(lockPath, "wx"), i.e. O_CREAT|O_EXCL). The steal is not, because the unlink and the create are separate syscalls with a window between them.
Interleaving
A: reads lock L0, classifies stale
B: reads lock L0, classifies stale # both agree — L0 really is stale
A: unlink(L0)
A: open("wx") -> succeeds, writes L_A # A now holds a FRESH lock
B: unlink(L_A) # B removes A's fresh lock, not L0
B: open("wx") -> succeeds, writes L_B
Result: both A and B believe they hold the lock; only L_B exists on disk.
B's unlink targets a path, not the inode it classified, so it destroys whatever is there — including a lock created microseconds earlier by a winner that already moved on.
Severity
Low-moderate. Requires two sessions to classify the same lock stale within one syscall window, so it needs a genuinely abandoned lock plus near-simultaneous acquires. But the consequence is the exact failure the locks exist to prevent — two holders — and it is silent.
Fix sketch
Make the steal a compare-and-swap: rename() the stale lock to a unique temp name and treat "my rename won" as the right to create. Only the process whose rename succeeded proceeds to openSync("wx"); the loser re-reads and either accepts the new holder or retries. rename is atomic on POSIX and the loser's rename fails with ENOENT, which distinguishes it cleanly.
Alternatively, drop the unlink entirely and have the stale path use openSync(tmp, "wx") + rename(tmp, lockPath) — last-writer-wins, but never leaves the window where two processes both think they created the file.
Test plan
The existing multi-process race test cannot pin this — CLI startup serializes the processes long before the syscall window (documented in checkout-lock.test.ts's atomic-create block, which reaches for writeAtomic directly for the same reason). Drive the window directly: two CheckoutLock instances, both past the staleness read, interleaved by hand.
Found during adversarial validation of #906 (Design v2, "Adjacent defects"), and deliberately kept out of that PR. Verified against
mainate2e57a7c.Bug
Stealing a stale lock is unlink-then-create, not compare-and-swap, in both lock classes:
src/lib/locks/checkout-lock.ts—acquire()callsunlinkSafe(lockPath)whenstaleness()reports stale, then falls through towriteAtomic.src/lib/locks/lock-manager.ts— same shape.The create half is genuinely atomic (
openSync(lockPath, "wx"), i.e.O_CREAT|O_EXCL). The steal is not, because the unlink and the create are separate syscalls with a window between them.Interleaving
B's unlink targets a path, not the inode it classified, so it destroys whatever is there — including a lock created microseconds earlier by a winner that already moved on.
Severity
Low-moderate. Requires two sessions to classify the same lock stale within one syscall window, so it needs a genuinely abandoned lock plus near-simultaneous acquires. But the consequence is the exact failure the locks exist to prevent — two holders — and it is silent.
Fix sketch
Make the steal a compare-and-swap:
rename()the stale lock to a unique temp name and treat "my rename won" as the right to create. Only the process whose rename succeeded proceeds toopenSync("wx"); the loser re-reads and either accepts the new holder or retries.renameis atomic on POSIX and the loser's rename fails withENOENT, which distinguishes it cleanly.Alternatively, drop the unlink entirely and have the stale path use
openSync(tmp, "wx")+rename(tmp, lockPath)— last-writer-wins, but never leaves the window where two processes both think they created the file.Test plan
The existing multi-process race test cannot pin this — CLI startup serializes the processes long before the syscall window (documented in
checkout-lock.test.ts's atomic-create block, which reaches forwriteAtomicdirectly for the same reason). Drive the window directly: twoCheckoutLockinstances, both past the staleness read, interleaved by hand.