console/worker_threads: keep fd 1/2 blocking across worker start; poll on EAGAIN in the console writer

[robobun]

Repro

// bun r.mjs 2>/dev/null | { sleep 1; cat; } | wc -l
//   bun  -> ~700  (of 20000; exit 0, torn line at the 64 KiB pipe boundary)
//   node -> 20000
import { Worker } from "node:worker_threads";
const w = new Worker('setTimeout(() => {}, 2500)', { eval: true }); // worker logs NOTHING
await new Promise(r => w.on("online", r));
for (let i = 0; i < 20000; i++) console.log("line", String(i).padStart(6, "0"), "p".repeat(80));
await new Promise(r => w.on("exit", r));

The worker never writes a byte. Its mere creation flips the process-wide fd 1 to O_NONBLOCK, after which the main thread's native console.log silently drops on EAGAIN when piped to a slow reader (| tee, | grep, CI log collectors behind any worker pool such as piscina/tinypool).

strace signature from the worker's JS thread, before any user eval runs:

fcntl(1, F_DUPFD_CLOEXEC, 0) = 11
fcntl(11, F_GETFL)           = O_WRONLY
fcntl(11, F_SETFL, O_WRONLY|O_NONBLOCK) = 0

No thread ever issues a restoring F_SETFL.

Cause

O_NONBLOCK is an open-file-description flag, not an fd flag. dup() makes a new fd number pointing at the same description, and worker threads share the fd table, so an F_SETFL on the worker's dup is a mode change of the parent's fd 1.

The flip is reached because the worker's stdio rebind (setupWorkerStdio) uses Object.defineProperty(process, "stdout"/"stderr", {...}). process.stdout/stderr are static PropertyCallback slots, and JSC's defineOwnProperty reifies a lazy static property before defining over it. Reification runs constructStdout → getStdioWriteStream → Bun.file(1).writer() → FileSink::setup() → open_for_writing, which dups fd 1 and calls set_nonblocking on the dup. The worker immediately discards that fd-backed stream (the port-backed writable replaces it), but the O_NONBLOCK on the shared description is the residue.

The intended undo lives in Bun__ForceFileSinkToBeSynchronousForProcessObjectStdio, which is gated on self.fd != INVALID. FileSink::setup() never wrote self.fd (the fd went only to self.writer), so that branch is dead code. void process.stdout on a pipe therefore leaves fd 1 nonblocking even on the main thread with no worker involved.

The drop is fd_write_all_quiet (the native console / output writer) treating any Err(_), including EAGAIN, as terminal and discarding the unwritten tail.

Fix

Three small changes, each closing one seam:

• src/js/node/worker_threads.ts: rebind worker stdio with plain assignment (process.stdout = ...) instead of Object.defineProperty. put() replaces a PropertyCallback slot without reifying it and yields the same {writable,enumerable,configurable}=true descriptor, so the fd-backed constructor is never run in a worker and fd 1/2 are never touched.

• src/sys/lib.rs: fd_write_all_quiet polls POLLOUT and retries on EAGAIN instead of giving up. Anything sharing the open file description (a worker, a parent shell, libuv in a co-process, node itself) can flip the flag at any time; blocking on writability is what makes this class safe in Node.

• src/runtime/webcore/FileSink.rs: setup() records the dup'd/opened fd in self.fd, so the process-stdio force-sync hook's update_nonblocking(self.fd, false) actually runs, and _getFd() returns the real fd on the Bun.file(fd).writer() path.

Verification

New tests in test/js/node/process/process-stdio.test.ts (POSIX):

• reading process.stdout / process.stderr leaves fd 1/2 blocking: fcntl(F_GETFL) on fd 1/2 before/after the lazy getter fires.
• starting a node:worker_threads Worker leaves fd 1/2 blocking: same, across an idle worker coming online.
• console.log delivers every byte when fd 1 is O_NONBLOCK and the pipe is full: a raw pipe(2) under the test's control; the child fills it to EAGAIN, signals on stderr, then console.logs markers into the still-full pipe. Without the fix the markers are dropped (payload "", 5/5); with the fix the writer polls until the parent drains and all markers arrive (5/5).

All three fail on main and pass with this change. The original 20000-line worker repro delivers 20000/20000.

Related

