Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 25 additions & 3 deletions packages/cli/src/browser/manager.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,13 +62,14 @@ interface FsMockOptions {
/** map of dir path -> entries returned by readdirSync */
dirs?: Record<string, string[]>;
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) => {
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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");
Expand Down
26 changes: 25 additions & 1 deletion packages/cli/src/browser/manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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();
Expand All @@ -113,6 +132,11 @@ export async function withInstallLock<T>(
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;
}
Expand All @@ -127,7 +151,7 @@ export async function withInstallLock<T>(
`[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
Expand Down
Loading