Skip to content
Open
Show file tree
Hide file tree
Changes from 9 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
82 changes: 81 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,83 @@ 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);
});
});
Loading