diff --git a/packages/cli/src/browser/manager.test.ts b/packages/cli/src/browser/manager.test.ts index 1b9bf0481a..d0f46af023 100644 --- a/packages/cli/src/browser/manager.test.ts +++ b/packages/cli/src/browser/manager.test.ts @@ -62,13 +62,14 @@ interface FsMockOptions { /** map of dir path -> entries returned by readdirSync */ dirs?: Record; touchError?: Error; + initialMtimeMs?: number; } -function installFsMocks({ existing, dirs, touchError }: FsMockOptions) { +function installFsMocks({ existing, dirs, touchError, initialMtimeMs = 0 }: FsMockOptions) { // Mutable, and returned, so tests can pre-seed a "lock already held" path or // assert the lock dir doesn't leak after ensureBrowser resolves. const paths = new Set(existing); - const mtimes = new Map([...existing].map((p) => [p, 0])); + const mtimes = new Map([...existing].map((p) => [p, initialMtimeMs])); vi.doMock("node:fs", () => ({ existsSync: (p: string) => paths.has(p), readdirSync: (p: string) => { @@ -174,6 +175,7 @@ describe("findBrowser — cache resolution", () => { }); afterEach(() => { + vi.useRealTimers(); Object.defineProperty(process, "platform", { value: origPlatform, configurable: true }); Object.defineProperty(process, "arch", { value: origArch, configurable: true }); vi.restoreAllMocks(); @@ -347,6 +349,23 @@ describe("findBrowser — cache resolution", () => { expect(paths.has(HF_LOCK)).toBe(false); }); + it("withInstallLock recovers when a crashed reclaimer leaves both lock directories", async () => { + vi.useFakeTimers(); + const paths = installFsMocks({ + existing: new Set([CACHE_ROOT, HF_LOCK, HF_RECLAIM_LOCK]), + }); + + const { withInstallLock } = await import("./manager.js"); + const acquisition = withInstallLock(async () => "done", TEST_LOCK_TIMINGS); + await vi.advanceTimersByTimeAsync(TEST_LOCK_TIMINGS.pollMs * 2); + + await expect(Promise.race([acquisition, Promise.resolve("still waiting")])).resolves.toBe( + "done", + ); + expect(paths.has(HF_LOCK)).toBe(false); + expect(paths.has(HF_RECLAIM_LOCK)).toBe(false); + }); + it("withInstallLock does not reclaim another waiter's fresh lock after this waiter timed out", async () => { // Regression guard for the timeout-reclaim race: if multiple waiters cross // the stale-lock deadline together, waiter A can reclaim the stale lock and @@ -392,7 +411,10 @@ describe("findBrowser — cache resolution", () => { }); it("withInstallLock reports progress while waiting instead of staying silent", async () => { - const paths = installFsMocks({ existing: new Set([CACHE_ROOT, HF_LOCK]) }); + const paths = installFsMocks({ + existing: new Set([CACHE_ROOT, HF_LOCK]), + initialMtimeMs: Date.now(), + }); const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); const { withInstallLock } = await import("./manager.js"); diff --git a/packages/cli/src/browser/manager.ts b/packages/cli/src/browser/manager.ts index ff18e230d5..5fdb637400 100644 --- a/packages/cli/src/browser/manager.ts +++ b/packages/cli/src/browser/manager.ts @@ -77,6 +77,15 @@ function tryAcquireDirLock(lockDir: string): boolean { } } +function isDirLockStale(lockDir: string, timeoutMs: number): boolean { + try { + return Date.now() - statSync(lockDir).mtimeMs > timeoutMs; + } catch (err) { + if (isErrno(err, "ENOENT")) return false; + throw err; + } +} + function reclaimStaleInstallLock(timeoutMs: number): void { if (!tryAcquireDirLock(INSTALL_RECLAIM_LOCK_DIR)) return; try { @@ -91,6 +100,16 @@ function reclaimStaleInstallLock(timeoutMs: number): void { } } +function reclaimAbandonedReclaimLock(timeoutMs: number): void { + try { + if (isDirLockStale(INSTALL_RECLAIM_LOCK_DIR, timeoutMs)) { + rmSync(INSTALL_RECLAIM_LOCK_DIR, { recursive: true, force: true }); + } + } catch (err) { + if (!isErrno(err, "ENOENT")) throw err; + } +} + function touchInstallLock(): void { try { const now = new Date(); @@ -113,6 +132,11 @@ export async function withInstallLock( let lastNoticeMs = 0; for (;;) { if (existsSync(INSTALL_RECLAIM_LOCK_DIR)) { + // A process can die after acquiring the reclaim gate but before its + // synchronous cleanup runs. Without aging out that gate, every future + // installer sleeps here forever and never reaches the install-lock + // timeout/reclaim path below. + reclaimAbandonedReclaimLock(timings.staleMs); await sleep(timings.pollMs); continue; } @@ -127,7 +151,7 @@ export async function withInstallLock( `[browser] Waiting for another hyperframes process to finish installing chrome-headless-shell (${Math.round(waitedMs / 1000)}s elapsed)...`, ); } - if (Date.now() > deadline) { + if (isDirLockStale(INSTALL_LOCK_DIR, timings.staleMs) || Date.now() > deadline) { // The reclaim gate matters when multiple waiters cross the timeout at // once: without it, waiter A can delete the stale lock and acquire a // fresh one, then waiter B (whose old deadline also expired) can delete