diff --git a/browse/src/browser-manager.ts b/browse/src/browser-manager.ts index 2ae2221d8..c976511ed 100644 --- a/browse/src/browser-manager.ts +++ b/browse/src/browser-manager.ts @@ -182,6 +182,41 @@ function launchedProcess(browser: Browser | null | undefined): ChildProcess | nu return typeof withProcess?.process === 'function' ? withProcess.process() : null; } +/** + * #2709: PID of the Chromium browser process gstack launched, read over CDP + * (`SystemInfo.getProcessInfo`, type "browser") because Playwright 1.62's + * Browser has no `process()`. Call it only for a launch gstack owns; a + * connected browser belongs to someone else and must never be reaped. Bounded: + * a hung or failed CDP call logs once and returns null, which means no reap. + */ +export async function launchedChromiumPid(browser: Browser, timeoutMs = 2_000): Promise { + let timer: ReturnType | undefined; + const query = async (): Promise => { + const session = await browser.newBrowserCDPSession(); + try { + const { processInfo } = await session.send('SystemInfo.getProcessInfo'); + const pid = processInfo.find((p) => p.type === 'browser')?.id; + return Number.isInteger(pid) && pid! > 0 ? pid! : null; + } finally { + await session.detach().catch(() => {}); + } + }; + try { + return await Promise.race([ + query(), + new Promise((_, reject) => { + timer = setTimeout(() => reject(new Error(`timed out after ${timeoutMs}ms`)), timeoutMs); + }), + ]); + } catch (err) { + const msg = err instanceof Error ? err.message : String(err); + console.warn(`[browse] Could not record the Chromium PID (CDP SystemInfo.getProcessInfo: ${msg}); browse stop cannot reap a surviving Chromium.`); + return null; + } finally { + clearTimeout(timer); + } +} + /** * Resolve why the underlying Chromium ChildProcess is going away. * @@ -669,12 +704,10 @@ export class BrowserManager { }); // #2709: record the child's identity so the CLI can reap a survivor after - // daemon shutdown. `.process()` exists here — we launched this browser. + // daemon shutdown. We launched this browser, so its PID is ours to record. { - const proc = launchedProcess(this.browser); - this.chromiumProcInfo = proc?.pid - ? { pid: proc.pid, startTime: readPidStartTime(proc.pid) } - : null; + const pid = await launchedChromiumPid(this.browser); + this.chromiumProcInfo = pid ? { pid, startTime: readPidStartTime(pid) } : null; } this.context = await this.browser.newContext(contextOptions); diff --git a/browse/test/headless-gpu-and-reap.test.ts b/browse/test/headless-gpu-and-reap.test.ts index cef5776e1..4b06423c1 100644 --- a/browse/test/headless-gpu-and-reap.test.ts +++ b/browse/test/headless-gpu-and-reap.test.ts @@ -14,10 +14,11 @@ */ import { describe, expect, test } from 'bun:test'; import { spawn } from 'node:child_process'; -import { headlessGpuArgs } from '../src/browser-manager'; +import { BrowserManager, headlessGpuArgs, launchedChromiumPid } from '../src/browser-manager'; import { reapRecordedChromium } from '../src/cli'; import { readPidStartTime } from '../src/xvfb'; import { isProcessAlive } from '../src/error-handling'; +import { chromium, type Browser } from 'playwright'; describe('headlessGpuArgs (#2709)', () => { test('darwin gets the validated four-flag set', () => { @@ -105,6 +106,83 @@ describe.skipIf(process.platform !== 'linux')('reapRecordedChromium identity gat }); }); +// E3: Playwright 1.62's Browser has no process(), so the recorded identity was +// always null and the reap above never ran. The PID now comes from CDP +// SystemInfo.getProcessInfo, only for a launch gstack owns. +describe.skipIf(process.platform !== 'linux')('Chromium PID via CDP on the pinned Playwright (#2709, E3)', () => { + const closeQuietly = (b: { close(): Promise } | null | undefined) => + Promise.race([b?.close().catch(() => {}), new Promise((r) => setTimeout(r, 3_000))]); + + test('an owned headless launch records the browser PID and start time, and the reap kills it', async () => { + const bm = new BrowserManager(); + await bm.launch(); + try { + const info = bm.getChromiumProcInfo(); + expect(info, 'headless launch recorded no Chromium identity').not.toBeNull(); + expect(isProcessAlive(info!.pid)).toBe(true); + expect(info!.startTime).toBe(readPidStartTime(info!.pid)); + expect(info!.startTime).not.toBe(''); + await reapRecordedChromium({ chromiumPid: info!.pid, chromiumStartTime: info!.startTime }); + expect(isProcessAlive(info!.pid)).toBe(false); + } finally { + await closeQuietly(bm); + } + }, 60_000); + + test('an externally owned Chromium is never reaped: its PID is not recorded and a stale identity never matches', async () => { + const external = await chromium.launch({ headless: true, chromiumSandbox: false }); + const bm = new BrowserManager(); + await bm.launch(); + try { + const externalPid = await launchedChromiumPid(external); + expect(externalPid).not.toBeNull(); + const ours = bm.getChromiumProcInfo(); + expect(ours?.pid).not.toBe(externalPid); + await reapRecordedChromium({ chromiumPid: ours!.pid, chromiumStartTime: ours!.startTime }); + expect(isProcessAlive(externalPid!)).toBe(true); + // A recycled PID: a live Chromium at the recorded PID with another start time. + await reapRecordedChromium({ chromiumPid: externalPid!, chromiumStartTime: 'Mon Jan 1 00:00:00 1990' }); + expect(isProcessAlive(externalPid!)).toBe(true); + } finally { + await closeQuietly(bm); + await closeQuietly(external); + } + }, 60_000); +}); + +describe('launchedChromiumPid bounded CDP failure (E3)', () => { + const stub = (newBrowserCDPSession: () => Promise) => ({ newBrowserCDPSession }) as unknown as Browser; + + test('a hung CDP session returns null within the bound and logs once', async () => { + const warn = console.warn; + const lines: string[] = []; + console.warn = (...args: unknown[]) => { lines.push(args.join(' ')); }; + try { + const t0 = Date.now(); + expect(await launchedChromiumPid(stub(() => new Promise(() => {})), 100)).toBeNull(); + expect(Date.now() - t0).toBeLessThan(2_000); + } finally { + console.warn = warn; + } + expect(lines).toEqual([ + '[browse] Could not record the Chromium PID (CDP SystemInfo.getProcessInfo: timed out after 100ms); browse stop cannot reap a surviving Chromium.', + ]); + }); + + test('a failing or browser-less answer returns null', async () => { + const warn = console.warn; + console.warn = () => {}; + try { + expect(await launchedChromiumPid(stub(async () => { throw new Error('Target closed'); }))).toBeNull(); + const session = (processInfo: unknown[]) => async () => ({ send: async () => ({ processInfo }), detach: async () => {} }); + expect(await launchedChromiumPid(stub(session([{ type: 'GPU', id: 4242 }])))).toBeNull(); + expect(await launchedChromiumPid(stub(session([{ type: 'browser', id: 4242 }])))).toBe(4242); + } finally { + console.warn = warn; + } + }); +}); + // ─── Source pins: the #2709 pieces stay WIRED ──────────────────────────── // The unit tests above prove headlessGpuArgs and reapRecordedChromium behave; // these pins prove the daemon persists the child's identity, the CLI reaps on @@ -195,4 +273,12 @@ describe('stop-reap wiring pins (#2709)', () => { const headless = src.slice(src.indexOf(' async launch() {'), src.indexOf(' async launchHeaded(')); expect(headless).toContain('launchArgs.push(...headlessGpuArgs(process.platform, process.env));'); }); + + test('launchedChromiumPid is called only by the owned headless launch()', () => { + const src = read('browser-manager.ts'); + const calls = src.match(/await launchedChromiumPid\(/g) ?? []; + expect(calls.length).toBe(1); + const headless = src.slice(src.indexOf(' async launch() {'), src.indexOf(' async launchHeaded(')); + expect(headless).toContain('const pid = await launchedChromiumPid(this.browser);'); + }); });