Skip to content
Open
Show file tree
Hide file tree
Changes from 11 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
9 changes: 6 additions & 3 deletions src/runtime/webcore/FileSink.rs
Original file line number Diff line number Diff line change
Expand Up @@ -239,9 +239,12 @@
{
this.force_sync.set(true);
// SAFETY(JsCell): single-field write; does not call into JS.
this.writer.with_mut(|w| w.force_sync = true);
if this.fd.get() != Fd::INVALID {
let _ = sys::update_nonblocking(this.fd.get(), false);
let fd = this.writer.with_mut(|w| {
w.force_sync = true;
w.get_fd()

Check failure on line 244 in src/runtime/webcore/FileSink.rs

View workflow job for this annotation

GitHub Actions / cargo clippy

method `get_fd` is private
});
if fd != Fd::INVALID {
let _ = sys::update_nonblocking(fd, false);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
Comment thread
robobun marked this conversation as resolved.
Outdated
}
}
#[cfg(windows)]
Expand Down
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 @@ fn fd_write_all_quiet(fd: Fd, mut bytes: &[u8]) -> bool {
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;
}
}
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
101 changes: 100 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,102 @@ describe.concurrent("process-stdio", () => {
);
});
});

// Materializing process.stdout dups fd 1 and flips it to O_NONBLOCK. The native
// console writer must not silently drop data on the resulting EAGAIN when the
// pipe is full. Kept outside the concurrent block so the extra spawned children
// don't push the already-slow stdin tests past their default timeout.
describe.skipIf(isWindows)("console.log after process.stdout is materialized on a pipe", () => {
test("many lines survive a slow reader", async () => {
const N = 1500;
const pad = Buffer.alloc(500, "x").toString();
using dir = tempDir("stdout-nonblock-loss", {
"child.mjs": `
if (process.argv[2] === "touch") void process.stdout.writableHighWaterMark;
const pad = ${JSON.stringify(pad)};
for (let i = 0; i < ${N}; i++) console.log("O" + i + " " + pad);
`,
});
// A separate `cat` reader starts 400ms late behind a shell fifo, so the
// 64 KiB pipe fills and write(2) on the now-nonblocking fd 1 returns EAGAIN
// mid-run. The pipeline's exit status is cat's, not bun's, so the delivered
// count is what proves the regression is gone.
await using proc = Bun.spawn({
cmd: [
"/bin/sh",
"-c",
'exec "$0" "$1" touch | { sleep 0.4; exec cat; }',
bunExe(),
path.join(String(dir), "child.mjs"),
],
env: bunEnv,
stdout: "pipe",
stderr: "inherit",
});
const stdout = await proc.stdout.text();
const delivered = stdout.split("\n").filter(l => /^O\d+ x+$/.test(l)).length;
expect(delivered).toBe(N);
});

test("a single 1 MiB line is not truncated", async () => {
// A single 1 MiB write into even a fast `| wc -c` reader exceeds the 64 KiB
// pipe, so write(2) on the now-nonblocking fd 1 returns a partial count and
// then EAGAIN before the reader drains.
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",
});
const stdout = (await proc.stdout.text()).trim();
expect(Number(stdout)).toBe((1 << 20) + 1);
});

test("parent console.log survives a bun child that touches inherited stdout", async () => {
Comment thread
robobun marked this conversation as resolved.
Outdated
// O_NONBLOCK lives on the shared open file description. A bun child with
// stdio:'inherit' that materializes its process.stdout flips the PARENT's
// fd 1 too; the parent's console writer must still deliver every line.
using dir = tempDir("stdout-nonblock-child-inherit", {
"parent.mjs": `
const { spawnSync } = require("node:child_process");
const r = spawnSync(process.execPath, ["-e", 'process.stdout.write("")'],
{ stdio: ["ignore", "inherit", "ignore"] });
if (r.error || r.status !== 0) throw new Error("child failed: " + (r.error ?? r.status));
const pad = Buffer.alloc(190, 120).toString();
for (let i = 0; i < 20000; i++) console.log("O" + i + " " + pad);
`,
});
await using proc = Bun.spawn({
cmd: ["/bin/sh", "-c", 'exec "$0" "$1" | { sleep 1; wc -l; }', bunExe(), path.join(String(dir), "parent.mjs")],
Comment thread
robobun marked this conversation as resolved.
Outdated
env: bunEnv,
stdout: "pipe",
stderr: "inherit",
});
const stdout = (await proc.stdout.text()).trim();
expect(Number(stdout)).toBe(20000);
});

test("console.log survives a raw O_NONBLOCK on fd 1 (isolates the writer)", async () => {
// Bun.file(1).writer() goes through FileSink.setup (dup + O_NONBLOCK) but is
// NOT the process.stdout getter path, so ForceFileSinkToBeSynchronous never
// clears the flag. This isolates fd_write_all_quiet's EAGAIN handling.
await using proc = Bun.spawn({
cmd: [
"/bin/sh",
"-c",
'exec "$0" -e "void Bun.file(1).writer(); console.log(Buffer.alloc(1<<20, 65).toString())" | { sleep 0.4; exec wc -c; }',
bunExe(),
],
env: bunEnv,
stdout: "pipe",
stderr: "inherit",
});
const stdout = (await proc.stdout.text()).trim();
expect(Number(stdout)).toBe((1 << 20) + 1);
});
});
Loading