diff --git a/packages/adapter-utils/src/directory-merge-lock.test.ts b/packages/adapter-utils/src/directory-merge-lock.test.ts index c05305fb8d6..a2b71aac55d 100644 --- a/packages/adapter-utils/src/directory-merge-lock.test.ts +++ b/packages/adapter-utils/src/directory-merge-lock.test.ts @@ -10,7 +10,13 @@ import { afterEach, describe, expect, it, vi } from "vitest"; import { withDirectoryMergeLock, WORKSPACE_RESTORE_LOCK_TIMEOUT_CODE } from "./workspace-restore-merge.js"; describe("directory merge lock process lifetime", () => { - const loader = fileURLToPath(new URL("../../../cli/node_modules/tsx/dist/cli.mjs", import.meta.url)); + // Load tsx as an --import hook, not tsx's own CLI entry point. The CLI entry + // point re-spawns the evaluated code in a further child process, so killing + // the process this file spawns would leave that further child as an orphan + // that still holds the lock. The --import hook runs the evaluated code in + // the process this file spawns directly, so killing it really does release + // the lock. + const loader = fileURLToPath(new URL("../../../cli/node_modules/tsx/dist/loader.mjs", import.meta.url)); const module = fileURLToPath(new URL("./workspace-restore-merge.ts", import.meta.url)); const directories: string[] = []; const children: ChildProcess[] = []; @@ -38,7 +44,7 @@ describe("directory merge lock process lifetime", () => { } async function holder(target: string, env: NodeJS.ProcessEnv) { - const child = spawn(process.execPath, [loader, "--eval", ` + const child = spawn(process.execPath, ["--import", loader, "--eval", ` import { withDirectoryMergeLock } from ${JSON.stringify(module)}; withDirectoryMergeLock(${JSON.stringify(target)}, async () => { process.send?.("locked"); @@ -63,10 +69,15 @@ describe("directory merge lock process lifetime", () => { return await access(`${lock}.owner.json`).then(() => `${lock}.owner.json`, () => path.join(lock, "owner.json")); } - function expireWait() { - const now = Date.now(); - return vi.spyOn(Date, "now").mockReturnValueOnce(now).mockReturnValue(now + 60_000); - } + // A short real wait budget for a test that asserts a timeout. 200 ms still + // lets the implementation's 50 ms retry interval run at least 4 retries, so + // the timeout is reached because the lock is genuinely held, not because + // the clock was replaced. + const TIMEOUT_ASSERTION_WAIT_MS = 200; + // A short real wait budget for the crash-recovery test, which asserts a + // successful acquisition. It must absorb ordinary scheduling jitter around + // the crash while still resolving well inside the test timeout below. + const CRASH_RECOVERY_WAIT_MS = 5_000; it("recovers a killed holder even when its recorded PID has been reused", async () => { const { target, env, lock } = await fixture(); @@ -77,10 +88,7 @@ describe("directory merge lock process lifetime", () => { const recordPath = await ownerPath(lock); const record = JSON.parse(await readFile(recordPath, "utf8")); await writeFile(recordPath, JSON.stringify({ ...record, pid: process.pid })); - const clock = expireWait(); - try { - await expect(withDirectoryMergeLock(target, async () => "restored", env)).resolves.toBe("restored"); - } finally { clock.mockRestore(); } + await expect(withDirectoryMergeLock(target, async () => "restored", env, undefined, CRASH_RECOVERY_WAIT_MS)).resolves.toBe("restored"); }, 15_000); it("protects a live holder in another process regardless of diagnostic PID or age", async () => { @@ -89,10 +97,7 @@ describe("directory merge lock process lifetime", () => { const recordPath = await ownerPath(lock); await writeFile(recordPath, JSON.stringify({ pid: 2_147_483_647, createdAt: "2000-01-01T00:00:00.000Z" })); const contender = vi.fn(); - const clock = expireWait(); - try { - await expect(withDirectoryMergeLock(target, contender, env)).rejects.toMatchObject({ code: WORKSPACE_RESTORE_LOCK_TIMEOUT_CODE }); - } finally { clock.mockRestore(); } + await expect(withDirectoryMergeLock(target, contender, env, undefined, TIMEOUT_ASSERTION_WAIT_MS)).rejects.toMatchObject({ code: WORKSPACE_RESTORE_LOCK_TIMEOUT_CODE }); expect(contender).not.toHaveBeenCalled(); const exited = once(child, "exit"); child.send("release"); @@ -109,18 +114,12 @@ describe("directory merge lock process lifetime", () => { it("keeps the OS lock when a same-process contender closes its connection", async () => { const { target, env } = await fixture(); await withDirectoryMergeLock(target, async () => { - const clock = expireWait(); - try { - await expect(withDirectoryMergeLock(target, async () => undefined, env)).rejects.toMatchObject({ code: WORKSPACE_RESTORE_LOCK_TIMEOUT_CODE }); - } finally { clock.mockRestore(); } + await expect(withDirectoryMergeLock(target, async () => undefined, env, undefined, TIMEOUT_ASSERTION_WAIT_MS)).rejects.toMatchObject({ code: WORKSPACE_RESTORE_LOCK_TIMEOUT_CODE }); // A same-process test alone cannot prove that the OS lock survived: on // POSIX, closing an unmanaged descriptor can drop process-wide locks. - const result = await promisify(execFile)(process.execPath, [loader, "--eval", ` + const result = await promisify(execFile)(process.execPath, ["--import", loader, "--eval", ` import { withDirectoryMergeLock, WORKSPACE_RESTORE_LOCK_TIMEOUT_CODE } from ${JSON.stringify(module)}; - const now = Date.now(); - let calls = 0; - Date.now = () => now + calls++ * 60_000; - withDirectoryMergeLock(${JSON.stringify(target)}, async () => "entered") + withDirectoryMergeLock(${JSON.stringify(target)}, async () => "entered", undefined, undefined, ${TIMEOUT_ASSERTION_WAIT_MS}) .then(() => { console.error("Entered a live holder's lock"); process.exit(1); }) .catch(error => { if (error.code !== WORKSPACE_RESTORE_LOCK_TIMEOUT_CODE) { console.error(error); process.exit(1); } @@ -162,10 +161,7 @@ describe("directory merge lock process lifetime", () => { await mkdir(lock, { recursive: true }); if (kind !== "missing") await writeFile(path.join(lock, "owner.json"), kind === "malformed" ? "{broken" : JSON.stringify({ pid: 2_147_483_647, createdAt: "2000-01-01T00:00:00.000Z" })); const contender = vi.fn(); - const clock = expireWait(); - try { - await expect(withDirectoryMergeLock(target, contender, env)).rejects.toMatchObject({ code: WORKSPACE_RESTORE_LOCK_TIMEOUT_CODE }); - } finally { clock.mockRestore(); } + await expect(withDirectoryMergeLock(target, contender, env, undefined, TIMEOUT_ASSERTION_WAIT_MS)).rejects.toMatchObject({ code: WORKSPACE_RESTORE_LOCK_TIMEOUT_CODE }); expect(contender).not.toHaveBeenCalled(); expect((await stat(lock)).isDirectory()).toBe(true); }); diff --git a/packages/adapter-utils/src/workspace-restore-merge.ts b/packages/adapter-utils/src/workspace-restore-merge.ts index 6b05b135b90..14c58371b35 100644 --- a/packages/adapter-utils/src/workspace-restore-merge.ts +++ b/packages/adapter-utils/src/workspace-restore-merge.ts @@ -341,9 +341,9 @@ export function describeWorkspaceRestoreFailure(code: WorkspaceRestoreFailureCod } } -async function acquireDirectoryMergeLock(lockDir: string, operation?: DirectoryMergeLockOperation): Promise<() => Promise> { +async function acquireDirectoryMergeLock(lockDir: string, operation?: DirectoryMergeLockOperation, waitMs: number = LOCK_WAIT_MS): Promise<() => Promise> { const startedAt = performance.now(); - const deadline = Date.now() + LOCK_WAIT_MS; + const deadline = Date.now() + waitMs; const databasePath = `${lockDir}.sqlite`; const ownerPath = `${lockDir}.owner.json`; async function waitForLock(diagnosticOwnerPath: string) { @@ -496,13 +496,16 @@ export async function withDirectoryMergeLock( fn: (canonicalTargetDir: string) => Promise, env: NodeJS.ProcessEnv = process.env, diagnosticOperation?: DirectoryMergeLockOperation, + // Test seam only: overrides how long acquisition waits before it reports a + // timeout. Production callers must omit this and keep the real budget. + waitMs: number = LOCK_WAIT_MS, ): Promise { // Canonicalize before we hash or lock: a retargeted symlink must not let the // lock protect one directory while the caller mutates another. const canonicalTargetDir = await fs.realpath(targetDir); const lockRoot = await resolveDirectoryMergeLockRoot(env); const lockKey = createHash("sha256").update(canonicalTargetDir).digest("hex"); - const releaseLock = await acquireDirectoryMergeLock(path.join(lockRoot, `${lockKey}.lock`), diagnosticOperation); + const releaseLock = await acquireDirectoryMergeLock(path.join(lockRoot, `${lockKey}.lock`), diagnosticOperation, waitMs); try { return await fn(canonicalTargetDir); } finally {