Skip to content

fix(fsUtils): fail the critical section on a compromised lock, not a timer throw - #100

Merged
1 commit merged into
masterfrom
fix/bind-registry-lock-compromise
Oct 8, 2026
Merged

1 commit merged into
masterfrom
fix/bind-registry-lock-compromise

Conversation

@valthon

@valthon valthon commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

Summary

withFileLock left proper-lockfile's default onCompromised in place, and that default re-throws from the library's own unref'd mtime-refresh timer. When a held lock's directory disappears under a live holder, the refresh's stat fails with ENOENT, proper-lockfile rewrites the error in place to ECOMPROMISED (keeping the ENOENT … wire-cluster-ports.lock.lock text), and the throw lands as an uncaught exception with no relationship to the caller that owns the lock. Under jest that fails whichever test happens to be running, which is why the bind-registry signature kept surfacing in a different suite each time (ClusterBuildDefaultsEpochBootstrap, NodeConfig, OperatorFunding, ClusterConfigProvider) and passed on retry.

The fix is the handler and nothing else:

  • withFileLock owns onCompromised: the in-flight critical section rejects with a NestedError carrying proper-lockfile's error as the cause, so the call that lost exclusivity is the call that fails. The section itself is not cancelled and its side effects are not undone; the public JSDoc says so.
  • A section that fails after the compromise already decided the outcome is logged at warn instead of vanishing. An ordinary section failure is not logged, since the caller receives it.
  • proper-lockfile's options (stale, update, retries) are unchanged from master. Every observed compromise on this host was the lock directory being deleted under a live holder (a test aborted at its 120 s budget while its fixture cleanup removed the sandboxed registry dir); the stale-steal signature appeared zero times, so no threshold tuning is warranted here.
  • The stale root jest.config.ts comment that still described the timer throw is updated.

Diff: 3 files, +161/−8. Mechanism verified against proper-lockfile 4.1.2 source and reproduced with a standalone probe before the fix.

Verification

Check Result
pnpm build, pnpm run lint clean
pnpm test (full, idle host, on master 2d703304) 229 suites, 2080 tests, all passing; zero lock.lock signatures in the run
tests/utils/fsUtils.test.ts 11 passing, including a deterministic compromise reproduction (lock dir removed under a held lock) that fails with the exact uncaught timer throw when the handler is removed

🤖 Generated with Claude Code

https://claude.ai/code/session_01CNDguHHWTwn1UBejDiTAGa

…timer throw

Mechanism, verified against [email protected] `lib/lockfile.js`:
`updateLock` refreshes a held lock's mtime on an `unref`'d timer; when that
`stat`/`utimes` ENOENTs it calls `setLockAsCompromised` with
`Object.assign(err, { code: 'ECOMPROMISED' })` — the error is rewritten IN
PLACE, so its `ENOENT … stat '…/<target>.lock'` text survives. The DEFAULT
`onCompromised` is `(err) => { throw err }`, i.e. an uncaught exception raised
from that timer callback, with no call-stack relationship to the code holding
the lock: under jest it fails whatever test happens to be running, while the
caller still inside the critical section is never told it lost exclusivity.
`setLockAsCompromised` also marks the holder released before invoking the
handler, which is why `release()` then rejects `ERELEASED` (already handled).

The fix: `withFileLock` owns `onCompromised`. It logs, then rejects a
`Deferred` the call races against the section, so the compromise fails THAT
call with a `NestedError` carrying proper-lockfile's error as the cause and
`{ lockPath }` as context. A compromise landing after the section already
settled is swallowed; a section that fails after a compromise decided the
outcome is logged at warn instead of vanishing. The section itself cannot be
cancelled — the public JSDoc now says so, since its side effects still land.

Deliberately NOT changed: `stale`, `update` and `retries` stay exactly as
master had them. Across four full cluster-tool runs the compromise signature
appeared 18–68 times while `Unable to update lock within the stale threshold`
— the signature a stale STEAL produces — appeared ZERO times: every observed
compromise was the lock directory being deleted under a live holder, so the
handler, not the threshold, is what fixes the reported failure. Raising
`stale` would only lengthen how long a crashed holder wedges every contender.

Gate on an idle host: `pnpm build` clean, `pnpm run lint` clean,
`pnpm --filter @wireio/cluster-tool test` 138/138 suites, 1505/1505 tests,
with no `.lock.lock` signature anywhere in the run. The new compromise test
fails against the unpatched handler (uncaught `ENOENT … .lock` from the
refresh timer; the call never settles), and the root `jest.config.ts` note
about that hook throwing is updated to what happens now.

Co-Authored-By: Claude Fable 5.1 <[email protected]>
Claude-Session: https://claude.ai/code/session_01CNDguHHWTwn1UBejDiTAGa
@valthon

valthon commented Sep 17, 2026

Copy link
Copy Markdown
Contributor Author

CI note: the Build and test failure here is master's, not this PR's. The job installs the published @wireio/sdk-core (1.0.89 in the log), which predates the setscorecfg / rank_score types #94 introduced, so packages/cluster-tool prepare fails in SystemContractSteps.ts and producerRankUtils.ts before any test runs. master's own last run on 2d703304 (this PR's base) fails with the identical TS2724 / TS2339 set; this PR touches only fsUtils.ts, its test and jest.config.ts. It clears once wire-libraries-ts publishes the sdk-core release carrying those types. Local gate on the same base, against the sibling-linked sources: 229 suites, 2080 tests, all passing.

valthon added a commit that referenced this pull request Oct 2, 2026
Combine owning-call compromise failures with PR #76's registry-scoped
external mutex and 30-second short-lock stale threshold. Preserve the
long-lock policy and document that lock loss cannot cancel ongoing work.

Retain faster wildcard port probing with IPv6 checks, early range rejection,
two Jest workers, and a supervised full commit gate. Isolate the validator
environment test and keep all test ports in the bind registry.

Cover real lock refresh and mutual exclusion after registry deletion,
late completion after compromise, network-family collisions, and watchdog
termination of descendants.
@valthon valthon closed this pull request by merging all changes into master in fd8352d Oct 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant