Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 16 additions & 3 deletions src/runtime/api/bun/subprocess.rs
Original file line number Diff line number Diff line change
Expand Up @@ -705,9 +705,10 @@ impl Subprocess<'_> {
sp.on_max_buffer(kind);
}

/// Close any still-open stdout/stderr pipe readers so the sync wait loop
/// stops waiting for EOF after timeout/maxBuffer. Matches Node.js
/// `SyncProcessRunner::Kill()`. Called outside any reader callback.
/// Close any still-open stdout/stderr pipe readers so we stop waiting for
/// EOF after a timeout/maxBuffer kill. A grandchild may still hold the
/// pipe's write end. Matches Node.js `SyncProcessRunner::Kill()`. Called
/// outside any reader callback.
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
pub fn close_readable_pipes(&self) {
if matches!(self.stdout.get(), Readable::Pipe(_)) {
self.stdout.with_mut(|s| s.close());
Expand Down Expand Up @@ -1033,6 +1034,18 @@ impl Subprocess<'_> {
}
}

// When Bun itself killed the child (timeout/maxBuffer), stop waiting on
// pipe EOF: a grandchild may still hold the write end and the caller
// has already opted into a bounded wait. The drain above buffers
// whatever was readable at exit; closing now delivers that buffer
// instead of blocking `proc.stdout` until the grandchild exits. Same
// semantics as the spawnSync wait loop's `close_readable_pipes()` call.
if self.event_loop_timer.get().state == EventLoopTimerState::FIRED
|| self.exited_due_to_maxbuf.get().is_some()
{
self.close_readable_pipes();
}
Comment thread
robobun marked this conversation as resolved.

if let Some(pipe_ptr) = stdin {
self.weak_file_sink_stdin_ptr.set(None);
self.update_flags(|f| f.insert(Flags::HAS_STDIN_DESTRUCTOR_CALLED));
Expand Down
25 changes: 25 additions & 0 deletions test/js/bun/spawn/spawn-maxbuf.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,31 @@ describe("timeout kills the process", () => {
expect(stderr).toBe("");
});

// When Bun kills the child on timeout, a grandchild that inherited the pipe
// may still hold the write end. Reading stdout/stderr after `proc.exited`
// must not wait for that grandchild to exit; Bun closes its read end and
// delivers whatever was buffered (same as `spawnSync`).
test.skipIf(isWindows)("Bun.spawn stdout does not hang when a grandchild outlives the timeout", async () => {
// `sh` starts fast enough that the background `sleep` is running before
// the timeout fires even under a debug build; the signal is delivered
// to `sh` alone, so `sleep` survives holding the pipe's write end.
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
await using proc = Bun.spawn({
cmd: ["sh", "-c", "echo from-child; sleep 60 & read _"],
env: bunEnv,
timeout: 200,
killSignal: "SIGTERM",
stdio: ["pipe", "pipe", "pipe"],
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
});
await proc.exited;
const [stdout, stderr] = await Promise.all([proc.stdout.text(), proc.stderr.text()]);
expect({ stdout, stderr, exitCode: proc.exitCode, signalCode: proc.signalCode }).toEqual({
stdout: "from-child\n",
stderr: "",
exitCode: null,
signalCode: "SIGTERM",
});
});

test("Bun.spawnSync", () => {
const timeStart = Date.now();
const proc = Bun.spawnSync([bunExe(), "exec", "sleep 5"], {
Expand Down
Loading