stdio: route process.stdout/stderr writes through Writable's buffer accounting

[robobun]

Repro

Every documented backpressure signal on process.stdout except the write() return value reports "nothing buffered".

import { spawn } from "node:child_process";
const N = 4 * 1024 * 1024;
if (process.argv[2] === "child") {
  const so = process.stdout, facts = { hwm: so.writableHighWaterMark };
  facts.writeRet = so.write(Buffer.alloc(N, 0x41), () => {});
  facts.writableLength = so.writableLength;
  facts.writableNeedDrain = so.writableNeedDrain;
  so.cork(); so.write(Buffer.alloc(1000, 0x42), () => {});
  facts.corkedLength = so.writableLength;
  facts.writableCorked = so.writableCorked; so.uncork();
  process.stderr.write("@@" + JSON.stringify(facts) + "@@\n");
} else {
  const p = spawn(process.execPath, [process.argv[1], "child"], { stdio: ["ignore", "pipe", "pipe"] });
  let err = ""; p.stderr.on("data", d => (err += d));
  p.stdout.pause();                                  // let the pipe fill
  setTimeout(() => p.stdout.resume(), 400);
  p.on("exit", () => console.log(err.match(/@@(.*)@@/)?.[1]));
}

node: {"hwm":65536,"writeRet":false,"writableLength":4194304,"writableNeedDrain":true,"corkedLength":4195304,"writableCorked":1}
bun : {"hwm":65536,"writeRet":false,"writableLength":0,      "writableNeedDrain":false,"corkedLength":0,      "writableCorked":1}

Producers that throttle on writableLength or writableNeedDrain (loggers, NDJSON/CSV writers, hand-rolled pump loops) see an empty buffer forever and keep writing at full speed into an unread pipe.

Cause

process.stdout, process.stderr, tty.WriteStream and a child's stdin all take the fs.WriteStream $fastPath, which installs an own write() (writeFast) that hands the chunk straight to the native FileSink and never calls Writable.prototype.write. _writableState is therefore never touched, so writeOrBuffer() / onwrite() / afterWrite() never run. Five separate contracts break off that one bypass:

symptom	cause
writableLength / writableNeedDrain stuck at 0 / false	state.length and the needDrain flag are only maintained by writeOrBuffer()/onwrite()
cork() buffers nothing	corked chunks are supposed to land in state.buffered
write(chunk, encoding) ignores encoding	writeFast passed the raw string to the sink
write callbacks can complete out of order under backpressure	writeFast ran the callback synchronously when the sink accepted outright, jumping the queue of callbacks already parked on the sink's promise
no 'error' event when a write callback is supplied	writeFast only called the callback; onwriteError() does both
a write after end()/destroy() silently succeeded	Writable.prototype.write's ERR_STREAM_WRITE_AFTER_END / ERR_STREAM_DESTROYED checks were never reached

Fix

