Skip to content

Commit 43b934c

Browse files
fix: exit on stdin EOF and SIGTERM/SIGINT/SIGHUP, closing the browser cleanly (#2117)
Fixes #2116. `chrome-devtools-mcp-main.ts` currently has no shutdown handler. After a session calls `navigate_page` (or anything else that launches Chrome), the Chrome subprocess keeps the Node event loop ref'd, so closing stdin (the stdio MCP convention for "I'm done") doesn't make the server exit. Callers that close stdin to terminate the server have to fall back to SIGTERM / SIGKILL on every page-loaded session — deterministically, not flakily. This change: 1. Adds `closeBrowser()` in `browser.ts` that calls `browser.close()` for launched instances (reaps the Chrome subprocess) and `browser.disconnect()` for attached instances (leaves the user's Chrome alive). No-op if no browser is active or the connection has already been dropped. 2. Registers shutdown handlers in `chrome-devtools-mcp-main.ts` for: - `stdin.on('end' | 'close')` — stdio MCP transport convention - `SIGTERM` / `SIGINT` / `SIGHUP` — clients that signal instead of closing stdin (`SIGHUP` for parity with `src/daemon/daemon.ts`) The handler is idempotent (guarded `shuttingDown` flag), and has an unref'd 10s timeout backstop in case Chrome teardown hangs (slow `beforeunload` handlers, many tabs, etc.). ### Note on scope This complements (does not replace) the client-side fixes filed against #1765, e.g. google-gemini/gemini-cli#13391 and anthropics/claude-code#42300. The MCP stdio convention is that closing stdin signals shutdown; a server that doesn't honor that forces every client to special-case it. The watchdog sub-process (`src/telemetry/watchdog/main.ts:145-146`) and the daemon (`src/daemon/daemon.ts:224-230`) both already implement this for the same reason — this PR extends the same pattern to the main entry point so all three execution paths behave consistently. ### Measurement Repro script in #2116, same env (chrome-devtools-mcp@1.0.1, Chrome 148.0.7778.178, Node v24.11.1, Linux), 10 iterations: | Scenario | Before | After | |---|---|---| | `tools/list only` (no navigation) | 10/10 clean, 30-37 ms | 10/10 clean, 29-40 ms | | `navigate example.com` | 10/10 SIGTERM at ~5080 ms | 10/10 clean at 145-180 ms | ### Notes - I didn't add a subprocess-based test for this; the existing `tests/utils.ts:runCli` infrastructure targets the `chrome-devtools` CLI, not the stdio MCP server, and a shutdown-timing test would introduce non-trivial Chrome-startup flakiness in CI. Happy to add one if maintainers want it — pointer to the right test directory appreciated.
1 parent 8ea5f09 commit 43b934c

3 files changed

Lines changed: 256 additions & 2 deletions

File tree

src/bin/chrome-devtools-mcp-main.ts

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import '../polyfill.js';
88

99
import process from 'node:process';
1010

11+
import {closeBrowser} from '../browser.js';
1112
import {createMcpServer, logDisclaimers} from '../index.js';
1213
import {logger, saveLogsToFile} from '../logger.js';
1314
import {ClearcutLogger} from '../telemetry/ClearcutLogger.js';
@@ -32,6 +33,44 @@ if (process.env['CHROME_DEVTOOLS_MCP_CRASH_ON_UNCAUGHT'] !== 'true') {
3233
});
3334
}
3435

