PR #37719 - oven-sh/bun

Problem

A Bun Shell builtin that prints several items (ls -d a b c, rm -v, mkdir -v) never finishes once the process's stdout has lost its reader, e.g. bun script.ts | head -2. The awaited $ promise never settles, so the script hangs (bun 1.4.0 too); one item, or echo, finishes.
When a write to the shell's stdout queue failed with EPIPE, the queue reported the failure once per child command and silently dropped that child's other queued chunks.
rm and the OutputTask builtins queue one chunk per argument under one child and finish only after counting a completion per chunk, so after the first EPIPE they waited for the other N-1 forever, and those tasks leaked.
The Zig shell gave each chunk its own child, so one error per child was one per chunk; the Rust port put them under one child without changing the error path.

Fix

When the queue fails, every chunk still queued gets its own error completion, oldest first. All three failure paths (async error, synchronous error during enqueue, end of file on the poll) share one function.
Property to check: every queued chunk either receives exactly one completion or was cancelled by its owner first. Single-chunk children see exactly what they saw before.
The two children that stop at their first error, cat and the subprocess output relay, now cancel their remaining chunks inside that first callback. For the relay this is required: the first completion can free the relay, so a second one is a use-after-free (ASAN report in the original). The cat line is covered by review only.
Verification: new tests close the stdout reader, then run ls -d with 16 entries, mkdir -v, rm -v and a relayed subprocess. Without the fix the three builtins time out in 6/6 runs; with it all pass on a debug ASAN build. Exit codes on EPIPE are deliberately not asserted (#32278 is changing them).

Background

Bun Shell ($) runs commands such as ls, rm, mkdir and cat in-process as builtins; their output reaches the process's stdout through the shell, not directly.
IOWriter is the shell's per-file-descriptor write queue. A child enqueues chunks tagged with its ChildPtr and normally gets one completion callback per chunk; cancel_chunks marks a child's queued chunks dead, so they are skipped with no callback.
rm and the OutputTask builtins produce output on a thread pool, one chunk per argument, all under the builtin's single ChildPtr, and finish when completions counted equals chunks queued.
CapturedWriter relays a spawned subprocess's piped output into the same queue, one chunk per read. It is embedded in the subprocess's pipe reader, so finishing the command from its callback can free it.
EPIPE is the error for writing to a pipe whose reader has closed. The shell learns of it either as a write error or as end of file from the poll, hence more than one failure path.

Original description

What does this PR do?

A Bun Shell builtin whose output consists of several chunks never finishes when the process's stdout has lost its reader, so the awaited $ promise never settles and the script hangs:

// hang.ts (dst_1, dst_2, dst_3 exist)
import { $ } from "bun";
$.nothrow();
for (let i = 0; i < 30; i++) {
  console.log("iteration", i);
  await $`ls -d dst_*`; // same with `rm -v a b c` or `mkdir -v a b c`
}
console.error("finished");

$ timeout 15 bash -c 'bun hang.ts | head -2 >/dev/null'; echo rc=$?
rc=124      # head exits after two lines; the next ls never finishes, "finished" is never printed (bun 1.4.0 too)

With a single entry (ls -d dst_1), or with echo, the command finishes. .quiet() avoids it because nothing is written to stdout at all.

Cause

IOWriter is the per-fd write queue the shell uses to echo command output. When a write fails, on_error (and the EndOfFile arm of on_write_pollable, via broken_pipe_for_writers) delivered one error completion per distinct child and dropped the rest of that child's queued chunks.

rm and the OutputTask builtins (ls, mkdir, touch, cp) queue one chunk per argument, all under the same ChildPtr ((cmd, WriterTag::Builtin)), and finish once they have counted a completion for every chunk (output_done >= output_waiting, output_queue FIFO). After the first EPIPE they received exactly one completion and waited for the other N-1 forever. The N-1 OutputTask boxes leaked as well.

In the Zig implementation each OutputTask was its own writer child, so "one error per child" was one error per chunk for these builtins; the Rust port collapsed them onto the builtin's ChildPtr (the output_queue FIFO is the stopgap for that) without changing the error path.

Fix

fail_pending_writers now gives every chunk that is still queued its own error completion, oldest first, and all three error paths (on_error, on_sync_error, EndOfFile) go through it; broken_pipe_for_writers is gone.

Two children stop at their first error while more of their chunks may still be queued: cat and the subprocess output relay (CapturedWriter). Both now cancel_chunks() their remaining chunks from inside that first error callback, and the queue is walked in place (liveness re-checked before each callback, cleared only at the end) so that the cancellation takes effect. This is the same cancel_chunks both already use on their other teardown paths. Nothing can be appended to the queue during the walk: err/broken_pipe are set before the first callback, so a re-entrant enqueue is answered by handle_dead_writer as before (#32946).

The CapturedWriter cancellation is required, not just tidy: its first error completion can go try_signal_done_to_cmd -> buffered_output_close -> close_io -> Readable::finalize -> detach, which drops the last Arc<PipeReader> and frees the CapturedWriter itself, so a second completion for it would be a use-after-free. ASAN confirmed that with the per-chunk delivery but without the cancellation (happens when the subprocess's output was fully read before the writer failed):

ASAN report (per-chunk delivery without the CapturedWriter cancellation)

ERROR: AddressSanitizer: heap-use-after-free
    #0 <bun_runtime::shell::subproc::CapturedWriter>::on_iowriter_chunk src/runtime/shell/subproc.rs
    #1 bun_runtime::shell::io_writer::on_io_writer_chunk src/runtime/shell/IOWriter.rs
    #2 <bun_runtime::shell::yield_::Yield>::run src/runtime/shell/Yield.rs
    #3 <IOWriter>::run_yield
    #4 <IOWriter>::fail_pending_writers
    #5 <IOWriter>::on_error
    ...
freed by thread T0 here:
    ...
    #12 <alloc::sync::Arc<bun_runtime::shell::subproc::PipeReader>>::drop_slow
    #15 <bun_runtime::shell::subproc::PipeReader>::detach
    #16 <bun_runtime::shell::subproc::Readable>::finalize
    #17 <bun_runtime::shell::subproc::ShellSubprocess>::close_io
    #18 <Cmd>::buffered_output_close_stdout
    #19 <Cmd>::buffered_output_close
    #20 <PipeReader>::try_signal_done_to_cmd
    #21 <CapturedWriter>::on_iowriter_chunk          <- first completion of the batch
    #25 <IOWriter>::fail_pending_writers

cat's case is the same shape (Builtin::done from the first completion retires the Cmd node, so a second completion would dispatch into a freed or reused node); it is the ExecStdin/ExecFilepathArgs write-error arm, one line away from the cancellation that its read-error arm already did. The builtin is currently behind BUN_ENABLE_EXPERIMENTAL_SHELL_BUILTINS on POSIX and I did not find a way to get more than one of its chunks queued at once there, so that line is covered by review rather than by a test.

The alternative of keeping "one error per child" and having the builtins drain their own queues does not work: their output_queue FIFO does not know which writer (stdout or stderr) a given chunk went to, so a single notification cannot tell them how many of their chunks just failed. Giving every OutputTask its own ChildPtr again (what the output_queue stopgap stands in for) would fix the OutputTask builtins but not rm, which counts chunks under the builtin's pointer too, so the contract is fixed at the queue instead and the two early-exit children opt out explicitly.

Other details:

on_sync_error keeps its contract: the enqueuing child's own chunk is withheld and returned so the trampoline that is already on the stack delivers it (#33008); other queued chunks, if any, are dispatched inline. It now comes back as Yield::done() only if one of those inline completions made the child cancel its chunks, i.e. the child has already finished.
The EndOfFile path previously built its EPIPE SystemError with to_system_error(); it now uses to_shell_system_error() like the on_error path (which is the one a real EPIPE takes, since the pipe writer reports it as an error). The handle_dead_writer rejections are unchanged.
The keepalive that on_error/on_sync_error held moved into the shared function, which is where the post-callback state access happens; the EndOfFile path gets it too.
Single-chunk children see exactly what they saw before (one chunk, one completion), and the exit codes of the affected builtins on EPIPE are unchanged from the single-chunk behavior (ls/mkdir/rm report 0, as ls -d one_entry already did; a relayed subprocess reports the errno, as before). What the exit code should be on a write error is a separate question (#32278 is moving these around), so the tests only assert that the command settles.

How did you verify your code works?

New tests in test/js/bun/shell/epipe.test.ts. A fixture waits until writes to its own stdout fail with EPIPE (the test closes the read end right after spawning), starts the command, blocks the main thread briefly so every argument's output is queued in one batch, and awaits it. ls -d with 16 entries (OutputTask path), mkdir -v (OutputTask path, and the directories must exist afterwards), rm -v (the counting path in rm, and the files must be gone), and a relayed head -c 1048576 /dev/zero subprocess (CapturedWriter; 4 to 6 of its chunks were queued when the writer failed in every local run).

Against the release build without the fix, ls, mkdir and rm time out in 6/6 runs and the subprocess test passes (it guards the cancellation described above, which ASAN catches when the ordering hits); with the fix, all pass on a debug ASAN build (5/5 runs). The rest of test/js/bun/shell (bunshell, commands/, yield, shell-write-fault, shell-pipe-read-fault, shelloutput, shell-blocking-pipe, pipeline_stack, throw, exec, shell-hang, file-io, leak) shows no new failures on the debug build; the only failures here are pre-existing in this environment (the ls permission-denied tests, which fail as root on the release build too, and leak tests that exceed their 100s budget under ASAN).