Skip to content
Closed
Show file tree
Hide file tree
Changes from 3 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
35 changes: 11 additions & 24 deletions src/js/node/worker_threads.ts
Original file line number Diff line number Diff line change
Expand Up @@ -446,37 +446,24 @@ function makePortWritable(port) {

function setupWorkerStdio(stdio) {
const { stdin, stdout, stderr } = stdio;
// Plain assignment: defineProperty would reify the lazy fd-backed stdio
// (JSC reifies a static PropertyCallback before defining over it).
Comment thread
robobun marked this conversation as resolved.
Outdated
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);
},
});
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
// 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 @@ describe.concurrent("process-stdio", () => {
`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() : "";
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;
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