shell(cat): restart the shared stdin reader for a later cat, notify each listener once

[robobun]

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 shell(IOReader): drain readers instead of iterating a snapshot of the list #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:

1. 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()).

2. 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).

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: cbc64305-a2b4-4888-b909-0db14af40735

📥 Commits

Reviewing files that changed from the base of the PR and between 2ffb8d4 and 5da50b6.

📒 Files selected for processing (2)

• src/runtime/shell/IOReader.rs
• test/js/bun/shell/commands/cat.test.ts

---

Walkthrough

Changes

IOReader lifecycle and cat behavior

Layer / File(s)	Summary
Reader dispatch and re-arming src/runtime/shell/IOReader.rs	IOReader documents callback access rules, checks pending reads on POSIX, and removes explicit callback re-arming.
Listener detachment during completion src/runtime/shell/IOReader.rs	Error and completion paths detach active-cycle listeners before callbacks and no longer store a cloned raw error.
Builtin cat reader reuse and recovery test/js/bun/shell/commands/cat.test.ts	POSIX and Linux tests cover shared stdin readers across commands, pipelines, pipes, EOF, read failures, and stdout failures.

Possibly related PRs

• oven-sh/bun#37743: Updates related cat read-error completion and queued-output handling.

Suggested reviewers: jarred-sumner

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly summarizes the primary reader-restart and one-time listener notification changes.
Description check	✅ Passed	The description explains the problem, implementation, scope, tests, verification results, and platform behavior.

---

_{Comment @coderabbitai help to get the list of available commands.}

[robobun]

^{Updated 2:05 PM PT - Aug 12th, 2026}

❌ @robobun, your commit 9bb5991 has 1 failures in Build #93362 (All Failures):

• test/cli/install/migration/complex-workspace.test.ts - code 1: error: Failed to install on 🍎 14 aarch64

---

🧪   To try this PR locally:

bunx bun-pr 37752

That installs a local version of the PR into your bun-37752 executable, so you can run:

bun-37752 --bun

[robobun]

