Skip to content
Open
Show file tree
Hide file tree
Changes from all 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
15 changes: 15 additions & 0 deletions src/sys/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9410,6 +9410,21 @@
match write(fd, bytes) {
Ok(0) => return false, // short write → give up
Ok(n) => bytes = &bytes[n..],
#[cfg(unix)]
Err(e) if e.get_errno() == E::EINTR => continue,
#[cfg(unix)]
Err(e) if e.get_errno() == E::EAGAIN => {
// fd 1/2 may be O_NONBLOCK once `process.stdout`/`stderr` is
// materialized; block until writable instead of dropping bytes.
Comment thread
robobun marked this conversation as resolved.
let mut pfd = [posix::PollFd {
fd: fd.native() as core::ffi::c_int,
events: posix::POLL_OUT,
revents: 0,
}];
if posix::poll(&mut pfd, -1).is_err() {
return false;
}
}

Check warning on line 9427 in src/sys/lib.rs

View check run for this annotation

Claude / Claude Code Review

PR title/description still describe the dropped FileSink fix

The title and description still describe the two-part fix, but commit 7d5bb800 dropped the `FileSink.rs` hunk per @alii's review — only the `fd_write_all_quiet` EAGAIN retry ships now. The O_NONBLOCK leak onto fd 1 (and the non-bun-child-inherit case alii cited) is *not* fixed by what remains; only bun's own console writer is now tolerant of it. Please retitle to just the "make console writer EAGAIN-safe" half and drop the "Two parts" / FileSink paragraph from the description before merge (CLAUD
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Comment thread
robobun marked this conversation as resolved.
Comment thread
robobun marked this conversation as resolved.
Err(_) => return false,
}
}
Expand Down
92 changes: 91 additions & 1 deletion test/js/node/process/process-stdio.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { spawn, spawnSync } from "bun";
import { describe, expect, test } from "bun:test";
import { bunEnv, bunExe } from "harness";
import { bunEnv, bunExe, isWindows, tempDir } from "harness";
import path from "path";
import { isatty } from "tty";
describe.concurrent("process-stdio", () => {
Expand Down Expand Up @@ -159,3 +159,93 @@ describe.concurrent("process-stdio", () => {
);
});
});

// Materializing process.stdout / process.stderr puts O_NONBLOCK on the shared
// open file description of fd 1 / fd 2. The native console writer behind
// console.log / console.error must then retry on EAGAIN instead of dropping the
// unwritten tail. A single 1 MiB write is larger than any pipe buffer, so the
// first write(2) is always short and the next one always sees EAGAIN: the
// unfixed binary delivers exactly one pipe buffer no matter how fast the reader
// is, which keeps these deterministic without a sleeping reader. Kept outside the
// concurrent block above so the extra children don't push the stdin tests past
// their timeout.
describe.skipIf(isWindows)("console output is not truncated once the stdio fd is nonblocking", () => {
const ONE_MIB_LINE = (1 << 20) + 1;

test("console.log after touching process.stdout", async () => {
await using proc = Bun.spawn({
cmd: [
"/bin/sh",
"-c",
'exec "$0" -e "void process.stdout.isTTY; console.log(Buffer.alloc(1<<20, 65).toString())" | wc -c',
bunExe(),
],
env: bunEnv,
stdout: "pipe",
stderr: "inherit",
});
expect(Number((await proc.stdout.text()).trim())).toBe(ONE_MIB_LINE);
});

test("console.error after touching process.stderr", async () => {
await using proc = Bun.spawn({
cmd: [
"/bin/sh",
"-c",
'exec "$0" -e "void process.stderr.isTTY; console.error(Buffer.alloc(1<<20, 66).toString())" 2>&1 >/dev/null | wc -c',
bunExe(),
],
env: bunEnv,
stdout: "pipe",
stderr: "inherit",
});
expect(Number((await proc.stdout.text()).trim())).toBe(ONE_MIB_LINE);
});

test("console.log in a parent after a bun child touched the inherited stdout", async () => {
// The flag lives on the open file description, so a child with
// stdio: 'inherit' that touches its process.stdout flips the parent's fd 1
// too, even though the parent never touched its own stream.
using dir = tempDir("stdout-nonblock-inherited", {
"parent.ts": `
const r = Bun.spawnSync({
cmd: [process.execPath, "-e", 'process.stdout.write("")'],
stdout: "inherit",
});
if (!r.success) throw new Error("child failed: " + r.exitCode);
console.log(Buffer.alloc(1 << 20, 65).toString());
`,
});
await using proc = Bun.spawn({
cmd: ["/bin/sh", "-c", 'exec "$0" "$1" | wc -c', bunExe(), path.join(String(dir), "parent.ts")],
env: bunEnv,
stdout: "pipe",
stderr: "inherit",
});
expect(Number((await proc.stdout.text()).trim())).toBe(ONE_MIB_LINE);
});

test("process.stdout.write to a pipe stays asynchronous", async () => {
// Pins the contract the console-writer fix must not disturb: like node, a
// write larger than the pipe returns false immediately and emits 'drain'
// once the reader catches up, rather than blocking the JS thread.
await using proc = Bun.spawn({
cmd: [
bunExe(),
"-e",
`
const ret = process.stdout.write(Buffer.alloc(1 << 20, 65));
process.stdout.once("drain", () => {
process.stderr.write(JSON.stringify({ ret, drained: true }));
});
`,
],
env: bunEnv,
stdout: "pipe",
stderr: "pipe",
});
const [stdout, stderr] = await Promise.all([proc.stdout.bytes(), proc.stderr.text()]);
expect(stdout.byteLength).toBe(1 << 20);
expect(JSON.parse(stderr)).toEqual({ ret: false, drained: true });
});
});