• FileSink: record the opened fd in setup() so stdio force-sync clears O_NONBLOCK #35956 also records self.fd in FileSink::setup() (and fixes a separate Bun.write EAGAIN spin); the one-line overlap here will drop out on rebase if that lands first.
• console: retry on EAGAIN instead of dropping output when stdout is nonblocking #33560 is the same fd_write_all_quiet EAGAIN poll; this PR adds the worker trigger and a deterministic pipe-full test alongside it.
• node:worker_threads: +48 Node.js tests passing — MessagePort, stdio, SHARE_ENV, exit codes, transfer semantics, postMessageToThread + inspector #31216 introduced the worker-side defineProperty that reaches the existing setup()/writer seams.

Fixes #21516.

---

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

[coderabbitai]

Walkthrough

Changes

Worker stdio rebinding now uses assignments, FileSink records started descriptors, Unix writes retry after readiness polling, and POSIX tests cover blocking state and complete pipe output.

stdio write handling

Layer / File(s)	Summary
stdio rebinding and descriptor tracking src/js/node/worker_threads.ts, src/runtime/webcore/FileSink.rs	Worker stdio streams are reassigned directly, while successfully started FileSink descriptors are stored on Windows and general startup paths.
retryable Unix writes src/sys/lib.rs	fd_write_all_quiet polls for POLL_OUT after retryable Unix write errors before continuing.
POSIX stdio regression coverage test/js/node/process/process-stdio.test.ts	POSIX subprocess tests inspect O_NONBLOCK state and verify complete console.log output through a full pipe.

Suggested reviewers: cirospaciari, jarred-sumner

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The changes address #21516 by preventing fd 1/2 nonblocking flips, retrying EAGAIN writes, and adding coverage for slow-pipe output.
Out of Scope Changes check	✅ Passed	All changed files support the console-output/stdio fix or its tests, with no unrelated features introduced.
Title check	✅ Passed	The title clearly summarizes the main changes: worker stdio blocking and console write retry behavior.
Description check	✅ Passed	The description is detailed and includes verification, but it does not follow the template headings exactly.

---

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

[github-actions]

Found 2 issues this PR may fix:

1. process.stdout.write outputs nothing, while console.log works #9573 - process.stdout.write outputs nothing in CI/piped environments; the EAGAIN-retry fix in fd_write_all_quiet prevents silent data loss when stdout is non-blocking
2. readline corrupts console.log #21516 - readline corrupts console.log output when stdout is piped to a slow consumer; the O_NONBLOCK + EAGAIN discard caused truncation and missing data

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

Fixes #9573
Fixes #21516

🤖 Generated with Claude Code

[github-actions]

This PR may be a duplicate of:

1. FileSink: record the opened fd in setup() so stdio force-sync clears O_NONBLOCK #35956 - Both record self.fd in FileSink::setup() so the force-sync hook can clear O_NONBLOCK (fix Fix calling #private() functions in classes #2 in this PR is identical: same file, same field, same purpose)
2. console: retry on EAGAIN instead of dropping output when stdout is nonblocking #33560 - Both fix fd_write_all_quiet in src/sys/lib.rs to poll on POLLOUT and retry instead of treating EAGAIN as fatal (fix Copy source lines when generating error messages #3 in this PR is identical: same file, same function, same fix)

🤖 Generated with Claude Code

[coderabbitai]

Actionable comments posted: 5

🤖 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 `@src/js/node/worker_threads.ts`:
- Around line 449-466: Update the stdio replacement logic around process.stdout,
process.stderr, and process.stdin to use Object.defineProperty with
Node-compatible writable descriptors or setters, rather than plain assignment.
Ensure each worker-local stream actually replaces the cache-backed getter,
including the port-backed and immediately-EOF stdin paths.

In `@src/sys/lib.rs`:
- Around line 9414-9424: Update the write error handling around the #[cfg(unix)]
Err(e) if e.is_retry() arm so EINTR is treated as retryable before entering the
EAGAIN polling path. Ensure an interrupted write retries the write operation and
preserves the unwritten tail instead of reaching the final Err(_) => return
false path, while retaining existing polling behavior for EAGAIN.

In `@test/js/node/process/process-stdio.test.ts`:
- Around line 168-172: Restrict the libcPathForDlopen() guard in the
“stdout/stderr vs O_NONBLOCK on a pipe” describe block to the platforms
supported by that helper, Linux and Darwin, rather than using isPosix. Preserve
the empty-string fallback for unsupported platforms so the skipped tests do not
evaluate the throwing lookup.
- Around line 268-274: Update the process stdio test’s stderr-reading logic to
accumulate chunks until a newline-delimited byte count is complete, safely
handle done reads, and parse only the framed header. Preserve any bytes read
after the header by initializing stderrRest with the remaining data (including
rest.join("\n")). Move closeSync(w) into the existing finally cleanup so it also
runs when spawn fails, using the established guarded close pattern.
- Around line 176-179: Replace the fixed three-argument fcntl binding in the
dlopen setup and the nonblock helper with a non-variadic binding that matches
the exact F_SETFL ABI, using an FFIType.i64 flag argument where supported, or
route the flag update through a non-variadic syscall binding. Ensure the
existing F_GETFL/F_SETFL flow changes only O_NONBLOCK reliably across supported
architectures.

