callstack / callstack/agent-device
fix(host-kit): process lock proves ownership before release and never evicts a live owner
- Dominant language
- TypeScript
- Stars
- 4.6k
- Forks
- 299
- Avg merge
- 10h 17m
- Merged PRs (30d)
- 515
Description
## Defects in `packages/host-kit/src/internal/process-lock.ts` (verified at `eefe37b51e`)
- Release does an unconditional `fs.rmSync` (`:43`) without proving the caller still owns the lock.
- Stale-clear is check-then-act (`:70-95`): two contenders can both read "stale" and both proceed.
- `readProcessLockOwner` (`:97-103`) swallows every error, so a transiently unreadable `owner.json` for a live holder is indistinguishable from "not yet written" and ages out after `ownerGraceMs` (5 s).
Consumers (all release via the returned closure; none releases a lock it did not acquire): runner cache/lease/host/device-set, snapshot-source host (Apple snapshot bridge), managed-allocation `store-lock.ts`, `device-claim-store.ts`, web `agent-browser-tool.ts`, `swift-cache.ts`. Two holders of the runner cache lock or the allocation store lock can race a derived-data write or double-allocate a device. `process-lock.test.ts` has 5 cases; none covers ownership on release or an unreadable live owner. Unrelated to #2105 (claims layer).
## Fix
- Release checks `owner.json` pid equals the acquiring pid before `rmSync`.
- "Not written" means `ENOENT` only; any other read error is "live, unknown" and never routes to mtime deletion.
- Stale clear: rename the lock dir to a unique name, then remove it, so one contender wins. On `EPERM`/`EACCES`/`ENOTEMPTY` (win32 open handles) fall back to the existing `rmSync(force)`; document that stale-clear atomicity is best-effort on win32. The `mkdir` `EEXIST` arbiter, unchanged, provides exclusion.
Lock semantics only; no refactor in this PR.
## Done when
New tests: release by a non-owner pid is a no-op; a live owner with a malformed `owner.json` is not evicted; two concurrent stale-clears yield exactly one acquirer.
Contributor guide
Assessment
This issue has not been assessed yet.