child_process: deliver each stdio event after the nextTicks and promise jobs of the one before by robobun · Pull Request #43790 · oven-sh/bun

robobun · 2026-09-22T13:00:05Z

Problem

child.stdout and child.stderr emit 'end' and 'close' before the promise jobs of the last chunk's listeners, when the last bytes and the EOF arrive in one read. Readable.fromWeb(blob.stream()) always does. Promise jobs can also run before the listener's nextTicks, or after the next 'data' or 'close'.
Cause: src/js/internal/streams/native-readable.ts pushes what one native read gives (chunks, EOF) from one nextTick drain, or from a promise job.

Fix

New internal/process/after_tick_drain.ts: runAfterTickDrain(fn) runs fn from the tick loop when the nextTick queue and the promise job queue are both empty.
Child stdio delivers each chunk, the EOF, a read error and 'close' through it when a 'data' or 'readable' listener exists. Readable.fromWeb delivers its EOF through it.
Verified: test/js/node/child_process/child-process-stdio.test.js, test/js/node/stream/node-stream.test.js (21 new tests, 19 fail without the fix, Node v26.3.0 prints the same lines). Other suites: Notes.
Self-reviewed: 32 concerns raised, 28 addressed. The 4 left out are in the Notes.

Background

A NativeReadable is a Node Readable over a native source. One ptr.pull() can return the last bytes and the EOF.
Node reads child stdio from a libuv pipe: one callback per chunk, for the EOF and for the close. After each it runs nextTicks and promise jobs until both queues are empty (processTicksAndRejections).

Downsides

Child stdio: a throw from a 'data' listener is now always an uncaughtException, never an unhandledRejection, and finished() calls back before 'close'. Node does both.
One more nextTick and queue entry per child stdio chunk. 4 GiB through child.stdout: 1388 to 1671 MiB/s before, 1424 to 1683 after.
Readable.fromWeb chunks still come back to back.

Notes

Design question for a maintainer. This adds a small primitive to the process.nextTick loop. The Notes below say why it is there and what it costs. If you prefer another layer, say so and I change it.

Repro (bun c.cjs and node c.cjs). The async listener does its work after an await, so it is not done when 'end' runs:

const cp = require("node:child_process");
let bad = 0, n = 0;
const N = 40;
function one() {
  const c = cp.spawn("/bin/sh", ["-c", "printf AAAA; printf BB"], { stdio: ["ignore", "pipe", "ignore"] });
  let got = 0, sent = 0;
  c.stdout.on("data", async d => { sent += d.length; await null; got += d.length; });
  c.stdout.on("end", () => {
    if (got !== sent) bad++;
    if (++n < N) one();
    else console.log(`'end' ran before the last data listener's continuation: ${bad} of ${N}`);
  });
}
one();

Node v26.3.0 prints 0 of 40. A release build of main prints 1 to 4 of 40 on an idle machine and more under load. This branch prints 0 of 40.

Event order of that repro on main, 100 runs under CPU load. The listener logs data(n), a nextTick logs tick(n), the code after await null logs job(n).

  48x  data(4) job(4) tick(4) data(2) job(2) tick(2) end close exit
  30x  data(6) job(6) tick(6) end close exit
  19x  data(6) tick(6) end close job(6) exit
   3x  data(6) tick(6) job(6) end close exit
node v26.3.0: always data, tick, job, then end. The position of exit varies.

The test fixture (fixtures/child-process-stdio-event-order.js) makes each case deterministic with files that the parent and the child wait for. Each listener queues a nextTick, a promise job, and two more jobs that each wait for a nextTick. Some rows, with C for tick(x) job(x) job2(x) job3(x):

first-read data       the bytes and the EOF are in the pipe before the first read
  node, this branch   data(AAAABB) C end C close
  main                data(AAAABB) tick end tick close job job(end) job2 job2(end) job3 job3(end)
waiting-read data     the read already waits when the child writes
  node, this branch   data(AAAABB) C end C close
  main                data(AAAABB) job tick end tick(end) close job2 job(end) job3 job2(end) job3(end)
two-chunks data       the second chunk is in the pipe while the first 'data' listener runs
  node, this branch   data(AAAA) C data(BBBB) C end C close
  main                data(AAAA) job tick data(BBBB) tick end tick(end) close job2(AAAA) job(BBBB) ...
