Skip to content

node:fs, Bun.write, fetch: retry EINTR in the bun_sys wrappers instead of surfacing it - #45929

Open
robobun wants to merge 11 commits into
mainfrom
robobun/d6a78666/node-fs-eintr-retry
Open

robobun wants to merge 11 commits into
mainfrom
robobun/d6a78666/node-fs-eintr-retry

Conversation

@robobun

@robobun robobun commented Oct 18, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

  • Some calls throw EINTR: interrupted system call, unlink '<path>' (or their own syscall) when a signal interrupts them. They are fs.unlink, rmdir, link, truncate, fsync, fdatasync, mkdtemp, cp, Bun.write(file, file) and fetch with a Bun.file body. existsSync answers false, Bun.which answers null.
  • The cause: errno_sys* (src/runtime/node/node_fs.rs:33) maps one finished return code to an error. libuv and bun_sys call again.

Fix

  • bun_sys gains wrappers that retry EINTR. NodeFS, Bun.write, fetch and Bun.serve call them. errno_sys* is deleted.
  • The layer that issues the syscall issues it again. Other errors keep their shape on Linux (39 of 39 rows).
  • Verified: test/js/node/fs/fs-eintr-syscalls.test.ts. Its 4 Linux tests fail on main. Its 2 macOS tests pass on the darwin lanes. The FreeBSD arm has not run.
  • Self-reviewed twice: 58 concerns raised, 53 addressed. Rejected (reasons in Notes): a copy-loop helper, two real-signal tests, an EAGAIN issue, a FreeBSD run.

Background

  • EINTR: a signal handler ran while the syscall waited (FUSE, NFS, a file lease, a pipe).
  • Considered an EINTR arm per site. The next site that maps a return code has the bug again.

Downsides

  • An errno outside bun's table (524) reports err.errno -524 (was -134) on 8 calls. Every bun_sys wrapper reports it with code: "EUNKNOWN".
  • Instructions per call: +1 to +12 on 7 of 24 rows (Notes). The other 17 rows, mallocs and syscalls are equal or lower.
  • macOS: Bun.write reports EINTR when fcopyfile stops mid-copy (main calls again and truncates the file). A fetch upload changes in three results besides EINTR (Notes).
Notes

No GitHub issue: a shim audit found this, no user reported it. The closed precedent is #41085, fixed by #41086, which put the retry in the bun_sys wrappers.

Who can hit it

  • The handlers of process.on and the SIGCHLD handler have SA_RESTART (BunProcess.cpp, spawn/process.rs). The SIGINT handlers of the REPL and of vm with breakOnSigint do not (repl.rs, SigintWatcher.cpp). Native code in the process (an addon, bun:ffi, a preloaded library) can install a handler without it.
  • With such a handler, a copy on a local file system returns EINTR. The run: siginterrupt(SIGUSR1, 1) through bun:ffi, a storm of SIGUSR1 from a child, 40 copies of a 32 MB file on tmpfs. On 1.4.3-canary (367d939), cpSync throws EINTR 40 of 40 times, Bun.write(file, file) 2 of 40, and fetch with a Bun.file body rejects 3 of 40 uploads. On this branch all three are 0 of 40. With SA_RESTART kept, both builds are 0 of 40.
  • truncate behind a file lease and Bun.write between FIFOs also return EINTR from a real signal. The other calls need a file system that waits (a network mount, a FUSE daemon).

Messages on main

  • EINTR: interrupted system call, unlink '<path>', and the same with rmdir, truncate, mkdtemp, fsync, fdatasync, link '<from>' -> '<to>'
  • EINTR: interrupted system call, copy_file_range '<dest>' from fs.cp
  • EINTR: interrupted system call, copy_file_range (or sendfile, splice) from Bun.write
  • EINTR fetching "<url>" from fetch
  • No message: one interrupted FICLONE in copyFileSync or cp turns clones off for the rest of the process. The clone rows of the test show it.

