PR #37752 - oven-sh/bun

Problem

With the builtin cat (BUN_ENABLE_EXPERIMENTAL_SHELL_BUILTINS=1 on POSIX, the default on Windows), a script that reads stdin with two cats, such as cat; echo ---; cat, prints the first cat's output and then hangs forever. A real shell runs the second cat to immediate EOF and moves on.
When the first cat's read failed and the second cat is inside a subshell (cat; (echo ---; cat)), bun crashes instead: panic: expected Node::Cmd at Node#2, got Subshell.
The hang: the shared stdin reader decided whether to start a new read by asking whether its poll was registered. A finished read (EOF or error) leaves the one-shot poll registered but already fired, so the second cat was left waiting for a wakeup that never comes.
The panic: a cat that had been notified stayed in the reader's listener list, so the next read notified it again, by then under a node id that had been freed or reused by another node.

Fix

The POSIX reader starts a new read whenever no read is armed, so a cat that arrives after a finished read reads the fd again, as a system cat inheriting the fd would: EOF again on a pipe, more input on a tty, the same error on a failing fd. Windows is unchanged; a second cat there needs #29986.
The EOF and error callbacks take the listener list out before notifying it, so each listener is notified exactly once with the outcome of the read it registered for, and a cat started from inside a notification registers into the empty list and gets a read of its own.
The stored error that a later EOF used to replay to the listeners is removed: a read ends in exactly one of error or EOF, and after an error nobody is left to replay it to.
Verification: a new test file with 20 cases, each run in a child bun. Before the fix 17 fail on Linux (16 hang, one panics); all 20 pass with it. The read-error subshell case only passes with the listener change and the EOF cases only with the restart change. The read-error cases were also checked on a Windows debug build.

Background

Bun.$ is bun's shell. cat can run as a builtin inside the interpreter instead of as a subprocess (behind the flag on POSIX, always on Windows), so every cat in a script reads stdin through the interpreter rather than through its own process.
IOReader (src/runtime/shell/IOReader.rs) wraps one readable fd for a whole script, either the script's stdin or the stdin a pipeline creates for a stage. A builtin that wants input registers as a listener; the reader passes chunks and the final EOF or error to every listener.
On POSIX the reader is driven by a one-shot poll: it fires once per arm and must be re-registered to fire again, so "registered" does not mean "a read is in flight". has_pending_read() (is the poll being watched) means that, and IOWriter already uses the same check on its writable poll.
Listeners are recorded by interpreter node id. Ids are released when a command finishes and can be handed to later nodes, so dispatching to a stale entry reaches whatever node holds that id now.
Captured stdout (.quiet()) versus inherited stdout decides when the next cat registers: with captured output the first cat's completion runs the rest of the script synchronously, inside the reader's callback; with inherited output the next command starts later, from a write callback. The tests cover both.

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/shell/commands/cat.test.ts

Original description

Repro

Builtin cat (POSIX: BUN_ENABLE_EXPERIMENTAL_SHELL_BUILTINS=1), two of them reading the same stdin in one script:

printf 'hi\n' | BUN_ENABLE_EXPERIMENTAL_SHELL_BUILTINS=1 bun -e 'console.log((await Bun.$`cat; echo ---; cat`.nothrow()).exitCode)'
# prints "hi" and "---", then hangs forever (same with .quiet())

BUN_ENABLE_EXPERIMENTAL_SHELL_BUILTINS=1 bun -e 'await Bun.$`cat; (echo ---; cat)`.nothrow()' < /etc/hostname
# (stdin is a regular file, so the first cat fails with EPERM from epoll and the second one restarts the reader)
# panic: expected Node::Cmd at Node#2, got Subshell

