diff --git a/evals/browser/lib/process.mjs b/evals/browser/lib/process.mjs index 37cf7937..7f10b49d 100644 --- a/evals/browser/lib/process.mjs +++ b/evals/browser/lib/process.mjs @@ -14,10 +14,13 @@ export function runProcess(command, args, { cwd, env, timeoutMs = 300_000 } = {} let stderr = ""; let timedOut = false; let settled = false; + let forceKillTimeout; + const useProcessGroup = process.platform !== "win32"; const child = spawn(command, args, { cwd, env: { ...process.env, ...env }, shell: false, + detached: useProcessGroup, stdio: ["ignore", "pipe", "pipe"], }); @@ -28,16 +31,31 @@ export function runProcess(command, args, { cwd, env, timeoutMs = 300_000 } = {} stderr = appendCaptured(stderr, chunk); }); + function terminate(signal) { + if (!useProcessGroup || !child.pid) { + child.kill(signal); + return; + } + try { + // Descendants may keep the pipes open even after the launcher exits. + process.kill(-child.pid, signal); + } catch (error) { + if (error.code !== "ESRCH") child.kill(signal); + } + } + const timeout = setTimeout(() => { timedOut = true; - child.kill("SIGTERM"); - setTimeout(() => child.kill("SIGKILL"), 2_000).unref(); + terminate("SIGTERM"); + forceKillTimeout = setTimeout(() => terminate("SIGKILL"), 2_000); + forceKillTimeout.unref(); }, timeoutMs); const finish = (result) => { if (settled) return; settled = true; clearTimeout(timeout); + clearTimeout(forceKillTimeout); resolve({ command, args, diff --git a/evals/browser/tests/process.test.mjs b/evals/browser/tests/process.test.mjs index a884d711..62ea6ebc 100644 --- a/evals/browser/tests/process.test.mjs +++ b/evals/browser/tests/process.test.mjs @@ -33,3 +33,55 @@ test("process output flushes an incomplete final UTF-8 sequence", async () => { assert.equal(result.stdout, "�"); assert.equal(result.stderr, "�"); }); + +for (const parentExits of [false, true]) { + test(`process timeout closes inherited pipes when the launcher ${parentExits ? "has exited" : "is running"}`, { + skip: process.platform === "win32", + }, async () => { + const descendant = ` + process.on("SIGTERM", () => { + process.stdout.write("descendant terminated\\n"); + process.exit(0); + }); + process.stdout.write("descendant ready\\n"); + setTimeout(() => process.exit(0), 5_000); + `; + const source = ` + const { spawn } = require("node:child_process"); + const child = spawn(process.execPath, ["-e", ${JSON.stringify(descendant)}], { + stdio: ["ignore", "inherit", "inherit"], + }); + ${parentExits ? "child.unref();" : ""} + `; + const result = await runProcess(process.execPath, ["-e", source], { timeoutMs: 500 }); + + assert.equal(result.timedOut, true); + assert.match(result.stdout, /descendant ready/); + assert.match(result.stdout, /descendant terminated/); + assert.ok(result.durationMs < 3_000, `timeout took ${result.durationMs} ms`); + assert.equal(result.exitCode, parentExits ? 0 : null); + assert.equal(result.signal, parentExits ? null : "SIGTERM"); + }); +} + +test("process timeout escalates for descendants that ignore SIGTERM", { + skip: process.platform === "win32", +}, async () => { + const descendant = ` + process.on("SIGTERM", () => {}); + process.stdout.write("descendant ready\\n"); + setTimeout(() => process.exit(0), 6_000); + `; + const source = ` + const { spawn } = require("node:child_process"); + spawn(process.execPath, ["-e", ${JSON.stringify(descendant)}], { + stdio: ["ignore", "inherit", "inherit"], + }); + `; + const result = await runProcess(process.execPath, ["-e", source], { timeoutMs: 500 }); + + assert.equal(result.timedOut, true); + assert.match(result.stdout, /descendant ready/); + assert.ok(result.durationMs < 4_500, `timeout took ${result.durationMs} ms`); + assert.equal(result.signal, "SIGTERM"); +});