What changed

  • src/sys/Error.rs (own commit): Error::get_error_code_tag_name names an errno that the table does not declare EUNKNOWN. This is the hunk that Remove libuv on Windows #42819 carries.
  • src/sys/lib.rs:
    • new, on the check! loop: link, truncate, posix_rmdir, unlink_without_path, copy_file_range, splice, ioctl_ficlone
    • new, with their own loop: fdatasync (the error carries the fd), mkdtemp (writes the XXXXXX back before it calls again, because glibc leaves its candidate name in the template)
    • sendfile takes an offset. macOS and FreeBSD get a sendfile of the same shape: EINTR with no bytes sent calls again, and EAGAIN or EINTR after some bytes is a short count. libuv has the same rule (uv__fs_sendfile in src/unix/fs.c).
    • exists_z and exists_os_path become exists_at. Its Linux arm is the raw faccessat, which retries. Its other arm now loops.
    • lstatat shares fstatat_with_flags. realpath loops.
    • deleted: c::clonefile_rc, c::copyfile_rc, the c::truncate re-export, c::sendfile (macOS, FreeBSD), linux::sendfile, linux_syscall::sendfile, err_with_path. linux::copy_file_range and linux::ioctl_ficlone are private to the crate.
  • src/runtime/node/node_fs.rs: all 21 errno_sys* sites and mkdtemp call the wrappers. The trait is deleted. errno_sys lines: 32 to 0.
  • src/runtime/webcore/blob/copy_file.rs: the three raw calls of do_copy_file_range (copy_file_range, sendfile, splice).
  • src/http/SendFile.rs: one body for Linux, macOS and FreeBSD on bun_sys::sendfile. Before, only Bun.serve retried, and fetch rejected on all three.
  • src/runtime/server/FileResponseStream.rs, src/sys/copy_file.rs: bun_sys::sendfile.
  • src/jsc/bindings/c-bindings.cpp: is_executable_file calls stat or open again on EINTR.
  • Raw libc fs calls outside src/sys (unlink, rmdir, link, truncate, mkdtemp, access, fsync, fdatasync, splice): 7 to 1. Uses of the raw thunks outside src/sys: 24 to 0. Source: 10 files, +441/-743.

The wrappers that take a path and put none on the error (link, truncate, posix_rmdir, unlink_without_path) follow statx and lstatx: node:fs reports the path its caller passed, so it boxes that path once, as before. With the path on the wrapper's error it would be boxed twice.

fcopyfile

Apple's copyfile.c copies from the file positions of both fds and ends copyfile_data with ftruncate(dst_fd, totalCopied), where totalCopied counts one call. A second call after a partial copy copies the rest and truncates the destination to the length of the rest. bun_sys::fcopyfile has called again on every EINTR since #41086. It now calls again only while both positions are at 0. bun_sys::copy_file used its own single call and now uses the wrapper.

fetch uploads on macOS and FreeBSD

A review found that only the Linux arm of SendFile::write used the wrapper. The three arms are now one body on bun_sys::sendfile.

  • macOS: XNU returns EINTR from sendfile when the read of the file is interrupted before it read a byte, or when the wait for the send-buffer lock is interrupted (bsd/kern/uipc_syscalls.c). The read waits on NFS, SMB, FUSE or a dataless file. A local APFS file does not wait. On main the upload then rejects with EINTR fetching "<url>".
  • FreeBSD: by a reading of sys/kern/kern_sendfile.c, no path returns EINTR for the call that fetch makes (non-blocking socket, no flags). The FreeBSD arm is a parity change. It is compiled for x86_64-unknown-freebsd and has not run. No CI lane runs FreeBSD tests.
  • Three results besides EINTR change on macOS and FreeBSD, each to the Linux rule:
    1. A count of 0 sends nothing and issues no call. Main passes 0, which these kernels read as "the rest of the file", so an empty slice of a large file puts file bytes after Content-Length: 0. fetch: send an empty Bun.file() slice without sendfile and close its file #44896 fixes the same case in fetch.rs.
    2. A call that succeeds with fewer bytes than asked (the file got shorter) is followed by one more call, which sends 0 and ends the upload. Main ended at once.
    3. macOS: EAGAIN after the whole count was sent ends the upload. Main rejects with EAGAIN fetching "<url>" although every byte is queued. XNU tests the socket space before it tests the count.
  • Rows: the macOS test has a row for EINTR before a byte, EINTR after 4096 bytes, result 3 (the shim reports EAGAIN with the whole count) and result 1 (an empty slice, with an empty shim log). Result 2 has no row.
  • Not changed: a call that sends nothing and reports EAGAIN still fails the request, on all three systems. It is not EINTR. fetch: fall back to read+write when sendfile(2) refuses a Bun.file() body #41494 has an EAGAIN arm for the Linux arm. With the single body that arm covers all three.
  • Not changed: how XNU's sendfile waits for mbufs (Bun.serve: stop using sendfile(2) for file responses on macOS #33728). This PR takes no position on fetch: stop using sendfile(2) for file request bodies on macOS #41905, which removes the macOS call from fetch.

