Skip to content
Closed
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 10 additions & 24 deletions src/js/node/worker_threads.ts
Original file line number Diff line number Diff line change
Expand Up @@ -446,37 +446,23 @@ function makePortWritable(port) {

function setupWorkerStdio(stdio) {
const { stdin, stdout, stderr } = stdio;
// Not defineProperty: that reifies the lazy fd-backed stdio before replacing it.
if (stdout) {
Object.defineProperty(process, "stdout", {
value: makePortWritable(stdout),
writable: true,
configurable: true,
enumerable: true,
});
process.stdout = makePortWritable(stdout);
}
if (stderr) {
Object.defineProperty(process, "stderr", {
value: makePortWritable(stderr),
writable: true,
configurable: true,
enumerable: true,
});
process.stderr = makePortWritable(stderr);
}
// node always replaces a worker's process.stdin: port-backed when { stdin: true },
// otherwise an immediately-EOF'd stream — never the process-wide fd 0, which
// would race the main thread (and hang on a TTY).
Object.defineProperty(process, "stdin", {
value: stdin
? makePortReadable(stdin, true)
: new Readable({
read() {
this.push(null);
},
}),
writable: true,
configurable: true,
enumerable: true,
});
process.stdin = stdin
? makePortReadable(stdin, true)
: new Readable({
read() {
this.push(null);
},
});
// node routes console.log through process.stdout/stderr; Bun's global console
// writes the fd directly, so rebind it to the captured streams when present.
if (stdout || stderr) {
Expand Down
3 changes: 3 additions & 0 deletions src/runtime/webcore/FileSink.rs
Original file line number Diff line number Diff line change
Expand Up @@ -657,6 +657,7 @@ impl FileSink {
return sys::Result::Err(err);
}
sys::Result::Ok(()) => {
self.fd.set(fd);
self.writer
.with_mut(|w| w.update_ref(self.io_evtloop(), false));
}
Expand All @@ -672,6 +673,8 @@ impl FileSink {
return sys::Result::Err(err);
}
sys::Result::Ok(()) => {
// `get_fd()` and the stdio force-sync O_NONBLOCK undo read this.
self.fd.set(fd);
// Only keep the event loop ref'd while there's a pending write in progress.
// If there's no pending write, no need to keep the event loop ref'd.
self.writer
Expand Down
12 changes: 12 additions & 0 deletions src/sys/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9405,11 +9405,23 @@ fn qw_set_fd(qw: &mut bun_core::output::QuietWriter, fd: Fd) {

/// Best-effort write-all loop. Returns `false` on I/O error / zero-write so
/// `ScopedLogger::log` can disable the scope; "quiet" callers discard the bool.
/// EAGAIN polls: anything sharing the open file description can flip O_NONBLOCK.
fn fd_write_all_quiet(fd: Fd, mut bytes: &[u8]) -> bool {
while !bytes.is_empty() {
match write(fd, bytes) {
Ok(0) => return false, // short write → give up
Ok(n) => bytes = &bytes[n..],
#[cfg(unix)]
Err(e) if e.is_retry() => {
let mut pfd = [posix::PollFd {
fd: fd.native(),
events: posix::POLL_OUT,
revents: 0,
}];
if posix::poll(&mut pfd, -1).is_err() {
return false;
}
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Err(_) => return false,
}
}
Expand Down
147 changes: 146 additions & 1 deletion test/js/node/process/process-stdio.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import { spawn, spawnSync } from "bun";
import { dlopen, FFIType, ptr } from "bun:ffi";
import { describe, expect, test } from "bun:test";
import { bunEnv, bunExe } from "harness";
import { bunEnv, bunExe, isPosix, libcPathForDlopen } from "harness";
import { closeSync, readSync } from "node:fs";
import path from "path";
import { isatty } from "tty";
describe.concurrent("process-stdio", () => {
Expand Down Expand Up @@ -158,4 +160,147 @@
`hello worldhello again|😋 Get Emoji — All Emojis to ✂️ Copy and 📋 Paste 👌`.repeat(9999),
);
});

// O_NONBLOCK is an open-file-description flag: any co-process or thread
// sharing the description (worker threads, a parent shell, libuv) can flip it
// on the process-wide fd 1/2. Bun must (a) not flip it from the worker stdio
// path and (b) not drop output when something else has.
describe.skipIf(!isPosix)("stdout/stderr vs O_NONBLOCK on a pipe", () => {
// F_GETFL/F_SETFL are 3/4 on Linux and Darwin; O_NONBLOCK differs (2048 vs 4).
// describe.skipIf still evaluates this body on Windows, so guard the libc
// lookup (which throws there); the skipped tests never read the value.
const libc = isPosix ? libcPathForDlopen() : "";

Check warning on line 172 in test/js/node/process/process-stdio.test.ts

View check run for this annotation

Claude / Claude Code Review

isPosix guard on libcPathForDlopen() throws on FreeBSD

`isPosix` (harness.ts:23) is `isMacOS || isLinux || isFreeBSD`, but `libcPathForDlopen()` only handles `linux`/`darwin` and throws `new Error("TODO")` in its default branch — so on FreeBSD this line throws during describe-body evaluation and fails the whole file rather than skipping. FreeBSD is only in `buildPlatforms` (not `testPlatforms`) so CI won't hit this today, but the established pattern (e.g. `transpiler-truncated-utf8.test.ts:9`) is to gate on `isLinux || isMacOS` for both the `describ
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
Comment thread
robobun marked this conversation as resolved.
Outdated
const fcntlPrelude = `
const { dlopen, FFIType } = require("bun:ffi");
const { O_NONBLOCK } = require("node:constants");
const { fcntl } = dlopen(${JSON.stringify(libc)}, {
fcntl: { args: [FFIType.int, FFIType.int, FFIType.int], returns: FFIType.int },
}).symbols;
const nonblock = fd => (fcntl(fd, 3, 0) & O_NONBLOCK) !== 0;

Check failure on line 179 in test/js/node/process/process-stdio.test.ts

View check run for this annotation

Claude / Claude Code Review

Variadic fcntl via fixed-arg bun:ffi signature is unreliable on Apple arm64

Binding `fcntl` via bun:ffi as `{ args: [int, int, int] }` is unreliable on Apple arm64: `fcntl` is variadic (`int fcntl(int, int, ...)`), and Apple's arm64 ABI reads variadic arguments from the stack, not from `w2` where the TinyCC trampoline puts the third arg. On the darwin-aarch64 CI lanes, test 3's `fcntl(1, F_SETFL, ...|O_NONBLOCK)` therefore reads garbage stack bytes as the flags word — fd 1 may stay blocking (→ `fs.writeSync` blocks on the full pipe before the parent starts draining, and
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
Comment thread
robobun marked this conversation as resolved.
Outdated
`;

test("reading process.stdout / process.stderr leaves fd 1/2 blocking", async () => {
await using proc = spawn({
cmd: [
bunExe(),
"-e",
fcntlPrelude +
`
const before = [nonblock(1), nonblock(2)];
void process.stdout;
void process.stderr;
const after = [nonblock(1), nonblock(2)];
process.stderr.write(JSON.stringify({ before, after }));
`,
],
env: bunEnv,
stdout: "pipe",
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
expect(stdout).toBe("");
expect(JSON.parse(stderr)).toEqual({ before: [false, false], after: [false, false] });
expect(exitCode).toBe(0);
});

test("starting a node:worker_threads Worker leaves fd 1/2 blocking", async () => {
// The worker's stdio rebind must not reify the fd-backed stream
// (JSC defineOwnProperty on a lazy PropertyCallback would run it).
await using proc = spawn({
cmd: [
bunExe(),
"-e",
fcntlPrelude +
`
const { Worker } = require("node:worker_threads");
const before = [nonblock(1), nonblock(2)];
const w = new Worker("setTimeout(() => {}, 0)", { eval: true });
w.on("online", () => {
const after = [nonblock(1), nonblock(2)];
process.stderr.write(JSON.stringify({ before, after }));
w.on("exit", () => {});
});
`,
],
env: bunEnv,
stdout: "pipe",
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
expect(stdout).toBe("");
expect(JSON.parse(stderr)).toEqual({ before: [false, false], after: [false, false] });
expect(exitCode).toBe(0);
});

test("console.log delivers every byte when fd 1 is O_NONBLOCK and the pipe is full", async () => {
// The open file description can be flipped by anything sharing it; the
// native console writer must poll for writability on EAGAIN, not discard
// the unwritten tail.
// stdout: "pipe" pre-drains into the parent, so use a raw pipe(2) whose
// read end only this test drains: the child fills it to EAGAIN, signals
// the byte count on stderr, then console.log()s the markers into the
// still-full pipe; the parent starts draining only after the signal.
const { pipe } = dlopen(libc, {
pipe: { args: [FFIType.ptr], returns: FFIType.int },
}).symbols;
const fds = new Int32Array(2);
expect(pipe(ptr(fds))).toBe(0);
const [r, w] = fds;
try {
await using proc = spawn({
cmd: [
bunExe(),
"-e",
fcntlPrelude +
`
const fs = require("node:fs");
fcntl(1, 4, fcntl(1, 3, 0) | O_NONBLOCK);
const fill = Buffer.alloc(4096, 120);
let filled = 0;
for (let i = 0; i < 1000; i++) {
try { filled += fs.writeSync(1, fill); } catch { break; }
}
fs.writeSync(2, String(filled) + "\\n");
for (let i = 0; i < 10; i++) console.log("marker " + i);
`,
],
env: bunEnv,
stdio: ["ignore", w, "pipe"],
});
closeSync(w);
const reader = proc.stderr.getReader();
const first = await reader.read();
const filled = Number(Buffer.from(first.value).toString().trim());
expect(filled).toBeGreaterThan(0);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
const buf = Buffer.alloc(65536);
let total = Buffer.alloc(0);
for (;;) {
const n = readSync(r, buf);
if (n === 0) break;
total = Buffer.concat([total, buf.subarray(0, n)]);
}
let stderrRest = "";
for (;;) {
const { value, done } = await reader.read();
if (done) break;
stderrRest += Buffer.from(value).toString();
}
const exitCode = await proc.exited;
expect({
stderrRest,
filledOK: total.subarray(0, filled).equals(Buffer.alloc(filled, 120)),
payload: total.subarray(filled).toString(),
}).toEqual({
stderrRest: "",
filledOK: true,
payload: Array.from({ length: 10 }, (_, i) => `marker ${i}\n`).join(""),
});
expect(exitCode).toBe(0);
} finally {
try {
closeSync(r);
} catch {}
}
});
});
});
Loading