A real shell runs the second cat to immediate EOF (or to the same read error) and moves on. The subprocess cat, and two separate $ calls, both behave; the bug needs two builtin cats on the one IOReader a script has for its stdin (the root stdin reader, or the reader a pipeline creates for a stage's stdin).

Cause

Two things in src/runtime/shell/IOReader.rs:

start() decided whether to (re)start the read with FilePoll::is_registered(). The reader's poll is one-shot; after it fires, PollReadable stays set and NeedsRearm is set until somebody re-registers. A read that ends in EOF or an error does not re-register, so that is exactly the state a finished read leaves behind: is_registered() is true, the poll will never fire again, and the second cat's start() returns suspended without doing anything. IOWriter::write() already documents and avoids the same trap on the writable poll (it uses is_watching()).
on_reader_done_cb / on_reader_error iterated a clone of readers and left the entries in place, and nothing removed a cat that had been notified. Once a restart does happen (today already on the error path, since a poll that failed to register is not registered; after (1) also on the EOF path), the next read notifies the previous cat's entry again. By then its NodeId has been freed, or reused by whatever node was allocated next, hence the panic above (cat; echo ---; cat only survived because the second cat happened to get the first one's slot back and add_reader's dedup matched the stale entry).

Fix

start() keys off BufferedReader::has_pending_read() (handle is a poll and it is_watching()), which is false for a never-registered, failed, or fired-and-not-re-armed poll and true while a read is armed. A listener that arrives after a finished read gets a new read on the same fd, which is what the system cat does with the inherited fd: a pipe or socket at EOF reports EOF again right away, a tty waits for more input, a failing fd fails again with its own errno. Listeners that arrive while a read is armed attach to it as before.
The done/error callbacks take the listener list (take_readers) before notifying it. Each listener is notified exactly once, with the outcome of the read it registered for. A cat started synchronously from inside a notification (with captured output the first cat's completion runs the rest of the script on the spot) registers into the emptied list and restarts the reader, so it is served by its own read instead of being completed by the notification loop that is still running; nothing stays behind to be dispatched under a dead NodeId. This makes State::raw_err dead. It existed so that a done following an error could hand the error to the listeners again, but a read reports one or the other: on POSIX every read loop in PipeReader.rs ends in exactly one of on_error() (registration failure or read error) or done() (EOF), both in tail position; on Windows WindowsBufferedReader::on_read returns right after on_error() for an error and only reaches close() / done() for EOF. The one way a done can still follow an error is a Windows VM teardown closing the source of a reader that already failed, and by then the error notification has detached every listener, so there is nothing to carry the error to (before this change that teardown done would have re-notified the already finished cats). A listener added after an error is waiting on a new read whose outcome is its own.

Restarting from inside the EOF notification is safe with respect to the bun_io reader: every done() / on_error() dispatch in PosixBufferedReader is in tail position and copies the vtable out first, so nothing in the read loop touches the reader after the callback has re-registered it. When the second cat registers while a read is still running (first cat killed by a stdout write error in the middle of a chunk), has_pending_read() is false as well and the poll gets re-registered once more. That read still serves the new listener; the re-registration then produces one more wakeup, which either serves a cat that registered in the meantime (a third cat started from the second one's EOF notification finds has_pending_read() true and waits for it) or reads EOF again with nobody to notify. The /dev/full cases below cover both outcomes.

start() on Windows is unchanged: there the reader closes its libuv source at EOF, so a second cat needs a different answer, which #29986 provides (it currently crashes on start_with_current_pipe). Note for whichever of the two lands second: #29986 drains readers by popping until empty, which would also complete a listener that registered during the drain; on POSIX that listener now has a read in flight and must be left alone, which is why this change snapshots the list instead. #35337 (regular files are not pollable) is a separate problem; the error-path tests here use a directory as stdin so they do not depend on how that one is resolved.

Not changed here: while no cat is registered (the first one died on a write error and the next command has not started its cat yet), the reader keeps reading and throws the bytes away (on_read_chunk_cb returns should_continue regardless of whether anyone is listening), where a real shell would leave them for the next cat. Pre-existing and independent of the hang; it is the reason the tests below close stdin up front or use a tty rather than feeding more data into a pipe.

Tests

test/js/bun/shell/commands/cat.test.ts (new; the other builtins have a file here, cat did not. #37743 and #35337 create the same file for their own cat fixes; the helpers are independent, so whichever lands later just appends its cases, and I will rebase this one if it is not first). Each case spawns a child bun with the flag and checks the script's stdout plus an exit=<code> trailer:

second cat after EOF: cat; echo ---; cat, && chain, three cats, and the second cat inside ( ) / if so it gets a different node id than the first; each with captured stdout (restart happens inside the EOF callback) and with stdout going through an IOWriter (restart happens later, from a write callback)
300 KB input spanning several reads, a pipeline stage's stdin (echo hi | (cat; echo ---; cat)), and stdin as an actual pipe via sh -c (Bun.spawn's pipe is a socketpair)
stdin as a tty (Bun.Terminal): hi + ^D, wait for ---, then more + ^D, expecting hi, ---, more. On a pipe the new read only reports EOF again, which prints the same thing as completing the second cat on the spot would; a tty's ^D is used up by the read that sees it, so this case only passes if the second cat really reads the fd again (it hangs before the fix and would print no more with complete-on-the-spot semantics)
first cat failing its stdout write (cat > /dev/full, Linux), captured output: second cat served by the still-running read; plus a third cat served by the extra wakeup, plus a trailing subprocess so the extra wakeup arrives with nobody to notify while the reader is still alive (debug assertions); and with an IOWriter, where echo completes later, so the read ends with nobody listening and the second cat starts a new one (this one hung before)
first cat failing to read (directory as stdin), second cat flat and inside ( ) (the latter panicked before). This block also runs on Windows, where the builtin cat is the default and a failed read can be retried (only the EOF block is skipped there, see above): on a Windows x64 canary build the ( ) case panics with the same expected Node::Cmd at Node#3, got Subshell, and both cases pass with a Windows debug build of this branch

Before the fix 17 of the 20 cases fail on Linux (16 hang, the read-error ( ) case panics; the three that pass, the basic and the trailing-subprocess /dev/full cases and the flat read-error case, are guards for paths this touches). The two halves are independently load-bearing: the read-error ( ) case does not involve start() at all and only passes with take_readers, while the EOF cases only stop hanging with the start() change. With the fix all 20 pass on the debug build (also checked: the file fails the same way on a debug build with src/ stashed). For comparison, with #29986's IOReader.rs changes ported onto main instead of this change, the captured-output cases pass as a side effect of its drain loop and every case where the next cat registers after the notification has returned still hangs (see the comment below).