Node

  • Node v26.3.0 under the same shim completes unlink, rmdir, link, fsync, fdatasync and existsSync.
  • Node does not retry everywhere. Its cpSync over an existing file is std::filesystem::copy_file (CpSyncOverrideFile in src/node_file.cc), not a libuv request.
  • mkdtemp: libuv calls mkdtemp again on the same buffer. After a glibc call that consumed the XXXXXX, the second call answers EINVAL. Bun writes the XXXXXX back. If the first call did create a directory, the second creates another.
  • macOS fsync: libuv issues F_FULLFSYNC. bun_sys::fsync issues fsync(2). Not changed here (node:fs: run appendFile through writeFile's write path and report the { flush: true } fsync error #36485 lists it).

Error shape for an errno outside the table (524 through the shim, main to this branch)

  • fsyncSync, fdatasyncSync, unlinkSync, rmdirSync, linkSync, truncateSync, cpSync, copyFileSync: errno -134 to -524. code stays EUNKNOWN.
  • Calls that were on bun_sys wrappers (accessSync, mkdtempSync, and the rest): code absent to EUNKNOWN, message Unknown Error, x to EUNKNOWN: unknown error, x.
  • Bun.write(file, file): no change (-134).
  • Other consumers of the name: Error::name() and Output::err print EUNKNOWN where they printed UNKNOWN. Log::add_sys_error gains the prefix. bun lint (src/lint/driver/discover.rs:196) reports such a stat error where it read it as "nothing there". The shell cd message is unknown error. No change: Listener.rs:376, shell/Builtin.rs:1007.

Measurements

Release builds of main 017b243 and of this change on top of it. perf, strace and valgrind are not installed here, so gdb single-steps the call from the entry of its host function to its return, after 30 calls. It counts instructions, syscall instructions and calls of the mimalloc entry points. 5 calls per row, minimum shown.

call main branch
unlinkSync 772 750
unlinkSync ENOENT 6428 6427
rmdirSync 849 827
rmdirSync ENOENT 6191 6190
linkSync 1575 1530
linkSync EEXIST 7710 7697
truncateSync 964 944
truncateSync ENOENT 6586 6586
fsyncSync 203 203
fsyncSync EBADF 5116 5111
fdatasyncSync 203 206
fdatasyncSync EBADF 5440 5438
mkdtempSync 1448 1439
mkdtempSync ENOENT 6767 6779
existsSync hit 769 770
existsSync miss 803 800
accessSync (control) 832 832
copyFileSync 64 KiB 1838 1826
cpSync 64 KiB 1881 1883
Bun.write(file, file) copy loop 77 70
Bun.write(device, file) copy loop 74 65
Bun.which hit 622 624
Bun.which miss 484 489
fetch upload, one SendFile::write 57 68
  • mimalloc calls per call: equal on all 24 rows (6 mallocs and 1 free for a failed unlinkSync on both).
  • Syscalls per call: equal on all rows. existsSync issues faccessat(2) where it issued access(2).
  • size of the release binary, text: 106,738,870 to 106,738,102 bytes.
  • Errors other than EINTR (ENOENT, EEXIST, ENOTDIR, EISDIR, ENOTEMPTY, EBADF, sync, callback and promise forms, Bun.write, Bun.file().unlink()): 39 of 39 rows print the same message, code, errno, syscall, path, dest and fd on both builds.

Test

A preloaded shim (LD_PRELOAD on Linux, DYLD_INSERT_LIBRARIES on macOS) fails the first call of a libc function with an errno. The directory of a row names the function and the errno. The shim logs each call it fails, and a row reports that log, so a pass cannot come from a shim that did not fire.

  • existsSync on Linux: main calls libc access, the shim fails it, the answer is false. The branch issues the raw faccessat, which no preloaded library can fail. The row asserts the answer and an empty log. accessSync is the control that the access hook fires. The retry of the raw call is linux_syscall::retry, the same loop as open and stat.
  • The FICLONE rows: the shim fails the first clone of a row and answers the next one with success. It clones nothing. Each later row shows that the clone before it did not turn clones off for the process.
  • The two macOS tests pass on the darwin lanes of this PR (aarch64 and x64, at da658dc): 2 pass, 4 skip. They did not run on main.
  • The upload rows send to a socket that keeps every byte of the request. A row passes only when Content-Length and the bytes after the head are the file or the slice. In one Linux row the shim cuts each sendfile to 7777 bytes, so that SendFile::write runs about 30 times.
  • Not covered by a row: lstatat and is_executable_file on macOS, the FICLONE probe of bun_sys::copy_file, Bun.serve file responses (same wrapper as the fetch row, and the existing suite).
  • Rows on main that fail: 12 of 13 call rows, 8 of 10 copy rows, 4 of 5 clone rows, 2 of 6 error rows. The other rows are controls. The three upload rows that came later ran on 1.4.3-canary (367d939).
  • The shim is compiled while the tests are collected, as fs-stat-seccomp-linux.test.ts does. A compiler that is not in the page cache takes seconds to start, and a test has 5 seconds.
  • The Linux tests run one at a time. Four debug builds that start together each take 2.5 to 4 seconds on a loaded machine. One at a time, each test takes about 1 second.

Suites run on the debug build of this branch, after the rebase on 25ef693:

  • Pass: fs-eintr-syscalls.test.ts, cp.test.ts, fs-stat-seccomp-linux.test.ts, bun-serve-file.test.ts, fetch-file-upload.test.ts, which.test.ts, bun-file.test.ts, init.test.ts, the 250 node test-fs-* files (parallel and sequential) and the 14 node test-compile-cache-* files.
  • fs.test.ts (609 pass, 8 fail), bun-write.test.js (105 pass, 1 fail) and glob/scan.test.ts (200 pass, 6 fail): each failure is a time-out of the debug build on this machine. They are the scans of the whole repository and the tests that queue behind them. The rmdir, opendir, createReadStream and Bun.file(0) tests among them pass when run alone.
  • cargo check --workspace passes for linux gnu and musl, android, darwin x64 and arm64, freebsd and windows.
  • After the follow-up commits (the macOS and FreeBSD sendfile, the upload rows): fs-eintr-syscalls.test.ts (4 pass, 2 skip), fetch-file-upload.test.ts, cp.test.ts, fs-stat-seccomp-linux.test.ts and bun-serve-file.test.ts pass on the debug build. cargo check --workspace passes for darwin arm64 and freebsd, and cargo check -p bun_sys -p bun_http for linux gnu and musl, android, darwin x64 and windows.
  • After the merge of main at 0793250 (one conflict, in the macOS clone of the async cp walk): the same five suites and shell/commands/cp.test.ts pass on the debug build, and cargo check --workspace passes for darwin arm64, freebsd and windows.

Not in scope, each still single-shot

Open pull requests this touches (git merge-tree against main and this branch)

Questions for @Jarred-Sumner and @dylan-conway: is the retry right for the four wrappers of #40212, and can the Error.rs hunk of #42819 land here? The first commit holds only that hunk and can move to a PR of its own. One more for @Jarred-Sumner: does fetch keep sendfile(2) on macOS (#41905)? If not, the macOS block of the wrapper and its rows go, and the FreeBSD arm and the single body stay.

Self-review, rejected

  • One helper for the two Linux copy loops (copy_file_inner, copy_single_file_sync). It is a refactor of code that four open PRs rewrite, and Consolidate node fs/path/types helpers #32021 carries it. Both loops now call the same wrapper.
  • A real-signal test for splice (it finds the blocked thread in /proc/self/task and signals it) and one for truncate behind a file lease (it needs lease support on the CI file system, and the kernel waits up to 45 s when a step goes wrong). The shim rows for splice and truncate pin the same retry.
  • An issue for the upload that fails when sendfile sends nothing and reports EAGAIN. It is a separate bug, and fetch: fall back to read+write when sendfile(2) refuses a Bun.file() body #41494 is open for its Linux arm. This PR names it above.
  • A run of the FreeBSD arm in a FreeBSD VM (the freebsd-smoke workflow on a probe branch). Not done. The arm is compiled only, and the description says so.

Credits: the Error.rs hunk and the wrapper calls of unlink and fsync are from #42819. The wrapper names are from #40212.

robobun and others added 5 commits October 18, 2026 11:56
…ages

The kernel can return an errno that bun's table does not declare (ENOTSUPP
524, or a code a FUSE daemon replies with). Error::get_errno already decodes
such a number as EUNKNOWN. Error::get_error_code_tag_name returned None for
it, so a JS error had no code and the message "Unknown Error".

