Skip to content

fix(wallet): recover a session lock left without a pid - #146

Open
rayasa07 wants to merge 1 commit into
tempoxyz:mainfrom
rayasa07:fix/recover-unreadable-session-lock
Open

rayasa07 wants to merge 1 commit into
tempoxyz:mainfrom
rayasa07:fix/recover-unreadable-session-lock

Conversation

@rayasa07

Copy link
Copy Markdown

withSessionLock creates the lock with open(path, "wx") and records the pid as a separate step:

const handle = await open(path, "wx");
try {
  await handle.writeFile(`${process.pid}\n${new Date().toISOString()}\n`);

A holder that dies between those two statements leaves the lock file in place and empty. removeStaleLock then has no pid to check against a running process, and Number("") is 0, so it takes the pid <= 0 branch and reports the lock as live:

const pid = Number(text.split("\n")[0]);
if (!Number.isInteger(pid) || pid <= 0) return false;

From then on every tempo wallet run touching that origin spins for the full 30 seconds and fails with Timed out waiting for session lock: …, with no recovery short of deleting the file by hand. A truncated or otherwise unparseable first line behaves the same way.

Fix

Route the no-usable-pid case to removeAbandonedLock, which releases the lock once its mtime is older than a 5s grace period. The grace matters: the same empty file is legitimate for the moment between open and writeFile, so deleting it on sight would let two holders run at once. A lock that does carry a pid is untouched and still checked with process.kill(pid, 0).

This trades the permanent wedge for the usual lock-timeout assumption — that a holder cannot stall for more than the grace period between two adjacent statements. Closing the window entirely would mean linking the pid file into place instead of writing it after creation, which is a larger change and would still need this recovery path for a truncated lock.

Tests

Adds test/session-lock.test.ts: the empty lock and the unparseable-pid lock are recovered; a just-created empty lock is left alone and its holder still runs; a dead pid is still reclaimed; a live pid is still respected; the lock is removed after the callback. Ages are set with utimes so the cases are deterministic and the file runs in ~1.4s.

The two recovery tests fail on main, each hitting the vitest timeout while the loop runs down its 30s deadline; the other four pass either way and are there to pin the exclusion behaviour.

Validation

vitest run test/session-lock.test.ts — 6 passed. pnpm typecheck, pnpm test:types, pnpm check:lint and pnpm changelog:validate are clean.

I could not use the full suite as a signal: 10 of its files fail on my machine on an unmodified main for environmental reasons — spawn sqlite3 ENOENT (no sqlite3 available here, and I cannot install it) plus 10s timeouts — and the same 10 files fail with and without this change. pnpm check:format also fails on main, on .agents/friction-log/20260831180632-immutable-releases-were/friction.md, which this branch does not touch.

withSessionLock creates the lock with open(path, "wx") and records the pid as a
separate step, so a holder that died in between left an empty file. Stale-lock
recovery skipped it because there was no pid to check, and every later run on
that origin waited out the 30s deadline and failed with no way back. Release a
lock carrying no usable pid once it is older than a short grace period, so one
that is only momentarily empty still belongs to its live holder.

This branch has not been deployed

No deployments
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