mirror of
https://github.com/garrytan/gstack.git
synced 2026-10-31 14:06:50 +01:00
E3: record the owned Chromium's PID over CDP so the #2709 reap runs on Playwright 1.62
Playwright 1.62's Browser has no process(), so chromiumProcInfo was always null and browse stop never reaped a surviving headless Chromium. The owned headless launch now reads the browser PID from CDP SystemInfo.getProcessInfo (bounded at 2s; a hung or failed call logs once and records nothing). The reaper's start-time and cmdline identity checks are unchanged, so a recycled PID or an externally owned browser is never signalled.
This commit is contained in:
1 parent
850ac83997
commit
c58cd80317
2 files changed
+125
-6
No files matched your search
@@ -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<number | null> {
|
||||
let timer: ReturnType<typeof setTimeout> | undefined;
|
||||
const query = async (): Promise<number | null> => {
|
||||
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<never>((_, 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);
|
||||
|
||||
@@ -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<void> } | 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<unknown>) => ({ 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);');
|
||||
});
|
||||
});
|
||||
Reference in new issue
Block a user