🪄 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 Plus

Run ID: ed47fc71-0c59-48cd-b24d-6bd9567a46de

📥 Commits

Reviewing files that changed from the base of the PR and between 6c12afd and 2c86c87.

📒 Files selected for processing (4)

• src/js/node/worker_threads.ts
• src/runtime/webcore/FileSink.rs
• src/sys/lib.rs
• test/js/node/process/process-stdio.test.ts

[claude]

No new issues found; the earlier variadic-fcntl and isPosix/FreeBSD concerns are addressed in the current diff. Deferring to a human given this touches the console output hot path, FileSink fd tracking, and worker stdio bootstrap together, with noted overlap against #35956/#33560.

What was reviewed:

• fd_write_all_quiet: EINTR now retried before the EAGAIN→posix::poll arm; posix::poll itself is EINTR-retried, and poll(-1) returns POLLERR/POLLHUP on a dead reader so the next write hits the terminal Err(_) path rather than hanging.
• FileSink::setup now records self.fd on both the Windows start_sync and the POSIX start success arms; self.fd is read-only elsewhere (no double-close introduced).
• Test: cc-compiled non-variadic fcntl wrappers replace the fixed-arg dlopen binding; libcPathForDlopen() is now only reached inside a test callback under isLinux || isMacOS; tempDirWithFiles at describe-body level is Windows-safe; stderr header is framed to \n and w is closed in finally.

Extended reasoning...

Overview

Four files: src/js/node/worker_threads.ts (swap Object.defineProperty for plain assignment in setupWorkerStdio so the lazy fd-backed process.stdout/stderr/stdin PropertyCallback is not reified), src/runtime/webcore/FileSink.rs (record the dup'd/opened fd in self.fd after writer.start()/start_sync() succeed), src/sys/lib.rs (fd_write_all_quiet retries EINTR and polls POLLOUT on EAGAIN instead of dropping the tail), and three new POSIX-gated tests in test/js/node/process/process-stdio.test.ts.

Security risks

None identified. No new user-controlled input reaches a parser or filesystem path; the FFI usage is test-only in a spawned child. The runtime change makes console.log block on a full nonblocking pipe, which is the Node-matching behavior and not an availability regression relative to silently dropping output.

Level of scrutiny

High. fd_write_all_quiet backs the native console/output writer and the crash reporter; changing its error handling to an unbounded poll(-1) is correct for the stdio-pipe case but is a semantic shift on a very hot path. The FileSink.rs self.fd.set(fd) line overlaps a sibling PR (#35956) and enables a previously-dead update_nonblocking branch. The worker_threads change relies on a JSC-specific detail (put() vs defineOwnProperty on a static PropertyCallback slot) that was empirically verified in-thread but is subtle enough to warrant a maintainer glance.

Other factors

All prior review threads (CodeRabbit and my earlier inline findings on the Apple-arm64 variadic fcntl ABI and the FreeBSD libcPathForDlopen() gate) are resolved in the current revision: the tests now compile non-variadic C wrappers via bun:ffi cc, gate on isLinux || isMacOS, frame the stderr header, and close the write-end fd in finally. The PR's own evidence footer notes the platform-specific tests were not run locally and are deferred to CI, and the description flags overlap with #35956 and #33560 — both reasons a human should sign off on landing order.

[robobun]

^{Updated 2:30 AM PT - Jul 27th, 2026}

✅ @robobun, your commit f49470f0d8b079e3645fd0a3a8e0fb056b6f204d passed in Build #83205! 🎉

---

🧪   To try this PR locally:

bunx bun-pr 36066

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

bun-36066 --bun

[robobun]

Closing: #37128 lists this PR as superseded and covers all three seams here. worker_threads assigns process.stdout/stderr instead of using defineProperty, the console writer polls on EAGAIN instead of dropping, and the stdio force-sync / O_NONBLOCK handling is replaced outright. Its process-stdio tests include the same three cases (fd flags after materialising process.stdout, console.log into a full O_NONBLOCK pipe, idle worker), and the idle-worker repro from this PR leaves fd 1 and 2 blocking on its current head.