Status: reproduced on the release build with the commands in the description (first one hangs until killed, second one panics with expected Node::Cmd at Node#2, got Subshell).

Tests (test/js/bun/shell/commands/cat.test.ts, as of 9bb5991): 17 of 20 fail without the src/ change on Linux (16 hang, 1 panic) and all 20 pass with it on a debug build; on Windows the read-error block runs too, where the ( ) case panics on the unfixed canary and both cases pass with a debug build of this branch. bunshell.test.ts, pipeline_stack, file-io, shell-hang, shell-blocking-pipe and commands/ still pass locally apart from the ls permission tests, which fail as root regardless of this change.

CI (build 93362, 9bb5991, finished): 191 jobs passed, 1 failed, 2 expired. cat.test.ts passed on every lane that ran, including the Windows lanes (where the read-error block now runs) and darwin 14 (tty case). The failed job is a darwin 14 shard failing test/cli/install/migration/complex-workspace.test.ts (bun install of the migration fixture failed; nothing in this PR is involved, reported separately). The two expired jobs are the darwin 26 aarch64 test shards, which waited about five hours without an agent, the same as on the previous build, so a re-push would most likely just expire there again; retrying those three jobs in Buildkite once the queue has agents is the cheaper way to get a fully green build. Everything else in the annotations is marked flaky and passed on retry. From this PR's side the diff is done; it needs a maintainer look (see the description for the re-entrancy argument and the notes on #29986 / #35399 / #37743).

[github-actions]

This PR may be a duplicate of:

1. shell(IOReader): drain readers instead of iterating a snapshot of the list #29986 - Rewrites the same on_reader_done_cb/on_reader_error in src/runtime/shell/IOReader.rs to stop iterating a cloned snapshot and adds the same "listener registered after the read cycle finished" guard in start(), for the same shared-stdin multi-cat hang/recycled-NodeId bug.
2. refactor(rust): eliminate 57 borrowck-workaround allocs and unsafe launders #35399 - Lands the identical std::mem::take(&mut s.readers) change in both of those functions (as incidental cleanup in a borrowck refactor), so it overlaps the "notify each listener once" half of this fix line-for-line.

🤖 Generated with Claude Code

[robobun]

Not a duplicate of either, though both touch the same two functions:

• shell(IOReader): drain readers instead of iterating a snapshot of the list #29986 changes start() only inside the #[cfg(windows)] block (its is_done() check sits between the is_reading test and start_with_current_pipe(), and on POSIX is_done() is never true for the shell reader anyway, since CLOSE_HANDLE is cleared and done() clears CLOSED_WITHOUT_REPORTING). The POSIX start() still keys off is_registered() with it, so a cat that registers after the previous read finished still never re-arms the poll. Its pop-until-empty drain does complete a cat that registers synchronously from inside the notification, so it would cover the captured-output cases here as a side effect, but not the ones where the previous command completes later (stdout through an IOWriter, which is what the repro in the description does, or the first cat dying mid-read). The approach is also deliberately different, see the note in the description: on POSIX the fd can be read again (pipe: EOF again; tty: waits for input; failing fd: its own errno), so this PR restarts the read and must not complete a listener that registered during the notification, while shell(IOReader): drain readers instead of iterating a snapshot of the list #29986 completes late registrants, which is the right thing on Windows where the source is gone. The two compose, whichever lands first. Measured: with shell(IOReader): drain readers instead of iterating a snapshot of the list #29986's IOReader.rs changes ported onto current main (its draining flag, drain_readers() in both callbacks, the Windows is_done() check) and this PR's src/ change absent, this PR's test file gives 11 pass / 6 fail on a Linux debug build: the six failures are the five "inherited stdout" EOF cases and the non-quiet /dev/full case, all hanging until the test timeout, i.e. every case where the next cat registers after the notification loop has returned. The captured-output cases pass there for the side-effect reason above.
• refactor(rust): eliminate 57 borrowck-workaround allocs and unsafe launders #35399 contains the same mem::take of readers in the two callbacks as part of a 57-site allocation refactor (it keeps raw_err and does not touch start()), so it neither fixes the hang nor tests the crash; it is currently conflicting with main. If it lands first, the two hunks here rebase onto it trivially.

[claude]

I re-reviewed after f3aeaf6 and didn't find any bugs — the earlier nit about the reader() SAFETY comment is addressed. Because the fix leans on subtle re-entrancy/aliasing reasoning (restarting the BufferedReader from inside its own done/error dispatch) and the PR description flags composition constraints with #29986 and #35399, a maintainer look would still be worthwhile.

What was reviewed:

• has_pending_read() on POSIX is Poll(p) && p.is_watching(), so Closed/Fd(_) handles still take the start path as before.
• raw_err has no remaining readers; on_reader_error and on_reader_done_cb are the only paths that reported an error to listeners, and each read cycle now reports its own outcome.
• Tests: subprocess pipes drained concurrently, Buffer.alloc for the large input, test.concurrent throughout, directory-as-stdin avoids depending on #35337.

Extended reasoning...

Overview

The PR fixes two coupled bugs in src/runtime/shell/IOReader.rs that make a second builtin cat on a shared stdin IOReader hang or dispatch to a recycled NodeId: (1) start() gated on FilePoll::is_registered(), which stays true after a one-shot poll fires, so a listener arriving after EOF/error never re-armed the read; (2) the done/error callbacks iterated a clone of readers and left the entries in place, so a restarted read re-notified stale entries. The fix switches start() to has_pending_read() (which is is_watching() on POSIX) and replaces the clone with mem::take via a new take_readers() helper. The now-dead State::raw_err field is removed. A new test/js/bun/shell/commands/cat.test.ts adds 17 subprocess tests covering EOF restart (captured and inherited stdout, subshell/if for distinct node ids, multi-chunk input, pipeline stage stdin, real pipe via sh -c), mid-read unregistration via /dev/full, and read-error restart via a directory fd.

Security risks

None. The affected path is the experimental builtin cat (POSIX-only, gated behind BUN_ENABLE_EXPERIMENTAL_SHELL_BUILTINS=1) reading from an inherited fd. No parsing of untrusted input, no auth/crypto, no new external surface.

Level of scrutiny

High. Although the feature is behind an experimental flag, the change lives in native code with UnsafeCell interior mutability and depends on a specific aliasing contract with PosixBufferedReader: the fix now calls self.reader() (and r.start()) transitively from inside on_reader_done_cb/on_reader_error, which is only sound because those dispatches copy the vtable out and hold no &mut ReaderImpl across the callback. The updated SAFETY comment on reader() and the PR description both spell this out, and I traced it against PipeReader.rs — but this is exactly the kind of invariant a maintainer familiar with the bun_io read loop should sign off on. The PR also documents a semantic divergence from #29986's pop-until-empty drain (which would incorrectly complete a listener that has a fresh read in flight on POSIX); whichever lands second needs to reconcile that.

Other factors

• My earlier inline nit (stale SAFETY comment contradicting the new re-entry) was addressed in 5da50b6/f3aeaf6, and the comment-cop flags on comment length were resolved in the same push. All inline threads are marked resolved.
• The bug hunting system found nothing. I additionally checked that has_pending_read() returns false for PollOrFd::Closed and PollOrFd::Fd(_) (it's a matches! on Poll), so the previously-explicit Closed/Fd arms are still covered; and that no other code reads State::raw_err.
• Test quality is solid per REVIEW.md: concurrent subprocess spawns with all three pipes drained via Promise.all, combined-object assertions, Buffer.alloc instead of repeat, bunEnv spread, per-platform gating with reasons, and the error-path fixture (directory fd) chosen so it does not depend on how #35337 resolves regular-file polling. The description states 15/17 fail without the fix and all pass with it, verified on a debug build.
• CI build #92895 was still running at the time of this review.

[claude]

I reviewed this PR and didn't find any bugs; both of my earlier inline nits (the reader() SAFETY comment and the tty test's early-exit handling) are now addressed in f3aeaf6 and 9bb5991. Because the fix relies on re-entering reader() from inside a vtable callback under the UnsafeCell aliasing contract, drops the raw_err carry-over, and needs to compose with #29986 / #35399, a human look at the re-entrancy argument would still be worthwhile.

What was reviewed:

• start()'s switch from is_registered() to has_pending_read() — matches the documented pattern in IOWriter::write(); verified has_pending_read() is false on a fired-not-rearmed one-shot poll.
• take_readers() vs. the old clone — a listener that registers during the notification loop (captured-output path) is left for its own read; the removed raw_err was only ever consumed by re-notifying stale entries, so it is dead once the list is taken.
• The tty test's new exit() callback rejects whichever deferred is still pending with the accumulated output; on the happy path both are already resolved so the reject is a no-op.

Extended reasoning...

Overview

The PR fixes a hang and a panic when two builtin cat commands share one IOReader (the script's stdin, or a pipeline stage's stdin) in Bun.$. Two changes in src/runtime/shell/IOReader.rs: (1) start() now keys off BufferedReader::has_pending_read() instead of FilePoll::is_registered(), so a listener that arrives after a finished read gets a new read instead of attaching to a fired-and-not-rearmed one-shot poll; (2) on_reader_done_cb / on_reader_error now mem::take the listener list before iterating it, so each listener is notified exactly once and a cat started synchronously from inside a notification registers into an empty list for its own read. The now-dead State::raw_err field is removed. A new test/js/bun/shell/commands/cat.test.ts covers 20 cases across EOF/error paths, captured vs. inherited stdout, subshells, pipelines, ttys, and /dev/full.

Security risks

None identified. The change is confined to the experimental builtin shell's stdin reader (gated behind BUN_ENABLE_EXPERIMENTAL_SHELL_BUILTINS=1 on POSIX) and does not touch auth, crypto, network, or untrusted-input parsing. The listener list is a Vec<ChildPtr> of (NodeId, ReaderTag) copies; taking it instead of cloning it does not change what data is exposed to what.

Level of scrutiny

High. The fix is small (~30 lines of Rust) but sits in unsafe code: IOReader splits its state across two UnsafeCells, and the fix depends on start() (which derives &mut ReaderImpl via self.reader()) being reachable from inside on_reader_done_cb / on_reader_error when a completed cat synchronously drives the rest of the script. The PR description and the updated reader() SAFETY comment argue this is sound because PosixBufferedReader dispatches done/error through a copied vtable over a raw pointer with no borrow of *this live, and I traced that in PipeReader.rs for my earlier inline comment. That argument is convincing, but it is exactly the kind of aliasing/re-entrancy invariant a maintainer who owns bun_io should sign off on. The removal of raw_err also changes observable semantics on any hypothetical path where a read reports both on_error and then on_done — the description asserts no such path exists on either platform, which I did not independently exhaust.

Other factors

• Two open PRs (#29986, #35399) touch the same functions; the description and the author's follow-up comment lay out how they compose (snapshot vs. drain-loop semantics differ on POSIX), which a maintainer will want to reconcile at merge time.
• My two earlier inline nits (stale SAFETY comment on reader(); tty test not rejecting on early child exit) are both addressed in the current diff (f3aeaf6, 9bb5991). The comment-cop bot's length flags were also addressed in f3aeaf6.
• Test coverage is thorough: the description reports 17/20 cases fail on the unfixed build (16 hang, 1 panic) and all 20 pass with the fix, and the two halves of the fix are independently load-bearing per the described breakdown. CI on the previous build was green apart from two darwin agent-expiry lanes.
• The tty test now wires the terminal exit callback to reject the pending deferred with the accumulated output, so a regression surfaces as a fast failure with the panic text rather than a bare timeout.