browse: close returns when Chromium exits; Xvfb cleanup does not wait on its own zombie

BrowserManager.close() detached every 'disconnected' listener before
browser.close(). That also removed Playwright's internal listener that
resolves close(), so every launched-mode close waited the full 5 s race
timeout (measured 5,000 ms on origin/main and on this branch; raw Playwright
closes in ~50 ms), and Playwright 1.62 has no Browser.process() for the
SIGKILL fallback. The daemon's SIGINT shutdown therefore took over 5 s, which
reddened the CLI lifecycle test under load (1 of 8 on main) and left browsers
alive at shard cleanup. gstack now detaches only its own listeners: close
takes ~50 ms.

cleanupXvfb treated a terminated Xvfb child as alive until our own blocked
event loop reaped it, so every close waited a full second then sent SIGKILL
to a zombie. A zombie now counts as exited; the display test counts only
running Xvfb processes.

browse/src/browser-manager.ts

@@ -323,6 +323,21 @@ export class BrowserManager {
   private extensionContext = false;
   /** Pending and recorded navigation-guard verdicts per page (D2). */
   private navigationGuards = new WeakMap<Page, { pending: Set<Promise<void>>; blocked: string | null }>();
+  // Our own 'disconnected' listeners per browser. Detach only these: removing
+  // every 'disconnected' listener also strips Playwright's internal one that
+  // resolves browser.close(), so each close waited the full race timeout (and
+  // Playwright 1.62 has no Browser.process() for the SIGKILL fallback).
+  private disconnectHandlers = new WeakMap<Browser, Array<() => void>>();
+  private onBrowserDisconnected(browser: Browser | null | undefined, handler: () => void): void {
+    if (!browser) return;
+    browser.on('disconnected', handler);
+    this.disconnectHandlers.set(browser, [...(this.disconnectHandlers.get(browser) ?? []), handler]);
+  }
+  private detachDisconnectHandlers(browser: Browser | null | undefined): void {
+    if (!browser) return;
+    for (const handler of this.disconnectHandlers.get(browser) ?? []) browser.off('disconnected', handler);
+    this.disconnectHandlers.delete(browser);
+  }
 
   // ─── Dialog Handling (global, not per-tab) ──────────────────
   private dialogAutoAccept: boolean = true;
@@ -601,7 +616,7 @@ export class BrowserManager {
       }), { usesCustomExecutable: Boolean(executablePath) }).catch(nameCustomChromium);
       this.extensionContext = true;
       this.browser = this.context.browser();
