From 022e17c595d6938e49aab38e8b3b78404182b6c6 Mon Sep 17 00:00:00 2001 From: Alistair Smith Date: Wed, 15 Apr 2026 11:38:44 -0700 Subject: [PATCH 1/4] ci: kill in-flight test subprocesses on runner SIGINT/SIGTERM MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Track spawnSafe's in-flight subprocesses in a module-level Set, and on SIGINT/SIGTERM/SIGHUP kill them (taskkill /T on Windows, kill(9) on POSIX) before exiting. Currently onExit() just calls process.exit() without killing anything, so a Buildkite job-cancel orphans the running test process. This is the conservative subset of the original process-group-kill attempt — no spawn-option changes (no `detached`, no `kill(-pid)`), so it won't reap POSIX grandchildren on the timeout path; it only handles the cancel path. The full process-group story can follow in a separate PR. The activeSubprocesses Set is declared above the first top-level `await spawnSafe(...)` (the Linux-only coredump sysctl probe at ~line 223). spawnSafe is a hoisted function declaration; with the const placed next to spawnSafe lower in the file, Linux CI hit a TDZ ReferenceError before any tests ran. --- scripts/runner.node.mjs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/scripts/runner.node.mjs b/scripts/runner.node.mjs index c223ca3d2c4e..1b1ae26a7170 100755 --- a/scripts/runner.node.mjs +++ b/scripts/runner.node.mjs @@ -218,6 +218,12 @@ if (isBuildkite) { let coresDir; +// Declared before the first top-level `await spawnSafe(...)` below — spawnSafe +// is a hoisted function declaration that references this; with the const lower +// in the file the Linux-only coredump sysctl call hit the TDZ. +/** @type {Set} */ +const activeSubprocesses = new Set(); + if (options["coredump-upload"]) { // this sysctl is set in bootstrap.sh to /var/bun-cores-$distro-$release-$arch const sysctl = await spawnSafe({ command: "sysctl", args: ["-n", "kernel.core_pattern"] }); @@ -945,6 +951,7 @@ async function spawnSafe(options) { subprocess.kill(9); } } + activeSubprocesses.delete(subprocess); resolve(); }; await new Promise(resolve => { @@ -975,6 +982,7 @@ async function spawnSafe(options) { env, }); subprocess.on("spawn", () => { + activeSubprocesses.add(subprocess); timestamp = Date.now(); timer = setTimeout(() => done(resolve), timeout); }); @@ -2350,6 +2358,12 @@ function isAlwaysFailure(error) { function onExit(signal) { const label = `${getAnsi("red")}Received ${signal}, exiting...${getAnsi("reset")}`; startGroup(label, () => { + for (const proc of activeSubprocesses) { + try { + if (isWindows) spawnSync("taskkill", ["/pid", String(proc.pid), "/T", "/F"], { stdio: "ignore" }); + else proc.kill(9); + } catch {} + } process.exit(getExitCode("cancel")); }); } From f56f11235d0e2960e29a460a5e1a72e5222e970a Mon Sep 17 00:00:00 2001 From: Alistair Smith Date: Wed, 15 Apr 2026 11:43:49 -0700 Subject: [PATCH 2/4] Add 5s timeout to onExit's taskkill so a hung tree-walk can't block the cancel exit --- scripts/runner.node.mjs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/scripts/runner.node.mjs b/scripts/runner.node.mjs index 1b1ae26a7170..93a48805d7b0 100755 --- a/scripts/runner.node.mjs +++ b/scripts/runner.node.mjs @@ -2360,7 +2360,8 @@ function onExit(signal) { startGroup(label, () => { for (const proc of activeSubprocesses) { try { - if (isWindows) spawnSync("taskkill", ["/pid", String(proc.pid), "/T", "/F"], { stdio: "ignore" }); + if (isWindows) + spawnSync("taskkill", ["/pid", String(proc.pid), "/T", "/F"], { stdio: "ignore", timeout: 5000 }); else proc.kill(9); } catch {} } From 8c0034a369e76613fda88a0ba5c05628223e0e9b Mon Sep 17 00:00:00 2001 From: Alistair Smith Date: Wed, 15 Apr 2026 13:13:39 -0700 Subject: [PATCH 3/4] onExit: fire-and-forget taskkill instead of spawnSync MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit spawnSync serialized N×5s worst-case under --parallel (8 on Windows). spawn() with detached+unref launches each taskkill as an independent process that outlives our process.exit() — CreateProcess is synchronous inside spawn() so the child exists before it returns. --- scripts/runner.node.mjs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/scripts/runner.node.mjs b/scripts/runner.node.mjs index 93a48805d7b0..39db32df2906 100755 --- a/scripts/runner.node.mjs +++ b/scripts/runner.node.mjs @@ -2360,8 +2360,12 @@ function onExit(signal) { startGroup(label, () => { for (const proc of activeSubprocesses) { try { + // Windows: detached+unref so taskkill outlives our process.exit() + // below — CreateProcess is synchronous inside spawn(), so the child + // exists before this returns. Avoids serializing N×timeout under + // --parallel like spawnSync would. if (isWindows) - spawnSync("taskkill", ["/pid", String(proc.pid), "/T", "/F"], { stdio: "ignore", timeout: 5000 }); + spawn("taskkill", ["/pid", String(proc.pid), "/T", "/F"], { stdio: "ignore", detached: true }).unref(); else proc.kill(9); } catch {} } From 537efeff78fd11485a88f20164d3586909c3e27b Mon Sep 17 00:00:00 2001 From: robobun Date: Thu, 16 Apr 2026 22:56:18 +0000 Subject: [PATCH 4/4] Drop Windows taskkill in onExit; Job Object already reaps children On Windows, children spawned without `detached` are placed in the runner's Job Object with JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE, so process.exit() already terminates the whole tree. The taskkill by PID was redundant and, in the race where the child exits first, could hit an unrelated reused PID. POSIX keeps the explicit proc.kill(9) via the ChildProcess handle (safe against PID reuse) since Buildkite cancel signals only the runner PID, not the process group. --- scripts/runner.node.mjs | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/scripts/runner.node.mjs b/scripts/runner.node.mjs index 39db32df2906..d67fa4d6549b 100755 --- a/scripts/runner.node.mjs +++ b/scripts/runner.node.mjs @@ -2358,16 +2358,17 @@ function isAlwaysFailure(error) { function onExit(signal) { const label = `${getAnsi("red")}Received ${signal}, exiting...${getAnsi("reset")}`; startGroup(label, () => { - for (const proc of activeSubprocesses) { - try { - // Windows: detached+unref so taskkill outlives our process.exit() - // below — CreateProcess is synchronous inside spawn(), so the child - // exists before this returns. Avoids serializing N×timeout under - // --parallel like spawnSync would. - if (isWindows) - spawn("taskkill", ["/pid", String(proc.pid), "/T", "/F"], { stdio: "ignore", detached: true }).unref(); - else proc.kill(9); - } catch {} + // Windows: children spawned without `detached` are assigned to this + // process's Job Object with JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE, so + // process.exit() below already terminates the whole tree — nothing to + // do here. POSIX: Buildkite cancel sends SIGTERM to the runner PID only + // (not the group), so orphans survive unless we kill them explicitly. + if (!isWindows) { + for (const proc of activeSubprocesses) { + try { + proc.kill(9); + } catch {} + } } process.exit(getExitCode("cancel")); });