It now names the number EUNKNOWN. The errno in the error stays the number
that the kernel returned.

Co-authored-by: Dylan Conway <dylan.conway567@gmail.com>
…nc, mkdtemp, copy_file_range, splice and FICLONE

bun_sys has no retrying form of these calls, so node:fs and Bun.write issue
them with libc directly and finish on the first EINTR.

- link, truncate, posix_rmdir and unlink_without_path leave the path off
  the error. node:fs reports the path its caller passed.
- mkdtemp writes the XXXXXX back before it calls again. libc can leave its
  candidate name in the template when the call fails.
- copy_file_range, splice and ioctl_ficlone are the retrying forms of the
  raw Linux calls. bun_sys::copy_file now uses them.
- exists_z and exists_os_path become exists_at on the working directory.
  Its Linux arm already retries. The other arm now loops on EINTR. Before,
  an interrupted access() read as "does not exist".
- clonefile on macOS takes the flags.

Co-authored-by: Jarred Sumner <jarred@jarredsumner.com>
…sync, mkdtemp and the copy syscalls through bun_sys

On POSIX these calls went to libc directly and mapped the one return code
with errno_sys*, so a signal that interrupted the syscall surfaced as EINTR
in JavaScript. libuv runs an fs request again on EINTR.

- NodeFS::unlink, rmdir, link, truncate, fsync, fdatasync and mkdtemp call
  the bun_sys wrappers and attach the caller's path as before.
