Skip to content

fix(lock): sleep and report the open error when no guard was ever seen - #227

Merged
askalf merged 1 commit into
masterfrom
fix/audit-2026-09-26
Sep 26, 2026
Merged

askalf merged 1 commit into
masterfrom
fix/audit-2026-09-26

Conversation

@askalf

@askalf askalf commented Sep 26, 2026

Copy link
Copy Markdown
Owner

Follow-up to #197.

Problem

On a read-only lock directory, open(guard, 'wx') fails with EPERM/EACCES (treated as contention) and the following stat(guard) returns ENOENT because the guard was never created. The age === null branch did continue, skipping sleepSync, so acquire() busy-looped for the full 10 s window (measured with a deny-write ACL: 10002 ms elapsed, 9985 ms CPU; about 160k open attempts under injection). It then threw "timed out waiting for another truecopy process ... delete it if no truecopy is running" about a guard that never existed.

Fix

  • The age === null branch falls through to the same sleepSync(20) as every other wait.
  • acquire() tracks whether a guard was ever observed (EEXIST from open, or a successful stat) and the last open error. At the deadline, if no guard was observed, it throws the original open error instead of the holder timeout. The EEXIST-then-ENOENT race still ends in the timeout message.

Tests

test/lock-guard-windows.test.mjs: two new cases (EACCES, EPERM) inject the error on open(wx) with the real stat, and assert the original code surfaces (not "timed out waiting") and that open is attempted fewer than 100 times in a 300 ms window. Both fail on main with the phantom-holder timeout.

Full suite: 277 tests, 273 pass, 0 fail, 4 skipped (Windows 11, Node).

On a read-only lock directory open(wx) fails with EPERM/EACCES and the
following stat finds no guard (ENOENT). The age === null branch retried
without sleeping, so acquire() busy-looped a full core for the whole
10 s window and then reported a timeout waiting for another truecopy
process about a guard that never existed.

That branch now sleeps like every other wait. acquire() records whether
a guard was ever observed (EEXIST from open or a successful stat) and
the last open error; at the deadline with no guard observed it throws
the open error instead of the phantom-holder timeout.

Tests inject EACCES and EPERM on open(wx) with the real stat and assert
the original error surfaces and open is attempted fewer than 100 times
in a 300 ms window.
@github-actions github-actions Bot added lockfile Lock file, hashing, signing, keychain tests Test suite and CI size/M 50-199 hand-written lines labels Sep 26, 2026

@sprayberry-redline sprayberry-redline left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: approve. The age === null branch now falls through to the shared 20 ms sleep instead of continuing, and acquire() reports the original open error at the deadline when no guard was ever observed, while an EEXIST that races away still yields the holder timeout. Traced both paths through the loop; the new EACCES/EPERM tests fail on the previous code (which threw the timeout message) and the attempt-count bound proves the sleep.

@askalf
askalf merged commit 21cd7b2 into master Sep 26, 2026
23 checks passed
@askalf
askalf deleted the fix/audit-2026-09-26 branch September 26, 2026 14:17
@askalf askalf mentioned this pull request Oct 29, 2026
askalf added a commit that referenced this pull request Oct 29, 2026
Bumps the version to 0.10.5. On merge, auto-release.yml tags v0.10.5,
cuts the GitHub release from the new CHANGELOG section and dispatches
publish.yml to npm.

Ships the --version fix (#211), the read-only lock directory fix (#227,
now in the CHANGELOG) and the README rewrite (#316), which npm shows
only from a release. The pinned version in the docs and issue templates
moves with it; docker/ stays on 0.10.4 until 0.10.5 is on npm.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lockfile Lock file, hashing, signing, keychain size/M 50-199 hand-written lines tests Test suite and CI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants