From eed64761e2109bb657451c05aecadbed1c1cf096 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 21 Sep 2026 17:23:32 +0000 Subject: [PATCH 1/7] node:child_process: deliver each stdio event after the ticks and promise 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. --- src/js/builtins/ProcessObjectInternals.ts | 17 +- src/js/internal/async_hooks_tick.ts | 8 + src/js/internal/process/after_tick_drain.ts | 65 +++++++ src/js/internal/streams/native-readable.ts | 173 ++++++++++++++++-- src/js/internal/webstreams_adapters.ts | 2 +- src/js/node/child_process.ts | 2 +- .../spawn/spawn-stdio-syscall-error.test.ts | 43 ++++- .../child_process/child-process-stdio.test.js | 89 ++++++++- .../node/child_process/child_process.test.ts | 11 +- .../child-process-stdio-event-order.js | 129 +++++++++++++ test/js/node/stream/node-stream.test.js | 51 ++++++ 11 files changed, 562 insertions(+), 28 deletions(-) create mode 100644 src/js/internal/process/after_tick_drain.ts create mode 100644 test/js/node/child_process/fixtures/child-process-stdio-event-order.js diff --git a/src/js/builtins/ProcessObjectInternals.ts b/src/js/builtins/ProcessObjectInternals.ts index 45ea5cbbd6ce..0ce8aa15fdc0 100644 --- a/src/js/builtins/ProcessObjectInternals.ts +++ b/src/js/builtins/ProcessObjectInternals.ts @@ -348,12 +348,13 @@ export function initializeNextTickQueue(process: typeof globalThis.process, next var drainMicrotasks = drainMicrotasksFn; const { validateFunction } = require("internal/validators"); + const asyncHooksTick = require("internal/async_hooks_tick"); var setup; setup = () => { const { FixedQueue } = require("internal/fixed_queue"); queue = new FixedQueue(); - tickInitHooks = require("internal/async_hooks_tick").tickInitHooks; + tickInitHooks = asyncHooksTick.tickInitHooks; function processTicksAndRejections() { var tock; @@ -393,7 +394,8 @@ export function initializeNextTickQueue(process: typeof globalThis.process, next } drainMicrotasks(); - } while (!queue.isEmpty()); + // With both queues empty, internal/process/after_tick_drain.ts (once something loaded it) runs one callback. + } while (!queue.isEmpty() || asyncHooksTick.runAfterTickDrainCallback?.()); } $putInternalField(nextTickQueue, 0, 0); @@ -402,6 +404,16 @@ export function initializeNextTickQueue(process: typeof globalThis.process, next setup = undefined; }; + // For internal/process/after_tick_drain.ts: makes the next drain call the tick loop, without a call of + // process.nextTick, which fake timers replace. JSNextTickQueue::drain only calls in while field 0 is set. + asyncHooksTick.ensureTickLoop = () => { + if (setup) { + setup(); + process = globalThis.process; + } + $putInternalField(nextTickQueue, 0, 1); + }; + function nextTick(cb, ...args) { validateFunction(cb, "callback"); if (setup) { @@ -420,7 +432,6 @@ export function initializeNextTickQueue(process: typeof globalThis.process, next if (tickInitHooks.length !== 0) { // node fires one TickObject init per process.nextTick() call, at // construction time (before the callback runs). - const asyncHooksTick = require("internal/async_hooks_tick"); const asyncId = asyncHooksTick.newAsyncId(); // Snapshot: enable()/disable() from inside a hook must not affect the // in-flight dispatch (node stages such mutations in tmp_array until diff --git a/src/js/internal/async_hooks_tick.ts b/src/js/internal/async_hooks_tick.ts index ce97c1cd6a81..55def82f8d40 100644 --- a/src/js/internal/async_hooks_tick.ts +++ b/src/js/internal/async_hooks_tick.ts @@ -9,6 +9,10 @@ // Currently only TickObject `init` events are delivered (enough for // console.log/stream.write tick-coalescing tests); promise, timer and native // resource events are still unimplemented. +// +// `ensureTickLoop` and `runAfterTickDrainCallback` connect the tick loop and +// internal/process/after_tick_drain.ts. The tick loop does not load that +// module, so a process that never uses it pays one property read for each drain. const tickInitHooks: Array<(asyncId: number, type: string, triggerAsyncId: number, resource: object) => void> = []; let nextAsyncId = 1; @@ -17,4 +21,8 @@ export default { newAsyncId() { return ++nextAsyncId; }, + // Set when process.nextTick is first read, which creates the tick queue. + ensureTickLoop: undefined as (() => void) | undefined, + // Set by internal/process/after_tick_drain.ts. The tick loop calls it each time both queues are empty. + runAfterTickDrainCallback: undefined as (() => boolean) | undefined, }; diff --git a/src/js/internal/process/after_tick_drain.ts b/src/js/internal/process/after_tick_drain.ts new file mode 100644 index 000000000000..cb6d94534b06 --- /dev/null +++ b/src/js/internal/process/after_tick_drain.ts @@ -0,0 +1,65 @@ +// Node calls into JS once for each libuv event, and after each call it runs +// every process.nextTick callback and every promise job, until both queues +// are empty. So the next event of a handle (the next chunk, the EOF) never +// runs ahead of a tick or a promise job that a listener of the previous event +// queued, however long the chain of ticks and jobs is. +// https://github.com/nodejs/node/blob/v24.9.0/lib/internal/process/task_queues.js#L72-L109 +// +// A module that gets several of Node's events out of one native call gives +// each of them a drain of its own with `runAfterTickDrain`. The tick loop +// (processTicksAndRejections in builtins/ProcessObjectInternals.ts) runs one +// of these callbacks each time both queues are empty, where Node's loop calls +// processPromiseRejections(). That happens inside the drain that is running, +// so no other I/O callback runs in between. +// +// Only nextTicks and promise jobs run between two callbacks. Bun reports +// unhandled rejections after the whole drain, and Node reads a pipe once for +// each turn of the event loop, so there a setImmediate() of a listener also +// runs before the next event. A setImmediate() here would wait for that too, +// but fake timers replace it, and it costs a turn of the event loop for each +// chunk. +const asyncHooksTick = require("internal/async_hooks_tick"); + +type AfterTickDrainCallback = { + callback: (arg: any) => void; + arg: unknown; + frame: import("../../node/async_hooks").Frame | undefined; +}; + +// A queue: entries before `head` are done. Bounded by the number of callers, which each wait for their turn. +const entries: (AfterTickDrainCallback | undefined)[] = []; +let head = 0; + +function runAfterTickDrain(callback: (arg: any) => void, arg?: unknown) { + let ensureTickLoop = asyncHooksTick.ensureTickLoop; + if (ensureTickLoop === undefined) { + // The first read of process.nextTick creates the tick queue. Fake timers read it before they replace it. + void process.nextTick; + ensureTickLoop = asyncHooksTick.ensureTickLoop; + // process.nextTick was overwritten before its first read: there is no tick loop to wait for. + if (ensureTickLoop === undefined) return process.nextTick(callback, arg); + } + ensureTickLoop(); + $arrayPush(entries, { callback, arg, frame: $getInternalField($asyncContext, 0) }); +} + +// The tick loop calls this each time both queues are empty. False: nothing waited. +function runAfterTickDrainCallback() { + if (head === entries.length) return false; + const { callback, arg, frame } = entries[head]!; + entries[head++] = undefined; + if (head === entries.length) { + entries.length = 0; + head = 0; + } + const restore = $getInternalField($asyncContext, 0); + $putInternalField($asyncContext, 0, frame); + // No catch and no finally, as for a tick: JSNextTickQueue::drain reports a throw inside the + // callback's async context, puts the context back, and calls the tick loop again. + callback(arg); + $putInternalField($asyncContext, 0, restore); + return true; +} +asyncHooksTick.runAfterTickDrainCallback = runAfterTickDrainCallback; + +export default { runAfterTickDrain }; diff --git a/src/js/internal/streams/native-readable.ts b/src/js/internal/streams/native-readable.ts index 3d62e724416f..1b7e084c1ca6 100644 --- a/src/js/internal/streams/native-readable.ts +++ b/src/js/internal/streams/native-readable.ts @@ -12,6 +12,7 @@ const transferToNativeReadable = $newCppFunction( 1, ); const { errorOrDestroy } = require("internal/streams/destroy"); +const { runAfterTickDrain } = require("internal/process/after_tick_drain"); const kRefCount = Symbol("refCount"); const kCloseState = Symbol("closeState"); @@ -20,6 +21,15 @@ const kHighWaterMark = Symbol("highWaterMark"); const kPendingRead = Symbol("pendingRead"); const kHasResized = Symbol("hasResized"); const kRemainingChunk = Symbol("remainingChunk"); +const kSawEof = Symbol("sawEof"); +const kReadEvents = Symbol("readEvents"); +const kReadEventsHead = Symbol("readEventsHead"); +const kReadEventQueued = Symbol("readEventQueued"); + +// What a read gave, in the order Node would report it: see `kReadEvents`. +const kChunkEvent = 0; +const kEofEvent = 1; +const kErrorEvent = 2; const MIN_BUFFER_SIZE = 512; let dynamicallyAdjustChunkSize = (_?) => ( @@ -49,6 +59,12 @@ interface NativeReadable extends NodeReadable { [kHighWaterMark]: number; [kHasResized]: boolean; [kRemainingChunk]: Buffer | undefined; + [kSawEof]: boolean; + // "libuv-handle" only: pairs of (kind, value) that wait for a drain of their own, from `kReadEventsHead` on. + // At most the results of one `_read`: the next pull waits until they are delivered. + [kReadEvents]: unknown[] | undefined; + [kReadEventsHead]: number; + [kReadEventQueued]: boolean; debugId: number; } @@ -65,12 +81,34 @@ interface NativePtr { let debugId = 0; -function constructNativeReadable(readableStream: ReadableStream, options): NativeReadable { +// `nodeSource` names what feeds the equivalent stream in Node. +// +// "libuv-handle": child_process stdio, a net.Socket over a pipe in Node. Node +// reports each chunk, the EOF, a read error and the close from a libuv +// callback of its own, and runs every nextTick and promise job between two of +// them (internal/process/after_tick_drain.ts). One native read here can give +// several of those events, and `_read` runs inside a nextTick or a promise +// job. So each event waits for `runAfterTickDrain`, unless nobody listens. +// https://github.com/nodejs/node/blob/v24.9.0/lib/internal/stream_base_commons.js#L166-L233 +// +// "webstream": Readable.fromWeb. Node pushes each chunk from the promise job +// of a `reader.read()`, and gets the EOF from a later read. Here a chunk is +// pushed as it arrives: inside `_read` when the read is synchronous (so a +// 'data' listener's nextTicks run before its promise jobs, unlike Node), +// inside the promise job otherwise. Only the EOF waits for `runAfterTickDrain`. +// https://github.com/nodejs/node/blob/v24.9.0/lib/internal/webstreams/adapters.js#L544-L555 +function constructNativeReadable( + readableStream: ReadableStream, + options, + nodeSource: "libuv-handle" | "webstream", +): NativeReadable { $assert(typeof readableStream === "object" && readableStream instanceof ReadableStream, "Invalid readable stream"); const bunNativePtr = (readableStream as any).$bunNativePtr; $assert(typeof bunNativePtr === "object", "Invalid native ptr"); - const stream = new Readable(options) as NativeReadable; + const isHandle = nodeSource === "libuv-handle"; + // A net.Socket emits 'close' itself, from the close callback of its handle: see `destroy`. + const stream = new Readable(isHandle ? { ...options, emitClose: false } : options) as NativeReadable; stream._read = read; stream._destroy = destroy; @@ -86,6 +124,10 @@ function constructNativeReadable(readableStream: ReadableStream, options): Nativ stream[kPendingRead] = false; stream[kHasResized] = !dynamicallyAdjustChunkSize(); stream[kCloseState] = [false]; + stream[kSawEof] = false; + stream[kReadEvents] = isHandle ? [] : undefined; + stream[kReadEventsHead] = 0; + stream[kReadEventQueued] = false; const highWaterMark = options.highWaterMark; stream[kHighWaterMark] = typeof highWaterMark === "number" ? highWaterMark : 256 * 1024; @@ -142,7 +184,12 @@ function read(this: NativeReadable, maxToRead: number) { // not paused from a previous `push()===false` (readStart, like net.Socket). // Runs even when a pull promise is outstanding so that promise can resolve. if (ptr) ptr.setFlowing?.(true); - if (this[kPendingRead]) { + if (this[kPendingRead] || this[kSawEof]) { + return; + } + const events = this[kReadEvents]; + // The events of the last read come first, and push() has not said yet if it wants more. + if (events !== undefined && events.length !== 0) { return; } if (!ptr) { @@ -158,17 +205,25 @@ function read(this: NativeReadable, maxToRead: number) { this[kHighWaterMark] = Math.min(this[kHighWaterMark], result); } if ($isTypedArrayView(result) && result.byteLength > 0) { - pushAndCheck(this, result); + deliverChunk(this, result); } const drainResult = ptr.drain(); this[kConstructed] = true; $debug(`[${this.debugId}] drain result: ${drainResult?.byteLength ?? "null"}`); if ((drainResult?.byteLength ?? 0) > 0) { - pushAndCheck(this, drainResult); + deliverChunk(this, drainResult); } } const chunk = getRemainingChunk(this, maxToRead); - var result = ptr.pull(chunk, this[kCloseState]); + var result; + try { + result = ptr.pull(chunk, this[kCloseState]); + } catch (error) { + if (events === undefined) throw error; + // The chunks that this read already got come before the error. + queueReadEvent(this, events, kErrorEvent, error); + return; + } $assert(result !== undefined); $debug( `[${this.debugId}] pull ${chunk?.byteLength} bytes, result: ${$isPromise(result) ? "" : $isTypedArrayView(result) ? `<${result.byteLength} bytes>` : result}, closeState: ${this[kCloseState][0]}`, @@ -184,7 +239,8 @@ function read(this: NativeReadable, maxToRead: number) { this[kRemainingChunk] = handleResult(this, result, chunk, this[kCloseState][0]); }, reason => { - errorOrDestroy(this, reason); + if (events === undefined) errorOrDestroy(this, reason); + else queueReadEvent(this, events, kErrorEvent, reason); }, ); } else { @@ -201,7 +257,7 @@ function handleResult(stream: NativeReadable, result: any, chunk: Buffer | undef return handleNumberResult(stream, result, chunk, isClosed); } else if (typeof result === "boolean") { $debug(`[${stream.debugId}] handleResult(${result})`, chunk, isClosed); - process.nextTick(pushEof, stream); + deliverEof(stream); return (chunk?.byteLength ?? 0) > 0 ? chunk : undefined; } else if ($isTypedArrayView(result)) { if (result.byteLength >= stream[kHighWaterMark] && !stream[kHasResized] && !isClosed) { @@ -213,7 +269,8 @@ function handleResult(stream: NativeReadable, result: any, chunk: Buffer | undef } } -// EOF is pushed a tick after the last chunk. After a destroy() in between, Node emits 'close' without 'end'. +// The EOF waits for the nextTicks and promise jobs of the last chunk's listeners. After a destroy() +// in between, Node emits 'close' without 'end'. function pushEof(stream: NativeReadable) { if (!stream.destroyed) stream.push(null); } @@ -228,17 +285,92 @@ function pushAndCheck(stream: NativeReadable, chunk: any) { } } +function deliverChunk(stream: NativeReadable, chunk: any) { + const events = stream[kReadEvents]; + // Without a 'data' or 'readable' listener nobody sees an order, and a read() call must get the + // bytes now: Node reads a child's pipe from the start, so there they are in the buffer already. + if ( + events === undefined || + (events.length === 0 && stream.listenerCount("data") === 0 && stream.listenerCount("readable") === 0) + ) { + pushAndCheck(stream, chunk); + } else { + queueReadEvent(stream, events, kChunkEvent, chunk); + } +} + +// The EOF can come in the same read as the last chunk. Node gets it in a read +// of its own, after the nextTicks and promise jobs of that chunk's listeners. +function deliverEof(stream: NativeReadable) { + stream[kSawEof] = true; + const events = stream[kReadEvents]; + if (events === undefined) runAfterTickDrain(pushEof, stream); + else queueReadEvent(stream, events, kEofEvent, undefined); +} + +function queueReadEvent(stream: NativeReadable, events: unknown[], kind: number, value: unknown) { + $arrayPush(events, kind); + $arrayPush(events, value); + queueReadEventDelivery(stream); +} + +function queueReadEventDelivery(stream: NativeReadable) { + if (stream[kReadEventQueued]) return; + stream[kReadEventQueued] = true; + runAfterTickDrain(deliverReadEvent, stream); +} + +function deliverReadEvent(stream: NativeReadable) { + stream[kReadEventQueued] = false; + const events = stream[kReadEvents]!; + // Node reports nothing more for a handle that is closed. + if (stream.destroyed) { + events.length = 0; + stream[kReadEventsHead] = 0; + return; + } + const head = stream[kReadEventsHead]; + const kind = events[head]; + const value = events[head + 1]; + if (head + 2 === events.length) { + events.length = 0; + stream[kReadEventsHead] = 0; + } else { + stream[kReadEventsHead] = head + 2; + // Before the push, so that a listener that throws does not strand the events behind this one. + queueReadEventDelivery(stream); + } + if (kind === kEofEvent) { + stream.push(null); + } else if (kind === kErrorEvent) { + errorOrDestroy(stream, value); + } else { + let threw = true; + try { + pushAndCheck(stream, value); + threw = false; + } finally { + // The throw of a 'data' listener skipped the maybeReadMore() of push(). Node's handle reads on. + if (threw) process.nextTick(readAfterListenerThrew, stream); + } + } +} + +function readAfterListenerThrew(stream: NativeReadable) { + if (!stream.destroyed) stream.read(0); +} + function handleNumberResult(stream: NativeReadable, result: number, chunk: any, isClosed: boolean) { if (result > 0) { const slice = chunk.subarray(0, result); chunk = slice.byteLength < chunk.byteLength ? chunk.subarray(result) : undefined; if (slice.byteLength > 0) { - pushAndCheck(stream, slice); + deliverChunk(stream, slice); } } if (isClosed) { - process.nextTick(pushEof, stream); + deliverEof(stream); } return chunk; @@ -246,11 +378,11 @@ function handleNumberResult(stream: NativeReadable, result: number, chunk: any, function handleArrayBufferViewResult(stream: NativeReadable, result: any, chunk: any, isClosed: boolean) { if (result.byteLength > 0) { - pushAndCheck(stream, result); + deliverChunk(stream, result); } if (isClosed) { - process.nextTick(pushEof, stream); + deliverEof(stream); } return chunk; @@ -261,11 +393,18 @@ function adjustHighWaterMark(stream: NativeReadable) { stream[kHasResized] = true; } -function destroy(this: NativeReadable, error: any, cb: () => void) { +function destroy(this: NativeReadable, error: any, cb: (error?: any) => void) { const ptr = this.$bunNativePtr; if (ptr) { ptr.cancel(error); } + if (this[kReadEvents] !== undefined) { + // As net.Socket: `_destroy` calls back at once, so 'error' comes in the next tick, and 'close' + // comes from the close callback of the handle, a libuv callback of its own. + if (cb) cb(error); + runAfterTickDrain(emitClose, this); + return; + } dropReadAhead(this); if (cb) { // `_destroy` reports its error through the callback. @@ -273,7 +412,11 @@ function destroy(this: NativeReadable, error: any, cb: () => void) { } } -// `_read()` pushes synchronously, so flow() stays one chunk ahead of the 'data' listener. Node's async sources do not. +function emitClose(stream: NativeReadable) { + stream.emit("close"); +} + +// `_read()` of a "webstream" source pushes synchronously, so flow() stays one chunk ahead of the 'data' listener. Node's async sources do not. function dropReadAhead(stream: NativeReadable) { const state = stream._readableState; // Paused: Node has this buffered too, and a later read() returns it. diff --git a/src/js/internal/webstreams_adapters.ts b/src/js/internal/webstreams_adapters.ts index a59bea657e37..29c62c98b58c 100644 --- a/src/js/internal/webstreams_adapters.ts +++ b/src/js/internal/webstreams_adapters.ts @@ -38,7 +38,7 @@ function tryTransferToNativeReadable(stream, options) { if (!ptr || ptr === -1) { return undefined; } - return require("internal/streams/native-readable").constructNativeReadable(stream, options); + return require("internal/streams/native-readable").constructNativeReadable(stream, options, "webstream"); } class ReadableFromWeb extends Readable { diff --git a/src/js/node/child_process.ts b/src/js/node/child_process.ts index 68c9f580793e..475ee5af8fae 100644 --- a/src/js/node/child_process.ts +++ b/src/js/node/child_process.ts @@ -1297,7 +1297,7 @@ class ChildProcess extends EventEmitter { return stream; } - const pipe = require("internal/streams/native-readable").constructNativeReadable(value, {}); + const pipe = require("internal/streams/native-readable").constructNativeReadable(value, {}, "libuv-handle"); this.#closesNeeded++; pipe.once("close", () => this.#maybeClose()); if (autoResume) pipe.resume(); diff --git a/test/js/bun/spawn/spawn-stdio-syscall-error.test.ts b/test/js/bun/spawn/spawn-stdio-syscall-error.test.ts index a6f323796272..d552769950f0 100644 --- a/test/js/bun/spawn/spawn-stdio-syscall-error.test.ts +++ b/test/js/bun/spawn/spawn-stdio-syscall-error.test.ts @@ -236,6 +236,32 @@ child.on("close", () => { }); `; +// node:child_process: the first read gets bytes and then fails. Both come from one native read, and the +// bytes reach 'data' before 'error'. The child says when it has written, and the parent blocks until then. +const CHILD_PROCESS_FIRST_READ_FIXTURE = /* js */ ` +import { spawn } from "node:child_process"; +import { existsSync } from "node:fs"; +const marker = process.env.SPAWN_FAULT_MARKER; +const events = []; +const child = spawn("sh", ["-c", 'printf AAAABB; : > "$SPAWN_FAULT_MARKER"; exec sleep 60'], { stdio: ["ignore", "pipe", "ignore"] }); +child.stdout.on("data", chunk => events.push("stdout.data:" + chunk)); +child.stdout.on("error", e => events.push("stdout.error:" + e.code)); +child.stdout.on("close", () => { + events.push("stdout.close"); + child.kill(); +}); +child.on("close", () => { + events.push("close"); + console.log(JSON.stringify({ events })); +}); +const sleeper = new Int32Array(new SharedArrayBuffer(4)); +const deadline = Date.now() + 60_000; +while (!existsSync(marker)) { + if (Date.now() > deadline) throw new Error("the child did not write"); + Atomics.wait(sleeper, 0, 0, 5); +} +`; + // Bun.spawnSync / child_process.spawnSync / execFileSync: the lost output is an error, not a success. const SPAWN_SYNC_FIXTURE = /* js */ ` import { spawnSync, execFileSync } from "node:child_process"; @@ -274,6 +300,7 @@ beforeAll(async () => { "stdout-write.mjs": STDOUT_WRITE_FIXTURE, "child-process.mjs": CHILD_PROCESS_FIXTURE, "child-process-bytes.mjs": CHILD_PROCESS_BYTES_FIXTURE, + "child-process-first-read.mjs": CHILD_PROCESS_FIRST_READ_FIXTURE, "spawn-sync.mjs": SPAWN_SYNC_FIXTURE, }); shimPath = join(String(dir), "shim.so"); @@ -382,8 +409,7 @@ describe.skipIf(!isLinux || !cc)("subprocess stdio syscall errors", () => { }); }); - // child.stdout reads one chunk ahead of its 'data' listener, so once the stream flows a chunk is still buffered - // when a later read fails. The stream is destroyed with the error, and that chunk has to reach 'data' first. + // The stream is destroyed with the error of a later read. Every chunk that was read before it has to reach 'data' first. test.concurrent("node:child_process: stdout delivers every byte read before the error", async () => { const report = join(String(dir), "recv-report.txt"); expect( @@ -395,6 +421,19 @@ describe.skipIf(!isLinux || !cc)("subprocess stdio syscall errors", () => { }); }); + test.concurrent("node:child_process: stdout delivers the bytes of the read that fails before the error", async () => { + expect( + await runWithFault("child-process-first-read.mjs", { + SPAWN_FAULT_RECV_AT: "2", + SPAWN_FAULT_MARKER: join(String(dir), "first-read-written"), + }), + ).toEqual({ + parsed: { events: ["stdout.data:AAAABB", "stdout.error:EIO", "stdout.close", "close"] }, + stderr: "", + exitCode: 0, + }); + }); + describe.each([ ["the first write", "1"], ["a later write", "3"], diff --git a/test/js/node/child_process/child-process-stdio.test.js b/test/js/node/child_process/child-process-stdio.test.js index 78b6454f9243..d11640cda959 100644 --- a/test/js/node/child_process/child-process-stdio.test.js +++ b/test/js/node/child_process/child-process-stdio.test.js @@ -1,5 +1,5 @@ import { describe, expect, it } from "bun:test"; -import { bunEnv, bunExe } from "harness"; +import { bunEnv, bunExe, isWindows, tempDir } from "harness"; import { execSync, spawn } from "node:child_process"; import { once } from "node:events"; @@ -166,3 +166,90 @@ describe("child.stdin", () => { }); }); }); + +// Node reads a child's stdio from a libuv handle. It reports each chunk, the EOF and the close from +// a libuv callback of its own, and it runs every nextTick and promise job between two callbacks. +// So the nextTicks and the promise jobs that the listeners of an event queue, and all that those +// queue, run before the next event. The fixture prints the same lines under Node. +describe.concurrent("child.stdout and child.stderr event order", () => { + const fixture = import.meta.dir + "/fixtures/child-process-stdio-event-order.js"; + const chain = label => `tick(${label}) job(${label}) job2(${label}) job3(${label})`; + const end = `end ${chain("end")} close`; + const expected = { + data: `data(AAAABB) ${chain("AAAABB")} ${end}`, + readable: `read(AAAABB) read(null) ${chain("AAAABB")} read(null) ${end}`, + // The listener destroys the stream from a promise job: no 'end' follows, and the rest of that job comes before 'close'. + destroy: "data(AAAABB) destroy job(destroy) tick(destroy) job2(destroy) job3(destroy) close", + // The throw is an uncaught exception, and the stream reads on. + throw: `data(AAAABB) uncaughtException(uncaughtException) ${chain("AAAABB")} ${end}`, + // Nobody listens: a read() call gets the bytes at once. + "read-on-exit": `read(AAAABB) ${end}`, + both: `data(OUT) ${chain("OUT")} data(ERR) ${chain("ERR")}`, + "two-chunks": `data(AAAA) ${chain("AAAA")} data(BBBB) ${chain("BBBB")} ${end}`, + }; + + it.each([ + // first-read: the bytes and the EOF are in the pipe before the parent reads for the first time. + ["stdout", "first-read", "data"], + ["stdout", "first-read", "readable"], + ["stdout", "first-read", "destroy"], + ["stderr", "first-read", "data"], + ["both", "first-read", "data"], + // waiting-read: the parent's read already waits when the child writes. + ["stdout", "waiting-read", "data"], + ["stdout", "waiting-read", "readable"], + ["stdout", "waiting-read", "destroy"], + ["stdout", "waiting-read", "throw"], + // two-chunks: the second chunk and the EOF are in the pipe while the first 'data' listener runs. + ["stdout", "two-chunks", "data"], + // Not on Windows. There Node runs the promise jobs of a listener that threw after 'end', when + // the EOF is already in the pipe. And Bun reads a pipe asynchronously there, so a read() call + // in 'exit' finds no bytes yet. + ...(isWindows + ? [] + : [ + ["stdout", "first-read", "throw"], + ["stdout", "first-read", "read-on-exit"], + ]), + ])("%s, %s, %s", async (which, mode, consumer) => { + using dir = tempDir("child-stdio-event-order", {}); + await using proc = Bun.spawn({ + cmd: [bunExe(), fixture, which, mode, consumer, String(dir)], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect({ stdout: stdout.trim(), stderr, exitCode }).toEqual({ + stdout: expected[which === "both" ? which : mode === "two-chunks" ? mode : consumer], + stderr: "", + exitCode: 0, + }); + }); + + // The events above wait for the tick loop, not for process.nextTick(), which fake timers replace + // (they read it first). The loop that flushes the held ticks stands in for the fake clock. + it.each([ + ["before its first read", "process.nextTick = held;"], + ["after a read", "const real = process.nextTick; process.nextTick = held;"], + ])("child.stdout delivers when process.nextTick is replaced %s", async (_, replace) => { + const script = ` + const ticks = []; + const held = (fn, ...args) => void ticks.push(() => fn(...args)); + ${replace} + const flush = setInterval(() => { while (ticks.length) ticks.shift()(); }, 1); + const { spawn } = require("node:child_process"); + const events = []; + const child = spawn(process.execPath, ["-e", "process.stdout.write('hi')"], { stdio: ["ignore", "pipe", "ignore"] }); + child.stdout.on("data", chunk => events.push("data(" + chunk + ")")); + child.stdout.on("end", () => events.push("end")); + child.on("close", () => { + clearInterval(flush); + console.log(events.join(" ")); + }); + `; + await using proc = Bun.spawn({ cmd: [bunExe(), "-e", script], env: bunEnv, stdout: "pipe", stderr: "pipe" }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect({ stdout: stdout.trim(), stderr, exitCode }).toEqual({ stdout: "data(hi) end", stderr: "", exitCode: 0 }); + }); +}); diff --git a/test/js/node/child_process/child_process.test.ts b/test/js/node/child_process/child_process.test.ts index 212a12c9ba2f..a775b9b765d2 100644 --- a/test/js/node/child_process/child_process.test.ts +++ b/test/js/node/child_process/child_process.test.ts @@ -1406,12 +1406,13 @@ it("child.stdout.pause() after flowing stops native reads and blocks the child", } }); -// child.stdout and child.stderr read ahead: `_read()` pushes each native pull -// result synchronously, so while the stream flows Readable holds the next chunk -// in its buffer when a 'data' listener runs. destroy() left that chunk there and -// flow() emitted it after destroy() returned, with `destroyed === true`. Node's +// child.stdout and child.stderr used to read ahead: `_read()` pushed each native +// pull result synchronously, so while the stream flowed Readable held the next +// chunk in its buffer when a 'data' listener ran. destroy() left that chunk there +// and flow() emitted it after destroy() returned, with `destroyed === true`. Node's // child stdio is a net.Socket that pushes asynchronously, so no 'data' follows -// destroy() there. +// destroy() there. Now each read result waits for the listeners of the one +// before, and destroy() drops what waits. it.concurrent.each(["stdout", "stderr"] as const)( "child.%s.destroy() inside a 'data' listener stops 'data' and 'end'", async name => { diff --git a/test/js/node/child_process/fixtures/child-process-stdio-event-order.js b/test/js/node/child_process/fixtures/child-process-stdio-event-order.js new file mode 100644 index 000000000000..a9148958da04 --- /dev/null +++ b/test/js/node/child_process/fixtures/child-process-stdio-event-order.js @@ -0,0 +1,129 @@ +// Prints the order of a child's stdio stream events, together with the nextTicks and the promise +// jobs that the stream's listeners queue. Node prints the same line. +// +// usage: child-process-stdio-event-order.js +// +// mode: +// first-read the bytes and the EOF are both in the pipe before the parent reads for the first time +// waiting-read the parent's read already waits when the child writes +// two-chunks the second chunk and the EOF are in the pipe while the 'data' listener of the first chunk runs +// consumer: +// data a 'data' listener +// readable a 'readable' listener that calls read() +// destroy a 'data' listener that destroys the stream from a promise job +// throw a 'data' listener that throws +// read-on-exit no listener, and a read() call when the child exits +// "both" reads stdout and stderr, and prints only their 'data' events. +const { spawn } = require("node:child_process"); +const fs = require("node:fs"); +const path = require("node:path"); + +const [which, mode, consumer, dir] = process.argv.slice(2); +const fds = which === "both" ? [1, 2] : which === "stdout" ? [1] : [2]; +// The parent creates `go`. The child creates `written` after it wrote everything and closed the fds. +const go = path.join(dir, "go"); +const written = path.join(dir, "written"); + +const sleeper = new Int32Array(new SharedArrayBuffer(4)); +// Blocks the event loop, so that nothing is read or delivered until `file` exists. +function blockUntilExists(file) { + const deadline = Date.now() + 60_000; + while (!fs.existsSync(file)) { + // The test is over and has removed its directory. + if (!fs.existsSync(dir)) process.exit(1); + if (Date.now() > deadline) throw new Error(`${file} was not created`); + Atomics.wait(sleeper, 0, 0, 5); + } +} + +// The child closes the fd itself, so the EOF is in the pipe before `written` exists. (On Windows +// closeSync() of fd 1 or 2 does nothing, and the EOF follows when the child exits.) +const childSource = ` + const fs = require("node:fs"); + const sleeper = new Int32Array(new SharedArrayBuffer(4)); + const dir = ${JSON.stringify(dir)}; + ${blockUntilExists} + const mode = ${JSON.stringify(mode)}; + const fds = ${JSON.stringify(fds)}; + if (mode === "two-chunks") fs.writeSync(fds[0], "AAAA"); + if (mode !== "first-read") blockUntilExists(${JSON.stringify(go)}); + for (const fd of fds) fs.writeSync(fd, fds.length === 2 ? (fd === 1 ? "OUT" : "ERR") : mode === "two-chunks" ? "BBBB" : "AAAABB"); + for (const fd of fds) fs.closeSync(fd); + fs.writeFileSync(${JSON.stringify(written)}, ""); +`; + +const stdio = ["ignore", "ignore", "ignore"]; +for (const fd of fds) stdio[fd] = "pipe"; +const child = spawn(process.execPath, ["-e", childSource], { stdio }); + +const events = []; +// The chain goes from the promise job queue to the nextTick queue and back, twice. +async function queueTickAndJobs(label) { + process.nextTick(() => events.push(`tick(${label})`)); + await null; + events.push(`job(${label})`); + await new Promise(resolve => process.nextTick(resolve)); + events.push(`job2(${label})`); + await new Promise(resolve => process.nextTick(resolve)); + events.push(`job3(${label})`); +} + +for (const fd of fds) { + const stream = child.stdio[fd]; + if (consumer === "data") { + stream.on("data", chunk => { + events.push(`data(${chunk})`); + queueTickAndJobs(chunk); + if (mode === "two-chunks" && !fs.existsSync(go)) { + fs.writeFileSync(go, ""); + blockUntilExists(written); + } + }); + } else if (consumer === "readable") { + stream.on("readable", () => { + let chunk; + while ((chunk = stream.read()) !== null) { + events.push(`read(${chunk})`); + queueTickAndJobs(chunk); + } + events.push("read(null)"); + }); + } else if (consumer === "destroy") { + stream.on("data", async chunk => { + events.push(`data(${chunk})`); + await null; + stream.destroy(); + events.push("destroy"); + queueTickAndJobs("destroy"); + }); + } else if (consumer === "throw") { + // Node calls the listener from a libuv callback, so the throw is an uncaught exception, and the handle reads on. + process.on("uncaughtException", (error, origin) => events.push(`uncaughtException(${origin})`)); + process.on("unhandledRejection", () => events.push("unhandledRejection")); + stream.on("data", chunk => { + events.push(`data(${chunk})`); + queueTickAndJobs(chunk); + throw new Error("the listener threw"); + }); + } else { + // Node has read the pipe from the start, so the bytes are in the stream's buffer by now. + child.on("exit", () => events.push(`read(${stream.read()})`)); + } + if (which === "both") continue; + stream.on("end", () => { + events.push("end"); + queueTickAndJobs("end"); + }); + stream.on("close", () => events.push("close")); +} +child.on("close", () => { + // A job that runs after 'close' still gets into the line. + setImmediate(() => console.log(events.join(" "))); +}); + +if (mode === "first-read") { + blockUntilExists(written); +} else if (mode === "waiting-read") { + // The stream started to read in a nextTick, so the read waits by now. + setImmediate(() => fs.writeFileSync(go, "")); +} diff --git a/test/js/node/stream/node-stream.test.js b/test/js/node/stream/node-stream.test.js index 782ed9039c62..f0ad3e2cd76a 100644 --- a/test/js/node/stream/node-stream.test.js +++ b/test/js/node/stream/node-stream.test.js @@ -1,6 +1,7 @@ import { exposedInternals } from "bun:internal-for-testing"; import { describe, expect, it, jest } from "bun:test"; import { bunEnv, bunExe, bunRun, isGlibcVersionAtLeast, isMacOS, tempDir, tmpdirSync } from "harness"; +import { once } from "node:events"; import { createReadStream, mkdirSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { Duplex, duplexPair, finished, PassThrough, Readable, Stream, Transform, Writable } from "node:stream"; @@ -388,6 +389,56 @@ it("Readable.fromWeb", async () => { expect(Buffer.concat(chunks).toString()).toBe("Hello World!\n"); }); +// These web streams have a native source, which gives the last bytes and the EOF in one read. +// A nextTick of the listener is not in the order: Node pushes every fromWeb chunk from a promise +// job, so its promise jobs run first. Bun pushes a synchronous read from the nextTick that reads. +describe("Readable.fromWeb of a native stream", () => { + const sources = [ + ["Blob", () => new Blob(["AAAABB"]).stream()], + ["Response body", () => new Response("AAAABB").body], + ["Bun.file", dir => Bun.file(join(dir, "file.txt")).stream()], + ]; + + it.each(sources)("%s: 'end' comes after the promise jobs of the last 'data' listeners", async (_, makeWebStream) => { + using dir = tempDir("fromweb-end-order", { "file.txt": "AAAABB" }); + const events = []; + const readable = Readable.fromWeb(makeWebStream(String(dir))); + readable.on("data", async chunk => { + events.push(`data(${chunk})`); + await null; + events.push("job"); + await null; + events.push("job2"); + }); + readable.on("end", () => events.push("end")); + await once(readable, "close"); + expect(events).toEqual(["data(AAAABB)", "job", "job2", "end"]); + }); + + it.each(sources)( + "%s: no 'end' after a promise job of the last 'data' listener destroys it", + async (_, makeWebStream) => { + using dir = tempDir("fromweb-end-order", { "file.txt": "AAAABB" }); + const events = []; + const readable = Readable.fromWeb(makeWebStream(String(dir))); + readable.on("data", async chunk => { + events.push(`data(${chunk})`); + await null; + readable.destroy(); + events.push("destroy"); + }); + readable.on("end", () => events.push("end")); + const { promise, resolve } = Promise.withResolvers(); + finished(readable, resolve); + const [error] = await Promise.all([promise, once(readable, "close")]); + expect({ events, code: error?.code }).toEqual({ + events: ["data(AAAABB)", "destroy"], + code: "ERR_STREAM_PREMATURE_CLOSE", + }); + }, + ); +}); + // fromWeb assigns stream.$bunNativePtr on the node Readable. When user code grafts // ReadableStream.prototype into the node stream prototype chain, that put used to // reach ReadableStream's private custom setter with the Readable as the receiver, From 66871ac28900a2de0e0e7e51193688484deafaaa Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Tue, 22 Sep 2026 13:44:45 +0000 Subject: [PATCH 2/7] child_process: queue a stdio stream's 'close' before the destroy callback 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. --- src/js/internal/streams/native-readable.ts | 7 ++++--- .../js/node/child_process/child-process-stdio.test.js | 3 +++ .../fixtures/child-process-stdio-event-order.js | 11 +++++++++++ 3 files changed, 18 insertions(+), 3 deletions(-) diff --git a/src/js/internal/streams/native-readable.ts b/src/js/internal/streams/native-readable.ts index 1b7e084c1ca6..5f865daf9602 100644 --- a/src/js/internal/streams/native-readable.ts +++ b/src/js/internal/streams/native-readable.ts @@ -399,10 +399,11 @@ function destroy(this: NativeReadable, error: any, cb: (error?: any) => void) { ptr.cancel(error); } if (this[kReadEvents] !== undefined) { - // As net.Socket: `_destroy` calls back at once, so 'error' comes in the next tick, and 'close' - // comes from the close callback of the handle, a libuv callback of its own. - if (cb) cb(error); + // As net.Socket: 'close' comes from the close callback of the handle, a libuv callback of its + // own, and `_destroy` calls back at once, so 'error' comes in the next tick. 'close' is queued + // first: the callback runs the one that destroy() got, and that one can throw. runAfterTickDrain(emitClose, this); + if (cb) cb(error); return; } dropReadAhead(this); diff --git a/test/js/node/child_process/child-process-stdio.test.js b/test/js/node/child_process/child-process-stdio.test.js index d11640cda959..65300edf0c6b 100644 --- a/test/js/node/child_process/child-process-stdio.test.js +++ b/test/js/node/child_process/child-process-stdio.test.js @@ -180,6 +180,8 @@ describe.concurrent("child.stdout and child.stderr event order", () => { readable: `read(AAAABB) read(null) ${chain("AAAABB")} read(null) ${end}`, // The listener destroys the stream from a promise job: no 'end' follows, and the rest of that job comes before 'close'. destroy: "data(AAAABB) destroy job(destroy) tick(destroy) job2(destroy) job3(destroy) close", + // destroy() swallows the throw of its callback, and 'close' still comes. + "destroy-callback-throws": "data(AAAABB) destroy-callback close", // The throw is an uncaught exception, and the stream reads on. throw: `data(AAAABB) uncaughtException(uncaughtException) ${chain("AAAABB")} ${end}`, // Nobody listens: a read() call gets the bytes at once. @@ -200,6 +202,7 @@ describe.concurrent("child.stdout and child.stderr event order", () => { ["stdout", "waiting-read", "readable"], ["stdout", "waiting-read", "destroy"], ["stdout", "waiting-read", "throw"], + ["stdout", "waiting-read", "destroy-callback-throws"], // two-chunks: the second chunk and the EOF are in the pipe while the first 'data' listener runs. ["stdout", "two-chunks", "data"], // Not on Windows. There Node runs the promise jobs of a listener that threw after 'end', when diff --git a/test/js/node/child_process/fixtures/child-process-stdio-event-order.js b/test/js/node/child_process/fixtures/child-process-stdio-event-order.js index a9148958da04..97562d3ad546 100644 --- a/test/js/node/child_process/fixtures/child-process-stdio-event-order.js +++ b/test/js/node/child_process/fixtures/child-process-stdio-event-order.js @@ -12,6 +12,7 @@ // readable a 'readable' listener that calls read() // destroy a 'data' listener that destroys the stream from a promise job // throw a 'data' listener that throws +// destroy-callback-throws a 'data' listener that calls destroy() with a callback that throws // read-on-exit no listener, and a read() call when the child exits // "both" reads stdout and stderr, and prints only their 'data' events. const { spawn } = require("node:child_process"); @@ -96,6 +97,16 @@ for (const fd of fds) { events.push("destroy"); queueTickAndJobs("destroy"); }); + } else if (consumer === "destroy-callback-throws") { + // destroy() swallows what its callback throws, and 'close' still comes. + process.on("uncaughtException", () => events.push("uncaughtException")); + stream.on("data", chunk => { + events.push(`data(${chunk})`); + stream.destroy(undefined, () => { + events.push("destroy-callback"); + throw new Error("the callback threw"); + }); + }); } else if (consumer === "throw") { // Node calls the listener from a libuv callback, so the throw is an uncaught exception, and the handle reads on. process.on("uncaughtException", (error, origin) => events.push(`uncaughtException(${origin})`)); From 7b9dfc13ee8960aeb00cb1effd748eefb6683f54 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Tue, 22 Sep 2026 13:51:16 +0000 Subject: [PATCH 3/7] Shorten the comments of the after-tick-drain change to one line each --- src/js/builtins/ProcessObjectInternals.ts | 3 +- src/js/internal/async_hooks_tick.ts | 6 +--- src/js/internal/process/after_tick_drain.ts | 25 ++-------------- src/js/internal/streams/native-readable.ts | 33 ++++----------------- 4 files changed, 11 insertions(+), 56 deletions(-) diff --git a/src/js/builtins/ProcessObjectInternals.ts b/src/js/builtins/ProcessObjectInternals.ts index 0ce8aa15fdc0..9fe47b6d8f65 100644 --- a/src/js/builtins/ProcessObjectInternals.ts +++ b/src/js/builtins/ProcessObjectInternals.ts @@ -404,8 +404,7 @@ export function initializeNextTickQueue(process: typeof globalThis.process, next setup = undefined; }; - // For internal/process/after_tick_drain.ts: makes the next drain call the tick loop, without a call of - // process.nextTick, which fake timers replace. JSNextTickQueue::drain only calls in while field 0 is set. + // For internal/process/after_tick_drain.ts. JSNextTickQueue::drain only calls the tick loop while field 0 is set. asyncHooksTick.ensureTickLoop = () => { if (setup) { setup(); diff --git a/src/js/internal/async_hooks_tick.ts b/src/js/internal/async_hooks_tick.ts index 55def82f8d40..e597368dce07 100644 --- a/src/js/internal/async_hooks_tick.ts +++ b/src/js/internal/async_hooks_tick.ts @@ -9,10 +9,6 @@ // Currently only TickObject `init` events are delivered (enough for // console.log/stream.write tick-coalescing tests); promise, timer and native // resource events are still unimplemented. -// -// `ensureTickLoop` and `runAfterTickDrainCallback` connect the tick loop and -// internal/process/after_tick_drain.ts. The tick loop does not load that -// module, so a process that never uses it pays one property read for each drain. const tickInitHooks: Array<(asyncId: number, type: string, triggerAsyncId: number, resource: object) => void> = []; let nextAsyncId = 1; @@ -21,7 +17,7 @@ export default { newAsyncId() { return ++nextAsyncId; }, - // Set when process.nextTick is first read, which creates the tick queue. + // For internal/process/after_tick_drain.ts. Set when process.nextTick is first read, which creates the tick queue. ensureTickLoop: undefined as (() => void) | undefined, // Set by internal/process/after_tick_drain.ts. The tick loop calls it each time both queues are empty. runAfterTickDrainCallback: undefined as (() => boolean) | undefined, diff --git a/src/js/internal/process/after_tick_drain.ts b/src/js/internal/process/after_tick_drain.ts index cb6d94534b06..648435a99bcc 100644 --- a/src/js/internal/process/after_tick_drain.ts +++ b/src/js/internal/process/after_tick_drain.ts @@ -1,23 +1,4 @@ -// Node calls into JS once for each libuv event, and after each call it runs -// every process.nextTick callback and every promise job, until both queues -// are empty. So the next event of a handle (the next chunk, the EOF) never -// runs ahead of a tick or a promise job that a listener of the previous event -// queued, however long the chain of ticks and jobs is. -// https://github.com/nodejs/node/blob/v24.9.0/lib/internal/process/task_queues.js#L72-L109 -// -// A module that gets several of Node's events out of one native call gives -// each of them a drain of its own with `runAfterTickDrain`. The tick loop -// (processTicksAndRejections in builtins/ProcessObjectInternals.ts) runs one -// of these callbacks each time both queues are empty, where Node's loop calls -// processPromiseRejections(). That happens inside the drain that is running, -// so no other I/O callback runs in between. -// -// Only nextTicks and promise jobs run between two callbacks. Bun reports -// unhandled rejections after the whole drain, and Node reads a pipe once for -// each turn of the event loop, so there a setImmediate() of a listener also -// runs before the next event. A setImmediate() here would wait for that too, -// but fake timers replace it, and it costs a turn of the event loop for each -// chunk. +// Node empties the nextTick queue and the promise job queue between two libuv callbacks: https://github.com/nodejs/node/blob/v24.9.0/lib/internal/process/task_queues.js#L72-L109 const asyncHooksTick = require("internal/async_hooks_tick"); type AfterTickDrainCallback = { @@ -30,6 +11,7 @@ type AfterTickDrainCallback = { const entries: (AfterTickDrainCallback | undefined)[] = []; let head = 0; +// Runs `callback` once both queues are empty, inside the drain that is running. One callback for each such point. function runAfterTickDrain(callback: (arg: any) => void, arg?: unknown) { let ensureTickLoop = asyncHooksTick.ensureTickLoop; if (ensureTickLoop === undefined) { @@ -54,8 +36,7 @@ function runAfterTickDrainCallback() { } const restore = $getInternalField($asyncContext, 0); $putInternalField($asyncContext, 0, frame); - // No catch and no finally, as for a tick: JSNextTickQueue::drain reports a throw inside the - // callback's async context, puts the context back, and calls the tick loop again. + // No catch and no finally, as for a tick: JSNextTickQueue::drain reports the throw and calls the tick loop again. callback(arg); $putInternalField($asyncContext, 0, restore); return true; diff --git a/src/js/internal/streams/native-readable.ts b/src/js/internal/streams/native-readable.ts index 5f865daf9602..ff327d137072 100644 --- a/src/js/internal/streams/native-readable.ts +++ b/src/js/internal/streams/native-readable.ts @@ -60,8 +60,7 @@ interface NativeReadable extends NodeReadable { [kHasResized]: boolean; [kRemainingChunk]: Buffer | undefined; [kSawEof]: boolean; - // "libuv-handle" only: pairs of (kind, value) that wait for a drain of their own, from `kReadEventsHead` on. - // At most the results of one `_read`: the next pull waits until they are delivered. + // "libuv-handle" only: (kind, value) pairs from `kReadEventsHead` on. At most the results of one `_read`. [kReadEvents]: unknown[] | undefined; [kReadEventsHead]: number; [kReadEventQueued]: boolean; @@ -81,22 +80,7 @@ interface NativePtr { let debugId = 0; -// `nodeSource` names what feeds the equivalent stream in Node. -// -// "libuv-handle": child_process stdio, a net.Socket over a pipe in Node. Node -// reports each chunk, the EOF, a read error and the close from a libuv -// callback of its own, and runs every nextTick and promise job between two of -// them (internal/process/after_tick_drain.ts). One native read here can give -// several of those events, and `_read` runs inside a nextTick or a promise -// job. So each event waits for `runAfterTickDrain`, unless nobody listens. -// https://github.com/nodejs/node/blob/v24.9.0/lib/internal/stream_base_commons.js#L166-L233 -// -// "webstream": Readable.fromWeb. Node pushes each chunk from the promise job -// of a `reader.read()`, and gets the EOF from a later read. Here a chunk is -// pushed as it arrives: inside `_read` when the read is synchronous (so a -// 'data' listener's nextTicks run before its promise jobs, unlike Node), -// inside the promise job otherwise. Only the EOF waits for `runAfterTickDrain`. -// https://github.com/nodejs/node/blob/v24.9.0/lib/internal/webstreams/adapters.js#L544-L555 +// `nodeSource`: what feeds the equivalent stream in Node. "libuv-handle" is child_process stdio, a net.Socket there. function constructNativeReadable( readableStream: ReadableStream, options, @@ -269,8 +253,7 @@ function handleResult(stream: NativeReadable, result: any, chunk: Buffer | undef } } -// The EOF waits for the nextTicks and promise jobs of the last chunk's listeners. After a destroy() -// in between, Node emits 'close' without 'end'. +// After a destroy() before the EOF gets its turn, Node emits 'close' without 'end'. function pushEof(stream: NativeReadable) { if (!stream.destroyed) stream.push(null); } @@ -287,8 +270,7 @@ function pushAndCheck(stream: NativeReadable, chunk: any) { function deliverChunk(stream: NativeReadable, chunk: any) { const events = stream[kReadEvents]; - // Without a 'data' or 'readable' listener nobody sees an order, and a read() call must get the - // bytes now: Node reads a child's pipe from the start, so there they are in the buffer already. + // Nobody listens, so nobody sees an order, and a read() call gets the bytes now: Node has them buffered already. if ( events === undefined || (events.length === 0 && stream.listenerCount("data") === 0 && stream.listenerCount("readable") === 0) @@ -299,8 +281,7 @@ function deliverChunk(stream: NativeReadable, chunk: any) { } } -// The EOF can come in the same read as the last chunk. Node gets it in a read -// of its own, after the nextTicks and promise jobs of that chunk's listeners. +// Node gets the EOF in a read of its own, after the nextTicks and promise jobs of the last chunk's listeners. function deliverEof(stream: NativeReadable) { stream[kSawEof] = true; const events = stream[kReadEvents]; @@ -399,9 +380,7 @@ function destroy(this: NativeReadable, error: any, cb: (error?: any) => void) { ptr.cancel(error); } if (this[kReadEvents] !== undefined) { - // As net.Socket: 'close' comes from the close callback of the handle, a libuv callback of its - // own, and `_destroy` calls back at once, so 'error' comes in the next tick. 'close' is queued - // first: the callback runs the one that destroy() got, and that one can throw. + // As net.Socket: 'close' comes from the handle's close callback. It is queued before cb(), which can throw. runAfterTickDrain(emitClose, this); if (cb) cb(error); return; From 82080bfcc3142cccae0639fd260174f53353dbe7 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Tue, 22 Sep 2026 14:24:45 +0000 Subject: [PATCH 4/7] Use the shared FIFO for the after-tick-drain queue 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. --- src/js/internal/process/after_tick_drain.ts | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/src/js/internal/process/after_tick_drain.ts b/src/js/internal/process/after_tick_drain.ts index 648435a99bcc..a9208aae2abe 100644 --- a/src/js/internal/process/after_tick_drain.ts +++ b/src/js/internal/process/after_tick_drain.ts @@ -1,5 +1,6 @@ // Node empties the nextTick queue and the promise job queue between two libuv callbacks: https://github.com/nodejs/node/blob/v24.9.0/lib/internal/process/task_queues.js#L72-L109 const asyncHooksTick = require("internal/async_hooks_tick"); +const Dequeue = require("internal/fifo"); type AfterTickDrainCallback = { callback: (arg: any) => void; @@ -7,9 +8,7 @@ type AfterTickDrainCallback = { frame: import("../../node/async_hooks").Frame | undefined; }; -// A queue: entries before `head` are done. Bounded by the number of callers, which each wait for their turn. -const entries: (AfterTickDrainCallback | undefined)[] = []; -let head = 0; +const entries = new Dequeue(); // Runs `callback` once both queues are empty, inside the drain that is running. One callback for each such point. function runAfterTickDrain(callback: (arg: any) => void, arg?: unknown) { @@ -22,18 +21,14 @@ function runAfterTickDrain(callback: (arg: any) => void, arg?: unknown) { if (ensureTickLoop === undefined) return process.nextTick(callback, arg); } ensureTickLoop(); - $arrayPush(entries, { callback, arg, frame: $getInternalField($asyncContext, 0) }); + entries.push({ callback, arg, frame: $getInternalField($asyncContext, 0) }); } // The tick loop calls this each time both queues are empty. False: nothing waited. function runAfterTickDrainCallback() { - if (head === entries.length) return false; - const { callback, arg, frame } = entries[head]!; - entries[head++] = undefined; - if (head === entries.length) { - entries.length = 0; - head = 0; - } + const entry = entries.shift(); + if (entry === undefined) return false; + const { callback, arg, frame } = entry; const restore = $getInternalField($asyncContext, 0); $putInternalField($asyncContext, 0, frame); // No catch and no finally, as for a tick: JSNextTickQueue::drain reports the throw and calls the tick loop again. From 451e832a66014e345f3f8cee0812002c05af9277 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 16 Oct 2026 08:19:58 +0000 Subject: [PATCH 5/7] child_process: stdio 'close' carries hadError, and tests for a consumer that listens after its await A consumer that awaits the end of child.stdout (for await, or once(stdout, "end")) and then listens for 'close' gets it, and once(child, "close") after that settles. The fixture has the rows: for-await, await-end, and held-open, where a grandchild holds the pipe until the parent has seen the child's 'exit'. The 'close' of a child's stdout and stderr now has net.Socket's hadError argument. --- src/js/internal/streams/native-readable.ts | 9 +++- .../spawn/spawn-stdio-syscall-error.test.ts | 6 +-- .../child_process/child-process-stdio.test.js | 19 ++++++-- .../child-process-stdio-event-order.js | 43 ++++++++++++++++--- 4 files changed, 63 insertions(+), 14 deletions(-) diff --git a/src/js/internal/streams/native-readable.ts b/src/js/internal/streams/native-readable.ts index ff327d137072..1831b17872d3 100644 --- a/src/js/internal/streams/native-readable.ts +++ b/src/js/internal/streams/native-readable.ts @@ -381,7 +381,7 @@ function destroy(this: NativeReadable, error: any, cb: (error?: any) => void) { } if (this[kReadEvents] !== undefined) { // As net.Socket: 'close' comes from the handle's close callback. It is queued before cb(), which can throw. - runAfterTickDrain(emitClose, this); + runAfterTickDrain(error ? emitCloseAfterError : emitClose, this); if (cb) cb(error); return; } @@ -392,8 +392,13 @@ function destroy(this: NativeReadable, error: any, cb: (error?: any) => void) { } } +// The argument is net.Socket's `hadError`: https://github.com/nodejs/node/blob/v26.3.0/lib/net.js#L346-L358 function emitClose(stream: NativeReadable) { - stream.emit("close"); + stream.emit("close", false); +} + +function emitCloseAfterError(stream: NativeReadable) { + stream.emit("close", true); } // `_read()` of a "webstream" source pushes synchronously, so flow() stays one chunk ahead of the 'data' listener. Node's async sources do not. diff --git a/test/js/bun/spawn/spawn-stdio-syscall-error.test.ts b/test/js/bun/spawn/spawn-stdio-syscall-error.test.ts index a7e1f4f4abb1..504041c57a32 100644 --- a/test/js/bun/spawn/spawn-stdio-syscall-error.test.ts +++ b/test/js/bun/spawn/spawn-stdio-syscall-error.test.ts @@ -303,8 +303,8 @@ const events = []; const child = spawn("sh", ["-c", 'printf AAAABB; : > "$SPAWN_FAULT_MARKER"; exec sleep 60'], { stdio: ["ignore", "pipe", "ignore"] }); child.stdout.on("data", chunk => events.push("stdout.data:" + chunk)); child.stdout.on("error", e => events.push("stdout.error:" + e.code)); -child.stdout.on("close", () => { - events.push("stdout.close"); +child.stdout.on("close", hadError => { + events.push("stdout.close(" + hadError + ")"); child.kill(); }); child.on("close", () => { @@ -547,7 +547,7 @@ describe.skipIf(!isLinux || !cc)("subprocess stdio syscall errors", () => { SPAWN_FAULT_MARKER: join(String(dir), "first-read-written"), }), ).toEqual({ - parsed: { events: ["stdout.data:AAAABB", "stdout.error:EIO", "stdout.close", "close"] }, + parsed: { events: ["stdout.data:AAAABB", "stdout.error:EIO", "stdout.close(true)", "close"] }, stderr: "", exitCode: 0, }); diff --git a/test/js/node/child_process/child-process-stdio.test.js b/test/js/node/child_process/child-process-stdio.test.js index 3b188ebfa8ca..9696e442086a 100644 --- a/test/js/node/child_process/child-process-stdio.test.js +++ b/test/js/node/child_process/child-process-stdio.test.js @@ -228,20 +228,25 @@ describe.skipIf(!isPosix)("stdio handed to the child", () => { describe.concurrent("child.stdout and child.stderr event order", () => { const fixture = import.meta.dir + "/fixtures/child-process-stdio-event-order.js"; const chain = label => `tick(${label}) job(${label}) job2(${label}) job3(${label})`; - const end = `end ${chain("end")} close`; + const end = `end ${chain("end")} close(false)`; + // The consumer awaits the end of the stream. Then it listens for 'close' and awaits the child's 'close': both still come. + const awaited = "data(AAAABB) consumed close(false) late-close(false) child-close-settled"; const expected = { data: `data(AAAABB) ${chain("AAAABB")} ${end}`, readable: `read(AAAABB) read(null) ${chain("AAAABB")} read(null) ${end}`, // The listener destroys the stream from a promise job: no 'end' follows, and the rest of that job comes before 'close'. - destroy: "data(AAAABB) destroy job(destroy) tick(destroy) job2(destroy) job3(destroy) close", + destroy: "data(AAAABB) destroy job(destroy) tick(destroy) job2(destroy) job3(destroy) close(false)", // destroy() swallows the throw of its callback, and 'close' still comes. - "destroy-callback-throws": "data(AAAABB) destroy-callback close", + "destroy-callback-throws": "data(AAAABB) destroy-callback close(false)", // The throw is an uncaught exception, and the stream reads on. throw: `data(AAAABB) uncaughtException(uncaughtException) ${chain("AAAABB")} ${end}`, // Nobody listens: a read() call gets the bytes at once. "read-on-exit": `read(AAAABB) ${end}`, both: `data(OUT) ${chain("OUT")} data(ERR) ${chain("ERR")}`, "two-chunks": `data(AAAA) ${chain("AAAA")} data(BBBB) ${chain("BBBB")} ${end}`, + "for-await": awaited, + "await-end": awaited, + "held-open": `exit ${awaited}`, }; it.each([ @@ -249,6 +254,8 @@ describe.concurrent("child.stdout and child.stderr event order", () => { ["stdout", "first-read", "data"], ["stdout", "first-read", "readable"], ["stdout", "first-read", "destroy"], + ["stdout", "first-read", "for-await"], + ["stdout", "first-read", "await-end"], ["stderr", "first-read", "data"], ["both", "first-read", "data"], // waiting-read: the parent's read already waits when the child writes. @@ -262,11 +269,15 @@ describe.concurrent("child.stdout and child.stderr event order", () => { // Not on Windows. There Node runs the promise jobs of a listener that threw after 'end', when // the EOF is already in the pipe. And Bun reads a pipe asynchronously there, so a read() call // in 'exit' finds no bytes yet. + // held-open: a grandchild holds the pipe until the parent has seen the child's 'exit', so the 'close' + // of the pipe is also the 'close' of the child. On Windows the exit of the child ends the grandchild. ...(isWindows ? [] : [ ["stdout", "first-read", "throw"], ["stdout", "first-read", "read-on-exit"], + ["stdout", "held-open", "for-await"], + ["stdout", "held-open", "await-end"], ]), ])("%s, %s, %s", async (which, mode, consumer) => { using dir = tempDir("child-stdio-event-order", {}); @@ -278,7 +289,7 @@ describe.concurrent("child.stdout and child.stderr event order", () => { }); const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); expect({ stdout: stdout.trim(), stderr, exitCode }).toEqual({ - stdout: expected[which === "both" ? which : mode === "two-chunks" ? mode : consumer], + stdout: expected[which === "both" ? which : mode === "two-chunks" || mode === "held-open" ? mode : consumer], stderr: "", exitCode: 0, }); diff --git a/test/js/node/child_process/fixtures/child-process-stdio-event-order.js b/test/js/node/child_process/fixtures/child-process-stdio-event-order.js index 97562d3ad546..90b722161260 100644 --- a/test/js/node/child_process/fixtures/child-process-stdio-event-order.js +++ b/test/js/node/child_process/fixtures/child-process-stdio-event-order.js @@ -7,6 +7,7 @@ // first-read the bytes and the EOF are both in the pipe before the parent reads for the first time // waiting-read the parent's read already waits when the child writes // two-chunks the second chunk and the EOF are in the pipe while the 'data' listener of the first chunk runs +// held-open the child exits at once, and a grandchild holds the pipe: it writes when the parent has seen 'exit' // consumer: // data a 'data' listener // readable a 'readable' listener that calls read() @@ -14,8 +15,11 @@ // throw a 'data' listener that throws // destroy-callback-throws a 'data' listener that calls destroy() with a callback that throws // read-on-exit no listener, and a read() call when the child exits +// for-await a `for await` loop, and after it a 'close' listener and `await once(child, "close")` +// await-end a 'data' listener and `await once(stream, "end")`, and after it the same two // "both" reads stdout and stderr, and prints only their 'data' events. const { spawn } = require("node:child_process"); +const { once } = require("node:events"); const fs = require("node:fs"); const path = require("node:path"); @@ -39,7 +43,7 @@ function blockUntilExists(file) { // The child closes the fd itself, so the EOF is in the pipe before `written` exists. (On Windows // closeSync() of fd 1 or 2 does nothing, and the EOF follows when the child exits.) -const childSource = ` +const writerSource = ` const fs = require("node:fs"); const sleeper = new Int32Array(new SharedArrayBuffer(4)); const dir = ${JSON.stringify(dir)}; @@ -52,6 +56,11 @@ const childSource = ` for (const fd of fds) fs.closeSync(fd); fs.writeFileSync(${JSON.stringify(written)}, ""); `; +// held-open: the writer is a grandchild that inherits the pipe, and the child exits without a wait for it. +const childSource = + mode === "held-open" + ? `require("node:child_process").spawn(process.execPath, ["-e", ${JSON.stringify(writerSource)}], { stdio: "inherit" }).unref();` + : writerSource; const stdio = ["ignore", "ignore", "ignore"]; for (const fd of fds) stdio[fd] = "pipe"; @@ -69,6 +78,7 @@ async function queueTickAndJobs(label) { events.push(`job3(${label})`); } +const awaitsEnd = consumer === "for-await" || consumer === "await-end"; for (const fd of fds) { const stream = child.stdio[fd]; if (consumer === "data") { @@ -116,16 +126,39 @@ for (const fd of fds) { queueTickAndJobs(chunk); throw new Error("the listener threw"); }); + } else if (awaitsEnd) { + // Node emits 'close' from the close callback of the handle, after the promise jobs that 'end' started. + (async () => { + if (consumer === "for-await") { + for await (const chunk of stream) events.push(`data(${chunk})`); + } else { + stream.on("data", chunk => events.push(`data(${chunk})`)); + await once(stream, "end"); + } + events.push("consumed"); + stream.on("close", hadError => events.push(`late-close(${hadError})`)); + await once(child, "close"); + events.push("child-close-settled"); + })(); } else { // Node has read the pipe from the start, so the bytes are in the stream's buffer by now. child.on("exit", () => events.push(`read(${stream.read()})`)); } if (which === "both") continue; - stream.on("end", () => { - events.push("end"); - queueTickAndJobs("end"); + // Not for a consumer that awaits: how many promise jobs an await takes is not what the line is about. + if (!awaitsEnd) { + stream.on("end", () => { + events.push("end"); + queueTickAndJobs("end"); + }); + } + stream.on("close", hadError => events.push(`close(${hadError})`)); +} +if (mode === "held-open") { + child.on("exit", () => { + events.push("exit"); + fs.writeFileSync(go, ""); }); - stream.on("close", () => events.push("close")); } child.on("close", () => { // A job that runs after 'close' still gets into the line. From 421f79127e909c3ad376849ebc8e87c86b9691fb Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 16 Oct 2026 09:34:48 +0000 Subject: [PATCH 6/7] test: say that the held-open rows are not checked on Windows --- test/js/node/child_process/child-process-stdio.test.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/js/node/child_process/child-process-stdio.test.js b/test/js/node/child_process/child-process-stdio.test.js index 9696e442086a..523dcf7c64fc 100644 --- a/test/js/node/child_process/child-process-stdio.test.js +++ b/test/js/node/child_process/child-process-stdio.test.js @@ -266,16 +266,16 @@ describe.concurrent("child.stdout and child.stderr event order", () => { ["stdout", "waiting-read", "destroy-callback-throws"], // two-chunks: the second chunk and the EOF are in the pipe while the first 'data' listener runs. ["stdout", "two-chunks", "data"], - // Not on Windows. There Node runs the promise jobs of a listener that threw after 'end', when - // the EOF is already in the pipe. And Bun reads a pipe asynchronously there, so a read() call - // in 'exit' finds no bytes yet. - // held-open: a grandchild holds the pipe until the parent has seen the child's 'exit', so the 'close' - // of the pipe is also the 'close' of the child. On Windows the exit of the child ends the grandchild. ...(isWindows ? [] : [ + // Not on Windows. There Node runs the promise jobs of a listener that threw after 'end', when + // the EOF is already in the pipe. And Bun reads a pipe asynchronously there, so a read() call + // in 'exit' finds no bytes yet. ["stdout", "first-read", "throw"], ["stdout", "first-read", "read-on-exit"], + // held-open: a grandchild holds the pipe until the parent has seen the child's 'exit', so the + // 'close' of the pipe is also the 'close' of the child. Not checked on Windows. ["stdout", "held-open", "for-await"], ["stdout", "held-open", "await-end"], ]), From 1bbaee09335140fe9114faae37a10aa50c4bfa21 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 17 Oct 2026 05:24:00 +0000 Subject: [PATCH 7/7] test: keep the grandchild of the held-open rows alive under BUN_FEATURE_FLAG_NO_ORPHANS The test runner sets BUN_FEATURE_FLAG_NO_ORPHANS for ASAN builds. With it a Bun child ends its own children when it exits. So the grandchild that holds the pipe died before it wrote, and the stream ended before 'exit'. The fixture removes the variable from the child's environment in that mode. --- .../fixtures/child-process-stdio-event-order.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/test/js/node/child_process/fixtures/child-process-stdio-event-order.js b/test/js/node/child_process/fixtures/child-process-stdio-event-order.js index 90b722161260..b995ac8a27c9 100644 --- a/test/js/node/child_process/fixtures/child-process-stdio-event-order.js +++ b/test/js/node/child_process/fixtures/child-process-stdio-event-order.js @@ -64,7 +64,11 @@ const childSource = const stdio = ["ignore", "ignore", "ignore"]; for (const fd of fds) stdio[fd] = "pipe"; -const child = spawn(process.execPath, ["-e", childSource], { stdio }); +const env = { ...process.env }; +// The test runner sets this for ASAN builds. With it a Bun child ends its own children when it exits, and the +// grandchild has to outlive the child here. +if (mode === "held-open") delete env.BUN_FEATURE_FLAG_NO_ORPHANS; +const child = spawn(process.execPath, ["-e", childSource], { stdio, env }); const events = []; // The chain goes from the promise job queue to the nextTick queue and back, twice.