Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
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
2 changes: 2 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,7 @@ impl FileSink {
return sys::Result::Err(err);
}
sys::Result::Ok(()) => {
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
40 changes: 16 additions & 24 deletions src/runtime/webcore/blob/write_file.rs
Original file line number Diff line number Diff line change
Expand Up @@ -338,32 +338,24 @@
//
// On macOS, it is an error to use pwrite() on a
// non-seekable file.
let result: bun_sys::Result<usize> =
sys::write(fd, &self.bytes_blob.shared_view()[off..off + len]);

loop {
match &result {
bun_sys::Result::Ok(res) => {
*wrote = *res;
self.total_written += *res;
}
bun_sys::Result::Err(err) => {
if err.get_errno() == io::RETRY {
if !self.could_block {
// regular files cannot use epoll.
// this is fine on kqueue, but not on epoll.
continue;
}
self.wait_for_writable();
return false;
} else {
self.errno = Some(bun_errno::from_errno(err.errno as i32).into());
self.system_error = Some(err.to_system_error().into());
return false;
}
match sys::write(fd, &self.bytes_blob.shared_view()[off..off + len]) {
bun_sys::Result::Ok(res) => {
*wrote = res;
self.total_written += res;
}
bun_sys::Result::Err(err) => {
if err.get_errno() == io::RETRY {
// Regular files never return EAGAIN, so receiving it here
// means the fd is pollable regardless of what `could_block`
// inferred from the store.
Comment thread
robobun marked this conversation as resolved.
Resolved
self.could_block = true;
self.wait_for_writable();
return false;
}

Check warning on line 354 in src/runtime/webcore/blob/write_file.rs

View check run for this annotation

Claude / Claude Code Review

Sibling infinite-loop pattern left unfixed in ReadFile::do_read

The identical `loop { match &result { ... continue; ... } }` pattern this PR removes here also lives in the read twin at `src/runtime/webcore/blob/read_file.rs:510-556` — `result` is bound once at 511-516 and the `if !self.could_block { continue; }` arm at line 530 re-matches the same stale `Err(EAGAIN)` forever. Per REVIEW.md's "fix the whole class in the same PR" rule (which explicitly names sync/async twins and copy-pasted blocks), consider applying the same rewrite there — drop the loop and
Comment thread
claude[bot] marked this conversation as resolved.
Resolved
self.errno = Some(bun_errno::from_errno(err.errno as i32).into());
self.system_error = Some(err.to_system_error().into());
return false;
}
break;
}

true
Expand Down
90 changes: 90 additions & 0 deletions test/js/bun/util/filesink.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -622,3 +622,93 @@
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
expect({ stdout: stdout.trim(), stderr, exitCode }).toEqual({ stdout: "ok", stderr: "", exitCode: 0 });
});

// Accessing process.stdout creates a FileSink on a dup of fd 1. openForWriting
// sets O_NONBLOCK on that dup, which also flips fd 1 itself (shared open file
// description). The sink is then forced into synchronous mode, which is meant
// to clear O_NONBLOCK again, but FileSink.setup() never recorded the fd so the
// undo was a no-op. With fd 1 left non-blocking, console.log silently dropped
// bytes on EAGAIN and Bun.write(Bun.stdout, ...) spun forever in WriteFile.
describe.concurrent("process.stdout leaves fd 1 blocking on a pipe", () => {
it.skipIf(!isLinux)("O_NONBLOCK is cleared after process.stdout is materialized", async () => {
await using proc = Bun.spawn({
cmd: [
bunExe(),
"-e",
`
process.stdout.write("x");
const fs = require("fs");
const fdinfo = fs.readFileSync("/proc/self/fdinfo/1", "utf8");
const flags = parseInt(fdinfo.match(/flags:\\s*(\\d+)/)[1], 8);
process.stderr.write((flags & 0o4000) ? "nonblock" : "blocking");
`,
],
env: bunEnv,
stdout: "pipe",
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
expect(stdout).toBe("x");
expect(stderr).toBe("blocking");
expect(exitCode).toBe(0);
});

it.skipIf(isWindows)("Bun.write(Bun.stdout, ...) completes after process.stdout is materialized", async () => {
const SIZE = 1024 * 1024;
await using proc = Bun.spawn({
cmd: [
bunExe(),
"-e",
`
const _ = process.stdout;
await Bun.write(Bun.stdout, Buffer.alloc(${SIZE}, "A").toString());
process.stderr.write("done");
`,
],
env: bunEnv,
stdout: "pipe",
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
expect(stderr).toBe("done");
expect(stdout.length).toBe(SIZE);
expect(exitCode).toBe(0);
});

it.skipIf(isWindows)(
"console.log / process.stdout.write / Bun.write(Bun.stdout) interleaved deliver every line",
async () => {
const N = 200;
const PAD = 512;
await using proc = Bun.spawn({
cmd: [
bunExe(),
"-e",
`
const pad = Buffer.alloc(${PAD}, "x").toString();
for (let i = 0; i < ${N}; i++) {
console.log("A" + i + " " + pad);
process.stdout.write("B" + i + " " + pad + "\\n");
Bun.write(Bun.stdout, "C" + i + " " + pad + "\\n");
}

Check failure on line 693 in test/js/bun/util/filesink.test.ts

View check run for this annotation

Claude / Claude Code Review

Interleaved-stdout test can flake on macOS: lines exceed PIPE_BUF while multiple threads write fd 1

The interleaved-stdout test can flake on macOS: each line is 516–518 bytes (> macOS `PIPE_BUF=512`), and the un-awaited `Bun.write(Bun.stdout, ...)` runs `sys::write(1, ...)` on a WorkPool thread concurrently with the main thread's next-iteration `console.log`/`process.stdout.write` on the same fd. POSIX permits >PIPE_BUF writes from concurrent writers to interleave on arbitrary byte boundaries, so a line can be split across another's newline and the A/B/C prefix counts drop below `N`. Fix: shri
Comment thread
claude[bot] marked this conversation as resolved.
Resolved
await Bun.write(Bun.stdout, "");
process.stderr.write("done");
`,
],
env: bunEnv,
stdout: "pipe",
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
expect(stderr).toBe("done");
const lines = stdout.split("\n").filter(Boolean);
expect({
A: lines.filter(l => l.startsWith("A")).length,
B: lines.filter(l => l.startsWith("B")).length,
C: lines.filter(l => l.startsWith("C")).length,
total: lines.length,
}).toEqual({ A: N, B: N, C: N, total: 3 * N });
expect(exitCode).toBe(0);
},
);
});
Loading