From 714dc37d864073f48bb6b443be68c412687d32a2 Mon Sep 17 00:00:00 2001 From: Yigtwxx Date: Sun, 2 Aug 2026 21:54:24 +0300 Subject: [PATCH 1/5] fix(lock): retry Windows lock files denied mid-teardown Windows denies access to a lock file while a just-unlinked directory entry is still being torn down, so a contended acquire observed EPERM on a name that was already gone. At the failure lstat reports ENOENT and a zero-delay retry opens the file, which makes this contention rather than a permission failure. acquire() only treated EEXIST as contention, so the transient EPERM escaped from two touch points: the exclusive create, and readSidecarLockSnapshot, which maps only ENOENT to a vanished lock. The repository already applies this Windows equivalence in replace-file.ts, json.ts, and move-path.ts; sidecar-lock.ts was the outlier. Retries are bounded so a genuine denial still surfaces as EPERM instead of degrading into a lock timeout. Locally this failed 8 times in 85 runs before the change and 0 times in 110 runs after it. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01GGksu4AaxVTcTHZntKTyAs --- CHANGELOG.md | 4 ++ scripts/check-file-size.mjs | 1 + src/sidecar-lock-policy.ts | 11 +++++ src/sidecar-lock.ts | 25 ++++++++-- test/sidecar-lock-regression.test.ts | 69 ++++++++++++++++++++++++++++ 5 files changed, 106 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 45dae4f..755429b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +### Security and Correctness + +- Retry a contended file-lock acquisition when Windows denies access to a lock file whose directory entry is still being torn down. Both the exclusive create and the holder's snapshot read reported that transient `EPERM` as a hard failure, so concurrent `acquireFileLock()` calls failed intermittently on Windows even though the very next attempt would have succeeded. Retries stay bounded, so a genuine permission denial still surfaces as `EPERM` rather than a lock timeout. + ### Features - Suffix Windows reserved basenames with `_` in `sanitizeUntrustedFileName()` while preserving case and extensions on every platform, including dollar names and superscript COM/LPT variants; thanks @SebTardif (#67). diff --git a/scripts/check-file-size.mjs b/scripts/check-file-size.mjs index 9a35075..c41c471 100644 --- a/scripts/check-file-size.mjs +++ b/scripts/check-file-size.mjs @@ -7,6 +7,7 @@ const LINE_BUDGETS = new Map([ ["src/permissions.ts", 566], ["src/root-impl.ts", 1750], ["src/root-path.ts", 862], + ["src/sidecar-lock.ts", 520], ["test/api-coverage.test.ts", 983], ["test/new-primitives.test.ts", 1500], ]); diff --git a/src/sidecar-lock-policy.ts b/src/sidecar-lock-policy.ts index e8fbef4..9f7b780 100644 --- a/src/sidecar-lock-policy.ts +++ b/src/sidecar-lock-policy.ts @@ -10,6 +10,17 @@ export function computeSidecarLockDelayMs(retry: SidecarLockRetryOptions, attemp return Math.min(maxTimeout, Math.round(base * jitter)); } +// Windows denies access to a lock file while a just-unlinked directory entry +// is still being torn down, so a contended acquire sees EPERM on a name that is +// already gone -- both when creating it exclusively and when reading the +// holder's snapshot. The next attempt succeeds, so this is contention rather +// than a permission failure. +export const maxTransientLockDenials = 8; + +export function isTransientLockFileDenial(error: unknown): boolean { + return process.platform === "win32" && (error as NodeJS.ErrnoException | null)?.code === "EPERM"; +} + export function sidecarLockPayloadIsStale( payload: unknown, staleMs: number, diff --git a/src/sidecar-lock.ts b/src/sidecar-lock.ts index e6f0c28..552d607 100644 --- a/src/sidecar-lock.ts +++ b/src/sidecar-lock.ts @@ -30,6 +30,8 @@ import type { import { computeSidecarLockDelayMs, defaultSidecarLockShouldReclaim, + isTransientLockFileDenial, + maxTransientLockDenials, } from "./sidecar-lock-policy.js"; export type { SidecarLockStaleSnapshot } from "./sidecar-lock-reclaim.js"; export type { @@ -261,6 +263,10 @@ export function createSidecarLockManager(key: string) { const reclaimGuardPath = `${lockPath}.reclaim`; let ownsReclaimGuard = false; let attempt = 0; + // Bounded so a genuine denial still surfaces as EPERM, not a lock timeout. + let transientDenials = 0; + const isRetryableDenial = (error: unknown): boolean => + isTransientLockFileDenial(error) && ++transientDenials <= maxTransientLockDenials; const waitForRetry = async (): Promise => { const elapsed = Date.now() - startedAt; if ( @@ -370,6 +376,10 @@ export function createSidecarLockManager(key: string) { parsePayload: options.parsePayload, }); } + if (isRetryableDenial(err)) { + await waitForRetry(); + continue; + } if ((err as { code?: unknown }).code !== "EEXIST") { throw err; } @@ -379,10 +389,17 @@ export function createSidecarLockManager(key: string) { continue; } const nowMs = Date.now(); - const snapshot = await readSidecarLockSnapshot(lockPath, { - lockRoot: options.lockRoot, - parsePayload: options.parsePayload, - }); + let snapshot: SidecarLockSnapshot | null; + try { + snapshot = await readSidecarLockSnapshot(lockPath, { + lockRoot: options.lockRoot, + parsePayload: options.parsePayload, + }); + } catch (readErr) { + if (!isRetryableDenial(readErr)) throw readErr; + await waitForRetry(); + continue; + } if (!snapshot) { continue; } diff --git a/test/sidecar-lock-regression.test.ts b/test/sidecar-lock-regression.test.ts index 4c87536..66af246 100644 --- a/test/sidecar-lock-regression.test.ts +++ b/test/sidecar-lock-regression.test.ts @@ -5,6 +5,7 @@ import { afterEach, describe, expect, it, vi } from "vitest"; import { fileStore } from "../src/file-store.js"; import { acquireFileLock } from "../src/file-lock.js"; import { configureFsSafeLocks, getFsSafeLockConfig } from "../src/lock-config.js"; +import { configureFsSafeNative } from "../src/native-config.js"; import { createSidecarLockManager } from "../src/sidecar-lock.js"; const tempDirs: string[] = []; @@ -17,6 +18,7 @@ async function tempRoot(prefix: string): Promise { afterEach(async () => { vi.restoreAllMocks(); + configureFsSafeNative({ mode: "auto" }); configureFsSafeLocks({ retry: undefined, staleMs: undefined, @@ -27,6 +29,73 @@ afterEach(async () => { }); describe("sidecar lock regressions", () => { + // Windows denies access to a lock file while a just-unlinked directory entry is + // still being torn down. Both touch points below observed it in CI as EPERM. + const denial = (lockPath: string) => + Object.assign( + new Error(`EPERM: operation not permitted, open '${lockPath}'`), + { code: "EPERM", errno: -4048, syscall: "open", path: lockPath }, + ); + + it.runIf(process.platform === "win32")("retries an exclusive create denied mid-teardown", async () => { + const base = await tempRoot("fs-safe-sidecar-eperm-create-"); + const targetPath = path.join(base, "state.json"); + const lockPath = `${targetPath}.lock`; + configureFsSafeNative({ mode: "off" }); + const realOpen = fsp.open.bind(fsp) as typeof fsp.open; + let injected = 0; + vi.spyOn(fsp, "open").mockImplementation((async (...args: Parameters) => { + if (args[0] === lockPath && args[1] === "wx" && injected === 0) { + injected += 1; + throw denial(lockPath); + } + return await realOpen(...args); + }) as typeof fsp.open); + + const lock = await acquireFileLock(targetPath, { + managerKey: `eperm-create-${Date.now()}-${Math.random()}`, + staleMs: 60_000, + timeoutMs: 1_000, + retry: { minTimeout: 1, maxTimeout: 2 }, + payload: async () => ({ pid: process.pid }), + }); + await lock.release(); + + expect(injected).toBe(1); + await expect(fsp.stat(lockPath)).rejects.toMatchObject({ code: "ENOENT" }); + }); + + it.runIf(process.platform === "win32")("retries a contended snapshot read denied mid-teardown", async () => { + const base = await tempRoot("fs-safe-sidecar-eperm-read-"); + const targetPath = path.join(base, "state.json"); + const lockPath = `${targetPath}.lock`; + configureFsSafeNative({ mode: "off" }); + await fsp.writeFile(lockPath, JSON.stringify({ pid: process.pid, createdAt: new Date().toISOString() })); + const realReadFile = fsp.readFile.bind(fsp) as typeof fsp.readFile; + let injected = 0; + vi.spyOn(fsp, "readFile").mockImplementation((async (...args: Parameters) => { + if (args[0] === lockPath && injected === 0) { + injected += 1; + // The holder's unlink lands while the reader is being denied. + await fsp.rm(lockPath, { force: true }); + throw denial(lockPath); + } + return await realReadFile(...args); + }) as typeof fsp.readFile); + + const lock = await acquireFileLock(targetPath, { + managerKey: `eperm-read-${Date.now()}-${Math.random()}`, + staleMs: 60_000, + timeoutMs: 1_000, + retry: { minTimeout: 1, maxTimeout: 2 }, + payload: async () => ({ pid: process.pid }), + }); + await lock.release(); + + expect(injected).toBe(1); + await expect(fsp.stat(lockPath)).rejects.toMatchObject({ code: "ENOENT" }); + }); + it("does not delete a fresh sidecar lock during stale reclaim or old release", async () => { const base = await tempRoot("fs-safe-sidecar-token-"); const targetPath = path.join(base, "state.json"); From 0dcf0c49ffd273ec629c54f4c0c5b36712e5bda9 Mon Sep 17 00:00:00 2001 From: Yigtwxx Date: Sun, 2 Aug 2026 21:59:58 +0300 Subject: [PATCH 2/5] test(lock): resolve the temp root before injecting the denial Both new tests held the raw mkdtemp() result and matched the injected path by string equality. Windows runners return an 8.3 short name from os.tmpdir() while acquire() realpaths the target, so the injection never matched the path the lock code used. The create case then saw zero injections and the read case fell through to the real snapshot, which read as a live lock and timed out. Follow the suite convention and realpath the root. Verified locally by pointing the same scenario at a directory junction, which reproduces the CI mismatch: the raw path records zero injections and the resolved path records one. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01GGksu4AaxVTcTHZntKTyAs --- test/sidecar-lock-regression.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/sidecar-lock-regression.test.ts b/test/sidecar-lock-regression.test.ts index 66af246..c2f64cd 100644 --- a/test/sidecar-lock-regression.test.ts +++ b/test/sidecar-lock-regression.test.ts @@ -38,7 +38,7 @@ describe("sidecar lock regressions", () => { ); it.runIf(process.platform === "win32")("retries an exclusive create denied mid-teardown", async () => { - const base = await tempRoot("fs-safe-sidecar-eperm-create-"); + const base = await fsp.realpath(await tempRoot("fs-safe-sidecar-eperm-create-")); const targetPath = path.join(base, "state.json"); const lockPath = `${targetPath}.lock`; configureFsSafeNative({ mode: "off" }); @@ -66,7 +66,7 @@ describe("sidecar lock regressions", () => { }); it.runIf(process.platform === "win32")("retries a contended snapshot read denied mid-teardown", async () => { - const base = await tempRoot("fs-safe-sidecar-eperm-read-"); + const base = await fsp.realpath(await tempRoot("fs-safe-sidecar-eperm-read-")); const targetPath = path.join(base, "state.json"); const lockPath = `${targetPath}.lock`; configureFsSafeNative({ mode: "off" }); From f02f9705230aaf85829dedf37a90587522588434 Mon Sep 17 00:00:00 2001 From: Yigtwxx Date: Sun, 2 Aug 2026 22:19:21 +0300 Subject: [PATCH 3/5] fix(lock): scope the denial retry to the lock-file operations The classification sat in the catch that wraps the whole acquisition body, which also covers options.payload(), lockRoot.create/open, the payload write, and the stat. A Windows EPERM raised by any of those was reclassified as contention, so the loop reran the caller's payload callback and, once the retry budget was spent, replaced the caller's error with file_lock_timeout. That is the opposite of the fail-closed contract. Classify at the two operations the evidence actually covers: the direct exclusive create and the holder snapshot read. Everything else propagates unchanged. The loop now only owns the retry budget. Adds a regression asserting that an EPERM thrown by the caller's payload reaches the caller as EPERM and that the callback runs exactly once. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01GGksu4AaxVTcTHZntKTyAs --- scripts/check-file-size.mjs | 2 +- src/sidecar-lock.ts | 16 +++++++++++----- test/sidecar-lock-regression.test.ts | 24 ++++++++++++++++++++++++ 3 files changed, 36 insertions(+), 6 deletions(-) diff --git a/scripts/check-file-size.mjs b/scripts/check-file-size.mjs index c41c471..2919362 100644 --- a/scripts/check-file-size.mjs +++ b/scripts/check-file-size.mjs @@ -7,7 +7,7 @@ const LINE_BUDGETS = new Map([ ["src/permissions.ts", 566], ["src/root-impl.ts", 1750], ["src/root-path.ts", 862], - ["src/sidecar-lock.ts", 520], + ["src/sidecar-lock.ts", 525], ["test/api-coverage.test.ts", 983], ["test/new-primitives.test.ts", 1500], ]); diff --git a/src/sidecar-lock.ts b/src/sidecar-lock.ts index 552d607..8a39f5f 100644 --- a/src/sidecar-lock.ts +++ b/src/sidecar-lock.ts @@ -265,8 +265,7 @@ export function createSidecarLockManager(key: string) { let attempt = 0; // Bounded so a genuine denial still surfaces as EPERM, not a lock timeout. let transientDenials = 0; - const isRetryableDenial = (error: unknown): boolean => - isTransientLockFileDenial(error) && ++transientDenials <= maxTransientLockDenials; + const withinDenialBudget = (): boolean => ++transientDenials <= maxTransientLockDenials; const waitForRetry = async (): Promise => { const elapsed = Date.now() - startedAt; if ( @@ -296,6 +295,7 @@ export function createSidecarLockManager(key: string) { continue; } let handle: SidecarFileHandle | null = null; + let lockFileCreateDenied = false; try { const payload = await options.payload(); const { raw, ownershipToken } = serializeSidecarLockPayload(payload); @@ -311,7 +311,13 @@ export function createSidecarLockManager(key: string) { } handle = (await options.lockRoot.open(relativeLockPath)).handle; } else { - handle = (await createNativeExclusiveFile(lockPath, 0o600)) ?? await fs.open(lockPath, "wx"); + try { + handle = + (await createNativeExclusiveFile(lockPath, 0o600)) ?? (await fs.open(lockPath, "wx")); + } catch (createError) { + lockFileCreateDenied = isTransientLockFileDenial(createError); + throw createError; + } await handle.writeFile(raw, "utf8"); } const snapshot = { raw, payload, stat: await handle.stat(), ownershipToken }; @@ -376,7 +382,7 @@ export function createSidecarLockManager(key: string) { parsePayload: options.parsePayload, }); } - if (isRetryableDenial(err)) { + if (lockFileCreateDenied && withinDenialBudget()) { await waitForRetry(); continue; } @@ -396,7 +402,7 @@ export function createSidecarLockManager(key: string) { parsePayload: options.parsePayload, }); } catch (readErr) { - if (!isRetryableDenial(readErr)) throw readErr; + if (!isTransientLockFileDenial(readErr) || !withinDenialBudget()) throw readErr; await waitForRetry(); continue; } diff --git a/test/sidecar-lock-regression.test.ts b/test/sidecar-lock-regression.test.ts index c2f64cd..842cf23 100644 --- a/test/sidecar-lock-regression.test.ts +++ b/test/sidecar-lock-regression.test.ts @@ -37,6 +37,30 @@ describe("sidecar lock regressions", () => { { code: "EPERM", errno: -4048, syscall: "open", path: lockPath }, ); + it.runIf(process.platform === "win32")("propagates an EPERM raised outside the lock file", async () => { + // Only the lock-file create and snapshot read see the teardown window. An + // EPERM from the caller's payload must reach the caller unchanged instead + // of being retried, which would rerun the callback and hide the error. + const base = await fsp.realpath(await tempRoot("fs-safe-sidecar-eperm-payload-")); + const targetPath = path.join(base, "state.json"); + configureFsSafeNative({ mode: "off" }); + let payloadCalls = 0; + + await expect( + acquireFileLock(targetPath, { + managerKey: `eperm-payload-${Date.now()}-${Math.random()}`, + staleMs: 60_000, + timeoutMs: 1_000, + retry: { retries: 0 }, + payload: async () => { + payloadCalls += 1; + throw denial(`${targetPath}.lock`); + }, + }), + ).rejects.toMatchObject({ code: "EPERM" }); + expect(payloadCalls).toBe(1); + }); + it.runIf(process.platform === "win32")("retries an exclusive create denied mid-teardown", async () => { const base = await fsp.realpath(await tempRoot("fs-safe-sidecar-eperm-create-")); const targetPath = path.join(base, "state.json"); From 3d0ffe4bd7fc7f212893db831e9d0062c0936cef Mon Sep 17 00:00:00 2001 From: Yigtwxx Date: Sun, 2 Aug 2026 22:33:06 +0300 Subject: [PATCH 4/5] fix(lock): require the denial to name the lock file createNativeExclusiveFile() opens dirname(lockPath) before its native exclusive open, so a denial raised by that setup step was still classified as the teardown window. The outer loop then retried it and reran options.payload(), which is the same fail-closed violation as before, one level further in. Require the EPERM to name the lock file itself. The observed failures carry path set to the lock path, from fs.open(lockPath, "wx") and from the snapshot read, so the evidence maps exactly onto that condition. A parent directory denial names the directory and now propagates untouched, as does any native helper error that identifies no path. Adds a regression asserting that a denial naming the lock parent rejects with the original EPERM and runs the payload callback exactly once. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01GGksu4AaxVTcTHZntKTyAs --- src/sidecar-lock-policy.ts | 9 ++++++--- src/sidecar-lock.ts | 5 +++-- test/sidecar-lock-regression.test.ts | 30 ++++++++++++++++++++++++++++ 3 files changed, 39 insertions(+), 5 deletions(-) diff --git a/src/sidecar-lock-policy.ts b/src/sidecar-lock-policy.ts index 9f7b780..2a6ac65 100644 --- a/src/sidecar-lock-policy.ts +++ b/src/sidecar-lock-policy.ts @@ -14,11 +14,14 @@ export function computeSidecarLockDelayMs(retry: SidecarLockRetryOptions, attemp // is still being torn down, so a contended acquire sees EPERM on a name that is // already gone -- both when creating it exclusively and when reading the // holder's snapshot. The next attempt succeeds, so this is contention rather -// than a permission failure. +// than a permission failure. The error must name the lock file itself: the +// exclusive-create helper opens the parent directory first, and a denial from +// that setup step carries no teardown evidence and has to reach the caller. export const maxTransientLockDenials = 8; -export function isTransientLockFileDenial(error: unknown): boolean { - return process.platform === "win32" && (error as NodeJS.ErrnoException | null)?.code === "EPERM"; +export function isTransientLockFileDenial(error: unknown, lockPath: string): boolean { + const denial = error as NodeJS.ErrnoException | null; + return process.platform === "win32" && denial?.code === "EPERM" && denial.path === lockPath; } export function sidecarLockPayloadIsStale( diff --git a/src/sidecar-lock.ts b/src/sidecar-lock.ts index 8a39f5f..a95c6ad 100644 --- a/src/sidecar-lock.ts +++ b/src/sidecar-lock.ts @@ -315,7 +315,7 @@ export function createSidecarLockManager(key: string) { handle = (await createNativeExclusiveFile(lockPath, 0o600)) ?? (await fs.open(lockPath, "wx")); } catch (createError) { - lockFileCreateDenied = isTransientLockFileDenial(createError); + lockFileCreateDenied = isTransientLockFileDenial(createError, lockPath); throw createError; } await handle.writeFile(raw, "utf8"); @@ -402,7 +402,8 @@ export function createSidecarLockManager(key: string) { parsePayload: options.parsePayload, }); } catch (readErr) { - if (!isTransientLockFileDenial(readErr) || !withinDenialBudget()) throw readErr; + if (!isTransientLockFileDenial(readErr, lockPath) || !withinDenialBudget()) + throw readErr; await waitForRetry(); continue; } diff --git a/test/sidecar-lock-regression.test.ts b/test/sidecar-lock-regression.test.ts index 842cf23..fa2f8d0 100644 --- a/test/sidecar-lock-regression.test.ts +++ b/test/sidecar-lock-regression.test.ts @@ -37,6 +37,36 @@ describe("sidecar lock regressions", () => { { code: "EPERM", errno: -4048, syscall: "open", path: lockPath }, ); + it.runIf(process.platform === "win32")("propagates an EPERM that names the lock parent", async () => { + // createNativeExclusiveFile() opens dirname(lockPath) before the exclusive + // create, so a denial can name the parent. Only the teardown window on the + // lock file itself is contention; a parent denial must reach the caller. + const base = await fsp.realpath(await tempRoot("fs-safe-sidecar-eperm-parent-")); + const targetPath = path.join(base, "state.json"); + configureFsSafeNative({ mode: "off" }); + const realOpen = fsp.open.bind(fsp) as typeof fsp.open; + let payloadCalls = 0; + + vi.spyOn(fsp, "open").mockImplementation((async (...args: Parameters) => { + if (args[0] === `${targetPath}.lock` && args[1] === "wx") throw denial(base); + return await realOpen(...args); + }) as typeof fsp.open); + + await expect( + acquireFileLock(targetPath, { + managerKey: `eperm-parent-${Date.now()}-${Math.random()}`, + staleMs: 60_000, + timeoutMs: 1_000, + retry: { retries: 0 }, + payload: async () => { + payloadCalls += 1; + return { pid: process.pid }; + }, + }), + ).rejects.toMatchObject({ code: "EPERM", path: base }); + expect(payloadCalls).toBe(1); + }); + it.runIf(process.platform === "win32")("propagates an EPERM raised outside the lock file", async () => { // Only the lock-file create and snapshot read see the teardown window. An // EPERM from the caller's payload must reach the caller unchanged instead From 46038173f1cccafeec4d46cb37c483d8ecaef7c5 Mon Sep 17 00:00:00 2001 From: Yigtwxx Date: Sun, 2 Aug 2026 22:42:52 +0300 Subject: [PATCH 5/5] fix(lock): keep the denial when no retry can be scheduled withinDenialBudget() only bounded the denial count, not the caller's retry and timeout limits. A lock-file EPERM still went through waitForRetry(), which throws file_lock_timeout under retries: 0 or an elapsed deadline, so the caller lost the permission diagnosis on exactly the settings that ask to fail fast. The PR claimed bounded denials preserved EPERM; they did not. Hand the original denial back when waiting cannot schedule another attempt. Any other waitForRetry() failure still propagates as itself, and both the create and snapshot-read branches route through the same helper. Adds a regression pinning EPERM with retries: 0 for the create branch. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01GGksu4AaxVTcTHZntKTyAs --- scripts/check-file-size.mjs | 3 ++- src/sidecar-lock.ts | 15 +++++++++++++-- test/sidecar-lock-regression.test.ts | 26 ++++++++++++++++++++++++++ 3 files changed, 41 insertions(+), 3 deletions(-) diff --git a/scripts/check-file-size.mjs b/scripts/check-file-size.mjs index 2919362..deb4492 100644 --- a/scripts/check-file-size.mjs +++ b/scripts/check-file-size.mjs @@ -7,9 +7,10 @@ const LINE_BUDGETS = new Map([ ["src/permissions.ts", 566], ["src/root-impl.ts", 1750], ["src/root-path.ts", 862], - ["src/sidecar-lock.ts", 525], + ["src/sidecar-lock.ts", 540], ["test/api-coverage.test.ts", 983], ["test/new-primitives.test.ts", 1500], + ["test/sidecar-lock-regression.test.ts", 540], ]); function walk(dir) { diff --git a/src/sidecar-lock.ts b/src/sidecar-lock.ts index a95c6ad..6ec7d9f 100644 --- a/src/sidecar-lock.ts +++ b/src/sidecar-lock.ts @@ -266,6 +266,17 @@ export function createSidecarLockManager(key: string) { // Bounded so a genuine denial still surfaces as EPERM, not a lock timeout. let transientDenials = 0; const withinDenialBudget = (): boolean => ++transientDenials <= maxTransientLockDenials; + // Waiting can fail on the caller's own retry or deadline limits. Classifying + // a denial as contention must not cost them the original diagnosis, so hand + // the denial back when no further attempt can be scheduled. + const retryOrRethrowDenial = async (denial: unknown): Promise => { + try { + await waitForRetry(); + } catch (waitError) { + if ((waitError as NodeJS.ErrnoException).code === "file_lock_timeout") throw denial; + throw waitError; + } + }; const waitForRetry = async (): Promise => { const elapsed = Date.now() - startedAt; if ( @@ -383,7 +394,7 @@ export function createSidecarLockManager(key: string) { }); } if (lockFileCreateDenied && withinDenialBudget()) { - await waitForRetry(); + await retryOrRethrowDenial(err); continue; } if ((err as { code?: unknown }).code !== "EEXIST") { @@ -404,7 +415,7 @@ export function createSidecarLockManager(key: string) { } catch (readErr) { if (!isTransientLockFileDenial(readErr, lockPath) || !withinDenialBudget()) throw readErr; - await waitForRetry(); + await retryOrRethrowDenial(readErr); continue; } if (!snapshot) { diff --git a/test/sidecar-lock-regression.test.ts b/test/sidecar-lock-regression.test.ts index fa2f8d0..a210432 100644 --- a/test/sidecar-lock-regression.test.ts +++ b/test/sidecar-lock-regression.test.ts @@ -37,6 +37,32 @@ describe("sidecar lock regressions", () => { { code: "EPERM", errno: -4048, syscall: "open", path: lockPath }, ); + it.runIf(process.platform === "win32")("keeps the lock-file EPERM when no retry is left", async () => { + // Classifying the denial as contention must not cost the caller the + // diagnosis: with no retry budget the original EPERM has to survive + // instead of being replaced by the loop's timeout error. + const base = await fsp.realpath(await tempRoot("fs-safe-sidecar-eperm-exhausted-")); + const targetPath = path.join(base, "state.json"); + const lockPath = `${targetPath}.lock`; + configureFsSafeNative({ mode: "off" }); + const realOpen = fsp.open.bind(fsp) as typeof fsp.open; + + vi.spyOn(fsp, "open").mockImplementation((async (...args: Parameters) => { + if (args[0] === lockPath && args[1] === "wx") throw denial(lockPath); + return await realOpen(...args); + }) as typeof fsp.open); + + await expect( + acquireFileLock(targetPath, { + managerKey: `eperm-exhausted-${Date.now()}-${Math.random()}`, + staleMs: 60_000, + timeoutMs: 1_000, + retry: { retries: 0 }, + payload: async () => ({ pid: process.pid }), + }), + ).rejects.toMatchObject({ code: "EPERM", path: lockPath }); + }); + it.runIf(process.platform === "win32")("propagates an EPERM that names the lock parent", async () => { // createNativeExclusiveFile() opens dirname(lockPath) before the exclusive // create, so a denial can name the parent. Only the teardown window on the