cherry-pick(file-lock): make file-lock actually lock and throw on contention
Cherry-pick of gsd-build/gsd-2 a09e01640 — withFileLockSync now actually acquires a proper-lockfile (was previously a no-op when proper-lockfile wasn't required) and throws on ELOCKED contention by default. Adds onLocked: 'skip' option for best-effort callers that tolerate dropped entries (audit, journal). Modernizes import style (createRequire/join from imports rather than ad-hoc require). Path-renames preserved (gsd-pi → sf-run). Co-Authored-By: Jeremy <jeremy@fluxlabs.net> Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
22d4579690
commit
bf727173e7
2 changed files with 172 additions and 25 deletions
|
|
@ -1,12 +1,21 @@
|
|||
import { existsSync } from "node:fs";
|
||||
import { createRequire } from "node:module";
|
||||
import { join } from "node:path";
|
||||
|
||||
function _require(name: string) {
|
||||
// The file-lock module is loaded in both CJS builds and ESM sources. Under ESM
|
||||
// the bare `require` identifier is not defined, so we always go through
|
||||
// createRequire. We try the current module's resolution context first and fall
|
||||
// back to the installed sf-run package if we are running from a consumer
|
||||
// project that does not hoist proper-lockfile.
|
||||
const localRequire = createRequire(import.meta.url);
|
||||
|
||||
function _require(name: string): any {
|
||||
try {
|
||||
return require(name);
|
||||
return localRequire(name);
|
||||
} catch {
|
||||
try {
|
||||
const sfPiRequire = require("module").createRequire(
|
||||
require("path").join(process.cwd(), "node_modules", "sf-run", "index.js")
|
||||
const sfPiRequire = createRequire(
|
||||
join(process.cwd(), "node_modules", "sf-run", "index.js"),
|
||||
);
|
||||
return sfPiRequire(name);
|
||||
} catch {
|
||||
|
|
@ -15,43 +24,107 @@ function _require(name: string) {
|
|||
}
|
||||
}
|
||||
|
||||
export function withFileLockSync<T>(filePath: string, fn: () => T): T {
|
||||
export type OnLocked = "fail" | "skip";
|
||||
|
||||
export interface FileLockOptions {
|
||||
/**
|
||||
* Behavior when the lock cannot be acquired after retries (ELOCKED).
|
||||
* - "fail" (default): rethrow the ELOCKED error so the caller can react.
|
||||
* - "skip": run fn() unlocked. Only choose this for best-effort writes
|
||||
* that genuinely tolerate contention (e.g. high-frequency audit appends
|
||||
* where dropping one entry is acceptable). Silent unlocked execution was
|
||||
* the legacy behavior and is a correctness hazard for shared state.
|
||||
*/
|
||||
onLocked?: OnLocked;
|
||||
/** proper-lockfile retries (default 5). */
|
||||
retries?: number;
|
||||
/** proper-lockfile stale threshold in ms (default 10000). */
|
||||
stale?: number;
|
||||
}
|
||||
|
||||
const DEFAULT_RETRIES = 5;
|
||||
const DEFAULT_STALE_MS = 10000;
|
||||
const SYNC_RETRY_DELAY_MS = 50;
|
||||
|
||||
// Block the thread for `ms` milliseconds without spinning the CPU.
|
||||
// Used by the sync lock retry loop, since proper-lockfile's lockSync does not
|
||||
// accept a `retries` option (only the async `lock` does).
|
||||
function sleepSync(ms: number): void {
|
||||
if (ms <= 0) return;
|
||||
Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, ms);
|
||||
}
|
||||
|
||||
function acquireLockSyncWithRetry(
|
||||
lockfile: any,
|
||||
filePath: string,
|
||||
retries: number,
|
||||
stale: number,
|
||||
): () => void {
|
||||
let lastErr: any;
|
||||
for (let attempt = 0; attempt <= retries; attempt++) {
|
||||
try {
|
||||
return lockfile.lockSync(filePath, { stale });
|
||||
} catch (err: any) {
|
||||
lastErr = err;
|
||||
if (err?.code !== "ELOCKED") throw err;
|
||||
if (attempt < retries) sleepSync(SYNC_RETRY_DELAY_MS);
|
||||
}
|
||||
}
|
||||
throw lastErr;
|
||||
}
|
||||
|
||||
export function withFileLockSync<T>(
|
||||
filePath: string,
|
||||
fn: () => T,
|
||||
opts: FileLockOptions = {},
|
||||
): T {
|
||||
const lockfile = _require("proper-lockfile");
|
||||
if (!lockfile) return fn();
|
||||
|
||||
if (!existsSync(filePath)) return fn();
|
||||
|
||||
const retries = opts.retries ?? DEFAULT_RETRIES;
|
||||
const stale = opts.stale ?? DEFAULT_STALE_MS;
|
||||
const onLocked: OnLocked = opts.onLocked ?? "fail";
|
||||
|
||||
try {
|
||||
const release = lockfile.lockSync(filePath, { retries: 5, stale: 10000 });
|
||||
const release = acquireLockSyncWithRetry(lockfile, filePath, retries, stale);
|
||||
try {
|
||||
return fn();
|
||||
} finally {
|
||||
release();
|
||||
}
|
||||
} catch (err: any) {
|
||||
if (err.code === "ELOCKED") {
|
||||
// Could not get lock after retries, let's fallback to un-locked instead of crashing the whole state machine
|
||||
if (err?.code === "ELOCKED" && onLocked === "skip") {
|
||||
return fn();
|
||||
}
|
||||
throw err;
|
||||
}
|
||||
}
|
||||
|
||||
export async function withFileLock<T>(filePath: string, fn: () => Promise<T> | T): Promise<T> {
|
||||
export async function withFileLock<T>(
|
||||
filePath: string,
|
||||
fn: () => Promise<T> | T,
|
||||
opts: FileLockOptions = {},
|
||||
): Promise<T> {
|
||||
const lockfile = _require("proper-lockfile");
|
||||
if (!lockfile) return await fn();
|
||||
|
||||
if (!existsSync(filePath)) return await fn();
|
||||
|
||||
const retries = opts.retries ?? DEFAULT_RETRIES;
|
||||
const stale = opts.stale ?? DEFAULT_STALE_MS;
|
||||
const onLocked: OnLocked = opts.onLocked ?? "fail";
|
||||
|
||||
try {
|
||||
const release = await lockfile.lock(filePath, { retries: 5, stale: 10000 });
|
||||
const release = await lockfile.lock(filePath, { retries, stale });
|
||||
try {
|
||||
return await fn();
|
||||
} finally {
|
||||
await release();
|
||||
}
|
||||
} catch (err: any) {
|
||||
if (err.code === "ELOCKED") {
|
||||
if (err?.code === "ELOCKED" && onLocked === "skip") {
|
||||
return await fn();
|
||||
}
|
||||
throw err;
|
||||
|
|
|
|||
|
|
@ -52,7 +52,7 @@ test("withFileLock: executes callback when file does not exist", async () => {
|
|||
}
|
||||
});
|
||||
|
||||
test("withFileLockSync: falls back to unlocked callback on ELOCKED", () => {
|
||||
test("withFileLockSync: throws ELOCKED by default (no silent fallback)", () => {
|
||||
if (!hasProperLockfile() || process.platform === "win32") {
|
||||
return;
|
||||
}
|
||||
|
|
@ -65,19 +65,56 @@ test("withFileLockSync: falls back to unlocked callback on ELOCKED", () => {
|
|||
const release = lockfile.lockSync(filePath, { retries: 0, stale: 10000 });
|
||||
try {
|
||||
let called = 0;
|
||||
const result = withFileLockSync(filePath, () => {
|
||||
called++;
|
||||
return "fallback-ok";
|
||||
});
|
||||
assert.equal(result, "fallback-ok");
|
||||
assert.equal(called, 1, "callback should run even when lock acquisition fails");
|
||||
assert.throws(
|
||||
() => {
|
||||
withFileLockSync(
|
||||
filePath,
|
||||
() => {
|
||||
called++;
|
||||
return "should-not-return";
|
||||
},
|
||||
{ retries: 0 },
|
||||
);
|
||||
},
|
||||
(err: any) => err?.code === "ELOCKED",
|
||||
);
|
||||
assert.equal(called, 0, "callback must not run when lock cannot be acquired");
|
||||
} finally {
|
||||
release();
|
||||
rmSync(dir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
test("withFileLock: falls back to unlocked callback on ELOCKED", async () => {
|
||||
test("withFileLockSync: onLocked=\"skip\" runs callback unlocked on ELOCKED", () => {
|
||||
if (!hasProperLockfile() || process.platform === "win32") {
|
||||
return;
|
||||
}
|
||||
|
||||
const lockfile = require("proper-lockfile");
|
||||
const dir = mkdtempSync(join(tmpdir(), "gsd-file-lock-test-"));
|
||||
const filePath = join(dir, "locked.jsonl");
|
||||
writeFileSync(filePath, "{}\n", "utf-8");
|
||||
|
||||
const release = lockfile.lockSync(filePath, { retries: 0, stale: 10000 });
|
||||
try {
|
||||
let called = 0;
|
||||
const result = withFileLockSync(
|
||||
filePath,
|
||||
() => {
|
||||
called++;
|
||||
return "fallback-ok";
|
||||
},
|
||||
{ retries: 0, onLocked: "skip" },
|
||||
);
|
||||
assert.equal(result, "fallback-ok");
|
||||
assert.equal(called, 1, "callback should run when onLocked is skip");
|
||||
} finally {
|
||||
release();
|
||||
rmSync(dir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
test("withFileLock: throws ELOCKED by default (no silent fallback)", async () => {
|
||||
if (!hasProperLockfile() || process.platform === "win32") {
|
||||
return;
|
||||
}
|
||||
|
|
@ -90,12 +127,49 @@ test("withFileLock: falls back to unlocked callback on ELOCKED", async () => {
|
|||
const release = await lockfile.lock(filePath, { retries: 0, stale: 10000 });
|
||||
try {
|
||||
let called = 0;
|
||||
const result = await withFileLock(filePath, async () => {
|
||||
called++;
|
||||
return "fallback-ok";
|
||||
});
|
||||
assert.equal(result, "fallback-ok");
|
||||
assert.equal(called, 1, "callback should run even when lock acquisition fails");
|
||||
await assert.rejects(
|
||||
async () => {
|
||||
await withFileLock(
|
||||
filePath,
|
||||
async () => {
|
||||
called++;
|
||||
return "should-not-return";
|
||||
},
|
||||
{ retries: 0 },
|
||||
);
|
||||
},
|
||||
(err: any) => err?.code === "ELOCKED",
|
||||
);
|
||||
assert.equal(called, 0, "callback must not run when lock cannot be acquired");
|
||||
} finally {
|
||||
await release();
|
||||
rmSync(dir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
test("withFileLock: onLocked=\"skip\" runs callback unlocked on ELOCKED", async () => {
|
||||
if (!hasProperLockfile() || process.platform === "win32") {
|
||||
return;
|
||||
}
|
||||
|
||||
const lockfile = require("proper-lockfile");
|
||||
const dir = mkdtempSync(join(tmpdir(), "gsd-file-lock-test-"));
|
||||
const filePath = join(dir, "locked.jsonl");
|
||||
writeFileSync(filePath, "{}\n", "utf-8");
|
||||
|
||||
const release = await lockfile.lock(filePath, { retries: 0, stale: 10000 });
|
||||
try {
|
||||
let called = 0;
|
||||
const result = await withFileLock(
|
||||
filePath,
|
||||
async () => {
|
||||
called++;
|
||||
return "fallback-ok";
|
||||
},
|
||||
{ retries: 0, onLocked: "skip" },
|
||||
);
|
||||
assert.equal(result, "fallback-ok");
|
||||
assert.equal(called, 1, "callback should run when onLocked is skip");
|
||||
} finally {
|
||||
await release();
|
||||
rmSync(dir, { recursive: true, force: true });
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue