diff --git a/src/probe.ts b/src/probe.ts index 5702163..f1b0806 100644 --- a/src/probe.ts +++ b/src/probe.ts @@ -196,14 +196,41 @@ export async function readAppLogTail(exec: ProbeExecFn, lines = 40): Promise` only ever worked + * under bash and busybox; on the Debian-based sandbox images it failed and + * the fallback signalled the group leader alone. + */ +const GROUP_MEMBERS_SH = + // After the ")" that closes the command name, /proc//stat reads + // " …"; a zombie (state Z) is dead and only unreaped. + 'members() { for d in /proc/[0-9]*; do [ -r "$d/stat" ] || continue; ' + + 'set -- $(sed "s/^.*) //" "$d/stat" 2>/dev/null); ' + + '[ "$3" = "$pid" ] && [ "$1" != "Z" ] && echo "${d#/proc/}"; done; }; ' + + // Without /proc (a host runtime on macOS) fall back to the shell's group kill. + 'signal() { if [ -d /proc/self ]; then m=$(members); [ -n "$m" ] || return 1; kill -s "$1" $m 2>/dev/null; ' + + 'else kill -s "$1" -- "-$pid" 2>/dev/null; fi; }; ' + + 'alive() { if [ -d /proc/self ]; then [ -n "$(members)" ]; else kill -0 -- "-$pid" 2>/dev/null; fi; }; '; + export async function stopBackgroundApp(exec: ProbeExecFn): Promise { + // SIGTERM the group, give it five seconds, then SIGKILL whatever is left. + // Dev servers that fork a worker outlive a TERM to the leader: Next.js's + // `start-server` child stayed bound to the port after `npm run dev` exited, + // so the next probe booted on another port while the stale server answered + // the probe's requests (500s on routes it had never compiled). await exec( [ "sh", "-c", - `pid="$(cat ${APP_PID_PATH} 2>/dev/null)"; [ -z "$pid" ] || kill -TERM -- "-$pid" 2>/dev/null || kill "$pid" 2>/dev/null || true`, + `pid="$(cat ${APP_PID_PATH} 2>/dev/null)"; [ -n "$pid" ] || exit 0; ` + + GROUP_MEMBERS_SH + + 'signal TERM || kill -s TERM "$pid" 2>/dev/null || true; ' + + 'i=0; while [ "$i" -lt 10 ] && alive; do sleep 0.5; i=$((i + 1)); done; ' + + 'signal KILL || kill -s KILL "$pid" 2>/dev/null || true', ], - { timeoutMs: 10_000 }, + { timeoutMs: 20_000 }, ); } diff --git a/tests/probe-stop.test.ts b/tests/probe-stop.test.ts new file mode 100644 index 0000000..c765015 --- /dev/null +++ b/tests/probe-stop.test.ts @@ -0,0 +1,89 @@ +/** + * The probe runner's app lifecycle: a dev server that shrugs off SIGTERM + * must still be gone after stopBackgroundApp, or the next probe boots on + * another port while the stale server keeps answering. The shell contract + * is pinned offline; the behavioural case runs where `setsid` exists (Linux, + * the sandbox images) and skips elsewhere. + */ + +import { describe, expect, test } from "bun:test"; +import { APP_PID_PATH, startAppInBackground, stopBackgroundApp, type ProbeExecFn } from "../src/probe.ts"; + +const realExec: ProbeExecFn = async (cmd, opts) => { + const proc = Bun.spawn(cmd, { stdout: "pipe", stderr: "pipe" }); + const timer = setTimeout(() => proc.kill(), opts?.timeoutMs ?? 30_000); + const [stdout, stderr] = await Promise.all([new Response(proc.stdout).text(), new Response(proc.stderr).text()]); + const exitCode = await proc.exited; + clearTimeout(timer); + return { exitCode, stdout, stderr, timedOut: false }; +}; + +describe("stopBackgroundApp", () => { + test("terms the group, waits, then kills what is left — never only a polite TERM", async () => { + const commands: string[][] = []; + await stopBackgroundApp(async (cmd) => { + commands.push(cmd); + return { exitCode: 0, stdout: "", stderr: "", timedOut: false }; + }); + expect(commands).toHaveLength(1); + const script = commands[0]![2]!; + expect(script).toContain(`cat ${APP_PID_PATH}`); + // The group is enumerated from /proc: dash's builtin kill cannot signal a + // negative pid, so `kill -- -` is only the no-/proc fallback. + expect(script).toContain("/proc/[0-9]*"); + expect(script).toContain("signal TERM"); + expect(script).toContain("signal KILL"); + expect(script.indexOf("signal TERM")).toBeLessThan(script.indexOf("signal KILL")); + }); + + test.skipIf(Bun.which("setsid") === null)( + "a server that ignores SIGTERM is gone after stop", + async () => { + // A process group that traps TERM and keeps a child alive, like a dev + // server whose worker outlives the parent. A script file keeps the + // start command free of quotes (startAppInBackground wraps it in `sh -c '…'`). + const script = `/tmp/quickstudy-stubborn-${process.pid}.sh`; + await Bun.write(script, 'trap "" TERM\nsh -c \'trap "" TERM; sleep 300\' &\nwait\n'); + await startAppInBackground(realExec, `sh ${script}`); + const pid = (await Bun.file(APP_PID_PATH).text()).trim(); + // Live group members by /proc, the way the stop itself finds them (zombies excluded). + const members = async () => + ( + await realExec([ + "sh", + "-c", + `for d in /proc/[0-9]*; do set -- $(sed "s/^.*) //" "$d/stat" 2>/dev/null); [ "$3" = "${pid}" ] && [ "$1" != "Z" ] && echo "\${d#/proc/}"; done; true`, + ]) + ).stdout + .trim() + .split("\n") + .filter(Boolean); + const settle = async (): Promise => { + const deadline = Date.now() + 5_000; + let left = await members(); + while (left.length > 0 && Date.now() < deadline) { + await new Promise((resolve) => setTimeout(resolve, 200)); + left = await members(); + } + return left; + }; + try { + expect(pid).toMatch(/^\d+$/); + await new Promise((resolve) => setTimeout(resolve, 300)); + expect((await members()).length).toBeGreaterThanOrEqual(2); + + const started = Date.now(); + await stopBackgroundApp(realExec); + expect(Date.now() - started).toBeLessThan(15_000); + // SIGKILL does not wait for the exit to land; give the kernel a moment. + expect(await settle()).toEqual([]); + } finally { + // Whatever the assertions did, leave no stubborn group or script behind. + const left = await members(); + if (left.length > 0) await realExec(["sh", "-c", `kill -s KILL ${left.join(" ")} 2>/dev/null; true`]); + await realExec(["rm", "-f", script, APP_PID_PATH]); + } + }, + 30_000, + ); +});