From 5b6d5cbcd278b39ef672171f38212896f602c462 Mon Sep 17 00:00:00 2001 From: Peter Amiri Date: Mon, 5 Oct 2026 16:45:56 -0700 Subject: [PATCH 1/2] fix(docs): bound every harness command so a stuck wheels can't hang the run runExec() had no default timeout and resolved only when the command's output pipes closed, which a leftover process can hold open forever: one stuck wheels call waited out a test's 300 s limit and then hung the verify job. - A default timeout (240 s, WHEELS_EXEC_TIMEOUT_MS) kills the command's whole process group and fails with 'timed out after Ns running '. - It returns 2 s after the command exits even if a leftover process still holds the pipes (not killed: a block may leave a server running). - Commands still running when the harness exits are stopped. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Peter Amiri --- .../guides/scripts/verify-docs/lib/exec.mjs | 81 +++++++++++++++++-- .../scripts/verify-docs/test/exec.test.mjs | 71 ++++++++++++++++ 2 files changed, 146 insertions(+), 6 deletions(-) diff --git a/web/sites/guides/scripts/verify-docs/lib/exec.mjs b/web/sites/guides/scripts/verify-docs/lib/exec.mjs index 1648dc39a7..b156b7ffb0 100644 --- a/web/sites/guides/scripts/verify-docs/lib/exec.mjs +++ b/web/sites/guides/scripts/verify-docs/lib/exec.mjs @@ -63,24 +63,70 @@ export async function wheelsBinaryAttestation() { return `wheels binary: ${RESOLVED_WHEELS} (via ${source}) — ${version} — mode: ${mode}`; } +/** + * How long a single command may run before runExec() gives up on it: WHEELS_EXEC_TIMEOUT_MS when it + * is a positive number, else 240 s. Below the harness tests' own 300 s limit, so a stuck command fails + * with its name instead of as an anonymous test timeout. + */ +export function defaultExecTimeout(env = process.env) { + const ms = Number(env.WHEELS_EXEC_TIMEOUT_MS); + return Number.isFinite(ms) && ms > 0 ? ms : 240_000; +} + +// After the command exits, how long to keep reading output a leftover process may still be writing. +const EXIT_GRACE_MS = 2_000; + +// Commands still running. Each leads its own process group, so a Ctrl-C at the terminal no longer +// reaches it; when the harness exits (normally, or via the signal handlers in isolated-home.mjs), +// stop whatever is left rather than leave it running. +const running = new Set(); +process.on('exit', () => { + for (const proc of running) killGroup(proc, 'SIGKILL'); +}); + +// Kill the child's whole process group (it leads one: spawned detached), so a JVM or anything else it +// started goes too; fall back to the child alone where groups aren't available. +function killGroup(proc, signal) { + try { + if (process.platform !== 'win32') { + process.kill(-proc.pid, signal); + return; + } + } catch { + // no group (already gone, or not a group leader): try the child itself + } + try { proc.kill(signal); } catch {} +} + /** * Launches `program` with the given argv array. Never invokes a shell. - * Returns `{ code, stdout, stderr }`. `code` is the process exit code, or -1 - * on spawn error (stderr will contain the Node error message in that case). + * Returns `{ code, stdout, stderr }`, plus `timedOut: true` when the command was stopped for running + * too long. `code` is the process exit code, or -1 on spawn error or timeout (stderr then carries the + * reason). * * Why no shell: the harness runs command strings pulled from MDX metadata. * Using `sh -c` would be a shell-injection surface. All callers must * pre-tokenize into program + args. + * + * Bounded, so one stuck `wheels` can't stall the run (a hung `wheels` call used to wait out a test's + * 300 s limit and then hang the job): + * - `opts.timeout` (default defaultExecTimeout()) stops the command and the whole process group it + * leads, and reports "timed out after Ns running ". + * - It returns when the command exits, after a short grace for the last output, even if a leftover + * process still holds its output pipes. That leftover is not killed: a block may legitimately leave + * a server running (`wheels start`). */ export function runExec(program, args = [], opts = {}) { - const { cwd, env, timeout } = opts; + const { cwd, env } = opts; + const timeout = opts.timeout ?? defaultExecTimeout(); const spawnOpts = { stdio: ['ignore', 'pipe', 'pipe'], shell: false, + // Its own process group, so a timeout can kill everything the command started. + detached: process.platform !== 'win32', }; if (cwd !== undefined) spawnOpts.cwd = cwd; if (env !== undefined) spawnOpts.env = env; - if (timeout !== undefined) spawnOpts.timeout = timeout; // Substitute the absolute `wheels` path resolved at module load. // Belt-and-braces: also protects against shells where PATH doesn't @@ -91,15 +137,38 @@ export function runExec(program, args = [], opts = {}) { // `wheels new` exited 0 despite a framework-not-found error. See // fixtures.mjs `createFixture` for the guard. const resolvedProgram = program === 'wheels' ? RESOLVED_WHEELS : program; + const label = [program, ...args].join(' '); return new Promise((resolve) => { const proc = spawn(resolvedProgram, args, spawnOpts); + running.add(proc); let stdout = ''; let stderr = ''; + let settled = false; + let graceTimer; + const finish = (result) => { + if (settled) return; + settled = true; + running.delete(proc); + clearTimeout(timeoutTimer); + clearTimeout(graceTimer); + proc.stdout.destroy(); + proc.stderr.destroy(); + resolve(result); + }; + const timeoutTimer = setTimeout(() => { + killGroup(proc, 'SIGKILL'); + const reason = `timed out after ${timeout / 1000}s running ${label}`; + finish({ code: -1, stdout, stderr: stderr ? `${stderr}\n${reason}` : reason, timedOut: true }); + }, timeout); proc.stdout.on('data', (d) => (stdout += d.toString())); proc.stderr.on('data', (d) => (stderr += d.toString())); - proc.on('error', (err) => resolve({ code: -1, stdout, stderr: stderr + err.message })); - proc.on('close', (code) => resolve({ code, stdout, stderr })); + proc.on('error', (err) => finish({ code: -1, stdout, stderr: stderr + err.message })); + proc.on('exit', (code, signal) => { + const exitCode = code ?? (signal ? -1 : 0); + graceTimer = setTimeout(() => finish({ code: exitCode, stdout, stderr }), EXIT_GRACE_MS); + }); + proc.on('close', (code, signal) => finish({ code: code ?? (signal ? -1 : 0), stdout, stderr })); }); } diff --git a/web/sites/guides/scripts/verify-docs/test/exec.test.mjs b/web/sites/guides/scripts/verify-docs/test/exec.test.mjs index 11b3068e02..5e660541d1 100644 --- a/web/sites/guides/scripts/verify-docs/test/exec.test.mjs +++ b/web/sites/guides/scripts/verify-docs/test/exec.test.mjs @@ -34,3 +34,74 @@ test('attestation defaults to as-installed mode when WHEELS_ATTEST_MODE is unset if (prev !== undefined) process.env.WHEELS_ATTEST_MODE = prev; } }); + +// A `wheels` call that never finishes used to stall the harness until the test's own timeout and then +// hang the job: runExec() had no default timeout and waited for its output pipes to close, which a +// leftover process can hold open forever. These use small `node` programs in place of `wheels`. +const NODE = process.execPath; + +// Starts a grandchild that inherits the output pipes and sleeps, prints its pid, then (optionally) exits. +function parentScript({ exit }) { + return [ + "const { spawn } = require('node:child_process');", + "const g = spawn(process.execPath, ['-e', 'setInterval(() => {}, 1000)'], { stdio: 'inherit' });", + "console.log(g.pid);", + exit ? "setTimeout(() => process.exit(0), 100);" : "setInterval(() => {}, 1000);", + ].join('\n'); +} + +function alive(pid) { + try { process.kill(pid, 0); return true; } catch { return false; } +} + +async function waitDead(pid, ms) { + const deadline = Date.now() + ms; + while (Date.now() < deadline && alive(pid)) await new Promise((r) => setTimeout(r, 100)); + return !alive(pid); +} + +test('a command that never exits fails fast with a timeout that names it', { timeout: 30_000 }, async () => { + const { runExec } = await import('../lib/exec.mjs'); + const started = Date.now(); + const r = await runExec(NODE, ['-e', 'setInterval(() => {}, 1000)'], { timeout: 800 }); + assert.ok(Date.now() - started < 10_000, 'runExec did not return promptly'); + assert.equal(r.code, -1); + assert.equal(r.timedOut, true); + assert.match(r.stderr, /timed out after 0\.8s running /); + assert.match(r.stderr, /setInterval/); +}); + +test('returns once the command exits, even when a leftover process holds its output pipes', { timeout: 30_000 }, async () => { + const { runExec } = await import('../lib/exec.mjs'); + let grandchild; + try { + const started = Date.now(); + const r = await runExec(NODE, ['-e', parentScript({ exit: true })]); + grandchild = Number(r.stdout.trim().split('\n')[0]); + assert.ok(Date.now() - started < 10_000, 'runExec waited for the leftover process'); + assert.equal(r.code, 0); + } finally { + if (grandchild) try { process.kill(grandchild, 'SIGKILL'); } catch {} + } +}); + +test('a timeout kills the whole process group, so nothing it started lingers', { timeout: 30_000 }, async () => { + const { runExec } = await import('../lib/exec.mjs'); + let grandchild; + try { + const r = await runExec(NODE, ['-e', parentScript({ exit: false })], { timeout: 1500 }); + grandchild = Number(r.stdout.trim().split('\n')[0]); + assert.equal(r.timedOut, true); + assert.ok(grandchild > 0, `no grandchild pid in stdout: ${r.stdout}`); + assert.ok(await waitDead(grandchild, 5000), `grandchild ${grandchild} survived the timeout`); + } finally { + if (grandchild) try { process.kill(grandchild, 'SIGKILL'); } catch {} + } +}); + +test('the default timeout comes from WHEELS_EXEC_TIMEOUT_MS', async () => { + const { defaultExecTimeout } = await import('../lib/exec.mjs'); + assert.equal(defaultExecTimeout({ WHEELS_EXEC_TIMEOUT_MS: '5000' }), 5000); + assert.equal(defaultExecTimeout({}), 240_000); + assert.equal(defaultExecTimeout({ WHEELS_EXEC_TIMEOUT_MS: 'nope' }), 240_000); +}); From 941f0189151b66eea1194eb332c1fb740dd49a6c Mon Sep 17 00:00:00 2001 From: Peter Amiri Date: Mon, 5 Oct 2026 16:47:54 -0700 Subject: [PATCH 2/2] fix(docs): stop a command's leftover processes when the harness exits A command that exited while something it started still held its pipes was dropped from the harness-exit cleanup, so a server a block started outlived the harness (and its isolated home). Those groups are now kept and stopped when the harness exits. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Peter Amiri --- .../guides/scripts/verify-docs/lib/exec.mjs | 21 ++++++++++------ .../scripts/verify-docs/test/exec.test.mjs | 25 +++++++++++++++++++ 2 files changed, 39 insertions(+), 7 deletions(-) diff --git a/web/sites/guides/scripts/verify-docs/lib/exec.mjs b/web/sites/guides/scripts/verify-docs/lib/exec.mjs index b156b7ffb0..aa7e2a9c04 100644 --- a/web/sites/guides/scripts/verify-docs/lib/exec.mjs +++ b/web/sites/guides/scripts/verify-docs/lib/exec.mjs @@ -76,12 +76,14 @@ export function defaultExecTimeout(env = process.env) { // After the command exits, how long to keep reading output a leftover process may still be writing. const EXIT_GRACE_MS = 2_000; -// Commands still running. Each leads its own process group, so a Ctrl-C at the terminal no longer -// reaches it; when the harness exits (normally, or via the signal handlers in isolated-home.mjs), -// stop whatever is left rather than leave it running. +// Commands still running, and commands that exited while something they started still held their +// output (a server a block started, say). Each leads its own process group, so a Ctrl-C at the +// terminal no longer reaches it; when the harness exits (normally, or via the signal handlers in +// isolated-home.mjs, which also delete the isolated home those processes run in), stop them all. const running = new Set(); +const leftovers = new Set(); process.on('exit', () => { - for (const proc of running) killGroup(proc, 'SIGKILL'); + for (const proc of [...running, ...leftovers]) killGroup(proc, 'SIGKILL'); }); // Kill the child's whole process group (it leads one: spawned detached), so a JVM or anything else it @@ -113,8 +115,8 @@ function killGroup(proc, signal) { * - `opts.timeout` (default defaultExecTimeout()) stops the command and the whole process group it * leads, and reports "timed out after Ns running ". * - It returns when the command exits, after a short grace for the last output, even if a leftover - * process still holds its output pipes. That leftover is not killed: a block may legitimately leave - * a server running (`wheels start`). + * process still holds its output pipes. That leftover keeps running while the harness does (a block + * may legitimately leave a server running, `wheels start`), and is stopped when the harness exits. */ export function runExec(program, args = [], opts = {}) { const { cwd, env } = opts; @@ -166,7 +168,12 @@ export function runExec(program, args = [], opts = {}) { proc.on('error', (err) => finish({ code: -1, stdout, stderr: stderr + err.message })); proc.on('exit', (code, signal) => { const exitCode = code ?? (signal ? -1 : 0); - graceTimer = setTimeout(() => finish({ code: exitCode, stdout, stderr }), EXIT_GRACE_MS); + graceTimer = setTimeout(() => { + // The pipes are still open, so something the command started is still running: let the + // harness carry on, but keep the group to stop when the harness exits. + leftovers.add(proc); + finish({ code: exitCode, stdout, stderr }); + }, EXIT_GRACE_MS); }); proc.on('close', (code, signal) => finish({ code: code ?? (signal ? -1 : 0), stdout, stderr })); }); diff --git a/web/sites/guides/scripts/verify-docs/test/exec.test.mjs b/web/sites/guides/scripts/verify-docs/test/exec.test.mjs index 5e660541d1..af624206d1 100644 --- a/web/sites/guides/scripts/verify-docs/test/exec.test.mjs +++ b/web/sites/guides/scripts/verify-docs/test/exec.test.mjs @@ -105,3 +105,28 @@ test('the default timeout comes from WHEELS_EXEC_TIMEOUT_MS', async () => { assert.equal(defaultExecTimeout({}), 240_000); assert.equal(defaultExecTimeout({ WHEELS_EXEC_TIMEOUT_MS: 'nope' }), 240_000); }); + +test('a leftover from a command that exited is stopped when the harness exits', { timeout: 30_000 }, async () => { + // A "harness" process runs a command that exits while its grandchild (think: a `wheels start` server) + // keeps running, then exits itself. The grandchild must not outlive it: the isolated home it runs in + // is deleted on exit, and a detached group no longer gets the terminal's Ctrl-C. + const lib = new URL('../lib/exec.mjs', import.meta.url).href; + const harness = [ + `const { runExec } = await import(${JSON.stringify(lib)});`, + `const r = await runExec(process.execPath, ['-e', ${JSON.stringify(parentScript({ exit: true }))}]);`, + "console.log(r.stdout.trim().split('\\n')[0]);", + 'process.exit(0);', + ].join('\n'); + const { spawn } = await import('node:child_process'); + const proc = spawn(NODE, ['--input-type=module', '-e', harness], { stdio: ['ignore', 'pipe', 'inherit'] }); + let out = ''; + proc.stdout.on('data', (d) => (out += d)); + await new Promise((resolve) => proc.once('exit', resolve)); + const grandchild = Number(out.trim()); + try { + assert.ok(grandchild > 0, `no grandchild pid: ${out}`); + assert.ok(await waitDead(grandchild, 5000), `leftover ${grandchild} outlived the harness`); + } finally { + if (grandchild) try { process.kill(grandchild, 'SIGKILL'); } catch {} + } +});