first-read destroy    a promise job of the listener destroys the stream
  node, this branch   data(AAAABB) destroy job(destroy) tick(destroy) job2(destroy) job3(destroy) close
  main                data(AAAABB) end tick(end) close destroy ...
waiting-read throw    the listener throws, an uncaughtException handler goes on
  node, this branch   data(AAAABB) uncaughtException(uncaughtException) C end C close
  main                data(AAAABB) unhandledRejection, then no 'end' and no 'close'
both first-read data  stdout and stderr both have their bytes before the first read
  node, this branch   data(OUT) C data(ERR) C
  main                data(OUT) data(ERR) tick(OUT) tick(ERR) job(OUT) job(ERR) ...

On Windows main only fails the data, throw and both rows, because libuv reports the bytes and the EOF separately there. Two rows run on POSIX only. first-read throw: Node on Windows runs the promise jobs of a listener that threw after 'end' when the EOF is already in the pipe. first-read read-on-exit: Bun reads a pipe asynchronously on Windows, so a read() call in 'exit' finds no bytes yet (the same on main).

Why a hook in the tick loop. A first version of this fix pushed the EOF from a promise job that queued a nextTick. A review found its limits. It needs the global queueMicrotask, which fake timers replace, so 'end' never came. A listener that awaits a promise which a nextTick settles still saw 'end' first. A chunk pushed from a nextTick let the next chunk run before the jobs of the one before. setImmediate would wait long enough, but fake timers replace it too, and it costs a turn of the event loop for each chunk. The tick loop is the one place that knows when both queues are empty. A native queue after the drain exists (DeferredTaskQueue in event_loop.rs), but it is unordered, and nothing drains the ticks and jobs that one of its tasks queues before the next task runs. The JS hook is the smaller change.

What the tick loop pays. The queue and its code are in internal/process/after_tick_drain.ts, which only native-readable.ts loads. The tick loop and that module meet through the object that internal/async_hooks_tick.ts exports, which the nextTick initializer already loads. The tick loop gets one optional call for each drain, in the loop condition, where Node's loop calls processPromiseRejections(). ensureTickLoop() creates the tick loop and marks it as having work without a call of process.nextTick, so the callbacks also run when fake timers replaced process.nextTick (they read it first, and that read creates the tick queue). When process.nextTick was overwritten before its first read, no tick queue exists, and runAfterTickDrain falls back to process.nextTick.

Nobody listens: push now. Without a 'data' or 'readable' listener nobody sees an order, and a read() call must get the bytes at once. Node reads a child's pipe from the start, so there the bytes are in the buffer already. child.on("exit", () => child.stdout.read()) depends on that.

'close'. A child's stdio stream is a net.Socket in Node: _destroy calls back at once, so 'error' comes in the next tick, and 'close' comes from the close callback of the handle. The "libuv-handle" kind does the same: emitClose: false, the callback at once, and 'close' through runAfterTickDrain. So 'close' comes after the promise jobs of the 'end' listeners, and finished() calls back at 'end'.

