Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
22 changes: 19 additions & 3 deletions src/runtime/api/bun/subprocess.rs
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,9 @@ bitflags::bitflags! {
/// by the caller). Owned terminals are closed when the subprocess exits
/// so the exit callback fires; borrowed terminals are left open for reuse.
const OWNS_TERMINAL = 1 << 6;
/// `handle_abort_signal` sent `kill_signal`; `on_process_exit` closes
/// pipe readers instead of waiting on EOF a grandchild may never send.
const ABORT_SIGNAL_KILLED = 1 << 7;
}
}

Expand All @@ -322,6 +325,9 @@ impl Subprocess<'_> {
#[bun_uws::uws_callback(thunk = "on_abort_signal_c")]
fn handle_abort_signal(&self, _reason: JSValue) {
self.clear_abort_signal();
if !self.has_exited() {
self.update_flags(|f| f.insert(Flags::ABORT_SIGNAL_KILLED));
}
let _ = self.try_kill(self.kill_signal);
}
}
Expand Down Expand Up @@ -705,9 +711,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 +1039,16 @@ impl Subprocess<'_> {
}
}

// When Bun itself killed the child (timeout/maxBuffer/AbortSignal) 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.flags.get().contains(Flags::ABORT_SIGNAL_KILLED)
{
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
33 changes: 33 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,39 @@ describe("timeout kills the process", () => {
expect(stderr).toBe("");
});

// A grandchild that inherited the pipe may still hold the write end after
// Bun kills the child. Reading stdout/stderr after `proc.exited` must
// deliver what was buffered instead of waiting for that grandchild to exit.
describe.each([
["timeout", () => ({ timeout: 200 })],
["AbortSignal", () => ({ signal: AbortSignal.timeout(200) })],
] as const)("via %s", (_, opt) => {
test.skipIf(isWindows)("Bun.spawn stdout does not hang when a grandchild outlives the kill", 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,
...opt(),
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",
});
Comment thread
robobun marked this conversation as resolved.
expect(stderr).toMatch(/^\d+\n$/);
});
});

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