shell: reject the promise and stop the pipeline when a member throws a JS error

Merged
RO
2mo ago
+526-67

Fixes #42282. Supersedes #42251 (its commits are in this branch).

Problem

  • A pipeline member that throws a JS error (cmd | cat > ${new Response("r")}) rejects the promise, but the process never exits. The members that already run keep running, the pipe ends of the failed member and of the members after it stay open, and a producer that writes more than the pipe buffer blocks forever.
  • A state that throws returned Yield::Failed with no node. The failed member never reported to its parent, so Pipeline::child_done (src/runtime/shell/states/Pipeline.rs) never ran for it, nothing released its IO, and the members after it were never started or freed. On main the trampoline also left the exception pending (Yield.rs:134), so after the first suspension the promise never settled.

Fix

  • Yield::Failed carries the NodeId. The trampoline calls Interpreter::fail(id): it takes the exception, kills every subprocess with SIGKILL, reports id to its parent as finished with exit code 1, and then rejects the promise. The rejection runs user JS, so it comes last.
  • The tree then winds down through the normal child_done path. The sequencing states stop on the failed flag (interrupted()), a pipeline frees the members it did not start (release_unstarted), a Cmd whose expansion completes later does not spawn, and an expansion stops after a failed command substitution. Each freed member drops its pipe ends, so the members that still run see EOF or EPIPE.
  • finish runs once every member has exited. It settles nothing when the script failed, but it still disables the keep-alive and drops the pending-activity count. No node is freed while a callback can still reach it, which is what #33996 got wrong.
  • Verified: test/js/bun/shell/yield.test.ts (20 new cases, 17 hang or abort on 1.4.3) and shell-worker-terminate-leak.test.ts (1 new ASAN case). Also bunshell, pipeline_stack, epipe, throw, lazy, shell-hang, exec, file-io, shell-worker-terminate-leak, commands/ and the rest of test/js/bun/shell.

Background

  • The shell interpreter is a tree of state nodes in an arena (Interpreter::nodes). A node reports to its parent with child_done, and the parent frees it. Yield::run is the trampoline that steps nodes until one suspends.
  • A pipeline creates one socketpair per | and gives each member an IOReader (read end) and an IOWriter (write end). The fds close when the last Arc drops, that is, when the member node is freed.
  • hasPendingActivity keeps the ShellInterpreter wrapper alive until finish. A subprocess exit, a pipe read, or a thread-pool task can call back into the interpreter until then.
Notes

Relation to #42251: that PR rejects the promise but leaves the failed pipeline's interpreter pinned and its members running, and names #42282 as the follow-up. Landing it alone ships an interim design, landing this one alone strands it, so this PR carries its commits and #42251 is closed. Here reject_with_pending_exception only takes the exception and rejects. The keep-alive and the pending-activity count are released by finish, as on the success path.

Why kill instead of wait: sleep 100 | bad > ${blob} would keep the process open for 100 s. A member that does write the pipe gets EPIPE once the failed member's read end closes, so the kill only matters for members that do not touch the pipe. SIGKILL is what Cmd::deinit already uses.

In-process tests check the mechanism, not only that the process exits: the member that does not touch the pipe writes its pid, the failing member waits for that file in a command substitution before it throws, and after the rejection the pid is gone (killed and reaped). Two members that fail reject with the first error, and a non-quiet pipeline rejects and leaves the shell usable. A worker that is terminated while a failed pipeline winds down runs clean under ASAN and LSan.

Shapes covered by the tests: producer bigger than the pipe buffer, a member that never touches the pipe, members after the failed one, the failed member first, a glob-expanding member whose glob completes after the failure (${bun} -e ... * | bad spawned the command before the guard in Cmd::next), a subshell member, a command substitution in another member, a builtin producer on the thread pool, (cmd; touch a) | bad; touch b leaves no file, and a failed pipeline is collected by the GC.

The other producers of Yield::Failed take the same path: a failed dup() for a subshell or command substitution, a failed write of the command not found message, an OOM pin of an ArrayBuffer redirect. #40033 changes the command not found write failure to exit 1 instead of a throw; this PR keeps the throw.

Self-review: 10 concerns raised, 3 addressed (the Cmd::next guard, Fixes line, fold #42251). Rejected: a parse-time check of the redirect target instead of a runtime wind-down, because the other producers above reach the same path with no misuse of the API, and waiting instead of killing, because a member that does not touch the pipe would pin the process for its lifetime.

Found while testing, not changed here: ls -R dir | <consumer that exits early> hangs on 1.4.3 because IOWriter reports a failed write once per child while ls queues one chunk per directory. #37719 fixes that. The test here uses ls dir (one chunk).

Probes on the ASAN debug build: BUN_JSC_validateExceptionChecks=1 and BUN_GARBAGE_COLLECTOR_LEVEL=2 over yield.test.ts, all green. leak.test.ts memleak cases and shell-blocking-pipe.test.ts time out the same way on a main build in this container. commands/ls.test.ts permission cases fail as root and the node_modules case needs the registry.

cargo check -p bun_runtime passes for x86_64-pc-windows-msvc and aarch64-apple-darwin.

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