mirror of
https://github.com/garrytan/gstack.git
synced 2026-09-19 19:32:18 +02:00
fix(browse): SIGKILL abandoned Chromium on close-race timeout (suite wedge)
close()'s launched-mode path raced browser.close() against 5s and on timeout ABANDONED the child: this.browser nulled, process handle lost, Chromium alive holding keep-alive connections into test servers whose stop() then waits forever. Reproduced twice as an intermittent (~50%) whole-suite wedge — a 44min 0.1%-CPU hang pinned by a leaked LISTEN socket, and a 400s hang with commands.test.ts teardown in flight. The child handle is now captured BEFORE the race and SIGKILLed on race-timeout (launched mode only; headed keeps context.close). Race timers are unref'd so a successful close stops pinning the caller's event loop for the window. The four browse test servers force-close keep-alives (stop(true)) as belt-and-braces. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5
parent
576112e7cf
commit
6a2e589cbd
@@ -727,6 +727,12 @@ export class BrowserManager {
|
|||||||
}
|
}
|
||||||
|
|
||||||
async close() {
|
async close() {
|
||||||
|
// unref'd race timer: without unref, every successful close still pins
|
||||||
|
// the caller's event loop for the full window.
|
||||||
|
const raceTimeout = (ms: number) => new Promise<false>((resolve) => {
|
||||||
|
const t = setTimeout(() => resolve(false), ms);
|
||||||
|
(t as { unref?: () => void }).unref?.();
|
||||||
|
});
|
||||||
if (this.browser || (this.connectionMode === 'headed' && this.context)) {
|
if (this.browser || (this.connectionMode === 'headed' && this.context)) {
|
||||||
if (this.connectionMode === 'headed') {
|
if (this.connectionMode === 'headed') {
|
||||||
// Headed/persistent context mode: close the context (which closes the browser)
|
// Headed/persistent context mode: close the context (which closes the browser)
|
||||||
@@ -734,15 +740,24 @@ export class BrowserManager {
|
|||||||
if (this.browser) this.browser.removeAllListeners('disconnected');
|
if (this.browser) this.browser.removeAllListeners('disconnected');
|
||||||
await Promise.race([
|
await Promise.race([
|
||||||
this.context ? this.context.close() : Promise.resolve(),
|
this.context ? this.context.close() : Promise.resolve(),
|
||||||
new Promise(resolve => setTimeout(resolve, 5000)),
|
raceTimeout(5000),
|
||||||
]).catch(() => {});
|
]).catch(() => {});
|
||||||
} else {
|
} else {
|
||||||
// Launched mode: close the browser we spawned
|
// Launched mode: close the browser we spawned.
|
||||||
this.browser.removeAllListeners('disconnected');
|
this.browser.removeAllListeners('disconnected');
|
||||||
await Promise.race([
|
// Grab the child handle BEFORE the race: nulling this.browser after a
|
||||||
this.browser.close(),
|
// race-timeout used to ABANDON a live Chromium whose sockets kept the
|
||||||
new Promise(resolve => setTimeout(resolve, 5000)),
|
// caller's event loop (and keep-alive connections into test servers)
|
||||||
]).catch(() => {});
|
// open forever — the intermittent whole-suite wedge. If graceful close
|
||||||
|
// doesn't finish in time, the child gets SIGKILL, not freedom.
|
||||||
|
const child = this.browser.process?.();
|
||||||
|
const closed = await Promise.race([
|
||||||
|
this.browser.close().then(() => true as const),
|
||||||
|
raceTimeout(5000),
|
||||||
|
]).catch(() => false as const);
|
||||||
|
if (closed === false && child && child.exitCode === null && !child.killed) {
|
||||||
|
try { child.kill('SIGKILL'); } catch { /* already gone */ }
|
||||||
|
}
|
||||||
}
|
}
|
||||||
this.browser = null;
|
this.browser = null;
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -43,7 +43,7 @@ beforeAll(async () => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
afterAll(async () => {
|
afterAll(async () => {
|
||||||
try { testServer.server.stop(); } catch {}
|
try { testServer.server.stop(true); } catch {} // force-close keep-alives — a lingering Chromium connection otherwise blocks stop() forever
|
||||||
// Close only this file's own browser — never process.exit(): bun test runs
|
// Close only this file's own browser — never process.exit(): bun test runs
|
||||||
// all files in one process, so a delayed exit kills the whole suite
|
// all files in one process, so a delayed exit kills the whole suite
|
||||||
// (see test/no-suicide-exit.test.ts). close() can hang when the browser
|
// (see test/no-suicide-exit.test.ts). close() can hang when the browser
|
||||||
|
|||||||
@@ -95,7 +95,7 @@ beforeAll(async () => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
afterAll(async () => {
|
afterAll(async () => {
|
||||||
try { testServer.server.stop(); } catch {}
|
try { testServer.server.stop(true); } catch {} // force-close keep-alives — a lingering Chromium connection otherwise blocks stop() forever
|
||||||
// Close only this file's own browser — never process.exit(): bun test runs
|
// Close only this file's own browser — never process.exit(): bun test runs
|
||||||
// all files in one process, so a delayed exit kills the whole suite
|
// all files in one process, so a delayed exit kills the whole suite
|
||||||
// (see test/no-suicide-exit.test.ts). close() can hang when the browser
|
// (see test/no-suicide-exit.test.ts). close() can hang when the browser
|
||||||
|
|||||||
@@ -27,7 +27,7 @@ beforeAll(async () => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
afterAll(async () => {
|
afterAll(async () => {
|
||||||
try { testServer.server.stop(); } catch {}
|
try { testServer.server.stop(true); } catch {} // force-close keep-alives — a lingering Chromium connection otherwise blocks stop() forever
|
||||||
// Close only this file's own browser — never process.exit(): bun test runs
|
// Close only this file's own browser — never process.exit(): bun test runs
|
||||||
// all files in one process, so a delayed exit kills the whole suite
|
// all files in one process, so a delayed exit kills the whole suite
|
||||||
// (see test/no-suicide-exit.test.ts). close() can hang when the browser
|
// (see test/no-suicide-exit.test.ts). close() can hang when the browser
|
||||||
|
|||||||
@@ -32,7 +32,7 @@ beforeAll(async () => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
afterAll(async () => {
|
afterAll(async () => {
|
||||||
try { testServer.server.stop(); } catch {}
|
try { testServer.server.stop(true); } catch {} // force-close keep-alives — a lingering Chromium connection otherwise blocks stop() forever
|
||||||
// Close only this file's own browser — never process.exit(): bun test runs
|
// Close only this file's own browser — never process.exit(): bun test runs
|
||||||
// all files in one process, so a delayed exit kills the whole suite
|
// all files in one process, so a delayed exit kills the whole suite
|
||||||
// (see test/no-suicide-exit.test.ts). close() can hang when the browser
|
// (see test/no-suicide-exit.test.ts). close() can hang when the browser
|
||||||
|
|||||||
Reference in New Issue
Block a user