Bun.spawn: close stdout/stderr pipes after a timeout kill

[robobun]

What

Reading proc.stdout / proc.stderr after await proc.exited could hang indefinitely when the process was killed by timeout, maxBuffer, or an AbortSignal and a grandchild still held the pipe's write end.

Repro

await using proc = Bun.spawn(["sh", "-c", "sleep 60 & echo hi; read _"], {
  timeout: 200,
  killSignal: "SIGTERM",
  stdio: ["pipe", "pipe", "pipe"],
});
await proc.exited;            // resolves after ~200ms
await proc.stdout.text();     // hangs until `sleep 60` exits

This is what made test/js/bun/spawn/spawn-maxbuf.test.ts "timeout kills the process > Bun.spawn" flaky: bun exec "sleep 5" is killed at 100ms but the sleep grandchild survives the signal and keeps the stdout write end open for ~5s, racing the 5s test timeout.

Cause

on_process_exit keeps reading stdout/stderr until EOF so that output from grandchildren written after the direct child is reaped is not lost. When Bun itself kills the child via timeout/maxBuffer/AbortSignal, the caller has already opted into a bounded wait, so waiting on a grandchild's EOF is the wrong trade-off. spawnSync already handles this by calling close_readable_pipes() in its wait loop; spawn (async) did not.

Fix

After draining the pipes in on_process_exit, call close_readable_pipes() when the exit was caused by timeout (event_loop_timer.state == FIRED), maxBuffer, or an AbortSignal (new ABORT_SIGNAL_KILLED flag set in handle_abort_signal). This closes the read end and delivers whatever was buffered instead of blocking on EOF.

Scope

This only covers the case where proc.stdout/proc.stderr are read after proc.exited resolves. If the getter is called before exit (e.g. Promise.all([proc.stdout.text(), proc.exited])), the reader has already been handed to a detached FileReader that Subprocess no longer tracks, so it continues to wait for EOF. Node.js never closes the stream in either ordering, and that case is intentionally left unchanged here.

Verification

# before: times out after 5s
USE_SYSTEM_BUN=1 bun test test/js/bun/spawn/spawn-maxbuf.test.ts -t "grandchild"

# after: passes in ~220ms per variant
bun bd test test/js/bun/spawn/spawn-maxbuf.test.ts -t "grandchild"

The new test uses sh -c "sleep 60 & echo $! >&2; echo from-child; read _" so the grandchild is spawned before the stdout marker even under a debug build, and its PID is reaped after the assertions.

---

no test proof · iteration 0 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/js/bun/spawn/spawn-maxbuf.test.ts

[robobun]

^{Updated 6:27 PM PT - Jul 21st, 2026}

❌ @robobun, your commit 0835776 has some failures in Build #77245 (All Failures)

---

🧪   To try this PR locally:

bunx bun-pr 35012

That installs a local version of the PR into your bun-35012 executable, so you can run:

bun-35012 --bun

[coderabbitai]

Walkthrough

Changes

Timeout pipe teardown

Layer / File(s)	Summary
Close pipes on forced exit src/runtime/api/bun/subprocess.rs, test/js/bun/spawn/spawn-maxbuf.test.ts	Timeout and maxBuffer exits close stdout/stderr readers without waiting for EOF, with regression coverage for a grandchild-held pipe.

Possibly related PRs

• oven-sh/bun#33832: Addresses related close_readable_pipes behavior for timeout/maxBuffer subprocess handling.
• oven-sh/bun#34971: Changes stdout/stderr pipe-reader lifecycle during subprocess exit.
• oven-sh/bun#33882: Updates subprocess exit handling for another output-related file descriptor cleanup path.

