Skip to content

Bun.write: poll instead of spin when a nonblocking stdout pipe returns EAGAIN #35953

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
robobun wants to merge 8 commits into
base: main
Choose a base branch
from
Open

Bun.write: poll instead of spin when a nonblocking stdout pipe returns EAGAIN #35953

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
e2ebe02
Bun.write: poll instead of spin when a nonblocking stdout pipe return…
robobun Sep 5, 2026
2f15c6d
WriteFile: use the duped fd for the readiness check and preallocation…
robobun Sep 5, 2026
085d290
Bun.write: resume the async write at the fast path's offset instead o…
robobun Sep 6, 2026
b4d2b0c
trim comments
robobun Sep 6, 2026
8fd2609
test: report which fd is left open and where the FIFO fixture stalls
robobun Sep 6, 2026
2a65b14
test: record FIFO inodes in the fixture; run it for a seeded and a di…
robobun Sep 6, 2026
9129a14
test: count only dups of stdout's pipe; validate the FIFO byte sequen…
robobun Sep 6, 2026
6371782
ci: retrigger
robobun Sep 6, 2026
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
46 changes: 30 additions & 16 deletions src/runtime/webcore/Blob.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4427,11 +4427,14 @@ fn write_file_with_empty_source_to_destination(
))
}

/// `already_written` is the byte count the synchronous fast path committed to
/// the destination before it fell back; the bytes source resumes after it.
Comment thread
robobun marked this conversation as resolved.
Resolved
pub(crate) fn write_file_with_source_destination(
ctx: &JSGlobalObject,
source_blob: &mut Blob,
destination_blob: &mut Blob,
options: &WriteFileOptions,
already_written: usize,
) -> JsResult<JSValue> {
let destination_store = destination_blob
.store
Expand Down Expand Up @@ -4461,6 +4464,8 @@ pub(crate) fn write_file_with_source_destination(
// `WriteFile::create` takes its own ref.
#[cfg(windows)]
{
// Windows has no synchronous fast path, so nothing was written yet.
debug_assert_eq!(already_written, 0);
let promise = JSPromise::create(ctx);
let promise_value = promise.as_value(ctx);
promise_value.ensure_still_alive();
Expand All @@ -4483,14 +4488,15 @@ pub(crate) fn write_file_with_source_destination(

#[cfg(not(windows))]
{
let file_copier = write_file_mod::WriteFile::create(
let mut file_copier = write_file_mod::WriteFile::create(
destination_blob.borrowed_view(),
source_blob.borrowed_view(),
write_file_promise,
WriteFilePromise::run,
options.mkdirp_if_not_exists.unwrap_or(true),
)
.expect("unreachable");
file_copier.total_written = already_written;
// Defer promise creation until we're just about to schedule the task.
// SAFETY: write_file_promise was just produced by heap::alloc above; sole owner.
unsafe { (*write_file_promise).promise = jsc::JSPromiseStrong::init(ctx) };
Expand Down Expand Up @@ -4781,9 +4787,15 @@ pub(crate) fn write_file_internal(
// This is a heuristic, but it's a good one.
//
// except if you're on Windows. Windows I/O is slower. Let's not even try.
//
// When the fast path hits EAGAIN after a partial write it hands off to the
// async `WriteFile`, which resumes at `already_written` instead of
// re-sending the prefix.
Comment thread
robobun marked this conversation as resolved.
Resolved
#[cfg(windows)]
let already_written: usize = 0;
#[cfg(not(windows))]
{
let mut needs_async = false;
let already_written: usize = {
let mut resume_async_at: Option<usize> = None;
let fast_path_ok = matches!(*path_or_blob, PathOrBlob::Path(_))
|| (matches!(*path_or_blob, PathOrBlob::Blob(ref b)
if b.offset.get() == 0 && !b.is_s3()
Expand All @@ -4810,17 +4822,17 @@ pub(crate) fn write_file_internal(
global_this,
pathlike,
&str,
&mut needs_async,
&mut resume_async_at,
)
} else {
write_string_to_file_fast::<false>(
global_this,
pathlike,
&str,
&mut needs_async,
&mut resume_async_at,
)
};
if !needs_async {
if resume_async_at.is_none() {
return Ok(result);
}
}
Expand All @@ -4841,23 +4853,24 @@ pub(crate) fn write_file_internal(
global_this,
pathlike,
buffer_view.byte_slice(),
&mut needs_async,
&mut resume_async_at,
)
} else {
write_bytes_to_file_fast::<false>(
global_this,
pathlike,
buffer_view.byte_slice(),
&mut needs_async,
&mut resume_async_at,
)
};
if !needs_async {
if resume_async_at.is_none() {
return Ok(result);
}
}
}
}
}
resume_async_at.unwrap_or(0)
};

// if path_or_blob is a path, convert it into a file blob
let mut destination_blob: Blob = match path_or_blob {
Expand Down Expand Up @@ -5134,6 +5147,7 @@ pub(crate) fn write_file_internal(
&mut *source_blob,
&mut destination_blob,
&options,
already_written,
)
}

Expand Down Expand Up @@ -5262,7 +5276,7 @@ fn write_string_to_file_fast<const NEEDS_OPEN: bool>(
global_this: &JSGlobalObject,
pathlike: &PathOrFileDescriptor,
str: &BunString,
needs_async: &mut bool,
resume_async_at: &mut Option<usize>,
) -> JSValue {
let fd: Fd = if !NEEDS_OPEN {
pathlike.fd()
Expand All @@ -5278,7 +5292,7 @@ fn write_string_to_file_fast<const NEEDS_OPEN: bool>(
bun_sys::Result::Ok(result) => result,
bun_sys::Result::Err(err) => {
if err.get_errno() == bun_sys::E::ENOENT {
*needs_async = true;
*resume_async_at = Some(0);
return JSValue::ZERO;
}
return JSPromise::dangerously_create_rejected_promise_value_without_notifying_vm(
Expand Down Expand Up @@ -5321,7 +5335,7 @@ fn write_string_to_file_fast<const NEEDS_OPEN: bool>(
bun_sys::Result::Err(err) => {
truncate.set(false);
if err.get_errno() == bun_sys::E::EAGAIN {
*needs_async = true;
*resume_async_at = Some(written.get());
return JSValue::ZERO;
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
let err_js = if !NEEDS_OPEN {
Expand All @@ -5346,7 +5360,7 @@ fn write_bytes_to_file_fast<const NEEDS_OPEN: bool>(
global_this: &JSGlobalObject,
pathlike: &PathOrFileDescriptor,
bytes: &[u8],
_needs_async: &mut bool,
_resume_async_at: &mut Option<usize>,
) -> JSValue {
let fd: Fd = if !NEEDS_OPEN {
pathlike.fd()
Expand All @@ -5366,7 +5380,7 @@ fn write_bytes_to_file_fast<const NEEDS_OPEN: bool>(
bun_sys::Result::Err(err) => {
#[cfg(not(windows))]
if err.get_errno() == bun_sys::E::ENOENT {
*_needs_async = true;
*_resume_async_at = Some(0);
return JSValue::ZERO;
}
return JSPromise::dangerously_create_rejected_promise_value_without_notifying_vm(
Expand Down Expand Up @@ -5396,7 +5410,7 @@ fn write_bytes_to_file_fast<const NEEDS_OPEN: bool>(
bun_sys::Result::Err(err) => {
#[cfg(not(windows))]
if err.get_errno() == bun_sys::E::EAGAIN {
*_needs_async = true;
*_resume_async_at = Some(written);
return JSValue::ZERO;
}
let err_js = if !NEEDS_OPEN {
Expand Down
70 changes: 29 additions & 41 deletions src/runtime/webcore/blob/read_file.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -538,53 +538,41 @@ impl ReadFile {
read_len: &mut usize,
retry: &mut bool,
) -> bool {
let result: bun_sys::Result<usize> = 'brk: {
if bun_sys::S::ISSOCK(self.file_store.mode) {
break 'brk bun_sys::recv_non_block(self.opened_fd, buf);
}
break 'brk bun_sys::read(self.opened_fd, buf);
let result: bun_sys::Result<usize> = if bun_sys::S::ISSOCK(self.file_store.mode) {
bun_sys::recv_non_block(self.opened_fd, buf)
} else {
bun_sys::read(self.opened_fd, buf)
};

loop {
match &result {
Ok(res) => {
*read_len = *res as usize; // @truncate — usize→usize is identity here
self.read_eof = *res == 0;
match result {
Ok(res) => {
*read_len = res;
self.read_eof = res == 0;
true
}
Err(err) => match err.get_errno() {
e if e == io::RETRY => {
// EAGAIN is impossible on a regular file, so the fd is pollable.
self.could_block = true;
*retry = true;
self.read_eof = false;
true
}
Err(err) => {
match err.get_errno() {
e if e == io::RETRY => {
if !self.could_block {
// regular files cannot use epoll.
// this is fine on kqueue, but not on epoll.
continue;
}
*retry = true;
self.read_eof = false;
return true;
}
_ => {
self.errno = Some(bun_errno::from_errno(err.errno as i32).into());
self.system_error = Some(err.to_system_error().into());
if self.system_error.as_ref().unwrap().path.is_empty() {
self.system_error.as_mut().unwrap().path =
if self.file_store.pathlike.is_path() {
BunString::clone_utf8(
self.file_store.pathlike.path().slice(),
)
} else {
BunString::EMPTY
};
}
return false;
}
_ => {
self.errno = Some(bun_errno::from_errno(err.errno as i32).into());
self.system_error = Some(err.to_system_error().into());
if self.system_error.as_ref().unwrap().path.is_empty() {
self.system_error.as_mut().unwrap().path =
if self.file_store.pathlike.is_path() {
BunString::clone_utf8(self.file_store.pathlike.path().slice())
} else {
BunString::EMPTY
};
}
false
}
}
break;
},
}

true
}

pub(crate) fn then(
Expand Down
87 changes: 51 additions & 36 deletions src/runtime/webcore/blob/write_file.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -341,21 +341,20 @@ impl WriteFile {
//
// On macOS, it is an error to use pwrite() on a
// non-seekable file.
loop {
match sys::write(fd, &self.bytes_blob.shared_view()[off..off + len]) {
Ok(wrote) => {
self.total_written += wrote;
return WriteStep::Wrote(wrote);
}
// regular files cannot use epoll.
// this is fine on kqueue, but not on epoll.
Err(err) if err.get_errno() == io::RETRY && !self.could_block => continue,
Err(err) if err.get_errno() == io::RETRY => return WriteStep::WouldBlock,
Err(err) => {
self.errno = Some(bun_errno::from_errno(err.errno as i32).into());
self.system_error = Some(err.to_system_error().into());
return WriteStep::Failed;
}
match sys::write(fd, &self.bytes_blob.shared_view()[off..off + len]) {
Ok(wrote) => {
self.total_written += wrote;
WriteStep::Wrote(wrote)
}
Err(err) if err.get_errno() == io::RETRY => {
// EAGAIN is impossible on a regular file, so the fd is pollable.
self.could_block = true;
WriteStep::WouldBlock
}
Err(err) => {
self.errno = Some(bun_errno::from_errno(err.errno as i32).into());
self.system_error = Some(err.to_system_error().into());
WriteStep::Failed
}
}
Comment thread
robobun marked this conversation as resolved.
Resolved
}
Expand Down Expand Up @@ -400,15 +399,20 @@ impl WriteFile {

#[cfg(not(windows))]
pub(crate) fn is_allowed_to_close(&self) -> bool {
self.file_blob
match self
.file_blob
.store
.get()
.as_ref()
.unwrap()
.data
.as_file()
.pathlike
.is_path()
{
PathOrFileDescriptor::Path(_) => true,
// opened_fd differs from the caller's fd only when run_with_fd duped it.
PathOrFileDescriptor::Fd(fd) => self.opened_fd != Fd::INVALID && self.opened_fd != fd,
}
}

#[cfg(not(windows))]
Expand All @@ -433,28 +437,38 @@ impl WriteFile {
return;
}

let fd = self.opened_fd;

self.could_block = 'brk: {
if let Some(store) = self.file_blob.store.get().as_ref() {
if let blob::store::Data::File(file) = &store.data {
if file.pathlike.is_fd() {
// If seekable was set, then so was mode
if file.seekable.is_some() {
// This is mostly to handle pipes which were passsed to the process somehow
// such as stderr, stdout. Bun.stdin and Bun.stderr will automatically set `mode` for us.
break 'brk !bun_sys::is_regular_file(file.mode);
}
let caller_supplied_fd = match self.file_blob.store.get().as_ref() {
Some(store) => match &store.data {
blob::store::Data::File(file) => match file.pathlike {
PathOrFileDescriptor::Fd(_) => {
self.could_block = file.mode != 0 && !bun_sys::is_regular_file(file.mode);
true
}
PathOrFileDescriptor::Path(_) => {
// Opened with O_NONBLOCK; don't fstat.
self.could_block = false;
false
}
},
_ => false,
},
None => false,
};

// The IO thread's epoll/kqueue keys interest by fd number, so concurrent WriteFiles on
// one caller-supplied fd would collide; dup so this instance polls a private fd number.
Comment thread
robobun marked this conversation as resolved.
Resolved
if caller_supplied_fd {
match bun_sys::dup(self.opened_fd) {
bun_sys::Result::Ok(duped) => self.opened_fd = duped,
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
bun_sys::Result::Err(err) => {
self.errno = Some(bun_errno::from_errno(err.errno as i32).into());
self.system_error = Some(err.to_system_error().into());
self.on_finish();
return;
}
}
Comment thread
robobun marked this conversation as resolved.
Resolved

// We opened the file descriptor with O_NONBLOCK, so we
// shouldn't have to worry about blocking reads/writes
//
// We do not call fstat() because that is very expensive.
false
};
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
Comment thread
robobun marked this conversation as resolved.
Resolved
let fd = self.opened_fd;

// We have never supported offset in Bun.write().
// and properly adding support means we need to also support it
Expand Down Expand Up @@ -1303,6 +1317,7 @@ impl WriteFileWaitFromLockedValueTask {
mkdirp_if_not_exists: Some(this.mkdirp_if_not_exists),
..Default::default()
},
0,
) {
Ok(p) => p,
Err(err) => {
Expand Down
Loading
Loading