From fa2f429aa247057a9127eee9448fc00a8bed1620 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Mon, 27 Jul 2026 00:44:17 -0700 Subject: [PATCH] fix(lock): scope reentrancy to logical owners --- CHANGELOG.md | 2 +- docs/migrating-to-0.5.md | 13 +- docs/sidecar-lock.md | 50 +++++- src/file-lock-sync.ts | 128 ++++++++++++--- src/sidecar-lock-handle.ts | 49 ++++++ src/sidecar-lock-types.ts | 1 + src/sidecar-lock.ts | 63 +++++-- test/file-lock-reentrancy.test.ts | 263 ++++++++++++++++++++++++++++++ 8 files changed, 514 insertions(+), 55 deletions(-) create mode 100644 src/sidecar-lock-handle.ts create mode 100644 test/file-lock-reentrancy.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 7a02b2c6..0f61f817 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,7 +32,7 @@ ### Compatibility -- Remove the unsound process-scoped `allowReentrant` async file-lock option. Callers that passed it should delete the property; same-process contention now follows the normal retry and timeout policy, while nested same-file `jsonStore` mutations fail immediately instead of deadlocking. +- Remove the unsound process-scoped `allowReentrant` async file-lock option and replace it with owner-scoped `reentrantOwner` for async and sync locks: only matching, explicitly defined logical owners reuse a canonical in-process lock, releases are reference-counted and idempotent, and different or absent owners contend normally. Callers that passed the boolean must either remove it or migrate intentional nesting to a per-operation owner key; `jsonStore` remains ownerless and rejects nested same-file mutations immediately. - Remove the persistent Python helper and its `pythonPath` configuration. Replace `configureFsSafePython`, `FS_SAFE_PYTHON_MODE`, and the OpenClaw Python aliases with `configureFsSafeNative` and `FS_SAFE_NATIVE_MODE`; 0.5 warns once and maps the former `auto`, `require`, and `off` policies solely as an upgrade bridge for shipped 0.4 consumers. - Add `publishFileExclusive({ strategy: "rename-noreplace" })`; this strategy requires the native helper, atomically moves the source, and never replaces an existing destination. diff --git a/docs/migrating-to-0.5.md b/docs/migrating-to-0.5.md index 5e4527b0..f077e0d4 100644 --- a/docs/migrating-to-0.5.md +++ b/docs/migrating-to-0.5.md @@ -140,11 +140,14 @@ The default directory-error policy remains `throw`. See - Use `acquireFileLockSync()` only in synchronous boot or migration code; retry waits block the thread. Request-serving paths should use `withFileLock()`. -- Remove `allowReentrant` from async file-lock options. Same-process contention - now follows the ordinary retry and timeout policy. Locked and unlocked - `jsonStore` mutations serialize by canonical file path; nested same-file - mutations from an update callback fail with `store-reentrant-update`, so - return the complete value from the outer callback instead. +- Remove the `allowReentrant` boolean from async file-lock options. If a logical + holder intentionally nests acquisition, pass the same operation-scoped + `reentrantOwner` string to each acquisition; different or missing owners + contend normally. Never replace the boolean with a process-wide constant. + Locked and unlocked `jsonStore` mutations serialize by canonical file path + and do not opt into lock reentrancy; nested same-file mutations from an update + callback fail with `store-reentrant-update`, so return the complete value from + the outer callback instead. - Use `createSecretFileAtomic()` for first-writer-wins credentials and catch `secret-exists`; use `writeSecretFileAtomic()` only when replacement is the intended protocol. diff --git a/docs/sidecar-lock.md b/docs/sidecar-lock.md index e04f64b7..b8fad781 100644 --- a/docs/sidecar-lock.md +++ b/docs/sidecar-lock.md @@ -59,6 +59,7 @@ type FileLockAcquireOptions> = { timeoutMs?: number; // overall acquire deadline; default unbounded retry?: FileLockRetryOptions; staleRecovery?: "fail-closed" | "remove-if-unchanged"; // default "fail-closed" + reentrantOwner?: string; // logical holder identity for owner-scoped nesting payload: () => TPayload | Promise; shouldReclaim?: (params: { lockPath: string; @@ -95,11 +96,50 @@ type FileLockRetryOptions = { result is passed to `shouldReclaim` and `shouldRemoveStaleLock`, allowing PID, process-start, argv, or role schemas to remain application-owned. -Async lock acquisition is not reentrant. Another acquisition for the same path, -including one from the same process and manager, follows the normal retry and -timeout policy until the current holder releases it. Version 0.5 removes the -unsound process-scoped `allowReentrant` option; callers that passed it should -delete the property. +## Owner-scoped reentrancy + +Version 0.5 removes the unsound process-scoped `allowReentrant` boolean and +replaces it with `reentrantOwner`. When a manager already holds the canonical +target path, another acquisition reuses that sidecar only when both acquisitions +provide the same owner string. Each acquisition gets an idempotent release +handle; the sidecar remains until the last reference is released. A different or +missing owner waits under the normal contention, retry, and timeout policy. A +known live in-process holder is never stale-reclaimed by its own manager. + +This supports logical session writers that may reach one file through real and +symlinked parent paths: + +```ts +const managerKey = "session-write-locks"; +const reentrantOwner = `session:${sessionId}:operation:${operationId}`; + +const outer = await acquireFileLock(realSessionPath, { + managerKey, + reentrantOwner, + staleMs: 60_000, + payload: () => ({ pid: process.pid, operationId }), +}); +const nested = await acquireFileLock(symlinkedSessionPath, { + managerKey, + reentrantOwner, + staleMs: 60_000, + payload: () => ({ pid: process.pid, operationId }), +}); + +await nested.release(); // sidecar remains for outer +await outer.release(); // final reference removes it +``` + +The manager domain and canonical target path are part of the identity, so +aliased paths must use the same `managerKey`. The owner key must identify one +logical holder or call chain. **Never use a process-wide or other shared constant +for unrelated tasks**: doing so would admit concurrent work to the same critical +section and recreate the lost-update bug that removed `allowReentrant`. + +Omit `reentrantOwner` for ordinary acquisitions. `jsonStore` does so and keeps +its separate canonical-path mutation queue. The synchronous APIs implement the +same owner/refcount rules; a mismatched synchronous acquisition blocks the +calling thread according to its retry and timeout options. Pass `lockRoot` to place sidecar create, read, verification, and removal behind an existing `Root` capability. `lockPath` must resolve inside that root. diff --git a/src/file-lock-sync.ts b/src/file-lock-sync.ts index eafe6ac3..d77deed9 100644 --- a/src/file-lock-sync.ts +++ b/src/file-lock-sync.ts @@ -27,6 +27,7 @@ export type FileLockSyncAcquireOptions> timeoutMs?: number; retry?: SidecarLockRetryOptions; staleRecovery?: SidecarLockStaleRecovery; + reentrantOwner?: string; payload: () => TPayload; shouldReclaim?: (params: { lockPath: string; @@ -51,6 +52,65 @@ export type FileLockSyncHandle = { [Symbol.dispose](): void; }; +type SyncHeldLock = { + fd: number; + lockPath: string; + normalizedTargetPath: string; + parsePayload?: (raw: string) => unknown; + refCount: number; + reentrantOwner?: string; + snapshot: SidecarLockSnapshot; + timer?: NodeJS.Timeout; +}; + +const SYNC_HELD_LOCKS_KEY = Symbol.for("fsSafe.syncSidecarLocks"); + +function getSyncHeldLocks(): Map { + const globalWithState = globalThis as typeof globalThis & { + [SYNC_HELD_LOCKS_KEY]?: Map; + }; + if (!globalWithState[SYNC_HELD_LOCKS_KEY]) { + globalWithState[SYNC_HELD_LOCKS_KEY] = new Map(); + } + return globalWithState[SYNC_HELD_LOCKS_KEY]; +} + +function verifySyncHeldLock(held: SyncHeldLock): boolean { + const current = readSidecarLockSnapshotSync(held.lockPath, held.parsePayload); + return !!current && sidecarLockSnapshotMatches(current, held.snapshot); +} + +function releaseSyncHeldLock(held: SyncHeldLock): boolean { + const heldLocks = getSyncHeldLocks(); + if (heldLocks.get(held.normalizedTargetPath) !== held) return false; + held.refCount -= 1; + if (held.refCount > 0) return false; + heldLocks.delete(held.normalizedTargetPath); + if (held.timer) { + clearInterval(held.timer); + held.timer = undefined; + } + fs.closeSync(held.fd); + removeSidecarLockIfUnchangedSync(held.lockPath, held.snapshot); + return true; +} + +function createSyncHeldLockHandle(held: SyncHeldLock): FileLockSyncHandle { + let released = false; + const release = () => { + if (released) return; + released = true; + releaseSyncHeldLock(held); + }; + return { + lockPath: held.lockPath, + normalizedTargetPath: held.normalizedTargetPath, + verifyStillHeld: () => verifySyncHeldLock(held), + release, + [Symbol.dispose]: release, + }; +} + function normalizeTargetPath(targetPath: string): string { const resolved = path.resolve(targetPath); fs.mkdirSync(path.dirname(resolved), { recursive: true }); @@ -93,6 +153,17 @@ export function acquireFileLockSync>( ): FileLockSyncHandle { const normalizedTargetPath = normalizeTargetPath(targetPath); const lockPath = boundedLockPath(options.lockPath ?? `${normalizedTargetPath}.lock`, options.lockRoot); + const heldLocks = getSyncHeldLocks(); + const held = heldLocks.get(normalizedTargetPath); + if ( + held && + options.reentrantOwner !== undefined && + held.reentrantOwner !== undefined && + options.reentrantOwner === held.reentrantOwner + ) { + held.refCount += 1; + return createSyncHeldLockHandle(held); + } const staleMs = options.staleMs ?? 30_000; const retry = options.retry ?? {}; const startedAt = Date.now(); @@ -120,38 +191,29 @@ export function acquireFileLockSync>( stat: fs.fstatSync(fd), ownershipToken, }; - const heldFd = fd; - let released = false; - let timer: NodeJS.Timeout | undefined; - const verifyStillHeld = () => { - const current = readSidecarLockSnapshotSync(lockPath, options.parsePayload); - return !!current && sidecarLockSnapshotMatches(current, snapshot); - }; - const release = () => { - if (released) return; - released = true; - if (timer) clearInterval(timer); - fs.closeSync(heldFd); - fd = undefined; - removeSidecarLockIfUnchangedSync(lockPath, snapshot); + const createdHeld: SyncHeldLock = { + fd, + lockPath, + normalizedTargetPath, + parsePayload: options.parsePayload, + refCount: 1, + reentrantOwner: options.reentrantOwner, + snapshot, }; + heldLocks.set(normalizedTargetPath, createdHeld); + const returnedHandle = createSyncHeldLockHandle(createdHeld); if (options.onCompromised && (options.compromiseCheckIntervalMs ?? 0) > 0) { - timer = setInterval(() => { - if (!verifyStillHeld()) { - if (timer) clearInterval(timer); - timer = undefined; + createdHeld.timer = setInterval(() => { + if (!returnedHandle.verifyStillHeld()) { + if (createdHeld.timer) clearInterval(createdHeld.timer); + createdHeld.timer = undefined; options.onCompromised?.({ lockPath, normalizedTargetPath }); } }, options.compromiseCheckIntervalMs); - timer.unref(); + createdHeld.timer.unref(); } - return { - lockPath, - normalizedTargetPath, - verifyStillHeld, - release, - [Symbol.dispose]: release, - }; + fd = undefined; + return returnedHandle; } catch (error) { if (fd !== undefined) { const failed = { payload: null, stat: fs.fstatSync(fd) } satisfies SidecarLockSnapshot; @@ -160,6 +222,20 @@ export function acquireFileLockSync>( removeSidecarLockIfUnchangedSync(lockPath, failed); } if ((error as NodeJS.ErrnoException).code !== "EEXIST") throw error; + if (heldLocks.has(normalizedTargetPath)) { + const elapsed = Date.now() - startedAt; + const timedOut = options.timeoutMs !== undefined && elapsed >= options.timeoutMs; + if (timedOut || (retry.retries !== undefined && attempt >= retry.retries)) { + throw Object.assign(new Error(`file lock timeout for ${normalizedTargetPath}`), { + code: "file_lock_timeout", + lockPath, + normalizedTargetPath, + }); + } + sleep(computeSidecarLockDelayMs(retry, attempt)); + attempt += 1; + continue; + } const snapshot = readSidecarLockSnapshotSync(lockPath, options.parsePayload); if (!snapshot) continue; const nowMs = Date.now(); diff --git a/src/sidecar-lock-handle.ts b/src/sidecar-lock-handle.ts new file mode 100644 index 00000000..03a72787 --- /dev/null +++ b/src/sidecar-lock-handle.ts @@ -0,0 +1,49 @@ +import type { Root } from "./root-impl.js"; +import { + sidecarLockSnapshotStillPresent, + type SidecarLockSnapshot, +} from "./sidecar-lock-reclaim.js"; +import type { SidecarLockHandle } from "./sidecar-lock-types.js"; + +export function createSidecarLockHandle(params: { + lockPath: string; + normalizedTargetPath: string; + verifyStillHeld: () => Promise; + release: () => Promise; +}): SidecarLockHandle { + let released = false; + const release = async (): Promise => { + if (released) return; + released = true; + await params.release(); + }; + return { + lockPath: params.lockPath, + normalizedTargetPath: params.normalizedTargetPath, + verifyStillHeld: params.verifyStillHeld, + release, + [Symbol.asyncDispose]: release, + }; +} + +export function createHeldSidecarLockHandle(params: { + normalizedTargetPath: string; + held: { + lockPath: string; + snapshot: SidecarLockSnapshot; + lockRoot?: Root; + parsePayload?: (raw: string) => unknown; + }; + release: () => Promise; +}): SidecarLockHandle { + return createSidecarLockHandle({ + lockPath: params.held.lockPath, + normalizedTargetPath: params.normalizedTargetPath, + verifyStillHeld: async () => + await sidecarLockSnapshotStillPresent(params.held.lockPath, params.held.snapshot, { + lockRoot: params.held.lockRoot, + parsePayload: params.held.parsePayload, + }), + release: params.release, + }); +} diff --git a/src/sidecar-lock-types.ts b/src/sidecar-lock-types.ts index 6f8b2880..7225602c 100644 --- a/src/sidecar-lock-types.ts +++ b/src/sidecar-lock-types.ts @@ -23,6 +23,7 @@ export type SidecarLockAcquireOptions> timeoutMs?: number; retry?: SidecarLockRetryOptions; staleRecovery?: SidecarLockStaleRecovery; + reentrantOwner?: string; payload: () => TPayload | Promise; shouldReclaim?: (params: { lockPath: string; diff --git a/src/sidecar-lock.ts b/src/sidecar-lock.ts index 887faea3..1f123cf5 100644 --- a/src/sidecar-lock.ts +++ b/src/sidecar-lock.ts @@ -19,6 +19,7 @@ import { import { FsSafeError } from "./errors.js"; import { createNativeExclusiveFile, type NativeFileHandle } from "./native-operations.js"; import type { Root } from "./root-impl.js"; +import { createHeldSidecarLockHandle } from "./sidecar-lock-handle.js"; import type { SidecarLockAcquireOptions, SidecarLockHandle, @@ -41,6 +42,8 @@ export type { WithSidecarLockOptions, } from "./sidecar-lock-types.js"; type HeldLock = { + refCount: number; + reentrantOwner?: string; handle: NativeFileHandle; lockPath: string; snapshot: SidecarLockSnapshot; @@ -84,6 +87,9 @@ function resolveManagerState(key: string): SidecarLockManagerState { // Backfill state created by fs-safe versions that predate reclaim guards. state.reclaimCleanupRegistered ??= false; state.reclaimGuards ??= new Set(); + for (const held of state.held.values()) { + held.refCount ??= 1; + } } return state; } @@ -169,11 +175,20 @@ async function releaseHeldLock( state: SidecarLockManagerState, normalizedTargetPath: string, held: HeldLock, + options: { force?: boolean } = {}, ): Promise { const current = state.held.get(normalizedTargetPath); if (current !== held) { return false; } + if (options.force) { + held.refCount = 0; + } else { + held.refCount -= 1; + if (held.refCount > 0) { + return false; + } + } if (held.releasePromise) { await held.releasePromise.catch(() => undefined); return true; @@ -198,6 +213,14 @@ async function releaseHeldLock( } } +function handleForHeldLock(state: SidecarLockManagerState, normalizedTargetPath: string, held: HeldLock) { + return createHeldSidecarLockHandle({ + normalizedTargetPath, + held, + release: async () => await releaseHeldLock(state, normalizedTargetPath, held), + }); +} + export function createSidecarLockManager(key: string) { const state = resolveManagerState(key); @@ -220,6 +243,16 @@ export function createSidecarLockManager(key: string) { ensureExitCleanupRegistered(); const normalizedTargetPath = await resolveNormalizedTargetPath(options.targetPath); const lockPath = options.lockPath ?? `${normalizedTargetPath}.lock`; + const held = state.held.get(normalizedTargetPath); + if ( + held && + options.reentrantOwner !== undefined && + held.reentrantOwner !== undefined && + options.reentrantOwner === held.reentrantOwner + ) { + held.refCount += 1; + return handleForHeldLock(state, normalizedTargetPath, held); + } const startedAt = Date.now(); const retry = options.retry ?? {}; @@ -276,6 +309,8 @@ export function createSidecarLockManager(key: string) { } const snapshot = { raw, payload, stat: await handle.stat(), ownershipToken }; const createdHeld: HeldLock = { + refCount: 1, + reentrantOwner: options.reentrantOwner, handle, lockPath, snapshot, @@ -290,21 +325,15 @@ export function createSidecarLockManager(key: string) { await releaseSidecarReclaimGuard(state.reclaimGuards, reclaimGuardPath); ownsReclaimGuard = false; } catch (err) { - await releaseHeldLock(state, normalizedTargetPath, createdHeld); + await releaseHeldLock(state, normalizedTargetPath, createdHeld, { force: true }); throw err; } } - const release = () => - releaseHeldLock(state, normalizedTargetPath, createdHeld).then(() => undefined); - const verifyStillHeld = async () => - await sidecarLockSnapshotStillPresent(lockPath, snapshot, { - lockRoot: options.lockRoot, - parsePayload: options.parsePayload, - }); + const returnedHandle = handleForHeldLock(state, normalizedTargetPath, createdHeld); const interval = options.compromiseCheckIntervalMs; if (options.onCompromised && interval !== undefined && interval > 0) { createdHeld.compromiseTimer = setInterval(() => { - void verifyStillHeld().then((stillHeld) => { + void returnedHandle.verifyStillHeld().then((stillHeld) => { if (!stillHeld && createdHeld.compromiseTimer) { clearInterval(createdHeld.compromiseTimer); createdHeld.compromiseTimer = undefined; @@ -314,13 +343,7 @@ export function createSidecarLockManager(key: string) { }, interval); createdHeld.compromiseTimer.unref(); } - return { - lockPath, - normalizedTargetPath, - verifyStillHeld, - release, - [Symbol.asyncDispose]: release, - }; + return returnedHandle; } catch (err) { if (handle) { const failedSnapshot: SidecarLockSnapshot = { payload: null }; @@ -362,6 +385,10 @@ export function createSidecarLockManager(key: string) { if (!snapshot) { continue; } + if (state.held.has(normalizedTargetPath)) { + await waitForRetry(); + continue; + } const shouldReclaim = options.shouldReclaim ?? defaultSidecarLockShouldReclaim; if ( await shouldReclaim({ @@ -434,7 +461,7 @@ export function createSidecarLockManager(key: string) { async function drain(): Promise { for (const [normalizedTargetPath, held] of Array.from(state.held.entries())) { - await releaseHeldLock(state, normalizedTargetPath, held).catch( + await releaseHeldLock(state, normalizedTargetPath, held, { force: true }).catch( () => undefined, ); } @@ -450,7 +477,7 @@ export function createSidecarLockManager(key: string) { lockPath: held.lockPath, acquiredAt: held.acquiredAt, metadata: held.metadata, - forceRelease: () => releaseHeldLock(state, normalizedTargetPath, held), + forceRelease: () => releaseHeldLock(state, normalizedTargetPath, held, { force: true }), })); } diff --git a/test/file-lock-reentrancy.test.ts b/test/file-lock-reentrancy.test.ts new file mode 100644 index 00000000..e8cf87eb --- /dev/null +++ b/test/file-lock-reentrancy.test.ts @@ -0,0 +1,263 @@ +import { spawn } from "node:child_process"; +import fsSync from "node:fs"; +import fs from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import { setTimeout as delay } from "node:timers/promises"; +import { afterEach, describe, expect, it } from "vitest"; +import { + acquireFileLock, + acquireFileLockSync, + createFileLockManager, + withFileLockSync, +} from "../src/file-lock.js"; + +const tempDirs: string[] = []; + +async function tempRoot(prefix: string): Promise { + const directory = await fs.mkdtemp(path.join(os.tmpdir(), prefix)); + tempDirs.push(directory); + return directory; +} + +function payload(): { pid: number; createdAt: string } { + return { pid: process.pid, createdAt: new Date().toISOString() }; +} + +function deferred(): { promise: Promise; resolve: () => void } { + let resolve!: () => void; + const promise = new Promise((done) => { + resolve = done; + }); + return { promise, resolve }; +} + +afterEach(async () => { + await Promise.all(tempDirs.splice(0).map((dir) => fs.rm(dir, { recursive: true, force: true }))); +}); + +describe("owner-scoped file-lock reentrancy", () => { + it.runIf(process.platform !== "win32")( + "shares one reference-counted lock across symlinked target paths", + async () => { + const root = await tempRoot("fs-safe-reentrant-alias-"); + const realDir = path.join(root, "real"); + const linkDir = path.join(root, "link"); + await fs.mkdir(realDir); + await fs.symlink(realDir, linkDir, "dir"); + const realPath = path.join(realDir, "session.json"); + const linkPath = path.join(linkDir, "session.json"); + const managerKey = `alias-${Date.now()}-${Math.random()}`; + const reentrantOwner = "session:run-1"; + const first = await acquireFileLock(realPath, { + managerKey, + reentrantOwner, + staleMs: 60_000, + payload, + }); + const second = await acquireFileLock(linkPath, { + managerKey, + reentrantOwner, + staleMs: 60_000, + payload, + }); + + try { + expect(second.normalizedTargetPath).toBe(first.normalizedTargetPath); + expect(second.lockPath).toBe(first.lockPath); + await expect(fs.stat(`${realPath}.lock`)).resolves.toMatchObject({}); + await expect(fs.stat(`${linkPath}.lock`)).resolves.toMatchObject({}); + await first.release(); + await expect(fs.stat(first.lockPath)).resolves.toMatchObject({}); + await second.release(); + await expect(fs.stat(first.lockPath)).rejects.toMatchObject({ code: "ENOENT" }); + } finally { + await first.release(); + await second.release(); + } + }, + ); + + it.each([ + ["different", "owner-b"], + ["absent", undefined], + ] as const)("queues a %s owner so both read-modify-write operations land", async (_label, secondOwner) => { + const root = await tempRoot("fs-safe-reentrant-owner-isolation-"); + const targetPath = path.join(root, "state.json"); + await fs.writeFile(targetPath, JSON.stringify({ count: 0 })); + const manager = createFileLockManager(`owner-isolation-${Date.now()}-${Math.random()}`); + const firstEntered = deferred(); + const releaseFirst = deferred(); + let secondEntered = false; + + const update = async (owner: string | undefined, wait?: () => Promise): Promise => { + await manager.withLock( + targetPath, + { + reentrantOwner: owner, + staleMs: 60_000, + timeoutMs: 1_000, + retry: { minTimeout: 1, maxTimeout: 2 }, + payload, + }, + async () => { + if (owner !== "owner-a") secondEntered = true; + const current = JSON.parse(await fs.readFile(targetPath, "utf8")) as { count: number }; + await wait?.(); + await fs.writeFile(targetPath, JSON.stringify({ count: current.count + 1 })); + }, + ); + }; + + const first = update("owner-a", async () => { + firstEntered.resolve(); + await releaseFirst.promise; + }); + await firstEntered.promise; + const second = update(secondOwner); + try { + await delay(10); + expect(secondEntered).toBe(false); + } finally { + releaseFirst.resolve(); + } + await Promise.all([first, second]); + await expect(fs.readFile(targetPath, "utf8").then(JSON.parse)).resolves.toEqual({ count: 2 }); + await manager.drain(); + }); + + it("keeps a nested same-owner lock until the last idempotent release", async () => { + const root = await tempRoot("fs-safe-reentrant-release-"); + const targetPath = path.join(root, "state.json"); + const manager = createFileLockManager(`release-order-${Date.now()}-${Math.random()}`); + const options = { + reentrantOwner: "session:run-2", + staleMs: 60_000, + payload, + }; + const outer = await manager.acquire(targetPath, options); + const inner = await manager.acquire(targetPath, options); + + await inner.release(); + await inner.release(); + await expect(fs.stat(outer.lockPath)).resolves.toMatchObject({}); + await outer.release(); + await expect(fs.stat(outer.lockPath)).rejects.toMatchObject({ code: "ENOENT" }); + }); + + it("continues to arbitrate with a separate process", async () => { + const root = await tempRoot("fs-safe-reentrant-process-"); + const targetPath = path.join(root, "state.json"); + const lockPath = `${targetPath}.lock`; + const childCode = ` + const fs = require("node:fs"); + const lockPath = process.argv[1]; + const fd = fs.openSync(lockPath, "wx", 0o600); + fs.writeFileSync(fd, JSON.stringify({ pid: process.pid, createdAt: new Date().toISOString() })); + process.stdout.write("ready\\n"); + process.stdin.once("data", () => { + fs.closeSync(fd); + fs.unlinkSync(lockPath); + process.stdout.write("released\\n"); + }); + `; + const child = spawn(process.execPath, ["-e", childCode, lockPath], { + stdio: ["pipe", "pipe", "pipe"], + }); + let output = ""; + const ready = deferred(); + const released = deferred(); + child.stdout.on("data", (chunk) => { + output += String(chunk); + if (output.includes("ready\n")) ready.resolve(); + if (output.includes("released\n")) released.resolve(); + }); + await ready.promise; + + let acquired = false; + const pending = acquireFileLock(targetPath, { + managerKey: `cross-process-${Date.now()}-${Math.random()}`, + reentrantOwner: "parent-operation", + staleMs: 60_000, + timeoutMs: 1_000, + retry: { minTimeout: 1, maxTimeout: 2 }, + payload, + }).then((lock) => { + acquired = true; + return lock; + }); + await delay(10); + expect(acquired).toBe(false); + child.stdin.write("release\n"); + await released.promise; + const lock = await pending; + await lock.release(); + await new Promise((resolve, reject) => { + child.once("error", reject); + child.once("exit", (code) => (code === 0 ? resolve() : reject(new Error(`child exited ${code}`)))); + child.stdin.end(); + }); + }); + + it("provides synchronous owner and reference-count parity", async () => { + const root = await tempRoot("fs-safe-reentrant-sync-"); + const targetPath = path.join(root, "state.json"); + const options = { + reentrantOwner: "sync-session:run-1", + staleMs: 60_000, + timeoutMs: 0, + retry: { retries: 0 }, + payload, + }; + const first = acquireFileLockSync(targetPath, options); + const second = acquireFileLockSync(targetPath, options); + + expect(() => + acquireFileLockSync(targetPath, { + ...options, + reentrantOwner: "sync-session:other-run", + }), + ).toThrow(/timeout/u); + second.release(); + second.release(); + expect(fsSync.existsSync(first.lockPath)).toBe(true); + first.release(); + expect(fsSync.existsSync(first.lockPath)).toBe(false); + + withFileLockSync(targetPath, options, () => { + expect(fsSync.existsSync(first.lockPath)).toBe(true); + withFileLockSync(targetPath, options, () => { + expect(fsSync.existsSync(first.lockPath)).toBe(true); + }); + expect(fsSync.existsSync(first.lockPath)).toBe(true); + }); + expect(fsSync.existsSync(first.lockPath)).toBe(false); + }); + + it.runIf(process.platform !== "win32")( + "provides synchronous canonical-alias parity", + async () => { + const root = await tempRoot("fs-safe-reentrant-sync-alias-"); + const realDir = path.join(root, "real"); + const linkDir = path.join(root, "link"); + await fs.mkdir(realDir); + await fs.symlink(realDir, linkDir, "dir"); + const options = { + reentrantOwner: "sync-session:aliased-run", + staleMs: 60_000, + timeoutMs: 0, + retry: { retries: 0 }, + payload, + }; + const first = acquireFileLockSync(path.join(realDir, "session.json"), options); + const second = acquireFileLockSync(path.join(linkDir, "session.json"), options); + + expect(second.normalizedTargetPath).toBe(first.normalizedTargetPath); + expect(second.lockPath).toBe(first.lockPath); + first.release(); + expect(fsSync.existsSync(first.lockPath)).toBe(true); + second.release(); + expect(fsSync.existsSync(first.lockPath)).toBe(false); + }, + ); +});