fix(lock): retry Windows lock files denied mid-teardown

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).

scripts/check-file-size.mjs

@@ -7,8 +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", 540],
   ["test/api-coverage.test.ts", 983],
   ["test/new-primitives.test.ts", 1500],
+  ["test/sidecar-lock-regression.test.ts", 540],
 ]);
 
 function walk(dir) {

src/sidecar-lock-policy.ts

@@ -10,6 +10,20 @@ 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. 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, lockPath: string): boolean {
+  const denial = error as NodeJS.ErrnoException | null;
+  return process.platform === "win32" && denial?.code === "EPERM" && denial.path === lockPath;
+}
+
 export function sidecarLockPayloadIsStale(
   payload: unknown,
   staleMs: number,

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,20 @@ 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 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<void> => {
+      try {
+        await waitForRetry();
+      } catch (waitError) {
+        if ((waitError as NodeJS.ErrnoException).code === "file_lock_timeout") throw denial;
+        throw waitError;
+      }
+    };
     const waitForRetry = async (): Promise<void> => {
       const elapsed = Date.now() - startedAt;
       if (
@@ -290,6 +306,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);
@@ -305,7 +322,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, lockPath);
+              throw createError;
+            }
             await handle.writeFile(raw, "utf8");
           }
           const snapshot = { raw, payload, stat: await handle.stat(), ownershipToken };
@@ -370,6 +393,10 @@ export function createSidecarLockManager(key: string) {
               parsePayload: options.parsePayload,
             });
           }
+          if (lockFileCreateDenied && withinDenialBudget()) {
+            await retryOrRethrowDenial(err);
+            continue;
+          }
           if ((err as { code?: unknown }).code !== "EEXIST") {
             throw err;
           }
@@ -379,10 +406,18 @@ 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 (!isTransientLockFileDenial(readErr, lockPath) || !withinDenialBudget())
+              throw readErr;
+            await retryOrRethrowDenial(readErr);
+            continue;
+          }
           if (!snapshot) {
             continue;
           }

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<string> {
 
 afterEach(async () => {
   vi.restoreAllMocks();
+  configureFsSafeNative({ mode: "auto" });
   configureFsSafeLocks({
     retry: undefined,
     staleMs: undefined,
@@ -27,6 +29,153 @@ 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")("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<typeof fsp.open>) => {
+      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
+    // 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<typeof fsp.open>) => {
+      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
+    // 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");
+    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<typeof fsp.open>) => {
+      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 fsp.realpath(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<typeof fsp.readFile>) => {
+      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");