From 602bc0baa9c88a20db773328d428dd031dfa89cb Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Sat, 20 Jun 2026 12:30:28 +0200 Subject: [PATCH] fix(bench): clean timed-out sample process groups --- scripts/bench-cli-startup.ts | 132 ++++++++++++++++++++----- test/scripts/bench-cli-startup.test.ts | 74 ++++++++++++++ 2 files changed, 184 insertions(+), 22 deletions(-) diff --git a/scripts/bench-cli-startup.ts b/scripts/bench-cli-startup.ts index b7a2d2db6be4..cebcd9dc7755 100644 --- a/scripts/bench-cli-startup.ts +++ b/scripts/bench-cli-startup.ts @@ -105,6 +105,8 @@ type CliOptions = { const DEFAULT_RUNS = 5; const DEFAULT_WARMUP = 1; const DEFAULT_TIMEOUT_MS = 30_000; +const TIMEOUT_KILL_GRACE_MS = 1_000; +const PROCESS_GROUP_EXIT_POLL_MS = 25; const DEFAULT_ENTRY = "openclaw.mjs"; const MAX_RSS_MARKER = "__OPENCLAW_MAX_RSS_KB__="; const VALUE_FLAGS = new Set([ @@ -708,12 +710,16 @@ async function runSample(params: { let stderr = ""; let settled = false; let timedOut = false; + let forceKillAt: number | null = null; + let forceKillTimer: ReturnType | null = null; const maxOutputLength = 32 * 1024 * 1024; try { return await new Promise((resolve) => { + const useProcessGroup = process.platform !== "win32"; const proc = spawn(process.execPath, nodeArgs, { cwd: process.cwd(), + detached: useProcessGroup, env: { ...process.env, HOME: runRoot, @@ -733,6 +739,10 @@ async function runSample(params: { return; } settled = true; + if (forceKillTimer) { + clearTimeout(forceKillTimer); + forceKillTimer = null; + } const ms = Number(process.hrtime.bigint() - started) / 1e6; resolve({ ms, @@ -751,18 +761,11 @@ async function runSample(params: { const timeout = setTimeout(() => { timedOut = true; - try { - proc.kill("SIGTERM"); - } catch { - // Best-effort timeout cleanup. - } - setTimeout(() => { - try { - proc.kill("SIGKILL"); - } catch { - // Best-effort timeout cleanup. - } - }, 1_000).unref?.(); + signalSampleProcess(proc, "SIGTERM", useProcessGroup); + forceKillAt = Date.now() + TIMEOUT_KILL_GRACE_MS; + forceKillTimer = setTimeout(() => { + signalSampleProcess(proc, "SIGKILL", useProcessGroup); + }, TIMEOUT_KILL_GRACE_MS).unref?.(); }, params.timeoutMs); timeout.unref?.(); @@ -790,16 +793,27 @@ async function runSample(params: { }); proc.once("close", (code, signal) => { clearTimeout(timeout); - finish({ - exitCode: code, - signal, - ...(code === 0 && signal == null - ? {} - : { - stdoutTail: tailLines(stdout, 20), - stderrTail: tailLines(stderr, 20), - }), - }); + const complete = () => + finish({ + exitCode: code, + signal, + ...(code === 0 && signal == null + ? {} + : { + stdoutTail: tailLines(stdout, 20), + stderrTail: tailLines(stderr, 20), + }), + }); + if (timedOut && isSampleProcessGroupAlive(proc, useProcessGroup)) { + void finishAfterTimeoutCleanup({ + complete, + forceKillAt, + proc, + useProcessGroup, + }); + return; + } + complete(); }); }); } finally { @@ -807,6 +821,80 @@ async function runSample(params: { } } +async function finishAfterTimeoutCleanup(params: { + complete: () => void; + forceKillAt: number | null; + proc: ReturnType; + useProcessGroup: boolean; +}): Promise { + const graceRemainingMs = + params.forceKillAt === null + ? TIMEOUT_KILL_GRACE_MS + : Math.max(0, params.forceKillAt - Date.now()); + if (graceRemainingMs > 0) { + await waitForSampleProcessGroupExit(params.proc, params.useProcessGroup, graceRemainingMs); + } + if (isSampleProcessGroupAlive(params.proc, params.useProcessGroup)) { + signalSampleProcess(params.proc, "SIGKILL", params.useProcessGroup); + } + await waitForSampleProcessGroupExit(params.proc, params.useProcessGroup, TIMEOUT_KILL_GRACE_MS); + params.complete(); +} + +function signalSampleProcess( + proc: ReturnType, + signal: NodeJS.Signals, + useProcessGroup: boolean, +): void { + if (!proc.pid) { + return; + } + try { + if (useProcessGroup) { + process.kill(-proc.pid, signal); + } else { + proc.kill(signal); + } + } catch (error) { + const code = (error as NodeJS.ErrnoException | undefined)?.code; + if (code !== "ESRCH" && code !== "EPERM") { + throw error; + } + } +} + +function isSampleProcessGroupAlive( + proc: ReturnType, + useProcessGroup: boolean, +): boolean { + if (!useProcessGroup || !proc.pid) { + return false; + } + try { + process.kill(-proc.pid, 0); + return true; + } catch (error) { + return (error as NodeJS.ErrnoException | undefined)?.code === "EPERM"; + } +} + +async function waitForSampleProcessGroupExit( + proc: ReturnType, + useProcessGroup: boolean, + timeoutMs: number, +): Promise { + const deadlineAt = Date.now() + timeoutMs; + while (Date.now() < deadlineAt) { + if (!isSampleProcessGroupAlive(proc, useProcessGroup)) { + return true; + } + await new Promise((resolvePoll) => { + setTimeout(resolvePoll, PROCESS_GROUP_EXIT_POLL_MS); + }); + } + return !isSampleProcessGroupAlive(proc, useProcessGroup); +} + async function runCase(params: { entry: string; commandCase: CommandCase; diff --git a/test/scripts/bench-cli-startup.test.ts b/test/scripts/bench-cli-startup.test.ts index 49bf742faf04..2b99e9485db7 100644 --- a/test/scripts/bench-cli-startup.test.ts +++ b/test/scripts/bench-cli-startup.test.ts @@ -29,6 +29,15 @@ function withEnv(env: Record, callback: () => T): } } +function isProcessAlive(pid: number): boolean { + try { + process.kill(pid, 0); + return true; + } catch { + return false; + } +} + describe("bench-cli-startup", () => { it("rejects unknown CLI options before running benchmarks", () => { expect(() => testing.validateCliArgs(["--wat"])).toThrow("Unknown argument: --wat"); @@ -49,6 +58,71 @@ describe("bench-cli-startup", () => { expect(result.stderr).not.toContain("\n at "); }); + it.runIf(process.platform !== "win32")( + "cleans timed-out benchmark process groups when the leader exits first", + () => { + const tempDirs = createTempDirTracker(); + const tmpDir = tempDirs.make("openclaw-cli-startup-timeout-group-"); + const entryPath = join(tmpDir, "entry.mjs"); + const childPidPath = join(tmpDir, "child.pid"); + let childPid = 0; + try { + writeFileSync( + entryPath, + [ + "import { spawn } from 'node:child_process';", + "import { writeFileSync } from 'node:fs';", + "process.on('SIGTERM', () => process.exit(0));", + "const child = spawn(process.execPath, [", + " '-e',", + " \"process.on('SIGTERM',()=>{});setInterval(()=>{},1000);\",", + "], { stdio: 'ignore' });", + `writeFileSync(${JSON.stringify(childPidPath)}, String(child.pid));`, + "setInterval(() => {}, 1000);", + "", + ].join("\n"), + "utf8", + ); + + const result = spawnSync( + process.execPath, + [ + "--import", + "tsx", + "scripts/bench-cli-startup.ts", + "--entry", + entryPath, + "--case", + "version", + "--runs", + "1", + "--warmup", + "0", + "--timeout-ms", + "100", + "--json", + ], + { + cwd: join(__dirname, "../.."), + encoding: "utf8", + timeout: 8_000, + }, + ); + + childPid = Number(readFileSync(childPidPath, "utf8")); + expect(result.status).toBe(1); + expect(result.signal).toBeNull(); + expect(result.stderr).toContain("version sample 1: timed out"); + expect(isProcessAlive(childPid)).toBe(false); + } finally { + if (childPid && isProcessAlive(childPid)) { + process.kill(childPid, "SIGKILL"); + } + tempDirs.cleanup(); + } + }, + ); + it("writes compare-mode JSON output and creates parent directories", () => { const tempDirs = createTempDirTracker(); const tmpDir = tempDirs.make("openclaw-cli-startup-compare-output-");