From 1cafadca54c6fbefffd7ea4ec3bedc3b3e5ebe9c Mon Sep 17 00:00:00 2001 From: u7-u7 <116776533+u7-u7@users.noreply.github.com> Date: Wed, 28 Oct 2026 19:56:35 +0800 Subject: [PATCH 1/2] fix(evals): terminate POSIX process groups on timeout --- evals/browser/lib/process.mjs | 22 ++++++++++- evals/browser/tests/process.test.mjs | 56 ++++++++++++++++++++++++++++ 2 files changed, 76 insertions(+), 2 deletions(-) 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..8856546c 100644 --- a/evals/browser/tests/process.test.mjs +++ b/evals/browser/tests/process.test.mjs @@ -33,3 +33,59 @@ 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"); + }, +); From 92b6d0bbe5c8f3c627273d6deec876449ee5d4ae Mon Sep 17 00:00:00 2001 From: u7-u7 <116776533+u7-u7@users.noreply.github.com> Date: Wed, 28 Oct 2026 20:04:47 +0800 Subject: [PATCH 2/2] style(evals): match the locked Biome formatter --- evals/browser/tests/process.test.mjs | 52 +++++++++++++--------------- 1 file changed, 24 insertions(+), 28 deletions(-) diff --git a/evals/browser/tests/process.test.mjs b/evals/browser/tests/process.test.mjs index 8856546c..62ea6ebc 100644 --- a/evals/browser/tests/process.test.mjs +++ b/evals/browser/tests/process.test.mjs @@ -35,11 +35,10 @@ test("process output flushes an incomplete final UTF-8 sequence", async () => { }); 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 = ` + 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); @@ -47,45 +46,42 @@ for (const parentExits of [false, true]) { process.stdout.write("descendant ready\\n"); setTimeout(() => process.exit(0), 5_000); `; - const source = ` + 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 }); + 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"); - }, - ); + 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 = ` +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 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 }); + 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"); - }, -); + 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"); +});