Delete the write() override and let the standard Writable machinery do the accounting. _write() stays as the bridge into the FileSink: it completes the write synchronously when the sink took the whole chunk (so back-to-back writes don't pile up in the Writable buffer), and defers to the sink's promise when the sink had to buffer, which is exactly what makes writableLength and writableNeedDrain honest. The hand-rolled emit("drain") and the kWriteMonkeyPatchDefense hack go away with it; afterWrite() emits 'drain' and write() is now inherited from the prototype, like node's.

decodeStrings is set to false on this path so UTF-8 strings still reach the sink without a Buffer.from() round-trip; other encodings are decoded first. node's process.stdout over a pipe is a net.Socket, which also runs with decodeStrings: false.

decodeStrings aside, the one thing the Writable machinery needs that it didn't have is a _writev, so a corked burst reaches the sink as a single write instead of one per chunk (see below).

Supersedes #33474, #31538, #33484, #33485, #33500, #33557, #34267, #34268 (and #33511, already closed into this branch)

Each of those PRs patches one of the symptoms above from inside writeFast. Removing the bypass fixes them at the root; every one is verified against main and node below, not assumed.

PR	check	main	this PR	node
#33474	write("48490a","hex") + write("QUJD","base64") + setDefaultEncoding("hex") + write("21")	48490aQUJD21	HI\nABC!	HI\nABC!
#31538	process.stdout.write === process.stdout.constructor.prototype.write (sonic-boom/pino tamper check)	false	true	true
#33485	events seen when a pty master closes mid-write	["cb:EIO", ...]	["cb:EIO","error:EIO"]	["cb:EIO","error:EIO"]
#33500	write callbacks in queue order under backpressure	reordered 6/6 runs	in order 6/6	in order 6/6
#33484	write("A",cb) / moveCursor(0,0,cb) / write("B",cb) / cursorTo(3,cb) on a pty	w1,w2,c,m0	w1,m0,w2,c	w1,m0,w2,c
#33511	write() after end() on process.stdout	callback fired with no error, bytes still reached the fd	ERR_STREAM_WRITE_AFTER_END	ERR_STREAM_WRITE_AFTER_END
#33557	write() after end() on piped stdout/stderr: write("A"); end("B"); write("C")	pipe reader received "ABC"	pipe reader receives "AB", ERR_STREAM_WRITE_AFTER_END	pipe reader receives "AB", ERR_STREAM_WRITE_AFTER_END
#34267	child.stdin.write() after child 'close'	{ret: true, cb: undefined}	{ret: false, cb: 'ERR_STREAM_DESTROYED'}	{ret: false, cb: 'ERR_STREAM_DESTROYED'}
#34268	child.stdin write failure (EPIPE) with a callback: is 'error' emitted and the stream destroyed?	'error' never fired when a callback was supplied	'error' fires, stream destroyed	'error' fires, stream destroyed

Conflict resolution for #33557, #34267 and #34268 (all merged): each of those PRs added a guard or error-path tweak inside writeFast/the old underscoreWriteFast, the functions this PR deletes. The resolution is to keep the deletion: Writable.prototype.write implements the ERR_STREAM_WRITE_AFTER_END / ERR_STREAM_DESTROYED checks; #34268's errorOrDestroy call is reached structurally because _write's cb(err) is state.onwrite, and onwrite(err) calls onwriteError → errorOrDestroy(stream, err). Verified by running each PR's own test (process-stdout-write-after-end.test.ts, child-process-stdio.test.js, child_process.test.ts -t "stdin write failure") against the resolved tree: all pass. The child-process-stdio.test.js conflict was a both-sides-add-tests; both sets were kept.

One caveat for whoever closes #33500: its fixture (process-stdout-write-order-fixture.js) only terminates against the old semantics. It sizes the in-flight data against write() returning false at the first buffered byte; once write() returns false at the high-water mark instead (node's rule), more bytes are in flight than the FIFO can hold and the fixture stalls. node stalls on it too. A drain loop in the fixture makes it terminate, and then it shows the reordering on main and in-order completion here and on node. This PR ports the deterministic half of that coverage (the readline-cursor interleave) instead.

What this PR does not fix

Two bot suggestions to add Fixes #... lines, both checked and both wrong:

• bytesWritten property always returns 0 for process.stdout and process.stderr #23061 (bytesWritten always 0) is untouched. Nothing on this path increments bytesWritten, before or after, and Writable doesn't track it. Still 0 here. (For whoever picks it up: node only exposes it when stdout is a pipe/TTY, where it's a net.Socket; over a file it's SyncWriteStream and bytesWritten is undefined, not 0. node:process: track bytesWritten for stdio streams #31627 is the open PR for this, and it patches the function this PR deletes, so it needs a small rebase onto underscoreWriteFast.)
• tty.WriteStream fails with EINVAL: invalid argument, kqueue on macOS when opening /dev/tty #24158 (EINVAL: invalid argument, kqueue from new tty.WriteStream(fs.openSync("/dev/tty","w")) on macOS) is untouched. The failure is inside Bun.file(fd).writer(), and this PR leaves that line exactly where it was; only the write() override is removed. The bot read this.write = writeFast out of the minified stack-trace line and assumed the whole fast path was gone.

Also related and not superseded: #29232 fixes the same encoding bug as #33474, but by routing every string through Buffer.from(). That costs ~3x on string writes, which is why this PR keeps decodeStrings: false and only decodes the non-UTF-8 encodings.

One behavior change, on purpose

The old fast path handed the chunk straight to the native sink without type validation, so an ArrayBuffer written to stdio "worked". Writable.prototype.write validates it, so it now throws ERR_INVALID_ARG_TYPE. This is not a new restriction so much as the end of an inconsistency — bun's own fs.WriteStream already rejected it, only stdio didn't:

	fs.createWriteStream(...).write(ab)	process.stdout.write(ab)
node	throws	throws
bun, today	throws	accepted
this PR	throws	throws

A view over the same buffer (new Uint8Array(ab)) is the supported spelling and is unaffected. Two in-tree test helpers were leaning on the old leniency (they wrote bun:jsc serialize()'s SharedArrayBuffer to stdout) and are updated; the new behavior is pinned by process.stdout - write() rejects an ArrayBuffer like node. Flagging it here rather than burying it: if Bun would rather keep accepting ArrayBuffer on stdio, that has to be a deliberate extension, and it cannot live on this path without reintroducing the write() override.

Known gap: Windows does not report backpressure

writableLength / writableNeedDrain still read as "nothing buffered" on Windows, and that is not fixed here. Its writer hands the chunk to uv_write and reports completion the moment libuv accepts it, so the sink never tells the stream it had to buffer and writableLength cannot see libuv's queue. This predates the PR — process.stdout.write() returns true for a 4 MB write into a blocked pipe on main too — and closing it means teaching WindowsBufferedWriter to report Pending while a uv_write is in flight, which is a native change I can't test here. cork() accounting does work on Windows. The two backpressure tests carry skipIf(isWindows) with that reason recorded inline.

Verification

writableLength / writableNeedDrain / writableCorked now match node byte-for-byte on both destinations stdio can have:

                 stdout -> pipe (Socket / WriteStream)                      stdout -> file (SyncWriteStream / WriteStream)
node   writeRet=false len=4194304 needDrain=true corked=4195304   |   writeRet=true len=0 needDrain=false corked=1000
bun    writeRet=false len=4194304 needDrain=true corked=4195304   |   writeRet=true len=0 needDrain=false corked=1000

New tests, every one verified failing on main and passing here:

test/js/node/process/process-stdio.test.ts

• writableLength, writableNeedDrain and cork() track the buffered bytes
• 'drain' resets writableLength and writableNeedDrain
• uncork() flushes a corked burst in a single write syscall (Linux-only)
• process.stdout - write() decodes the encoding argument
• process.stdout - write() is inherited, not an own property
• process.stdout - write callbacks run in call order with readline cursor callbacks
• process.stdout - write() rejects an ArrayBuffer like node
• process.stdout - write after end()

test/js/node/child_process/child-process-stdio.test.js

• child.stdin write after end() / destroy(), with and without a callback

913 of node's test/parallel stream / fs / child_process / process / net / tty / console tests run identically before and after (894 pass, 19 pre-existing failures, zero drift), as do test/js/node/stream, test/js/node/child_process, test/js/node/fs, test/js/node/tty and test/regression/issue/1632.test.ts (stdout EPIPE).

_writev: cork() has to batch, not just buffer

Review raised this and it was a real gap. With cork() finally buffering, clearBuffer() was flushing the backlog one chunk at a time, so a corked burst of 1000 writes cost 1000 write(2) calls where node coalesces the same burst into a single writev(2). Giving the FileSink-backed streams a _writev closes it. Counted with /proc/self/io's syscw, 1000 × 64-byte writes:

	writableLength while corked	syscalls, corked burst	syscalls, uncorked control
main	0 (cork was a no-op)	1000	1000
this PR, before _writev	64000	1000	1000
this PR	64000	1	1000
node (stdout → pipe)	64000	1	1000

To be precise about the "regression" framing: cork never batched before this PR either, because writeFast ignored state.corked and pushed every chunk straight at the sink. Nothing regressed; the batching simply never existed, and now it does.

_writev also flushes the post-backpressure backlog in one sink write, which is where it earns back some of the cost in the table above.

The regression test for it is Linux-only (syscw is the only way to observe a syscall count) and points stdout at a regular file rather than a pipe: a pipe that fills up makes the sink coalesce on its own, which would sink both numbers and leave the test asserting nothing. Verified it fails in both directions it needs to:

src at main                  -> both cork tests fail
fix present, _writev removed -> accounting passes, syscall test fails  (so it pins _writev, not the accounting)
full fix                     -> both pass

Performance

process.stdout.write() now pays the Writable bookkeeping node pays, so it gets slower. Release builds, same commit, 300k × 64-byte writes:

destination	payload	main	this PR	node
/dev/null	Buffer	4.69M ops/s	3.55M ops/s (-24%)	2.11M ops/s
/dev/null	string	4.05M ops/s	3.15M ops/s (-22%)	1.65M ops/s
pipe (drained)	Buffer	1.35M ops/s	1.19M ops/s (-12%)	1.13M ops/s
pipe (drained)	string	1.27M ops/s	1.21M ops/s (-4%)	1.04M ops/s

Against a real pipe, where the syscall dominates, the cost is 4-12% and Bun stays ahead of node. Keeping decodeStrings: false is what holds the string column up; routing strings through Buffer.from() like a plain fs.WriteStream would have cost ~3x instead. I did not find a way to keep a fast path here and stay correct: whether the sink buffers is only knowable after the write, and by then the bytes are already in the sink and the accounting has to be reconstructed by hand.

---

no test proof · iteration 15 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/js/node/process/process-stdio.test.ts

[robobun]

^{Updated 4:21 PM PT - Jul 16th, 2026}

❌ @robobun, your commit 842a0e3 has 3 failures in Build #74124 (All Failures):

• test/cli/run/require-cache.test.ts - code 1 on 🍎 26 aarch64
• test/js/node/test/parallel/test-worker-message-port-transfer-terminate.js - SIGABRT on 🐧 13 x64-asan
• test/js/node/worker_threads/worker-transfer-terminate-stress.test.ts - SIGABRT on 🐧 13 x64-asan

---

🧪   To try this PR locally:

bunx bun-pr 33508

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

bun-33508 --bun

[github-actions]

Found 2 issues this PR may fix:

1. bytesWritten property always returns 0 for process.stdout and process.stderr #23061 - bytesWritten always returns 0 for process.stdout/stderr because writeFast bypassed Writable's accounting entirely; this PR routes writes through Writable's buffer tracking
2. tty.WriteStream fails with EINVAL: invalid argument, kqueue on macOS when opening /dev/tty #24158 - tty.WriteStream fails with EINVAL: invalid argument, kqueue on macOS at the exact line initializing kWriteStreamFastPath / Bun.file(fd).writer(); this PR removes that fast-path entirely

If this is helpful, copy the block below into the PR description to auto-close these issues on merge.

Fixes #23061
Fixes #24158

🤖 Generated with Claude Code

[github-actions]

This PR may be a duplicate of:

1. fs: decode write(chunk, encoding) on the WriteStream fast path #33474 - Fixes encoding on the same writeFast path that this PR deletes entirely (explicitly superseded)
2. fs: keep WriteStream.prototype.write on the prototype so pino uses its fast path #31538 - Modifies the same writeFast override in streams.ts for pino compatibility (explicitly superseded)
3. stdio: emit 'error' on a failed write even when a write callback is passed #33485 - Fixes missing 'error' event on the custom write path that this PR removes (explicitly superseded)
4. stdio: run write completion callbacks in write order #33484 - Fixes write callback ordering in streams.ts, same as superseded stdio: run write completion callbacks in write order #33500
5. fix: Pipe process.stdout.write through Buffer to fix node incompatibility #29232 - Pipes process.stdout.write through Buffer to fix encoding/buffer handling, a subset of this PR's fix
6. node:process: track bytesWritten for stdio streams #31627 - Tracks bytesWritten for stdio streams by modifying the same custom write path this PR replaces

🤖 Generated with Claude Code

[claude]

I didn't find any bugs, but this trades a documented ~22-24% process.stdout.write throughput regression for correctness on a very hot path and supersedes four other open PRs — that's a maintainer-level call worth a human sign-off.

Extended reasoning...

Overview

This PR removes the own-property write() fast path (writeFast) from FileSink-backed WriteStreams (process.stdout/process.stderr, tty.WriteStream, child stdin) and routes writes through the standard Writable.prototype.write machinery instead. _write remains as the bridge to the native FileSink, now with decodeStrings: false and a manual non-UTF-8 decode branch. The kWriteMonkeyPatchDefense hack and hand-rolled emit('drain') are deleted. Net -78/+24 in src/js/internal/fs/streams.ts, plus five new subprocess-based tests in test/js/node/process/process-stdio.test.ts.

Security risks

None identified. No auth, crypto, permissions, or untrusted-input parsing is involved; the change is confined to Node stream compatibility semantics.

Level of scrutiny

High. process.stdout.write() is on the hot path of essentially every Bun program, and the fast path being removed was added deliberately for performance. The author transparently documents a 22-24% throughput drop on the /dev/null microbenchmark (4-12% against a real pipe, still ahead of Node). Whether that correctness-for-speed trade is acceptable — and whether the decodeStrings: false + in-_write encoding branch is the right shape versus reconstructing the accounting by hand — is a design/perf judgment a maintainer should make, not a bot.

Other factors

• The PR claims to supersede #33474, #31538, #33485, and #33500; someone needs to coordinate closing those and confirm nothing from them is lost (the author flags that #33500's fixture stalls under the new semantics and on Node).
• The change interacts with _writev (still set to undefined on the fast path) and with cork()/uncork() batching behavior — worth a second pair of eyes on whether corked writes now go through _write one-by-one where they previously batched.
• Test coverage is thorough and follows repo conventions (subprocess spawns, await using, marker-based readiness instead of sleeps, exit code asserted last), and the author reports zero drift across 913 Node parallel tests. The implementation itself reads correct to me; my hesitation is purely about the perf trade-off and cross-PR coordination, not about bugs.

[coderabbitai]

Walkthrough

The FileSink-backed write fast path is rewritten to use lazy writer setup and uniform callback handling. Stdio tests add coverage for child stdin lifecycle errors and process stdout buffering, encoding, inheritance, and callback ordering.

Changes

stdio write path and coverage

Layer / File(s)	Summary
WriteStream fast path src/js/internal/fs/streams.ts	Forces decodeStrings = false, routes vector writes through the fast path, and rewrites fast writes to lazily create the FileSink writer, normalize non-UTF8 strings to Buffer, and settle callbacks through promise/error handling.
child.stdin lifecycle errors test/js/node/child_process/child-process-stdio.test.js	Adds child.stdin tests for writes after end() and destroy(), asserting return values, emitted errors, callback codes, and child input handling.
process.stdout stdio behavior test/js/node/process/process-stdio.test.ts	Adds process.stdout tests for write-after-end handling, backpressure and cork accounting, drain behavior, encoding decoding, prototype inheritance, and callback ordering with readline.

Sequence Diagram(s)

sequenceDiagram
  participant WriteStream
  participant FileSink

  WriteStream->>FileSink: writer() lazily initialized
  WriteStream->>FileSink: write(chunk or Buffer.concat(chunks))
  FileSink-->>WriteStream: promise or thrown error
  WriteStream-->>WriteStream: cb(null) or cb(err)

Loading

Related issues: #33500

Related PRs: None identified

Suggested labels: node.js, tests

Suggested reviewers: alii

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The fix and tests address #33500 by restoring ordered write callbacks under backpressure and readline interleaving.
Out of Scope Changes check	✅ Passed	The extra write-after-end/destroy tests are still directly related to restoring Writable semantics, so no clear out-of-scope changes stand out.
Title check	✅ Passed	The title clearly summarizes the main change: stdio writes now use Writable buffer accounting.
Description check	✅ Passed	The description covers what the PR does and how it was verified, though it doesn't use the template headings verbatim.

---

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

[robobun]

I arrived at the same writeFast bypass from a different symptom (write-after-end) and opened #33511 before this PR existed. Yours supersedes it: deleting the override restores Writable.prototype.write, which is where the ERR_STREAM_WRITE_AFTER_END / ERR_STREAM_DESTROYED checks live, so that sixth contract falls out of your change for free.

I pushed the regression coverage for it onto this branch in ceb1c9b1 (tests only, no source change):

• test/js/node/child_process/child-process-stdio.test.js — write after end() returns false and the callback gets ERR_STREAM_WRITE_AFTER_END, the bytes never reach the child; a callback-less write after end() emits 'error'; write after destroy() reports ERR_STREAM_DESTROYED
• test/js/node/process/process-stdio.test.ts — process.stdout write after end() is dropped, returns false, and reports through both the callback and 'error'

All four fail on 1.4.0 (today process.stdout.end() followed by write(x) actually writes x and reports success) and pass on this branch unmodified. Your backpressure tests still pass alongside them: 23 pass / 0 fail across both files.

Closing #33511.

[claude]

I didn't find any bugs, but this deliberately removes the stdio writeFast bypass and takes a measured 4–24% hit on process.stdout.write() in exchange for correct Writable accounting — that's the right architectural call IMO, but it's a perf-vs-correctness tradeoff on one of the hottest paths in the runtime and it supersedes 4+ competing PRs, so a maintainer should sign off on the direction.

Extended reasoning...

Overview

The PR deletes the own-property write() override (writeFast) that process.stdout/stderr, tty.WriteStream, and child stdin install on the $fastPath, and instead lets Writable.prototype.write do the buffer accounting while _write (underscoreWriteFast) bridges into the native FileSink. It also flips decodeStrings to false on this path and decodes non-UTF-8 encodings inside _write. Net -78/+24 in src/js/internal/fs/streams.ts, plus ~250 lines of new test coverage for backpressure accounting, encoding, prototype inheritance, callback ordering, and write-after-end/destroy.

Security risks

None. This is Node-compat stream plumbing with no auth, crypto, or untrusted-input parsing surface changes.

Level of scrutiny

High. process.stdout.write() is one of the most-called functions in any Bun program, and the writeFast bypass was presumably added intentionally for throughput. The PR itself measures a 22–24% regression to /dev/null and 4–12% to a drained pipe (still faster than Node). The author argues convincingly that no fast path can stay correct here — whether the sink buffers is only knowable after the write — but whether the perf cost is acceptable, and whether this is the right consolidation vs. the four PRs it supersedes (#33474, #31538, #33485, #33500) plus #33511, is a maintainer-level architectural call.

Other factors

• The change is well-reasoned, thoroughly tested (9 new tests, all verified failing on main), and the description shows zero drift across 913 Node parallel tests.
• The bug-hunting pass found nothing; the simplified _write looks correct (sync cb(null) when the sink accepts outright is fine — Writable's onwrite handles the sync-completion case and defers via nextTick).
• bytesWritten is still not maintained on this path (issue #23061) — the fast-path _write doesn't increment it — but that's pre-existing and orthogonal to this PR's goal.
• No prior human review comments to address; CI build #69103 was still in progress at the time of this review.

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@test/js/node/process/process-stdio.test.ts`:
- Around line 336-337: Remove the uncorked lower-bound assertion from the
process stdio test and keep the rest of the buffering checks intact. The control
case in the test around the uncorked write loop can legitimately coalesce writes
because process.stdout uses Bun.file(fd).writer(), so do not require uncorked to
be one syscall per write; leave the corked threshold check and the byte-count
assertion in place, and update the test logic near the uncorked/corked
comparisons accordingly.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 4387cb64-7159-4a8a-895f-1e693864dad6

📥 Commits

Reviewing files that changed from the base of the PR and between a728010 and f3086e6.

📒 Files selected for processing (2)

• test/js/node/child_process/child-process-stdio.test.js
• test/js/node/process/process-stdio.test.ts

[robobun]

Thanks — one of these found a real gap. Going through all four.

@coderabbitai: the uncorked control could coalesce — correct, and it mattered

You were right, and the failure mode was worse than "the control is weak": on a pipe, a slow reader makes the sink coalesce both numbers, so the corked < 10 assertion would have passed vacuously. Reproduced it by pausing the reader for 600 ms:

slow reader, stdout -> pipe:  {"corked": 1, "uncorked": 204}   # uncorked fell to 204 of 1000

Rather than drop the control, I pointed stdout at a regular file. A file never applies backpressure, so every sink write is exactly one write(2) and both numbers are deterministic:

	corked	uncorked
main	1000	1000
this PR, _writev removed	1000	1000
this PR	1	1000

So the control is now expect(uncorked).toBe(chunks) exactly, and the test is pinned from both sides — it fails with the fix present but _writev removed, which is the property that makes it worth having. Fixed in f9d0d9c.

@claude: "do corked writes now go through _write one-by-one where they previously batched?"

Half right, and the half that was right was the important one.

Cork never batched before this PR — writeFast ignored state.corked entirely and pushed every chunk straight at the sink, which is why writableLength read 0 while corked. So nothing regressed. But node does batch (its stdio over a pipe is a net.Socket with _writev, so a corked burst becomes one writev(2)), and my first revision did not. Shipping "cork() works now, at 1000x the syscalls" is a half-finished job, so _writev is now part of the PR: a corked burst (and a post-backpressure backlog) reaches the sink as one write.

Measured with /proc/self/io's syscw, 1000 × 64-byte writes: 1000 syscalls → 1, matching node. Good catch — it wouldn't have been caught by any behavioral test.

@github-actions (find-issues): both suggested Fixes #... lines are wrong

I checked both against a build rather than taking them on faith, and did not add either.

• bytesWritten property always returns 0 for process.stdout and process.stderr #23061 (bytesWritten always 0) — not fixed. Nothing on this path increments bytesWritten, before or after; Writable doesn't track it either. Still 0 here. (@claude's second pass independently reached the same conclusion.) Worth noting for whoever picks it up: node only exposes it when stdout is a pipe/TTY, where it's a net.Socket — over a file it's SyncWriteStream and bytesWritten is undefined, not 0.
• tty.WriteStream fails with EINVAL: invalid argument, kqueue on macOS when opening /dev/tty #24158 (EINVAL ... kqueue on macOS) — not fixed. The failure is inside Bun.file(fd).writer(), and this PR leaves that line exactly where it was; only the write() override is removed. The minified builtin puts the whole fast-path block on one source line, so the stack trace shows this.write = writeFast next to the call that actually throws.

@github-actions (find-duplicate-prs)

Accurate. #33474, #31538, #33484, #33485 and #33500 are all symptoms of the same bypass and are superseded; each is verified against main and node in the table in the description, not assumed. #33511 was closed into this branch already.

Two I'd flag rather than steamroll, both from outside contributors:

• node:process: track bytesWritten for stdio streams #31627 (@EffortlessSteven) is the real fix for bytesWritten and is not superseded — but it patches the function this PR deletes, so it needs a rebase onto underscoreWriteFast. Happy to do that on top if that's preferred.
• fix: Pipe process.stdout.write through Buffer to fix node incompatibility #29232 (@lindskogen) fixes the same encoding bug as fs: decode write(chunk, encoding) on the WriteStream fast path #33474 by routing every string through Buffer.from(). That costs ~3x on string writes, which is why this PR keeps decodeStrings: false and only decodes the non-UTF-8 encodings.

---

Revalidated after _writev: the 913 test/parallel stream / fs / child_process / process / net / tty / console tests still show zero drift (894 pass, 19 pre-existing failures, identical before and after), and the stdio suites are green.

[coderabbitai]

@robobun Thanks for digging into this — the reproduction with the 600ms slow reader ({"corked": 1, "uncorked": 204}) is a great concrete demonstration of exactly why the pipe-based control was unreliable. Pointing stdout at a regular file to get deterministic one-syscall-per-write semantics, then pinning the assertion to expect(uncorked).toBe(chunks) on both sides (present/absent _writev), is the right fix — it now actually fails without _writev, which is the whole point of having the test.

Nice catch also on the _writev batching gap from Claude's review, and thorough due diligence on the two find-issues suggestions and the find-duplicate-prs list (especially flagging #31627 and #29232 rather than silently superseding them). This all looks solid.

(◕‿◕)ノ nice work tracking that one down

---

✏️ Learnings added

Learnt from: robobun
URL: https://github.com/oven-sh/bun/pull/33508

Timestamp: 2026-07-06T16:40:56.334Z
Learning: In `test/js/node/process/process-stdio.test.ts` (bun repo), tests that assert on the number of underlying sink write syscalls (e.g., comparing corked vs. uncorked write counts to verify `_writev` batching) should target a regular file rather than a pipe. On a pipe, a slow reader can cause the sink to coalesce multiple writes into fewer syscalls, making count-based assertions pass vacuously. A regular file never applies backpressure, so each sink write corresponds to exactly one `write(2)`, making the assertion deterministic.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

✅ Action performed

Comments resolved. Approval is disabled; enable reviews.request_changes_workflow to allow explicit top-level @coderabbitai resolve or @coderabbitai approve commands.

[robobun]

CI surfaced two things that change what this PR claims, so flagging them here rather than leaving them in a silent description edit. Branch is rebased onto main (the cookie-map failures were #33425's, already fixed there).

1. process.stdout.write(arrayBuffer) now throws, on purpose

Writable.prototype.write validates the chunk type; the old fast path didn't, and fed whatever the native sink accepted straight through. So this is the end of an inconsistency rather than a new restriction — Bun's own fs.WriteStream already rejected it, only stdio didn't:

	fs.createWriteStream(...).write(ab)	process.stdout.write(ab)
node	throws ERR_INVALID_ARG_TYPE	throws ERR_INVALID_ARG_TYPE
bun, today	throws ERR_INVALID_ARG_TYPE	accepted
this PR	throws ERR_INVALID_ARG_TYPE	throws ERR_INVALID_ARG_TYPE

A view over the buffer (new Uint8Array(ab)) is the supported spelling and is unaffected. Two in-tree helpers were leaning on the old leniency — they wrote bun:jsc serialize()'s SharedArrayBuffer to stdout — and are updated; the behavior is now pinned by process.stdout - write() rejects an ArrayBuffer like node, which fails on main with "accepted,accepted".

This one genuinely wants a maintainer's opinion. It cannot be avoided on this path: the validation lives in Writable.prototype.write, so keeping ArrayBuffer working would mean reintroducing the write() override this PR exists to delete. If Bun wants to keep accepting it on stdio, that has to be a deliberate extension somewhere else.

2. Windows still does not report backpressure, and this PR does not fix it

writableLength / writableNeedDrain remain 0 / false on Windows. Its writer hands the chunk to uv_write and reports completion the moment libuv accepts it, so the sink never tells the stream it had to buffer, and writableLength cannot see libuv's queue. On main, a 4 MB write into a blocked pipe returns true there too, so this predates the change and is native-side — closing it means teaching WindowsBufferedWriter to report Pending while a uv_write is in flight, which I can't build or test from here.

cork() accounting does work on Windows (the burst buffers correctly). The two backpressure tests now carry skipIf(isWindows) with that reason recorded inline, so the gap is visible rather than silently green. POSIX (Linux + macOS) is unaffected and matches node byte-for-byte.

---

Everything else CI flagged was the cookie-map Expires format, unrelated and fixed by the rebase. Locally, the four files CI named are green, the new ArrayBuffer test fails on main and passes here, and the suites I touched show no new failures against a main baseline.

[robobun]

Status: diff is green, CI is red on known main breaks

Rebased onto current main. All touched files pass locally, 913 node test/parallel stream/fs/child_process/process/net/tty/console tests show zero drift vs main, and all superseded PRs' own test files (#33557, #34267, #34268) pass against the resolved tree.

Build 74124 (current, 842a0e38, finalized): three test failures, all owned by other main-break fix sessions.

• test-worker-message-port-transfer-terminate.js + worker-transfer-terminate-stress.test.ts SIGABRT on x64-asan — the known worker-terminate exception-scope race. The stress test is node:http/https/http2: raise Node v26.3.0 compat to ~94%, sync the upstream suites, and fix the Windows/macOS transport-layer teardown bugs they exposed #32488's amplified harness for the same race (its own comment: "SIGABRTs intermittently on x64-asan only, 0/115 local repros").
• require-cache.test.ts on darwin-26 — an RSS-threshold memory-leak test (71 MB vs 64 MB cap) that flakes on 8/39 recent builds across unrelated branches (win-appcontainer, sliceansi, port-node-domain-tests, …). Module caching; zero stdio overlap.

Build 73936 (b3010760): grpc-js/test-resolver.test.ts Rust null-deref at exit on x64-asan, after all tests passed. 0/60 local reproductions on release-asan (30× main, 30× this branch). Has not recurred on 73993 or 74124.

The two items that need a human decision are in the description: the deliberate ArrayBuffer-now-throws behavior change, and the still-open Windows backpressure-reporting gap (pre-existing, native-side). Happy to rebase or adjust on request.

[EffortlessSteven]

• node:process: track bytesWritten for stdio streams #31627 (@EffortlessSteven) is the real fix for bytesWritten and is not superseded — but it patches the function this PR deletes, so it needs a rebase onto underscoreWriteFast. Happy to do that on top if that's preferred.

Happy to rebase, just say the word.

[robobun]

Appreciated. To set expectations though: I'm not a maintainer, so the call on whether to fold #31627 in here or land it separately on top isn't mine to give. That's for whoever merges.

What I can do is make the rebase target precise so it's a small move whenever it happens. After this PR, the whole stdio write path funnels through one function in src/js/internal/fs/streams.ts:

function underscoreWriteFast(this: FSStream, chunk, encoding, cb) {
  ...
  maybePromise = fileSink.write(chunk);   // <- the single point bytes reach the sink
  ...
}

writeFast (the old own-write override your PR patched) is gone, and _writev now routes through underscoreWriteFast too (underscoreWritevFast just coalesces then calls it), so a bytesWritten increment in that one spot covers plain writes, corked bursts, and the post-backpressure backlog. That's fewer touch points than your original patch needed against the pre-split path.

One thing to keep in mind for the counting: on this path a UTF-8 string reaches fileSink.write() still as a string (only non-UTF-8 encodings get a Buffer.from first), so the increment wants the encoded length (Buffer.byteLength(chunk, encoding) for a string, chunk.byteLength for a view), not chunk.length. Your description already mentions counting by encoded byte length, so you've likely got this.

bytesWritten is explicitly out of scope for this PR (I noted it under "What this PR does not fix" in the description), so there's no overlap to resolve. Happy to give your rebase a look once this lands if that's useful.

[robobun]

Closing: #37128 reworks the stdio sinks and lists this PR as superseded (it removes the own write = writeFast override the same way, with _write/_writev bridging into the sink and decodeStrings: false, so writableLength / writableNeedDrain / cork() / 'drain' / the encoding argument / write-after-end are covered; the one deliberate difference is that process.stdout.write(arrayBuffer) stays accepted there).

Tests from this PR that #37128 does not have (corked burst reaching the fd as one write, child.stdin write after end()/destroy(), write callback order under backpressure) are listed over there.

If something here turns out not to be covered once #37128 lands, this can be reopened.