Suggested reviewers: jarred-sumner

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly captures the main change: closing Bun.spawn pipes after timeout kills.
Description check	✅ Passed	The description covers the PR purpose and verification, though it uses different headings than the repository template.

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/runtime/api/bun/subprocess.rs`:
- Around line 708-711: Condense the comments at
src/runtime/api/bun/subprocess.rs:708-711,
src/runtime/api/bun/subprocess.rs:1037-1042, and
test/js/bun/spawn/spawn-maxbuf.test.ts:190-193 to three lines or fewer each,
retaining only the durable, non-obvious rationale about closing inherited pipe
writers after timeout/maxBuffer termination and the related regression coverage.

In `@test/js/bun/spawn/spawn-maxbuf.test.ts`:
- Around line 200-205: Update the `Bun.spawn` command in this test so the
intentionally surviving grandchild exits when the managed shell’s stdin closes,
or explicitly terminate that grandchild after the output assertions. Preserve
the timeout and signal behavior while ensuring successful runs leave no
descendant process running.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: af89f441-f6fd-4ffd-8c99-a4e48cea96e1

📥 Commits

Reviewing files that changed from the base of the PR and between f599a1b and 29ab902.

📒 Files selected for processing (2)

• src/runtime/api/bun/subprocess.rs
• test/js/bun/spawn/spawn-maxbuf.test.ts

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@test/js/bun/spawn/spawn-maxbuf.test.ts`:
- Around line 195-197: Remove the fixture-narration comment near the spawn
timeout regression test, leaving only the tracked issue URL as the
regression-test comment.
- Around line 198-203: Update the Bun.spawn fixture command in the timeout test
so the background grandchild is started before the readiness marker is emitted.
Keep the marker after the `sleep 60 &` launch, ensuring the test proceeds only
once a grandchild can retain the pipe writer.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 78047ee8-43d5-49a3-be23-29d605a085ea

📥 Commits

Reviewing files that changed from the base of the PR and between 29ab902 and 346d43c.

📒 Files selected for processing (1)

• test/js/bun/spawn/spawn-maxbuf.test.ts

[claude]

LGTM — the AbortSignal gap from the earlier round is now covered and the getter-before-exit ordering is explicitly scoped out in the description.

What was reviewed:

• close_readable_pipes() placement in on_process_exit: runs after the drain reads, mirrors the spawnSync wait-loop call site, and re-entry via on_close_io only touches stdout/stderr + update_has_pending_activity (this_jsvalue was already captured).
• ABORT_SIGNAL_KILLED is only set when !has_exited(), and on_process_exit clears the abort listener first, so a natural exit followed by a late abort won't spuriously close pipes.
• Test: describe.each thunk defers AbortSignal.timeout(200) creation to test time; grandchild PID is reaped before assertions so failure paths don't leak it.

Extended reasoning...

Overview

Two files: src/runtime/api/bun/subprocess.rs adds a Flags::ABORT_SIGNAL_KILLED bit, sets it in handle_abort_signal, and calls the pre-existing close_readable_pipes() from on_process_exit when the exit was Bun-initiated (timeout timer FIRED, maxBuffer, or the new abort flag). test/js/bun/spawn/spawn-maxbuf.test.ts adds a describe.each over timeout: and signal: AbortSignal.timeout() that spawns a sh grandchild holding the pipe write end and asserts stdout resolves with buffered data instead of hanging.

Security risks

None. No user-controlled input parsing, no auth/crypto, no path handling. The change tightens resource release on a path where Bun already decided to kill the child.

Level of scrutiny

Medium. on_process_exit is delicate (ref counting, re-entry through reader callbacks), but the added call reuses the exact helper spawnSync's wait loop already calls at the equivalent point, so the teardown sequence is not novel. I traced close_readable_pipes() → Readable::close() → PipeReader::close() → reader.close() → on_reader_done/on_close_io: the only Subprocess fields it touches are stdout/stderr (already drained) and this_value via update_has_pending_activity(), and on_process_exit captured this_jsvalue locally before this point, so downgrading the JsRef mid-body is harmless.

Other factors

A previous review pass from me flagged two sibling gaps; the author addressed the AbortSignal one with the new flag + test parametrization and explicitly scoped out the getter-before-exit ordering in the PR description with a Node.js comparison. CodeRabbit's fixture-ordering and grandchild-reaping nits were also addressed. The bug-hunting system found nothing on the current revision. The intentional behavior change — dropping grandchild output after a Bun-initiated kill instead of waiting for EOF — is the point of the fix and matches the spawnSync semantics the doc comment cites.

[robobun]

CI on build 77245: 193/196 green. The one red lane is :darwin: 14 x64 - test-bun failing at git clone with No space left on device on agent macOS-13-x64-1 (infra, not this diff). The changed test file test/js/bun/spawn/spawn-maxbuf.test.ts passes on every lane that ran it. The four retry-pass flakes (jsc-stress, test-repl-close, test-http-server-connections-checking-leak, and the spawn.test.ts kill/unref permutation stress on Windows) use explicit proc.kill() with stdio: "ignore" or are entirely unrelated modules; none touch the timeout/maxBuffer/AbortSignal path changed here.

Ready for review/merge; the darwin x64 lane needs its disk cleared or the job retried on a different agent.