- The copy_file_range loops of copyFile and cp, the forced and the
  opportunistic FICLONE, and Bun.write(file, file) (copy_file_range,
  sendfile, splice) call the retrying wrappers.
- The macOS clonefile and copyfile sites call bun_sys::clonefile and
  bun_sys::copyfile.
- errno_sys*, clonefile_rc, copyfile_rc and the c::truncate re-export have
  no caller left and are removed. linux::copy_file_range and
  linux::ioctl_ficlone are private to bun_sys.

Co-authored-by: Dylan Conway <dylan.conway567@gmail.com>
Co-authored-by: Jarred Sumner <jarred@jarredsumner.com>
…utable_file, and give fcopyfile one rule

More calls of the same class finish on the first EINTR:

- fetch with a Bun.file body issues the raw sendfile thunk and rejects with
  "EINTR fetching". bun_sys::sendfile now takes an offset, and fetch,
  Bun.serve and bun_sys::copy_file send files through it. The raw
  linux::sendfile thunk has no caller left and is removed.
- is_executable_file, the probe behind Bun.which and spawn, reads an
  interrupted stat or open as "not executable". It now calls again.
- lstatat outside Linux is a single fstatat. It now shares the loop of
  fstatat.
- realpath now calls again on EINTR. err_with_path has no caller left.
- fcopyfile(3) copies from the file positions and then truncates the
  destination to the bytes of that one call, so a second call after a
  partial copy leaves a short file. bun_sys::fcopyfile now calls again only
  while both positions are at 0, and bun_sys::copy_file uses it.
A preloaded shim fails one call of a named libc function in the directory
of a row and logs it. Linux rows: unlink, rmdir, link, truncate, fsync,
fdatasync, mkdtemp, exists, Bun.which, realpath, cp, copyFile, Bun.write
(copy_file_range, sendfile, splice), fetch with a Bun.file body, and the
FICLONE of copyFile and cp. One test pins the error of a call that fails
with ENOSPC or with an errno outside bun's table. macOS rows: exists
(faccessat), clonefile, copyfile and fcopyfile.

The shim is compiled while the tests are collected, so a compiler that is
not in the page cache does not use up the time of a test.
@robobun

robobun commented Oct 18, 2026 •

Copy link
Copy Markdown
Collaborator Author

Status

Reproduced on main in two ways.

With a preloaded shim (LD_PRELOAD) that fails the first call of a libc function with EINTR:

  • fs.unlinkSync, rmdirSync, linkSync, truncateSync, fsyncSync, fdatasyncSync, mkdtempSync and cpSync throw EINTR: interrupted system call.
  • Bun.write(file, file) and fetch with a Bun.file body reject with EINTR.
  • fs.existsSync answers false for a file that exists, and Bun.which answers null.