-      this.browser?.on('disconnected', () => {
+      this.onBrowserDisconnected(this.browser, () => {
         void handleChromiumDisconnect(this.browser);
       });
       for (const page of this.context.pages()) await page.close().catch(() => {});
@@ -651,7 +666,7 @@ export class BrowserManager {
     // means "user wanted this, don't restart"; non-zero means "crash, please
     // bring me back." Without this distinction every Cmd+Q gets treated as
     // a crash and the user-visible window keeps respawning.
-    this.browser.on('disconnected', () => {
+    this.onBrowserDisconnected(this.browser, () => {
       void handleChromiumDisconnect(this.browser);
     });
 
@@ -944,7 +959,7 @@ export class BrowserManager {
     // terminal agent, save session, clean profile locks + state file) so
     // crashes don't strand resources either.
     if (this.browser) {
-      this.browser.on('disconnected', () => {
+      this.onBrowserDisconnected(this.browser, () => {
         if (this.intentionalDisconnect) return;
         const browserRef = this.browser;
         void (async () => {
@@ -1003,14 +1018,14 @@ export class BrowserManager {
       if (this.connectionMode === 'headed' || this.handoffPrevious || this.extensionContext) {
         // Headed/persistent context mode: close the context (which closes the browser)
         this.intentionalDisconnect = true;
-        if (this.browser) this.browser.removeAllListeners('disconnected');
+        this.detachDisconnectHandlers(this.browser);
         await Promise.race([
           this.context ? this.context.close() : Promise.resolve(),
           raceTimeout(this.closeRaceMs),
         ]).catch(() => {});
       } else if (this.browser) {
         // Launched mode: close the browser we spawned.
-        this.browser.removeAllListeners('disconnected');
+        this.detachDisconnectHandlers(this.browser);
         // Grab the child handle BEFORE the race: nulling this.browser after a
         // race-timeout used to ABANDON a live Chromium whose sockets kept the
         // caller's event loop (and keep-alive connections into test servers)
@@ -1028,7 +1043,7 @@ export class BrowserManager {
       this.browser = null;
     }
     if (previousBrowser && previousBrowser !== currentBrowser) {
-      previousBrowser.removeAllListeners('disconnected');
+      this.detachDisconnectHandlers(previousBrowser);
       const child = launchedProcess(previousBrowser);
       const closed = await Promise.race([
         previousBrowser.close().then(() => true), raceTimeout(this.closeRaceMs),
@@ -2001,7 +2016,7 @@ export class BrowserManager {
 
       if (this.browser) {
         const browserRef = this.browser;
-        this.browser.on('disconnected', () => {
+        this.onBrowserDisconnected(this.browser, () => {
           if (this.intentionalDisconnect) return;
           void handleChromiumDisconnect(browserRef);
         });
@@ -2015,7 +2030,7 @@ export class BrowserManager {
       catch (err) { console.warn('[browse] Headed promotion callback failed:', err); }
 
       // 4. Close old headless browser (fire-and-forget)
-      previous.browser.removeAllListeners('disconnected');
+      this.detachDisconnectHandlers(previous.browser);
       previous.browser.close().catch(() => {});
 
       return [

browse/src/xvfb.ts

@@ -234,14 +234,23 @@ export async function spawnXvfb(displayNum: number): Promise<XvfbHandle> {
  *
  * Best-effort: never throws.
  */
+function isZombie(pid: number): boolean {
+  try {
+    const stat = fs.readFileSync(`/proc/${pid}/stat`, 'utf8');
+    return stat.slice(stat.lastIndexOf(')') + 2).startsWith('Z');
+  } catch { return false; }
+}
+
 export function cleanupXvfb(state: { pid: number; startTime: string; display: string }): void {
   if (!state.pid) return;
   if (!isOurXvfb(state.pid, state.startTime)) return;
   try { safeKill(state.pid, 'SIGTERM'); } catch { /* swallow */ }
   // Wait briefly for Xvfb to exit, then SIGKILL if still alive.
+  // An exited child stays a zombie until our event loop reaps it, and this
+  // synchronous wait blocks that loop, so a zombie counts as exited here.
   const deadline = Date.now() + 1000;
   while (Date.now() < deadline) {
-    if (!isProcessAlive(state.pid)) break;
+    if (!isProcessAlive(state.pid) || isZombie(state.pid)) break;
     Bun.sleepSync(10);
   }
   if (isOurXvfb(state.pid, state.startTime)) {

browse/test/handoff.test.ts

@@ -331,10 +331,12 @@ describe.skipIf(process.platform !== 'linux')('lazy owned display lifecycle', ()
   let root: string;
   let hbm: BrowserManager;
   const displays = () => {
-    const result = Bun.spawnSync(['ps', '--ppid', String(process.pid), '-o', 'comm='], {
+    // A terminated Xvfb can linger as a zombie until this process reaps it; it is no display.
+    const result = Bun.spawnSync(['ps', '--ppid', String(process.pid), '-o', 'stat=,comm='], {
       stdout: 'pipe', stderr: 'pipe', timeout: 2000,
     });
-    return result.stdout.toString().split('\n').filter(line => line.trim() === 'Xvfb').length;
+    return result.stdout.toString().split('\n').map(line => line.trim().split(/\s+/))
+      .filter(([stat, comm]) => comm === 'Xvfb' && !stat?.startsWith('Z')).length;
   };
 
   beforeEach(async () => {

browse/test/headless-launch-options.test.ts

@@ -51,6 +51,17 @@ describe.skipIf(process.platform !== 'linux')('headless launch options (Linux, r
     expect(cmdlines.some(c => c.includes('headless_shell'))).toBe(false);
   }, 60_000);
 
+  test('close returns when Chromium exits, not at the close race timeout', async () => {
+    // Removing every 'disconnected' listener used to strip Playwright's own,
+    // so browser.close() never resolved and every close waited 5 s.
+    bm = new BrowserManager();
+    await bm.launch();
+    const started = performance.now();
+    await bm.close();
+    expect(performance.now() - started).toBeLessThan(2_500);
+    bm = null;
+  }, 60_000);
+
   test('a broken GSTACK_CHROMIUM_PATH fails with the path named', async () => {
     const missing = path.join(scratch, 'no-such-chromium');
     process.env.GSTACK_CHROMIUM_PATH = missing;