Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 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
15 changes: 12 additions & 3 deletions src/runtime/api/bun/subprocess.rs
Original file line number Diff line number Diff line change
Expand Up @@ -705,9 +705,9 @@ 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 still-open stdout/stderr pipe readers after a timeout/maxBuffer
/// kill; a grandchild may still hold the write end (Node.js
/// `SyncProcessRunner::Kill()`). Called outside any reader callback.
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 +1033,15 @@ impl Subprocess<'_> {
}
}

// When Bun itself killed the child (timeout/maxBuffer) stop waiting on
// pipe EOF after the drain above: a grandchild may still hold the
// write end and the caller already opted into a bounded wait.
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("");
});

// A grandchild that inherited the pipe may still hold the write end after
// the timeout kill. Reading stdout/stderr after `proc.exited` must deliver
// what was buffered instead of waiting for that grandchild to exit.
test.skipIf(isWindows)("Bun.spawn stdout does not hang when a grandchild outlives the timeout", async () => {
// `sh` spawns `sleep` before the stdout marker so the assertion proves a
// grandchild holds the pipe's write end when the kill signal reaches `sh`.
await using proc = Bun.spawn({
cmd: ["sh", "-c", "sleep 60 & echo $! >&2; echo from-child; read _"],
env: bunEnv,
timeout: 200,
killSignal: "SIGTERM",
stdio: ["pipe", "pipe", "pipe"],
});
await proc.exited;
const [stdout, stderr] = await Promise.all([proc.stdout.text(), proc.stderr.text()]);
const grandchild = parseInt(stderr.trim(), 10);
if (Number.isInteger(grandchild)) try { process.kill(grandchild); } catch {}
expect({ stdout, exitCode: proc.exitCode, signalCode: proc.signalCode }).toEqual({
stdout: "from-child\n",
exitCode: null,
signalCode: "SIGTERM",
});
expect(stderr).toMatch(/^\d+\n$/);
});

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