With real signals and no shim: a SIGUSR1 handler without SA_RESTART (siginterrupt through bun:ffi), a storm of SIGUSR1 from a child, and 40 copies of a 32 MB file on tmpfs. On 1.4.3-canary, fs.cpSync throws EINTR 40 of 40 times, Bun.write(file, file) 2 of 40, and fetch with a Bun.file body rejects 3 of 40 uploads. On this branch all three are 0 of 40.

The repro is test/js/node/fs/fs-eintr-syscalls.test.ts. Its 4 Linux tests fail on main and pass on this branch.

PR: #45929

@robobun

robobun commented Oct 18, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 11:58 PM PT - Oct 21st, 2026

✅ @robobun, your commit 4bea0734582f71204eef96ece2c0b39b700ee332 passed in Build #130548! 🎉


🧪   To try this PR locally:

bunx bun-pr 45929

That installs a local version of the PR into your bun-45929 executable, so you can run:

bun-45929 --bun

@coderabbitai

coderabbitai Bot commented Oct 18, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 5 seconds.

Check out review usage here.

View limit details

Limit details: You’ve used all 10 included reviews currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: Repository: oven-sh/bun/.coderabbit.yaml
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: afe69e24-41a4-493f-97be-5f601e4abdb4
📥 Commits

Reviewing files that changed from the base of the PR and between da658dc and 4bea073.

📒 Files selected for processing: 12
  • src/http/SendFile.rs
  • src/jsc/bindings/c-bindings.cpp
  • src/runtime/node/node_fs.rs
  • src/runtime/server/FileResponseStream.rs
  • src/runtime/webcore/blob/copy_file.rs
  • src/standalone_graph/StandaloneModuleGraph.rs
  • src/sys/Error.rs
  • src/sys/copy_file.rs
  • src/sys/lib.rs
  • src/sys/linux_syscall.rs
  • test/js/node/fs/fs-eintr-syscalls-fixture.ts
  • test/js/node/fs/fs-eintr-syscalls.test.ts

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: oven-sh/bun/.coderabbit.yaml
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: 3a7d1f0b-e576-46f2-afd0-c25311d3914e
📥 Commits

Reviewing files that changed from the base of the PR and between acda140 and da658dc.

📒 Files selected for processing: 7
  • src/http/SendFile.rs
  • src/standalone_graph/StandaloneModuleGraph.rs
  • src/sys/Error.rs
  • src/sys/copy_file.rs
  • src/sys/lib.rs
  • test/js/node/fs/fs-eintr-syscalls-fixture.ts
  • test/js/node/fs/fs-eintr-syscalls.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 3 remain after this review.

Walkthrough

The change adds shared POSIX syscall wrappers, routes filesystem and copy operations through Result-based APIs, updates EINTR handling, and adds Linux/macOS fault-injection tests for retries, fallbacks, and error reporting.

Changes

Filesystem syscall handling

Layer / File(s) Summary
Syscall wrappers and retry behavior
src/sys/lib.rs, src/sys/Error.rs, src/sys/linux_syscall.rs, src/jsc/bindings/c-bindings.cpp
POSIX wrappers add retry and error handling. Linux/Android copy wrappers use Result-based APIs. Unknown errno values map to EUNKNOWN.
Node filesystem operations
src/runtime/node/node_fs.rs
Filesystem operations use shared wrappers for synchronization, linking, temporary-directory creation, path operations, cloning, and copying.
Copy and sendfile call paths
src/sys/copy_file.rs, src/runtime/webcore/blob/copy_file.rs, src/http/SendFile.rs, src/runtime/server/FileResponseStream.rs, src/standalone_graph/StandaloneModuleGraph.rs
Copy and sendfile callers use shared wrappers. Sendfile failures follow existing response error handling. Platform-specific copy fallbacks remain in place.
Filesystem fault-injection tests
test/js/node/fs/fs-eintr-syscalls-fixture.ts, test/js/node/fs/fs-eintr-syscalls.test.ts
Linux and macOS tests inject selected syscall errors and check filesystem, copy, clone, retry, fallback, and error-reporting results.

Suggested reviewers: jarred-sumner

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to da658

This change retries interrupted system calls inside Bun's syscall wrappers. File operations, copies, uploads and existence checks therefore no longer fail spuriously when a signal arrives. No concrete defect was identified at the reviewed head, and the change appears ready to merge. As the author noted, the macOS and FreeBSD paths have had less runtime validation than Linux.