36+
// Shutdown on stdin EOF (stdio MCP convention — the client closes the
37+
// transport to signal exit) and on standard termination signals. Without
38+
// this, an active Chrome subprocess keeps the Node event loop ref'd after
39+
// stdin closes and the server hangs until something else kills it.
40+
let shuttingDown = false;
41+
async function shutdown(reason: string): Promise<void> {
42+
if (shuttingDown) {
43+
return;
44+
}
45+
shuttingDown = true;
46+
logger(`Shutting down (${reason})`);
47+
// Backstop in case browser teardown hangs (e.g. unresponsive Chrome,
48+
// slow beforeunload handlers, many tabs). Exits 0 because we still
49+
// honored the shutdown request; the log line preserves observability.
50+
// Unref'd so it doesn't keep the loop alive on the clean path.
51+
setTimeout(() => {
52+
logger('Shutdown timeout exceeded, forcing exit');
53+
process.exit(0);
54+
}, 10000).unref();
55+
await closeBrowser();
56+
process.exit(0);
57+
}
58+
process.stdin.on('end', () => {
59+
void shutdown('stdin end');
60+
});
61+
process.stdin.on('close', () => {
62+
void shutdown('stdin close');
63+
});
64+
process.on('SIGTERM', () => {
65+
void shutdown('SIGTERM');
66+
});
67+
process.on('SIGINT', () => {
68+
void shutdown('SIGINT');
69+
});
70+
process.on('SIGHUP', () => {
71+
void shutdown('SIGHUP');
72+
});
73+
3574
logger(`Starting Chrome DevTools MCP Server v${VERSION}`);
3675
const {server} = await createMcpServer(args, {
3776
logFile,

src/browser.ts

Lines changed: 37 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import type {
1919
import {puppeteer} from './third_party/index.js';
2020

2121
let browser: Browser | undefined;
22+
let browserMode: 'launched' | 'connected' | undefined;
2223

2324
function makeTargetFilter(enableExtensions = false) {
2425
const ignoredPrefixes = new Set(['chrome://', 'chrome-untrusted://']);
@@ -120,7 +121,12 @@ export async function ensureBrowserConnected(options: {
120121

121122
logger('Connecting Puppeteer to ', JSON.stringify(connectOptions));
122123
try {
123-
browser = await puppeteer.connect(connectOptions);
124+
// Assign mode before browser so a concurrent closeBrowser() never sees
125+
// `browser` set with `browserMode` still undefined (would fall through
126+
// to the disconnect() path and orphan a launched Chrome).
127+
const connected = await puppeteer.connect(connectOptions);
128+
browserMode = 'connected';
129+
browser = connected;
124130
} catch (err) {
125131
throw new Error(
126132
`Could not connect to Chrome. ${autoConnect ? `Check if Chrome is running and remote debugging is enabled by going to chrome://inspect/#remote-debugging.` : `Check if Chrome is running.`}`,
@@ -266,8 +272,37 @@ export async function ensureBrowserLaunched(
266272
if (browser?.connected) {
267273
return browser;
268274
}
269-
browser = await launch(options);
275+
// Assign mode before browser; see the connect path above for rationale.
276+
const launched = await launch(options);
277+
browserMode = 'launched';
278+
browser = launched;
270279
return browser;
271280
}
272281

282+
/**
283+
* Shutdown hook for the active browser. Closes a launched browser (so the
284+
* Chrome subprocess is reaped) or disconnects from an attached browser (so
285+
* the user's Chrome instance stays alive). No-op if no browser is active or
286+
* the connection has already been dropped. Called from the server entrypoint
287+
* on stdin EOF / SIGTERM / SIGINT.
288+
*/
289+
export async function closeBrowser(): Promise<void> {
290+
const b = browser;
291+
const mode = browserMode;
292+
browser = undefined;
293+
browserMode = undefined;
294+
if (!b || !b.connected) {
295+
return;
296+
}
297+
if (mode === 'launched') {
298+
await b.close().catch(err => {
299+
logger('Failed to close browser', err);
300+
});
301+
return;
302+
}
303+
await b.disconnect().catch(err => {
304+
logger('Failed to disconnect from browser', err);
305+
});
306+
}
307+
273308
export type Channel = 'stable' | 'canary' | 'beta' | 'dev';

tests/shutdown.test.ts

Lines changed: 180 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,180 @@
1+
/**
2+
* @license
3+
* Copyright 2026 Google LLC
4+
* SPDX-License-Identifier: Apache-2.0
5+
*/
6+
7+
import assert from 'node:assert';
8+
import type {ChildProcessByStdio} from 'node:child_process';
9+
import {spawn} from 'node:child_process';
10+
import type {Readable, Writable} from 'node:stream';
11+
import {describe, it} from 'node:test';
12+
13+
import {executablePath} from 'puppeteer';
14+
15+
type Server = ChildProcessByStdio<Writable, Readable, Readable>;
16+
17+
// Once shutdown is signalled, the server should be fully gone within this
18+
// budget. The actual fast path is well under 500ms; the budget is set to be
19+
// generous against CI noise without being so loose that it would hide a hang.
20+
const SHUTDOWN_BUDGET_MS = 3000;
21+
// Outer test timeout. If exit doesn't happen within this, treat as a hang
22+
// (the bug we're guarding against) and SIGKILL the subprocess.
23+
const EXIT_TIMEOUT_MS = 15000;
24+
25+
async function spawnServer(): Promise<Server> {
26+
const child = spawn(
27+
'node',
28+
[
29+
'build/src/bin/chrome-devtools-mcp.js',
30+
'--headless',
31+
'--isolated',
32+
'--executable-path',
33+
await executablePath(),
34+
],
35+
{
36+
env: {
37+
...process.env,
38+
CHROME_DEVTOOLS_MCP_NO_USAGE_STATISTICS: 'true',
39+
},
40+
stdio: ['pipe', 'pipe', 'pipe'],
41+
},
42+
) as Server;
43+
// Drain stderr to avoid pipe-buffer backpressure stalling the server.
44+
child.stderr.on('data', () => {
45+
// discard
46+
});
47+
return child;
48+
}
49+
50+
async function waitForExit(
51+
child: Server,
52+
timeoutMs: number,
53+
): Promise<{
54+
code: number | null;
55+
signal: NodeJS.Signals | null;
56+
elapsedMs: number;
57+
}> {
58+
const start = Date.now();
59+
return await new Promise((resolve, reject) => {
60+
const timer = setTimeout(() => {
61+
child.kill('SIGKILL');
62+
reject(new Error(`server did not exit within ${timeoutMs}ms`));
63+
}, timeoutMs);
64+
child.once('exit', (code, signal) => {
65+
clearTimeout(timer);
66+
resolve({code, signal, elapsedMs: Date.now() - start});
67+
});
68+
});
69+
}
70+
71+
async function rpc(
72+
child: Server,
73+
msg: {method: string; params?: unknown},
74+
): Promise<unknown> {
75+
const id = Math.floor(Math.random() * 1e9);
76+
const payload = JSON.stringify({jsonrpc: '2.0', id, ...msg}) + '\n';
77+
return await new Promise((resolve, reject) => {
78+
let buf = '';
79+
const onData = (chunk: Buffer) => {
80+
buf += chunk.toString();
81+
const lines = buf.split('\n');
82+
buf = lines.pop() ?? '';
83+
for (const line of lines) {
84+
if (!line.trim()) {
85+
continue;
86+
}
87+
try {
88+
const parsed = JSON.parse(line) as {id?: number};
89+
if (parsed.id === id) {
90+
child.stdout.off('data', onData);
91+
resolve(parsed);
92+
return;
93+
}
94+
} catch {
95+
// Not a JSON message; ignore.
96+
}
97+
}
98+
};
99+
child.stdout.on('data', onData);
100+
const onExit = () => {
101+
child.stdout.off('data', onData);
102+
reject(new Error('server exited before RPC response'));
103+
};
104+
child.once('exit', onExit);
105+
child.stdin.write(payload);
106+
});
107+
}
108+
109+
function notify(child: Server, msg: {method: string; params?: unknown}): void {
110+
child.stdin.write(JSON.stringify({jsonrpc: '2.0', ...msg}) + '\n');
111+
}
112+
113+
async function initializeAndLaunchBrowser(child: Server): Promise<void> {
114+
await rpc(child, {
115+
method: 'initialize',
116+
params: {
117+
protocolVersion: '2024-11-05',
118+
capabilities: {},
119+
clientInfo: {name: 'shutdown-test', version: '0.0.1'},
120+
},
121+
});
122+
notify(child, {method: 'notifications/initialized'});
123+
// navigate_page forces a real Chrome launch — this is what reproduces
124+
// the hang in #2116. Without an active Chrome subprocess, stdin EOF
125+
// would close the event loop on its own and shutdown would look fine
126+
// even with broken handlers.
127+
await rpc(child, {
128+
method: 'tools/call',
129+
params: {
130+
name: 'navigate_page',
131+
arguments: {url: 'about:blank'},
132+
},
133+
});
134+
}
135+
136+
describe('shutdown', () => {
137+
it('exits within budget on stdin EOF after Chrome launch', async () => {
138+
const child = await spawnServer();
139+
await initializeAndLaunchBrowser(child);
140+
child.stdin.end();
141+
const {elapsedMs} = await waitForExit(child, EXIT_TIMEOUT_MS);
142+
assert.ok(
143+
elapsedMs < SHUTDOWN_BUDGET_MS,
144+
`stdin-EOF shutdown took ${elapsedMs}ms (budget ${SHUTDOWN_BUDGET_MS}ms)`,
145+
);
146+
});
147+
148+
it('exits within budget on SIGTERM after Chrome launch', async () => {
149+
const child = await spawnServer();
150+
await initializeAndLaunchBrowser(child);
151+
child.kill('SIGTERM');
152+
const {elapsedMs} = await waitForExit(child, EXIT_TIMEOUT_MS);
153+
assert.ok(
154+
elapsedMs < SHUTDOWN_BUDGET_MS,
155+
`SIGTERM shutdown took ${elapsedMs}ms (budget ${SHUTDOWN_BUDGET_MS}ms)`,
156+
);
157+
});
158+
159+
it('exits within budget on SIGINT after Chrome launch', async () => {
160+
const child = await spawnServer();
161+
await initializeAndLaunchBrowser(child);
162+
child.kill('SIGINT');
163+
const {elapsedMs} = await waitForExit(child, EXIT_TIMEOUT_MS);
164+
assert.ok(
165+
elapsedMs < SHUTDOWN_BUDGET_MS,
166+
`SIGINT shutdown took ${elapsedMs}ms (budget ${SHUTDOWN_BUDGET_MS}ms)`,
167+
);
168+
});
169+
170+
it('exits within budget on SIGHUP after Chrome launch', async () => {
171+
const child = await spawnServer();
172+
await initializeAndLaunchBrowser(child);
173+
child.kill('SIGHUP');
174+
const {elapsedMs} = await waitForExit(child, EXIT_TIMEOUT_MS);
175+
assert.ok(
176+
elapsedMs < SHUTDOWN_BUDGET_MS,
177+
`SIGHUP shutdown took ${elapsedMs}ms (budget ${SHUTDOWN_BUDGET_MS}ms)`,
178+
);
179+
});
180+
});

0 commit comments

Comments
 (0)