use setsid for process group cleanup

This commit is contained in:
Chris Wanstrath 2026-03-10 09:45:45 -07:00
parent 04abf17d60
commit 22d61d906e
3 changed files with 32 additions and 36 deletions

View File

@ -11,7 +11,7 @@ Transcript-based shell integration test runner. Bun + TypeScript.
## Architecture ## Architecture
- `src/parse.ts` — parses `.shout` files into `ShoutFile` (list of `Command`) - `src/parse.ts` — parses `.shout` files into `ShoutFile` (list of `Command`)
- `src/run.ts` — executes commands via `Bun.spawn(["/bin/sh"])`, captures output with sentinels - `src/run.ts` — executes commands via `Bun.spawn(["setsid", "/bin/sh"])`, captures output with sentinels
- `src/match.ts` — wildcard-aware output matching and diff generation - `src/match.ts` — wildcard-aware output matching and diff generation
- `src/format.ts` — evaluates pass/fail, formats failures and summary - `src/format.ts` — evaluates pass/fail, formats failures and summary
- `src/update.ts` — rewrites `.shout` files with actual output (`--update` mode) - `src/update.ts` — rewrites `.shout` files with actual output (`--update` mode)

View File

@ -26,11 +26,10 @@ const defaultOpts = {
async function isProcessRunning(pid: number): Promise<boolean> { async function isProcessRunning(pid: number): Promise<boolean> {
try { try {
const stat = await readFile(`/proc/${pid}/stat`, "utf-8") const stat = await readFile(`/proc/${pid}/stat`, "utf-8")
// State is the 3rd field: R=running, S=sleeping, Z=zombie, etc. // Find state after the comm field (which is wrapped in parens and may contain spaces)
const state = stat.split(" ")[2] const state = stat.charAt(stat.lastIndexOf(")") + 2)
return state !== "Z" && state !== "X" return state !== "Z" && state !== "X"
} catch { } catch {
// /proc entry doesn't exist — process is fully gone
return false return false
} }
} }
@ -39,44 +38,45 @@ describe("runFile", () => {
test("basic command", async () => { test("basic command", async () => {
const file = makeFile([{ command: "echo hello" }]) const file = makeFile([{ command: "echo hello" }])
const result = await runFile(file, defaultOpts) const result = await runFile(file, defaultOpts)
expect(result.results[0]?.actual).toEqual(["hello"]) try {
expect(result.results[0]?.exitCode).toBe(0) expect(result.results[0]?.actual).toEqual(["hello"])
await cleanupTmpDir(result.tmpDir) expect(result.results[0]?.exitCode).toBe(0)
} finally {
await cleanupTmpDir(result.tmpDir)
}
}) })
test("cleans up backgrounded processes after exit", async () => { test("cleans up backgrounded processes after exit", async () => {
const file = makeFile([ const file = makeFile([
{ command: "sleep 300 >/dev/null 2>&1 & echo $!" }, { command: "sleep 300 >/dev/null 2>&1 & echo $!" },
]) ])
const result = await runFile(file, defaultOpts) const result = await runFile(file, defaultOpts)
const pid = parseInt(result.results[0]?.actual[0] ?? "", 10) try {
expect(pid).toBeGreaterThan(0) const pid = parseInt(result.results[0]?.actual[0] ?? "", 10)
expect(pid).toBeGreaterThan(0)
// Give the SIGTERM a moment to be delivered await new Promise(r => setTimeout(r, 100))
await new Promise(r => setTimeout(r, 100)) expect(await isProcessRunning(pid)).toBe(false)
} finally {
expect(await isProcessRunning(pid)).toBe(false) await cleanupTmpDir(result.tmpDir)
await cleanupTmpDir(result.tmpDir) }
}) })
test("cleans up multiple backgrounded processes", async () => { test("cleans up multiple backgrounded processes", async () => {
const file = makeFile([ const file = makeFile([
{ command: "sleep 300 >/dev/null 2>&1 & P1=$!; sleep 300 >/dev/null 2>&1 & P2=$!; echo $P1 $P2" }, { command: "sleep 300 >/dev/null 2>&1 & P1=$!; sleep 300 >/dev/null 2>&1 & P2=$!; echo $P1 $P2" },
]) ])
const result = await runFile(file, defaultOpts) const result = await runFile(file, defaultOpts)
const pids = (result.results[0]?.actual[0] ?? "").split(" ").map(Number) try {
expect(pids).toHaveLength(2) const pids = (result.results[0]?.actual[0] ?? "").split(" ").map(Number)
expect(pids[0]).toBeGreaterThan(0) expect(pids).toHaveLength(2)
expect(pids[1]).toBeGreaterThan(0) expect(pids[0]).toBeGreaterThan(0)
expect(pids[1]).toBeGreaterThan(0)
await new Promise(r => setTimeout(r, 100)) await new Promise(r => setTimeout(r, 100))
for (const pid of pids) {
for (const pid of pids) { expect(await isProcessRunning(pid)).toBe(false)
expect(await isProcessRunning(pid)).toBe(false) }
} finally {
await cleanupTmpDir(result.tmpDir)
} }
await cleanupTmpDir(result.tmpDir)
}) })
}) })

View File

@ -175,14 +175,8 @@ export async function runFile(
error: err instanceof Error ? err.message : String(err), error: err instanceof Error ? err.message : String(err),
} }
} finally { } finally {
// Kill the shell's process group to clean up backgrounded children
if (proc.pid) { if (proc.pid) {
try { try { process.kill(-proc.pid, "SIGKILL") } catch {}
process.kill(-proc.pid, "SIGTERM")
} catch {}
setTimeout(() => {
try { process.kill(-proc.pid, "SIGKILL") } catch {}
}, 500)
} }
} }
} }
@ -194,8 +188,9 @@ async function readWithTimeout(
const reader = stream.getReader() const reader = stream.getReader()
const chunks: Uint8Array[] = [] const chunks: Uint8Array[] = []
let timerId: ReturnType<typeof setTimeout>
const timeout = new Promise<never>((_, reject) => const timeout = new Promise<never>((_, reject) =>
setTimeout(() => reject(new Error("Timeout reading output")), timeoutMs), timerId = setTimeout(() => reject(new Error("Timeout reading output")), timeoutMs),
) )
try { try {
@ -205,6 +200,7 @@ async function readWithTimeout(
if (value) chunks.push(value) if (value) chunks.push(value)
} }
} finally { } finally {
clearTimeout(timerId!)
reader.releaseLock() reader.releaseLock()
} }