Pre-merge checks: Passed 4
Check name Status Explanation
Title check Passed The title clearly identifies the main change: retrying EINTR in bun_sys wrappers for node:fs, Bun.write, and fetch.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Description check Passed The description clearly explains the problem, implementation, scope, limitations, and verification results. It does not use the template headings "What does this PR do?" and "How did you verify your c…
Full details: Description check

Explanation

The description clearly explains the problem, implementation, scope, limitations, and verification results. It does not use the template headings "What does this PR do?" and "How did you verify your code works?", but it provides the required information in equivalent sections.

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

Beyond the inline findings, I also checked the explicit-offset retries in the new bun_sys::sendfile and bun_sys::copy_file_range wrappers (callers in src/http/SendFile.rs, src/runtime/server/FileResponseStream.rs, and the cp/copyFile loops in src/runtime/node/node_fs.rs): the kernel only writes the offset out-param back on a positive return, so re-issuing with the same &mut i64 after EINTR neither skips nor double-sends bytes, and dropping the EINTR => continue arms at those call sites is equivalent.

Extended reasoning...

The change moves EINTR retry for unlink/rmdir/link/truncate/mkdtemp/fdatasync/sendfile/copy_file_range/splice/ioctl_ficlone/fcopyfile into src/sys/lib.rs wrappers and rewires node_fs, Bun.write, fetch, and Bun.serve callers onto them, with no security-sensitive surface. The inline findings already flag the mkdtemp double-create, the fcopyfile position gate, and the uneven SendFile.rs arms; this note only records the offset-retry check that was ruled out.