A destroyed stream gets no more events. A promise job of the last 'data' listener can destroy the stream before the EOF gets its turn. Node reports nothing more for a closed handle, so deliverReadEvent drops what waits, and pushEof (from #43739) drops the EOF of a fromWeb stream. #43739 also drops the chunk that a flowing stream holds when destroy() runs (dropReadAhead). Child stdio no longer reads ahead while someone listens, so that call is now for fromWeb only. For child stdio this removes the difference that #43739 accepted: chunks buffered while paused are emitted after destroy(), as in Node.

A read error. A read error is an event too, so the chunks that the same native read gave come first. test/js/bun/spawn/spawn-stdio-syscall-error.test.ts has a new case for that (Linux, LD_PRELOAD): it passes on main, and it fails when the error is not queued.

A destroy() callback that throws. _destroy queues 'close' before it calls back, as net.Socket registers the close callback of its handle before cb(exception). So a throw from the callback that destroy(err, cb) got does not lose 'close' (row destroy-callback-throws: Node and the branch print data(AAAABB) destroy-callback close, on main the stream never closes).

A listener that throws. The throw leaves push() before maybeReadMore(), so nothing read on, and 'end' never came. deliverReadEvent now reads on after a throw. This covers the child stdio part of #37171.

Cost. Release builds of main and of this branch at the same commit, with the same configuration, interleaved runs on a busy 16-core machine. No difference shows:

4 GiB from `head -c` through child.stdout, sync 'data' listener, 6 runs each
  main    1388 to 1671 MiB/s, parent user CPU 958 to 1316 ms
  branch  1424 to 1683 MiB/s, parent user CPU 966 to 1310 ms
300000 writes of 64 bytes from a child, 5 runs each
  main    0.34 to 0.45 s
  branch  0.31 to 0.46 s
300 short children, one after the other, 5 runs each
  main    0.67 to 0.74 ms per child
  branch  0.64 to 0.78 ms per child
first process.nextTick() call (it creates the tick loop), 10th percentile of 60 to 200 runs, 5 rounds
  main    1.00 to 1.05 ms
  branch  1.02 to 1.12 ms

An earlier layout of this change had the queue inside the nextTick initializer. That made the first process.nextTick() call about 90 us slower, so the queue moved to the lazy module.

Suites run with the debug build: test/js/node/child_process/, test/js/node/stream/, test/js/web/streams/streams.test.js, test/js/web/fetch/body.test.ts, test/js/web/fetch/fetch-backpressure.test.ts, test/js/bun/spawn/spawn-stdio-syscall-error.test.ts, test/js/node/process/process-nexttick.test.js, test/js/node/async_hooks/, test/js/node/readline/readline.node.test.ts, test/js/node/process/process-stdin.test.ts, and 457 of Node's test-child-process-*, test-stream*, test-readable*, test-readline*, test-next-tick*, test-microtask*, test-async-hooks*, test-domain*, test-promise*, test-stdio-* files. What fails also fails without the change: tests that need more time than the debug build gets on this machine, or an environment the container does not have. On Windows x64: both test files, child_process.test.ts, child_process-node.test.js and streams.test.js (841 tests).

Left out after the self-review (the first three items below), plus one naming point: the bridge stays in internal/async_hooks_tick.ts under that name, to keep the diff small.

Differences from Node that this PR does not change:

A child that writes faster than the parent reads keeps the tick loop busy while reads return bytes at once, so timers wait. This is on main too: there flow() loops inside one nextTick. Release builds at the same commit, yes at full speed for 1 s, a 10 ms interval: main fires it 23 to 82 times of 99 (worst lateness 23 to 157 ms), the branch 44 to 64 times (36 to 44 ms), Node 99 times (3 ms). A bound needs a way to yield to the event loop that fake timers do not replace, or a cap on consecutive synchronous reads in FileReader::on_pull.
Only nextTicks and promise jobs run between two events. Node reads a pipe once for each turn of the event loop, so there a setImmediate() from a listener also runs before the next event. Bun reports unhandled rejections after the whole drain, Node between two events.
process.stdin (a pipe) and fs.createReadStream run the promise jobs of a 'data' listener before its nextTicks, and process.stdin can emit 'end' before a late job. They do not use NativeReadable. process.stdin is the natural next user of runAfterTickDrain. process.stdin: deliver 'end' when pause() inside a 'data' handler races a buffered EOF #34613, process.stdin: do not reset the ended and destroyed state on resume() #43100 and process.stdin: end instead of throwing EISDIR when fd 0 is a directory #41484 are open on that code, so it is not part of this PR.
Readable.fromWeb() of a native stream runs nextTicks before promise jobs when pull() returns bytes at once, and emits available chunks back to back. Node's fromWeb pushes every chunk from a promise job.

A flaky test that main has too. test/js/bun/spawn/spawn-stdio-syscall-error.test.ts, "stdout delivers every byte read before the error", can report lost < 0: the listener got bytes that were read after the recv() that failed. PosixBufferedReader::read_loop (src/io/PipeReader.rs:758) gives the bytes of a partly filled buffer to on_read_chunk before it reports the error. on_read_chunk settles the waiting pull, and the promise jobs and nextTicks run before it returns. They pull again. The reader is still open and has no stored error, so the nested on_pull reads on (the shim of the test fails one call only). The error comes when a read finds the pipe empty. close_if_final closes the reader before the last chunk of an EOF. A read error has no such step. Release builds at the same commit, 16 busy cores: main 6 of 5400 runs, this branch 8 of 4600, and the recv() log of each bad run shows this sequence. No open PR covers it.

Related open PRs that touch native-readable.ts: #37171 (a throw from a 'data' listener: this PR covers its child stdio case, and its pushAndCheck change conflicts). #36316 and #42281 add a third parameter to constructNativeReadable. #42584 routes the push(null) sites through a new endOfSource(). #41608 touches the constructor. None of them changes the order described here. Each needs a small rebase if this lands first.

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/node/stream/node-stream.test.js, test/js/node/child_process/child_process.test.ts, test/js/node/child_process/child-process-stdio.test.js, test/js/bun/spawn/spawn-stdio-syscall-error.test.ts

…ise jobs of the one before A child's stdout and stderr got the last bytes and the EOF from one native read and pushed both from one nextTick drain, so 'end' and 'close' ran before the promise jobs that the listeners of the last chunk queued. A chunk from a pending read was pushed inside a promise job, so a listener's promise jobs ran before its nextTicks. Chunks that were available at once were emitted back to back, and 'close' ran before the promise jobs of the 'end' listeners. Node reports each chunk, the EOF, a read error and the close of a libuv handle from a callback of its own, and runs every nextTick and promise job between two callbacks. internal/process/after_tick_drain gives a callback such a point: the tick loop runs one of its callbacks each time the nextTick queue and the promise job queue are both empty. A child's stdio streams deliver their events from there while a 'data' or 'readable' listener can see the order. A throw from a 'data' listener is an uncaught exception now, and the stream reads on. Readable.fromWeb of a native stream pushes its EOF from there too, so a promise job of the last 'data' listener runs first, and can destroy the stream before the EOF.

robobun · 2026-09-22T13:01:30Z

Status: ready for review.

How I reproduced it

The repro in the PR body (bun c.cjs): main prints 1 to 4 of 40 on an idle machine and more under load. This branch prints 0 of 40, as Node v26.3.0 does.
test/js/node/child_process/fixtures/child-process-stdio-event-order.js makes each order deterministic. The parent and the child wait for files, so the bytes and the EOF are in the pipe before the first read, or the read already waits when the child writes. Node v26.3.0 prints the same lines as this branch, on Linux and on Windows x64.
With main's src/ and the debug build, 19 of the 21 new tests fail. With this branch all of them pass.

One thing needs a maintainer's decision: the change adds a small primitive to the process.nextTick loop (runAfterTickDrain). The Notes in the PR body say why it is in that layer, what it costs, and what the alternatives were.

coderabbitai · 2026-09-22T13:10:13Z

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: oven-sh/bun/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Essentials

Run ID: 80f8ebf2-a639-41b6-9a62-d0ff04d6edc9

📥 Commits

Reviewing files that changed from the base of the PR and between 66871ac and 7b9dfc1.

📒 Files selected for processing (4)

src/js/builtins/ProcessObjectInternals.ts
src/js/internal/async_hooks_tick.ts
src/js/internal/process/after_tick_drain.ts
src/js/internal/streams/native-readable.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 1 remains after this review.

Walkthrough

The change adds post-tick-drain callbacks and integrates them with native readable streams. Libuv-backed streams now queue chunk, error, EOF, and close events for ordered delivery. Child-process and webstream tests cover event ordering and destruction behavior.

Changes

Native stream event ordering

Layer / File(s)	Summary
Tick drain coordination `src/js/builtins/ProcessObjectInternals.ts`, `src/js/internal/async_hooks_tick.ts`, `src/js/internal/process/after_tick_drain.ts`	The tick loop now supports post-drain callbacks. The dispatcher preserves callback order, async context, queue state, and exception handling.
Native readable event delivery `src/js/internal/streams/native-readable.ts`, `src/js/internal/webstreams_adapters.ts`, `src/js/node/child_process.ts`	Libuv-handle streams queue and deliver chunk, error, EOF, and close events after the drain cycle. Webstream behavior remains source-specific. Adapters pass the source type.
Event-order regression coverage `test/js/bun/spawn/spawn-stdio-syscall-error.test.ts`, `test/js/node/child_process/*`, `test/js/node/stream/node-stream.test.js`	Tests cover failing reads, data and error ordering, promise jobs, destruction, replaced `process.nextTick`, and native-backed `Readable.fromWeb` streams.

Suggested reviewers: jarred-sumner

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to 7b9df

The change aligns native stream event delivery with Node-compatible queue ordering and adds regression coverage; no actionable merge-blocking risk is currently established.

🚥 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.
Description check	✅ Passed	The description clearly explains the problem, implementation, verification, tradeoffs, test coverage, and known limitations. It does not use the template headings exactly, but it provides the required…
Title check	✅ Passed	The title clearly and concisely describes the primary change: ordering each child-process stdio event after the preceding nextTicks and promise jobs.

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

claude

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

…back The callback of _destroy runs the one that destroy() got, and that one can throw. net.Socket registers the close callback of its handle before it calls back, so 'close' still comes.

claude

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

Beyond the inline finding, I also checked the two call sites of the new constructNativeReadable signature (child_process.ts:1300 and webstreams_adapters.ts:41 are the only callers) and the "webstream" path: with [kReadEvents] undefined, chunks still go through pushAndCheck, pull errors still reach errorOrDestroy, and only the EOF moves to runAfterTickDrain, so Readable.fromWeb keeps its base-branch behavior apart from the intended EOF ordering. The destroy reorder in 7b9dfc1 (queue emitClose before cb(error)) covers the destroy-callback-throw case I raised earlier.

Extended reasoning...

The change adds an after-tick-drain hook to the process.nextTick loop and reworks native-readable event delivery for child_process stdio and Readable.fromWeb; it touches no security-sensitive surface. Findings are posted inline, so this note only records what else was examined: the signature change has exactly two callers, and the non-handle path preserves prior behavior. The tick-loop primitive is a design decision the author explicitly asks a maintainer to weigh in on.

The array with a head index only shrank when it became empty. With two streams that each queue their next event before the other one is taken, it never became empty, and it grew by one slot for each event.

claude

I re-reviewed the latest push and found no bugs; the two earlier inline points ('close' queued before the destroy callback, the after-drain queue growth) are addressed in the current code. Because this adds a new hook into the process.nextTick loop and changes the delivery model for every child_process stdio stream, a maintainer decision on the tick-loop primitive (which the description itself asks for) is still worthwhile.

What was reviewed:

runAfterTickDrainCallback in processTicksAndRejections: only evaluated once the tick queue is empty, one entry per drain, and a throw leaves the async-context reset to JSNextTickQueue::drain the same way a throwing tick does.
The kReadEvents lifecycle in native-readable.ts: bounded to one _read's results, cleared on destroy, _read suppressed while events are pending or after EOF, and both constructNativeReadable callers pass the new nodeSource argument.
emitClose: false plus a direct emit("close") leaves closeEmitted unset, matching Node's net.Socket; the process.nextTick-overwritten-before-first-read fallback and the _exiting case were checked and do not strand the stream.

Extended reasoning...

The change touches src/js/builtins/ProcessObjectInternals.ts (the nextTick loop), a new src/js/internal/process/after_tick_drain.ts, src/js/internal/async_hooks_tick.ts, src/js/internal/streams/native-readable.ts, and the two callers in child_process.ts and webstreams_adapters.ts, plus new event-order tests. It touches no security-sensitive surface (no auth, crypto, injection, or path handling). The bug hunt ran dry with no findings, and the code addresses the prior inline comments, but the PR introduces a cross-cutting primitive in the tick loop and alters event timing for all child_process stdio streams and Readable.fromWeb EOF, which is a design decision a maintainer should weigh rather than something to approve automatically. I could not run the test suite here since no debug build exists in this checkout.

### Problem - `test/js/bun/spawn/spawn-stdio-syscall-error.test.ts` is red on alpine: `"lost": -82000`, not `0` (build 120303). #43739 added the case, not the bug. - `read_loop` (`src/io/PipeReader.rs:766`) delivers the bytes read before a failed read, then the error. The consumer asks for more inside that delivery, and the reader reads the fd past the error. The error comes late, or never. ### Fix - `read_once` sets `PosixFlags::READ_FAILED` on a fatal error, and `begin_read` does not read while it is set. The request parks, then `on_reader_error` rejects it. Nothing clears the flag. - Correct: libuv `uv__read` clears `UV_HANDLE_READABLE` before it reports a read error. - Verified: `test/js/bun/spawn/spawn-stdio-syscall-error.test.ts`, 17 pass. The four new cases fail without the fix. Suites: Notes. - Self-reviewed: 15 concerns raised, 14 addressed. Not taken: a flag reset in `start()`, which nothing on main needs. ### Background - `PosixBufferedReader` reads the fd behind subprocess stdio, file streams and the shell. Its parent gets `on_read_chunk`, then `on_reader_done` or `on_reader_error`. - `FileReader` is the parent behind a `ReadableStream`. `on_read_chunk` resolves the parked `pull()`, and the reaction runs before it returns. - Considered: a repeating shim failure hides the lost error. An error stored in `FileReader` first needs a callback in every parent. - #43920 is a newer PR with the same change. Its test trigger is here. ### Downsides - After a read error, a stream that ended with `'end'` now ends with `'error'`, as in Node. With no listener the process stops. - After a read error, `bun run --filter` no longer drains that pipe at exit. - Reads that do not fail pay nothing: `begin_read` tests one more bit. <details><summary>Notes</summary> **Trace without the fix** (bun 1.4.3-canary.1+367d939d9, shim logs each `recv()`, writer `dd bs=1025`). The debug build of main at 8d36bff does the same: 3 of 8 runs, two with no `'error'` and `lost` -7714150: ``` recv #5 len=262144 -> 95325 recv #6 len=166819 -> EIO (injected) same fill_scratch call as #5 JS data 95325 recv #7 len=65536 -> 65536 pull from inside the delivery, read_into recv #8 .. #116 to EOF {"received":452025,"got":8000125,"lost":-7548100,"events":["stdout.close","close"]} ``` No `'error'` event: the reads reached EOF before `on_reader_error` ran, and a stored error is only returned by a later pull. When the reads park first, `on_reader_error` rejects that pull and `'error'` comes late. That is the CI signature: the events match and `lost` is a negative multiple of 1025. **Why alpine.** In CI the failure is injected: the shim fails only the Nth `recv()`, so a later `recv()` succeeds and the extra bytes show. The failing `recv()` must follow bytes in the same wakeup. BusyBox `head` writes 1025 bytes at a time, so a wakeup often holds a short `recv()` and then the failing one. coreutils `head` fills the buffer in one `recv()`. The same run fails on debian when the writer is slow. With a writer that copies BusyBox `head` (stdio, 1024-byte buffer), the `RECV_AT=6` case fails 7 of 40 runs on bun 1.4.3-canary (release), and 36 of 40 when the writer also spins between chunks. This branch (debug): 0 of 120, and 0 of 60 with `dd bs=1025`. **With no shim.** A child with one AF_UNIX socket as fd 0 and fd 1 writes a line to stdout and reads stdin. The peer leaves that line unread, sends 8192 bytes and closes. The kernel gives the child 8192 bytes, then `ECONNRESET`, then EOF. | Runtime | stdin events | |---|---| | Node v26.3.0 | `'error'` `ECONNRESET` after 8192 bytes | | bun 1.4.3-canary.1+367d939d9 | `'end'` after 8192 bytes, no error | | this branch | `'error'` `ECONNRESET` after 8192 bytes | **The new cases.** Each one reaches the reader from inside the delivery in a different way. Whole-file runs of the describe block: | Case | Entry | Release, no fix | Debug, no fix | Debug, guard in `read_into` only | Debug, this branch | |---|---|---|---|---|---| | `child_process`, `'data'` listener | pull, `read_into` | fails 6 of 6 | fails 5 of 5 | passes | passes | | `child_process`, `'readable'` and `read()` | `set_flowing(true)`, `read` | fails 6 of 6 | fails 5 of 5 | fails 3 of 3 | passes | | `Bun.spawn`, `lazy` | pull, `read_into` | fails 6 of 6 | fails 5 of 5 | passes | passes | | `Bun.spawn`, reader started at spawn | pull, `read_into` | fails 6 of 6 | fails 5 of 5 | passes | passes | - The first three use counts: `SPAWN_FAULT_RECV_CAP=4096` makes every `recv()` short, so `fill_scratch` calls `recv()` again in the same wakeup. `SPAWN_FAULT_RECV_EAGAIN_AT=2` ends the first read loop, so the consumer's read parks and the next read is poll-driven. `SPAWN_FAULT_RECV_AT` then fails after bytes in that wakeup. For the `'readable'` case it is 19: #3 to #18 return 64 KiB, the highWaterMark, so the reader is stopped and `read()` starts it again. - The fourth uses state, and comes from #43920: the writer waits for a line on stdin, so the first read is parked when the bytes arrive. `SPAWN_FAULT_RECV_MID_FILL=1` fails the `recv()` that follows one that returned bytes, and `SPAWN_FAULT_READS_AFTER` counts the `recv()` calls after it. Without the fix it is 1. - With a count-based trigger and the reader started at spawn, the case passed without the fix on a debug build: the buffered reader that runs before JS reads `.stdout` took the bytes and the error. That is why this case uses state. - With the BusyBox-like writer at four speeds, the whole file passes 20 of 20 on this branch. **With the fix**, `CAP=4096 EAGAIN_AT=2 RECV_AT=5`: ``` recv #3 -> 4096, recv #4 -> 4096, recv #5 -> EIO JS data 8192 close(fd) JS error EIO ``` **Node.** libuv `uv__read` (`src/unix/stream.c`): on a read error other than `EAGAIN` it clears `UV_HANDLE_READABLE | UV_HANDLE_WRITABLE`, calls `read_cb` with the error, then stops the watcher. It calls `read_cb` once for each `read()`, so it never holds bytes and an error from one batch. **Placement.** EOF and the `maxBuffer` stop have the same guard at this site: `close_if_final` closes the reader before the final chunk is delivered. An error cannot use it, because a closed reader with no stored error reads as a clean end. For the same reason `READ_FAILED` is not part of `is_done()`. **Parents** (14 `BufferedReaderParent` implementations, what each does in `on_reader_error`): - 3 release the fd: `FileReader`, `SubprocessPipeReader`, `Terminal`. - 2 drop the reader: `FileResponseStream`, shell `subproc.rs`. - 8 only do accounting: `filter_run.rs`, `multi_run.rs`, `lifecycle_script_runner.rs`, `security_scanner.rs`, `git_runner.rs`, both cron jobs, test `Worker.rs`. `lifecycle_script_runner.rs` and `cron.rs` build a new reader with `init()` for each spawn. - 1 is shared and can restart: the shell `IOReader`. Only two read the same reader after an error. `filter_run.rs` `drain_and_close_pipes` reads once more at exit. That read is now a no-op, and `deinit()` follows. Before, it could reach a second terminal callback and decrement `remaining_fds` twice. The shell `IOReader::start()` does not restart a reader after a failed read on main, because the fired one-shot poll still counts as registered. The Windows reader gets one libuv callback for each read, so it has no bytes-then-error batch. **Left open.** - The flag is permanent. #39638 and #37901 change `IOReader::start()` to restart the shell's stdin reader. After this PR they must clear `READ_FAILED` there, or a `cat` that follows a stdin read error gets no data, no EOF and no error. - Not in this PR: a batch that stops because the buffer is full is labelled `ReadState::Eof` when the poll event carries the hangup (`read_state`, `None if received_hup`). `Bun.write(file, proc.stdout)` then writes 262144 of 400000 pending bytes and resolves. It is on main and in 1.4.3-canary, and this PR does not change it. It needs its own change. - Not in this PR: release the fd on a read error once, in `PosixBufferedReader::on_error`. #41456 names it as a follow-up. #41420, #41456 and #42150 did it for one parent each. **Suites run on the debug build:** `test/js/web/streams/streams.test.js` (624 pass), `test/js/node/stream/node-stream.test.js` (112 pass), `test/js/bun/spawn/spawn-streaming-stdout.test.ts` (pass), `test/js/node/child_process/child_process.test.ts` (81 pass, 2 fail in my container for reasons outside this change: "spawn in the default shell" reads an empty `$SHELL`, and "extra stdio pipes are not double-closed on GC" needs 5.0 s in a debug build against the 5 s timeout, its script prints `OK`). With this branch's build, the test file of #43920 passes 17 of 17 in 3 runs. #43790 is open and edits the comment above the failing case. It changes the event order in `native-readable.ts`, not the reader. </details>  --- **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/spawn/spawn-stdio-syscall-error.test.ts

robobun assigned Jarred-Sumner Sep 22, 2026

github-actions Bot added the claude label Sep 22, 2026

robobun mentioned this pull request Sep 22, 2026

node:stream: deliver child stdio 'data' listener throws as uncaughtException #37171

Open

claude Bot reviewed Sep 22, 2026

View reviewed changes

Comment thread src/js/internal/streams/native-readable.ts Outdated

Comment thread src/js/builtins/ProcessObjectInternals.ts

child_process: queue a stdio stream's 'close' before the destroy call…

66871ac

…back The callback of _destroy runs the one that destroy() got, and that one can throw. net.Socket registers the close callback of its handle before it calls back, so 'close' still comes.