fix(browser): recover abandoned install locks (#2328)

This commit is contained in:
Miguel Ángel
2026-07-13 16:52:04 -04:00
committed by GitHub
parent 5ff4ba13f4
commit 81a6edcb67
2 changed files with 50 additions and 4 deletions
+25 -3
View File
@@ -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) => {
@@ -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");
+25 -1
View File
@@ -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<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;
}
@@ -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