Additional findings (outside the current diff — GitHub can't attach inline comments there):

  • 🟣 src/http/SendFile.rs — Pre-existing, left by this partial fix: on macOS and FreeBSD, fetch with a Bun.file body still rejects with EINTR when a signal interrupts sendfile, exactly as on the base branch. Only the Linux arm of SendFile::write was moved onto the retrying bun_sys::sendfile; the macOS arm at src/http/SendFile.rs:113 and the FreeBSD arm at src/http/SendFile.rs:82 still call raw bun_sys::c::sendfile and turn every errno other than EAGAIN into Status::Err. Fix: make every arm of SendFile::write survive EINTR, e.g. treat EINTR like EAGAIN (offset and remain are already advanced from sbytes, so returning Status::Again resumes correctly) or loop on it, for both the macOS and FreeBSD arms.

    Why this was flagged

    On macOS or FreeBSD a user calls fetch(url, { method: "POST", body: Bun.file(path) }) with a file of 32 KB or more, which takes the sendfile upload path in src/http/lib.rs:3538. A signal arrives while the kernel sendfile waits (the PR names FUSE, NFS and file leases as realistic sources); Darwin sendfile returns -1 with errno EINTR and a partial sbytes. src/http/SendFile.rs:113 tests errcode != bun_sys::E::EAGAIN || ..., so EINTR falls into Status::Err at src/http/SendFile.rs:118 and the request fails with EINTR fetching "<url>", the message the PR says it removes. The base branch behaves the same on these platforms; only the Linux arm at src/http/SendFile.rs:37 was changed to the retrying wrapper, so this sibling site of the same bug class is left unfixed and the macOS test rows in test/js/node/fs/fs-eintr-syscalls.test.ts:298 do not cover fetch.

    Verification: Pre-existing (the base branch fails the same way by the same route; the PR fixes only the Linux arm). Triggering condition: on macOS a fetch with a Bun.file body of >= 32 KiB is interrupted by a signal while in sendfile(2). At src/http/SendFile.rs:113 EINTR falls through to Status::Err at line 118. The new macOS test does not interpose sendfile, so nothing covers this arm.

Comment thread src/sys/lib.rs Resolved
Comment thread src/sys/lib.rs Resolved
Comment thread src/http/SendFile.rs Resolved
…body

On macOS, fetch with a Bun.file body issues sendfile(2) through a raw thunk
and rejects with EINTR when the call is interrupted. XNU returns EINTR from
the read of the file on a file system that waits (NFS, SMB, FUSE). Only the
Linux arm of SendFile::write used the retrying wrapper. The FreeBSD arm had
the same code as the macOS arm.

- bun_sys::sendfile for macOS and FreeBSD has the shape of the Linux call.
  It returns the bytes sent and advances the offset. EINTR with no bytes
  sent calls again. EAGAIN or EINTR after some bytes is a short count, the
  rule of uv__fs_sendfile in libuv.
- SendFile::write has one body for the three systems. The raw c::sendfile
  thunks have no caller left and are removed.
- Three results other than EINTR change on macOS and FreeBSD, each to the
  Linux rule. A count of 0 sends nothing (these kernels read 0 as the rest
  of the file). A call that sent less than it was asked for is followed by
  another call. EAGAIN after the whole count was sent ends the upload and
  is not an error.
- The FreeBSD arm is compiled and has not run. No CI lane runs FreeBSD
  tests.
The upload rows send to a socket that keeps every byte of the request. A
row passes only when Content-Length and the bytes after the head are the
file or the slice.

- Linux: a whole file, a slice, and an upload that the shim cuts into
  parts of 7777 bytes, so that SendFile::write runs many times.
- macOS: EINTR before a byte is sent, EINTR after 4096 bytes, EAGAIN after
  the whole count, a slice, and an empty slice that must issue no call.
- A sendfile that fails with EIO rejects with EIO on both systems.

The Linux tests now run one at a time. Four debug builds that start
together each take most of the 5 seconds of a test on a loaded machine.
One at a time, each test takes about 1 second.
@robobun

robobun commented Oct 20, 2026

Copy link
Copy Markdown
Collaborator Author

2942926 covers the macOS and FreeBSD arms of SendFile::write that the review named.

  • bun_sys::sendfile now exists for macOS and FreeBSD with the shape of the Linux call. EINTR with no bytes sent calls again. EAGAIN or EINTR after some bytes is a short count. uv__fs_sendfile in libuv has the same rule.
  • SendFile::write has one body for the three systems. The raw c::sendfile thunks are deleted.
  • c3e6335: the upload rows compare the bytes that arrive. The macOS test has rows for EINTR before a byte, EINTR after 4096 bytes, EAGAIN after the whole count, a slice and an empty slice.
  • The description lists the three results besides EINTR that change on macOS and FreeBSD. The FreeBSD arm is compiled and has not run.
  • c9bc842 cuts the comments of this PR to one line each.

The three inline threads have their own replies.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reviewed the latest push (the unified macOS/FreeBSD sendfile wrapper, the single SendFile::write body, and the test changes) and found no new bugs; because this rewrites the syscall wrappers under bun install, Bun.write and fetch uploads across Linux/macOS/FreeBSD cfg arms that cannot all be type-checked from one host, and the description says the macOS test has not been run, a human look is still worthwhile.

What was reviewed:

  • The new macOS/FreeBSD sendfile wrapper against the two arms it replaces: EAGAIN/EINTR with bytes sent still yields a short count and Status::Again, zero bytes still errors, and the len == 0 early return avoids the whole-file send the old macOS arm would have issued for an exhausted range.
  • The force-clone site in copy_file_inner now tagged clonefile instead of copyfile: masked, because NodeFS::copy_file rewrites syscall/path/dest on every error, so the Node error contract is unchanged.
  • Stale callers of the deleted thunks (c::sendfile, linux::sendfile, clonefile_rc, copyfile_rc, c::truncate, errno_sys*): none remain outside src/sys.
  • get_error_code_tag_name's EUNKNOWN fallback: every platform errno table (linux, darwin, freebsd, windows) declares the variant.
Extended reasoning...

The change moves EINTR retry for unlink/rmdir/link/truncate/fsync/fdatasync/mkdtemp/realpath/faccessat/sendfile/copy_file_range/splice/FICLONE/fcopyfile into bun_sys wrappers, deletes the node_fs errno_sys shim and the raw sendfile/clonefile/copyfile thunks, and collapses SendFile::write and FileResponseStream's sendfile path onto the wrappers; it touches no auth, crypto or injection surface. It is roughly +1084/-743 across src/sys, node_fs, Bun.write, fetch and Bun.serve, with four cfg-gated platform arms, a user-visible error-shape change (unknown errnos now report EUNKNOWN) and a new LD_PRELOAD/DYLD_INSERT_LIBRARIES fault-injection test that compiles C at test time. The size, the untested macOS arm per the author, and my still-open inline findings from the prior review decided defer over approve.

…-fs-eintr-retry

# Conflicts:
#	src/runtime/node/node_fs.rs

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code review found no issues

No high-confidence issues detected in this change.

No change in behaviour. The comments name the placeholder as six X.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code review found no issues

No high-confidence issues detected in this change.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant