Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 34 additions & 0 deletions src/site-memory/file-lock.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,40 @@ describe('site memory file lock', () => {
.resolves.toBe('written');
});

it('does not steal a stale lock from a same-host owner that is still alive', async () => {
const target = await tempTarget();
const lockPath = lockPathFor(target);
await writeFile(lockPath, `${JSON.stringify({ pid: process.pid, host: hostname(), token: 'still-working' })}\n`);
const past = new Date(Date.now() - 60_000);
await utimes(lockPath, past, past);

await expect(withFileLock(target, async () => 'written', { staleMs: 10_000, timeoutMs: 50 }))
.rejects.toMatchObject({ code: 'SITE_MEMORY_BUSY' });

await expect(readFile(lockPath, 'utf8')).resolves.toContain('still-working');
});

it('does not let two critical sections overlap when one runs longer than staleMs', async () => {
const target = await tempTarget();
let insideFirst = false;
let overlapped = false;

const first = withFileLock(target, async () => {
insideFirst = true;
await new Promise((resolve) => { setTimeout(resolve, 120); });
insideFirst = false;
}, { staleMs: 30, timeoutMs: 5_000 });

await new Promise((resolve) => { setTimeout(resolve, 60); });

const second = withFileLock(target, async () => {
if (insideFirst) overlapped = true;
}, { staleMs: 30, timeoutMs: 5_000 });

await Promise.all([first, second]);
expect(overlapped).toBe(false);
});

it('reports a live holder as SITE_MEMORY_BUSY instead of writing anyway', async () => {
const target = await tempTarget();
await writeFile(lockPathFor(target), `${JSON.stringify({ pid: process.pid, host: hostname(), token: 'held' })}\n`);
Expand Down
20 changes: 14 additions & 6 deletions src/site-memory/file-lock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,14 @@
* marker the filesystem can see: `open(..., 'wx')` creates the lock file only
* when it does not already exist, atomically, on every platform we support.
*
* Abandoned locks never wedge site memory. A lock whose owner process is gone
* is broken on the next attempt, and any lock older than `staleMs` is broken
* regardless — the critical section itself is a small read plus a rename, which
* takes milliseconds. `timeoutMs` is deliberately longer than `staleMs` so an
* abandoned lock is always broken rather than surfaced to the user as an error.
* Abandoned locks never wedge site memory. A lock whose owner process is
* confirmed gone is broken on the next attempt. A lock older than `staleMs`
* is broken too, but only when we cannot confirm the owner is still alive on
* this host — a same-host owner that `process.kill(pid, 0)` finds alive is
* still working, no matter how long its critical section has run, and is
* never stolen on staleness alone. `timeoutMs` is deliberately longer than
* `staleMs` so a genuinely abandoned lock is broken rather than surfaced to
* the user as an error.
*/
import { AsyncLocalStorage } from 'node:async_hooks';
import { randomUUID } from 'node:crypto';
Expand Down Expand Up @@ -116,8 +119,13 @@ async function breakIfAbandoned(lockPath: string, staleMs: number): Promise<bool
const before = await statOrUndefined(lockPath);
if (!before) return true;
const owner = await readOwner(lockPath);
const checkable = owner.host === hostname() && isActionablePid(owner.pid);
const ownerAlive = checkable && isPidAlive(owner.pid);
// A same-host owner we can confirm is still alive is still working, no matter
// how long its critical section has run — staleness alone never steals its lock.
if (ownerAlive) return false;
const ownerGone = checkable && !ownerAlive;
const expired = Date.now() - before.mtimeMs > staleMs;
const ownerGone = owner.host === hostname() && isActionablePid(owner.pid) && !isPidAlive(owner.pid);
if (!expired && !ownerGone) return false;

const after = await statOrUndefined(lockPath);
Expand Down
Loading