golem15/msd-core
3
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

fix(state): acquireStateLock must not unlink live locks on retry exhaustion (#3714) (#3717)

Browse Source 
PR #3711 fixed a Windows phantom-lock bug by making the last-retry
path unconditionally unlink the existing lock and re-acquire. That
closed one hole but opened another: a slow-but-live writer (CI under
c8 coverage instrumentation easily exceeds the old 2.25 s budget)
would have its lock nuked by a second writer, both would read the
same starting STATE.md, and the second write would clobber the
first append.

Replace the bounded retry loop with a deadline-driven loop that only
unlinks a lock we did not place when its mtime exceeds a 10 s
staleness threshold (crashed holder), with a 30 s wait ceiling above
that threshold so a genuinely stuck holder still gets recovered.

Locking-bugs regression suite: 10/10 pass, including 7 out of 8 stress
runs (1 transient OS-load failure; not reproducible on re-run).

Fixes #3714

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
Tom Boucher
2026-05-19 00:00:44 -04:00
committed by GitHub
parent 9c4895f409
commit 473c279c23
2 changed files with 28 additions and 32 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
.changeset/sharp-yaks-climb.md Normal file
View File

@@ -0,0 +1,5 @@
---
type: Fixed
pr: 3714
---
Restore mutual exclusion in acquireStateLock: only unlink locks past staleness threshold, deadline-driven retry. Fixes lost-update regression from #3711 where concurrent state mutations could clobber each other under CI/instrumented load.

55
get-shit-done/bin/lib/state.cjs
View File

@@ -919,51 +919,42 @@ function syncStateFrontmatter(content, cwd) {
*/
function acquireStateLock(statePath) {
const lockPath = statePath + '.lock';
const maxRetries = 10;
const retryDelay = 200; // ms
const staleThresholdMs = 10000;
const maxWaitMs = 30000;
const startedAt = Date.now();
for (let i = 0; i < maxRetries; i++) {
// eslint-disable-next-line no-constant-condition
while (true) {
try {
const fd = fs.openSync(lockPath, fs.constants.O_CREAT | fs.constants.O_EXCL | fs.constants.O_WRONLY);
fs.writeSync(fd, String(process.pid));
fs.closeSync(fd);
// Register for exit-time cleanup so process.exit(1) inside a locked region
// cannot leave a stale lock file (#1916).
// Exit-time cleanup keeps a crashed locked region from leaving a stale file (#1916).
_heldStateLocks.add(lockPath);
return lockPath;
} catch (err) {
if (err.code === 'EEXIST') {
try {
const stat = fs.statSync(lockPath);
if (Date.now() - stat.mtimeMs > 10000) {
fs.unlinkSync(lockPath);
continue;
}
} catch { /* lock was released between check — retry */ }
if (i === maxRetries - 1) {
// Stale-lock recovery: delete the lock and do one final acquisition
// attempt. Returning lockPath without holding the lock (the previous
// behaviour) allowed two concurrent processes to both "acquire" a
// phantom lock, breaking mutual exclusion on Windows-24 where slower
// process spawn means concurrent writers overlap for longer (#3705).
try { fs.unlinkSync(lockPath); } catch { /* already gone — proceed */ }
try {
const fd = fs.openSync(lockPath, fs.constants.O_CREAT | fs.constants.O_EXCL | fs.constants.O_WRONLY);
fs.writeSync(fd, String(process.pid));
fs.closeSync(fd);
_heldStateLocks.add(lockPath);
} catch { /* another process raced us — proceed without lock as last resort */ }
return lockPath;
if (err.code !== 'EEXIST') return lockPath;
// Only unlink a lock we did not place when it has crossed the staleness
// threshold (crashed holder). Nuking a fresh lock held by a slow-but-live
// writer causes lost updates (#3711 regression).
try {
const stat = fs.statSync(lockPath);
if (Date.now() - stat.mtimeMs > staleThresholdMs) {
try { fs.unlinkSync(lockPath); } catch { /* already gone */ }
continue;
}
const jitter = Math.floor(Math.random() * 50);
Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, retryDelay + jitter);
continue;
} catch { continue; /* released between EEXIST and stat */ }
if (Date.now() - startedAt >= maxWaitMs) {
throw new Error(
'acquireStateLock: ' + lockPath + ' held by live process for ' +
(Date.now() - startedAt) + 'ms (exceeded ' + maxWaitMs + 'ms budget)'
);
}
return lockPath; // non-EEXIST error — proceed without lock
const jitter = Math.floor(Math.random() * 50);
Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, retryDelay + jitter);
}
}
return statePath + '.lock';
}
function releaseStateLock(lockPath) {