shell: a command substitution inherits the stdin of its command

RO
2mo ago
+78-19

Problem

  • A $(...) inside a pipeline member reads the stdin of the bun process, not the pipe. echo FROM_PROCESS_STDIN | bun -e 'await Bun.$echo hi | echo "[$(cat)]"' prints [FROM_PROCESS_STDIN]. bash and dash print [hi]. On a terminal the script blocks, because cat waits for terminal input.
  • The cause is in Expansion::next (src/runtime/shell/states/Expansion.rs:204). It builds the IO of the substitution from interp.root_io(). The root IO is the IO of the whole $ call. An Expansion node did not know the IO of the command it expands in.
  • On Windows the shell IOReader closed its fd at the first EOF. A second read of the same stdin panicked (Option::unwrap() on None), and a spawn after $(cat) failed with Bad file descriptor. The released bun has this for echo hi | (cat; cat) and echo hi | (cat; prog). With the pipe as stdin of the substitution, echo hi | prog "$(cat)" would hit it too.

Fix

  • Expansion gets an io: IO field. Each caller passes the IO of its own node: Cmd.io for arguments and redirect targets, CondExpr.io for [[ ]] operands, and a new Assigns.io for assignment values. The substitution's Script runs with that stdin and stderr. Interpreter::root_io() has no callers left and is removed.
  • On Windows the IOReader now borrows the fd as it does on POSIX (CLOSE_HANDLE cleared), attaches a new File when a read starts after EOF, and closes the fd in Drop.
  • A done or error notification takes the readers list. A cat that finished no longer stays registered, so the next EOF cannot reach its freed node.
  • Verified: test/js/bun/shell/bunshell.test.ts (seven new cases under "command substitution", one per expansion site plus a second read after the substitution, all fail on the released bun). Also all of test/js/bun/shell/ on Linux and bunshell.test.ts on Windows x64.

Background

  • The shell interpreter is a tree of state nodes (Script, Stmt, Pipeline, Cmd, Expansion, ...). Each node that runs something carries an IO (src/runtime/shell/IO.rs): stdin, stdout, stderr. Pipeline gives each member an IO whose stdin is the read end of the pipe, wrapped in one shared IOReader.
  • Expansion turns one word (an ast::Atom) into argv strings. For a $(...) it spawns a child Script with stdout: Pipe and collects the output.
  • IOReader (src/runtime/shell/IOReader.rs) is the shell's async stdin reader. Builtins such as cat register a ChildPtr on it and get chunk and done callbacks. On POSIX it already borrowed the fd and closed it in Drop. On Windows it used a libuv File that owned the fd.
Notes
  • Probes against the debug build with echo FROM_PROCESS_STDIN | on stdin, all match bash: top-level echo "[$(cat)]" prints [FROM_PROCESS_STDIN], echo hi | echo "[$(cat)]" < f.txt prints [hi] (expansion runs before redirections), echo hi | [[ $(cat) == hi ]] && echo yes prints yes, echo hi | echo "[$(cat)]" | cat prints [hi], echo hi | if true; then echo "[$(cat)]"; fi prints [hi], echo hi | (x=$(cat); cat) exits 0 with empty stdout.
  • Windows x64 debug build probes: echo hi | (cat; cat; echo after) prints hi\nafter, echo hi | (cat; bun -e ...) prints hi\nprog, echo "[$(cat)]"; cat; echo after prints [FROM_PROC]\nafter, echo hi | (A=$(cat); cat; echo "[$A]") prints [hi]. All of these crash or fail with EBADF on the released Windows bun.
  • On Linux the stale readers entry never fired because the freed node id was reused by the next cat, so the dispatch reached the right node by chance.
  • A bare assignment as a pipeline member (echo hi | A=$(cat)) is still skipped by Pipeline.rs, as before this change. The header comment in Assigns.rs describes that.
  • The state node is dropped in Interpreter::free_node (n[id.idx()] = Node::Free), so the extra Arc refs on the pipe reader held by Expansion.io are released when the expansion finishes.
  • Self-reviewed: the review asked to lead the body with the bash/dash divergence (done) and to reword the Assigns.rs header (kept, it is still accurate).
  • In test/js/bun/shell/ on the Linux machine, 15 pre-existing tests fail on the debug+ASAN build: 5s and 100s timeouts in leak.test.ts, shell-load.test.ts, shell-leak-args.test.ts, commands/rm.test.ts, and ls permission tests that cannot fail as root. On the Windows machine, 5 tilde_expansion tests fail with the released bun too. None use a command substitution.

no test proof · iteration 1 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/js/bun/shell/bunshell.test.ts