From a1259728a67ee9ecdc85e1ffe2c7ccb285615f05 Mon Sep 17 00:00:00 2001 From: Jarred Sumner Date: Sun, 23 Aug 2026 13:25:18 +0000 Subject: [PATCH 1/8] node:fs: remove unsafe from the fs implementation and binding Route the syscalls node:fs open-coded through typed bun_sys / sys_uv / sys::windows wrappers (link, truncate, mkdtemp, posix_rmdir, posix_fadvise, copy_file_range_fd, read_uninit, read_into_vec, iovecs_as_const, safe_libc::{fsync,fdatasync}; realpath/mkdtemp/utime/lutime/futime via libuv; GetFileAttributesW/CopyFileW/SetEndOfFile/FlushFileBuffers/ GetFinalPathNameByHandleW), and give libuv fs requests an owned type (OwnedFsReq) plus typed one-shot submitters in bun_io::uv_fs so the Windows UVFSRequest is a Box lent to libuv and re-queued with Task::from_boxed. fs.cp shares its state through CpTaskRef (last share posts the boxed task); recursive readdir keeps its cross-thread state in Arc with a lock-free BoxQueue of per-directory results; off-thread results are plain data (StringOrBytes, byte entries) so AsyncFSTask is Send by construction. The shell cp builtin hands over a ShellCpHandle instead of a raw pointer, Bun__mkdirp is a HOST_EXPORT, and ArgumentsSlice::hand_off_protection replaces the ManuallyDrop dance in the binding. --- src/bun_core/lib.rs | 7 + src/codegen/generate-host-exports.ts | 8 + src/event_loop/AnyTaskWithExtraContext.rs | 40 + src/event_loop/ConcurrentTask.rs | 28 + src/io/lib.rs | 2 + src/io/uv_fs.rs | 163 ++ src/jsc/array_buffer.rs | 18 + src/libuv_sys/libuv.rs | 110 +- src/runtime/dispatch.rs | 14 +- src/runtime/jsc_hooks.rs | 5 +- src/runtime/node/node_fs.rs | 2351 +++++++++------------ src/runtime/node/node_fs_binding.rs | 7 +- src/runtime/shell/builtin/cp.rs | 115 +- src/runtime/webcore/blob/copy_file.rs | 11 +- src/runtime/webcore/blob/write_file.rs | 6 +- src/sys/lib.rs | 150 +- src/sys/sys_uv.rs | 205 +- src/sys/windows/mod.rs | 39 + src/threading/lib.rs | 2 +- src/threading/unbounded_queue.rs | 80 + 20 files changed, 1859 insertions(+), 1502 deletions(-) create mode 100644 src/io/uv_fs.rs diff --git a/src/bun_core/lib.rs b/src/bun_core/lib.rs index aa42dd2f9f2d..660c0048caf5 100644 --- a/src/bun_core/lib.rs +++ b/src/bun_core/lib.rs @@ -552,6 +552,13 @@ pub mod vec { self.0.as_mut_ptr().cast::() } + /// The whole buffer as uninitialized bytes, for producers typed over `MaybeUninit`. + #[inline(always)] + pub fn as_uninit_mut(&mut self) -> &mut [core::mem::MaybeUninit] { + // SAFETY: `MaybeUninit<[u8; N]>` and `[MaybeUninit; N]` have identical layout and no validity invariant. + unsafe { core::slice::from_raw_parts_mut(self.0.as_mut_ptr().cast(), N) } + } + /// # Safety /// Write-only view, same contract as [`spare_bytes_mut`]: only a producer may store into it, and only the prefix it reports may be read back. #[inline(always)] diff --git a/src/codegen/generate-host-exports.ts b/src/codegen/generate-host-exports.ts index 5448dfb9372b..235354b379cb 100644 --- a/src/codegen/generate-host-exports.ts +++ b/src/codegen/generate-host-exports.ts @@ -144,6 +144,14 @@ function ptrify(ty: string): { cTy: string; deref: (n: string) => string; extraL `{\n // SAFETY: C++ caller passes \`${n}_len\` live elements at \`${n}\` (or 0).\n unsafe { ::bun_core::ffi::slice(${n}, ${n}_len) }\n }`, }; } + // `Option<&CStr>` — C passes a nullable `const char*`. + if (/^Option\s*<\s*&\s*(?:(?:core|std)::ffi::)?CStr\s*>$/.test(ty)) { + return { + cTy: `*const c_char`, + deref: n => + `{\n // SAFETY: C++ caller passes null or a NUL-terminated string live for the call.\n if ${n}.is_null() { None } else { Some(unsafe { ::core::ffi::CStr::from_ptr(${n}) }) }\n }`, + }; + } // Other slice shapes (`&mut [T]`, `&'a [T]`) and `&str` are NOT FFI-safe; reject. if (/^&[^\[]*\[/.test(ty) || /^&\s*str\b/.test(ty)) { throw new Error(`slice/str param \`${ty}\` is not FFI-safe; use \`&[T]\` (const) or (ptr, len)`); diff --git a/src/event_loop/AnyTaskWithExtraContext.rs b/src/event_loop/AnyTaskWithExtraContext.rs index e033e5cb6e5b..a1c20aee8f78 100644 --- a/src/event_loop/AnyTaskWithExtraContext.rs +++ b/src/event_loop/AnyTaskWithExtraContext.rs @@ -68,6 +68,46 @@ impl AnyTaskWithExtraContext { } } + /// Heap-allocates a task that owns `value`; when it runs it calls + /// `callback(value, extra)` and frees itself. The mini loop that receives + /// the returned pointer owns the allocation until then. + pub fn from_value( + value: T, + callback: fn(T, *mut c_void), + ) -> NonNull { + #[repr(C)] + struct Wrapper { + any_task: AnyTaskWithExtraContext, + value: T, + callback: fn(T, *mut c_void), + } + + fn function(this: *mut (), extra: *mut ()) { + // SAFETY: `this` is the `ctx` set below: the heap `Wrapper`, whose + // first (`repr(C)`) field is the task the loop just dequeued. + let that: Box> = unsafe { bun_core::heap::take(this.cast::>()) }; + let Wrapper { + value, callback, .. + } = *that; + callback(value, extra.cast::()); + } + + let task = bun_core::heap::into_raw(Box::new(Wrapper:: { + any_task: AnyTaskWithExtraContext { + callback: function::, + ctx: None, + next: bun_threading::Link::new(), + }, + value, + callback, + })); + // SAFETY: `task` was just produced by `into_raw`; valid and exclusive. + unsafe { + (*task).any_task.ctx = NonNull::new(task.cast::<()>()); + NonNull::new_unchecked(core::ptr::addr_of_mut!((*task).any_task)) + } + } + /// Initializes `self` in place to call `callback(of, extra)`. // The unit context means the callee is effectively `fn(*T)` only; mapped // to `*mut ()` to keep the two-arg stored ABI uniform. diff --git a/src/event_loop/ConcurrentTask.rs b/src/event_loop/ConcurrentTask.rs index 245c9500ea3c..8a484947ac29 100644 --- a/src/event_loop/ConcurrentTask.rs +++ b/src/event_loop/ConcurrentTask.rs @@ -195,6 +195,34 @@ impl Task { } } +/// `impl Taskable` for a payload that is queued as an owned `Box` (via +/// [`Task::from_boxed`]) and whose dispatch arm reclaims that box: an unrun +/// task is released by dropping it. Forms: +/// +/// ```ignore +/// boxed_taskable!(MyTask => task_tag::MyTask); +/// boxed_taskable!([const B: bool] MyTask => if B { task_tag::A } else { task_tag::C }); +/// boxed_taskable!([R, A] MyReq where [R: X, A: Y] => task_tag::Z); +/// ``` +#[macro_export] +macro_rules! boxed_taskable { + ([$($gen:tt)*] $ty:ty where [$($bounds:tt)*] => $tag:expr) => { + impl<$($gen)*> $crate::Taskable for $ty where $($bounds)* { + const TAG: $crate::TaskTag = $tag; + unsafe fn release_unrun(this: *mut Self) { + // SAFETY: fn contract — `this` is the box queued under `TAG`. + drop(unsafe { ::bun_core::heap::take(this) }); + } + } + }; + ([$($gen:tt)*] $ty:ty => $tag:expr) => { + $crate::boxed_taskable!([$($gen)*] $ty where [] => $tag); + }; + ($ty:ty => $tag:expr) => { + $crate::boxed_taskable!([] $ty where [] => $tag); + }; +} + // Taskable impls for the low-tier task wrappers defined in this crate. impl Taskable for crate::ManagedTask::ManagedTask { const TAG: TaskTag = task_tag::ManagedTask; diff --git a/src/io/lib.rs b/src/io/lib.rs index 4df690e14bf0..3884484d3474 100644 --- a/src/io/lib.rs +++ b/src/io/lib.rs @@ -471,6 +471,8 @@ pub use pipe_read_scratch::{PipeReadScratch, PipeReadScratchGuard}; #[cfg(windows)] #[path = "source.rs"] pub mod source; +#[cfg(windows)] +pub mod uv_fs; #[path = "write.rs"] pub mod write; diff --git a/src/io/uv_fs.rs b/src/io/uv_fs.rs new file mode 100644 index 000000000000..a9f75f94d0a4 --- /dev/null +++ b/src/io/uv_fs.rs @@ -0,0 +1,163 @@ +//! Typed one-shot libuv fs requests: a heap object that embeds its +//! `uv_fs_t`, is owned by libuv while the request is in flight, and comes +//! back as a `Box` when it completes. + +use core::ffi::c_int; + +use bun_core::ZStr; +use bun_sys::windows::libuv as uv; + +/// A heap object with an embedded [`uv::OwnedFsReq`]. +/// +/// The submitters below take it as a `Box`, hand the allocation to libuv for +/// the duration of the request (nothing else may touch it meanwhile), and give +/// it back to [`on_complete`](Self::on_complete) on the loop thread once libuv +/// has filled in `req().result` (and, per op, `req()`'s result accessors). +pub trait UvFsRequest: Sized + 'static { + fn req(&mut self) -> &mut uv::OwnedFsReq; + fn on_complete(this: Box); +} + +/// I/O parameters for [`read`]/[`write`], read off the boxed owner once it is +/// at its final address: `(request, fd, buffers, position)` in one split +/// borrow (`position` is `-1` for the current one). libuv copies the +/// descriptor array before returning; the memory the descriptors cover must +/// stay valid and otherwise untouched until completion, which the owner +/// guarantees by holding (and, for JS-backed buffers, pinning) it — the +/// `PlatformIoVec` convention used by every vectored-I/O entry point in +/// `bun_sys`. +pub trait UvFsIo: UvFsRequest { + fn io_parts(&mut self) -> (&mut uv::OwnedFsReq, uv::uv_file, IoBufs<'_>, i64); +} + +pub enum IoBufs<'a> { + One(uv::uv_buf_t), + Many(&'a [uv::uv_buf_t]), +} + +impl IoBufs<'_> { + #[inline] + fn as_slice(&self) -> &[uv::uv_buf_t] { + match self { + IoBufs::One(b) => core::slice::from_ref(b), + IoBufs::Many(s) => s, + } + } +} + +extern "C" fn on_uv_fs_done(req: *mut uv::fs_t) { + // SAFETY: `req.data` is the `Box` released in `start`; libuv is done + // with the request, so ownership returns here exactly once. + let owner: Box = unsafe { bun_core::heap::take((*req).data.cast::()) }; + T::on_complete(owner); +} + +/// Release `owner` to libuv and run `submit` on its embedded request. A +/// submission error comes back synchronously as `Err((owner, rc))` with +/// `req().result` also holding `rc` — libuv will not call back in that case. +fn start( + owner: Box, + submit: impl FnOnce(*mut uv::Loop, *mut uv::fs_t, uv::uv_fs_cb) -> uv::ReturnCode, +) -> Result<(), (Box, uv::ReturnCode)> { + let raw: *mut T = bun_core::heap::into_raw(owner); + // SAFETY: `raw` is the live allocation released above; exclusive here. + let req: *mut uv::fs_t = unsafe { + let req: &mut uv::fs_t = (*raw).req(); + req.data = raw.cast(); + req + }; + let rc = submit(uv::Loop::get(), req, Some(on_uv_fs_done::)); + if rc.is_err() { + // SAFETY: libuv rejected the request and will not call back; reclaim. + let mut owner = unsafe { bun_core::heap::take(raw) }; + owner.req().result = rc.into(); + return Err((owner, rc)); + } + Ok(()) +} + +/// `uv_fs_open`; libuv copies `path` before returning. +pub fn open( + owner: Box, + path: &ZStr, + flags: c_int, + mode: c_int, +) -> Result<(), (Box, uv::ReturnCode)> { + start(owner, |l, req, cb| { + // SAFETY: `req` is the owner's embedded request; `path` is NUL-terminated. + unsafe { uv::uv_fs_open(l, req, path.as_ptr(), flags, mode, cb) } + }) +} + +/// `uv_fs_close`. +pub fn close( + owner: Box, + fd: uv::uv_file, +) -> Result<(), (Box, uv::ReturnCode)> { + // SAFETY: `req` is the owner's embedded request. + start(owner, |l, req, cb| unsafe { + uv::uv_fs_close(l, req, fd, cb) + }) +} + +/// `uv_fs_statfs`; libuv copies `path` before returning. Read the result with +/// `req().statfs_result()`. +pub fn statfs(owner: Box, path: &ZStr) -> Result<(), (Box, uv::ReturnCode)> { + start(owner, |l, req, cb| { + // SAFETY: `req` is the owner's embedded request; `path` is NUL-terminated. + unsafe { uv::uv_fs_statfs(l, req, path.as_ptr(), cb) } + }) +} + +fn start_io( + owner: Box, + op: unsafe extern "C" fn( + *mut uv::Loop, + *mut uv::fs_t, + uv::uv_file, + *const uv::uv_buf_t, + core::ffi::c_uint, + i64, + uv::uv_fs_cb, + ) -> uv::ReturnCode, +) -> Result<(), (Box, uv::ReturnCode)> { + let raw: *mut T = bun_core::heap::into_raw(owner); + // SAFETY: `raw` is the live allocation released above; exclusive here (one + // `&mut` reborrow, split by `io_parts`). The descriptor array is copied by + // libuv during the call; what it describes is kept valid by `*raw` until + // completion (see `UvFsIo`). + let rc = unsafe { + let owner: &mut T = &mut *raw; + let (req, fd, bufs, position) = owner.io_parts(); + req.data = raw.cast(); + let req: *mut uv::fs_t = &mut **req; + let slice = bufs.as_slice(); + let (ptr, len) = (slice.as_ptr(), slice.len()); + op( + uv::Loop::get(), + req, + fd, + ptr, + core::ffi::c_uint::try_from(len).expect("int cast"), + position, + Some(on_uv_fs_done::), + ) + }; + if rc.is_err() { + // SAFETY: libuv rejected the request and will not call back; reclaim. + let mut owner = unsafe { bun_core::heap::take(raw) }; + owner.req().result = rc.into(); + return Err((owner, rc)); + } + Ok(()) +} + +/// `uv_fs_read` into the owner's buffers. +pub fn read(owner: Box) -> Result<(), (Box, uv::ReturnCode)> { + start_io(owner, uv::uv_fs_read) +} + +/// `uv_fs_write` from the owner's buffers. +pub fn write(owner: Box) -> Result<(), (Box, uv::ReturnCode)> { + start_io(owner, uv::uv_fs_write) +} diff --git a/src/jsc/array_buffer.rs b/src/jsc/array_buffer.rs index bc10d428e5b2..8c1c9e6328f1 100644 --- a/src/jsc/array_buffer.rs +++ b/src/jsc/array_buffer.rs @@ -960,6 +960,24 @@ impl MarkedArrayBuffer { } } + /// Take ownership of heap bytes (freed by `destroy`/`Drop`, or handed to + /// JSC by `to_node_buffer`/`to_js`). An empty slice yields [`Self::EMPTY`]. + pub fn from_owned_bytes(bytes: Box<[u8]>, typed_array_type: JSType) -> MarkedArrayBuffer { + if bytes.is_empty() { + return MarkedArrayBuffer { + buffer: ArrayBuffer { + typed_array_type, + ..ArrayBuffer::EMPTY + }, + owns_buffer: false, + }; + } + MarkedArrayBuffer { + buffer: ArrayBuffer::from_owned_bytes(bytes, typed_array_type), + owns_buffer: true, + } + } + pub const EMPTY: MarkedArrayBuffer = MarkedArrayBuffer { owns_buffer: false, buffer: ArrayBuffer::EMPTY, diff --git a/src/libuv_sys/libuv.rs b/src/libuv_sys/libuv.rs index 9175e67e2677..e943fdad09e3 100644 --- a/src/libuv_sys/libuv.rs +++ b/src/libuv_sys/libuv.rs @@ -91,6 +91,9 @@ pub type uv_uid_t = u8; pub type uv_gid_t = u8; pub type uv_req_type = c_uint; pub type uv_fs_type = c_int; +pub const UV_FS_READLINK: uv_fs_type = 25; +pub const UV_FS_REALPATH: uv_fs_type = 28; +pub const UV_FS_STATFS: uv_fs_type = 34; pub(crate) type uv_tty_mode_t = c_uint; /// `uv_tty_mode_t` (uv.h) — typed wrapper for `uv_tty_set_mode` callers. #[repr(u32)] @@ -1894,7 +1897,7 @@ pub struct fs_t { pub cb: uv_fs_cb, pub result: ReturnCodeI64, pub(crate) ptr: *mut c_void, - pub path: *const c_char, + pub(crate) path: *const c_char, pub statbuf: uv_stat_t, pub work_req: uv__work, pub(crate) flags: c_int, @@ -1904,6 +1907,62 @@ pub struct fs_t { } pub type uv_fs_t = fs_t; +/// An owned `uv_fs_t` that runs `uv_fs_req_cleanup` on drop if a `uv_fs_*` +/// call ever wrote it. Safe to move once libuv is done with it: the one +/// self-reference libuv leaves behind (`fs.info.bufs` pointing at the inline +/// `bufsml` for reads/writes of ≤ 4 buffers) is cleared before cleanup, so a +/// moved request never frees a stale interior pointer. While a request is in +/// flight libuv holds its address, so it must not move then (the async +/// submitters in `bun_io::uv_fs` keep it boxed). +#[repr(transparent)] +pub struct OwnedFsReq(fs_t); + +impl OwnedFsReq { + #[inline] + pub fn new() -> Self { + Self(fs_t::uninitialized()) + } +} +impl Default for OwnedFsReq { + #[inline] + fn default() -> Self { + Self::new() + } +} +impl Drop for OwnedFsReq { + #[inline] + fn drop(&mut self) { + if !self.0.is_initialized() { + return; + } + // SAFETY: `uv__fs_req_init` zeroes the `fs` union, and only + // `uv_fs_read`/`uv_fs_write` write the `info` arm's `nbufs`/`bufs`; + // with `nbufs <= 4` libuv uses (and `bufs` points at) the inline + // `bufsml`, which `uv_fs_req_cleanup` must not free — null it so the + // `bufs != bufsml` check there cannot misfire after a move. + unsafe { + let info = &mut self.0.fs.info; + if !info.bufs.is_null() && info.nbufs as usize <= info.bufsml.len() { + info.bufs = core::ptr::null_mut(); + } + } + self.0.deinit(); + } +} +impl core::ops::Deref for OwnedFsReq { + type Target = fs_t; + #[inline] + fn deref(&self) -> &fs_t { + &self.0 + } +} +impl core::ops::DerefMut for OwnedFsReq { + #[inline] + fn deref_mut(&mut self) -> &mut fs_t { + &mut self.0 + } +} + impl fs_t { #[cfg(debug_assertions)] const UV_FS_CLEANEDUP: c_int = 0x0010; @@ -1955,6 +2014,49 @@ impl fs_t { self.assert_initialized(); self.ptr.cast::() } + /// Whether a `uv_fs_*` call has written this request (vs. the + /// [`uninitialized`](Self::uninitialized) sentinel). + #[inline] + pub fn is_initialized(&self) -> bool { + self.loop_ as usize != 0xAAAA_AAAA_AAAA_0000usize + } + /// The `uv_statfs_t` a successful `uv_fs_statfs` left in `req.ptr`, copied + /// out (`None` before completion, on error, or after cleanup nulled it). + #[inline] + pub fn statfs_result(&self) -> Option { + if !self.is_initialized() || self.fs_type != UV_FS_STATFS || self.ptr.is_null() { + return None; + } + // SAFETY: for a completed `UV_FS_STATFS` request libuv points `ptr` at a + // heap `uv_statfs_t` it owns until `uv_fs_req_cleanup`; no alignment + // promise, hence the unaligned read. + Some(unsafe { core::ptr::read_unaligned(self.ptr.cast::()) }) + } + /// The NUL-terminated string a successful `uv_fs_realpath`/`uv_fs_readlink` + /// left in `req.ptr` (`None` before completion, on error, or after cleanup). + #[inline] + pub fn ptr_c_str(&self) -> Option<&core::ffi::CStr> { + if !self.is_initialized() + || !(self.fs_type == UV_FS_REALPATH || self.fs_type == UV_FS_READLINK) + || self.ptr.is_null() + { + return None; + } + // SAFETY: for these request types libuv stores a heap NUL-terminated + // string in `ptr`, owned by the request until `uv_fs_req_cleanup`. + Some(unsafe { core::ffi::CStr::from_ptr(self.ptr.cast::()) }) + } + /// `req.path` — the request's (UTF-8, NUL-terminated) path; for + /// `uv_fs_mkdtemp`/`uv_fs_mkstemp` libuv rewrites it to the created name. + #[inline] + pub fn path_c_str(&self) -> Option<&core::ffi::CStr> { + if !self.is_initialized() || self.path.is_null() { + return None; + } + // SAFETY: libuv keeps `path` pointing at a NUL-terminated copy it owns + // until `uv_fs_req_cleanup` nulls it. + Some(unsafe { core::ffi::CStr::from_ptr(self.path) }) + } /// `req.file.fd` union arm. The union is private /// because the active variant is path-dependent (`uv_fs_open` writes `fd`; /// path-taking ops write `pathw`); callers reading the wrong arm get UB. @@ -2322,6 +2424,12 @@ impl fmt::Display for ReturnCode { #[repr(transparent)] #[derive(Clone, Copy, PartialEq, Eq, Debug)] pub struct ReturnCodeI64(pub(crate) i64); +impl From for ReturnCodeI64 { + #[inline] + fn from(rc: ReturnCode) -> Self { + Self(i64::from(rc.int())) + } +} impl ReturnCodeI64 { #[inline] pub const fn int(self) -> i64 { diff --git a/src/runtime/dispatch.rs b/src/runtime/dispatch.rs index afae984518da..65ffbd3666c7 100644 --- a/src/runtime/dispatch.rs +++ b/src/runtime/dispatch.rs @@ -292,14 +292,14 @@ pub(crate) fn run_task( .run(); } task_tag::AsyncCpTask => { - // SAFETY: posted by `on_subtask_done` with the count at zero (exclusive). - unsafe { (*task.ptr.cast::()).run_from_js_thread()? }; + // SAFETY: boxed by `NewAsyncCpTask::on_all_done`; the arm consumes it. + unsafe { bun_core::heap::take(cast_ptr!(crate::node::fs::AsyncCpTask)) } + .run_from_js_thread(global)?; } task_tag::ShellAsyncCpTask => { // SAFETY: as above. - unsafe { - (*task.ptr.cast::()).run_from_js_thread()? - }; + unsafe { bun_core::heap::take(cast_ptr!(crate::node::fs::ShellAsyncCpTask)) } + .run_from_js_thread(global)?; } task_tag::StatWatcherHop => { // SAFETY: posted by `StatWatcher::post_to_js_thread` with a ref held. @@ -415,7 +415,9 @@ pub(crate) fn run_task( for_each_fs_uv_op!(__fs_pat) => { macro_rules! __fs_run { ($($tag:ident $ty:ident;)*) => { match task.tag { - $(task_tag::$tag => cast!(fs_async::$ty).run_from_js_thread()?,)* + // SAFETY: boxed by `UVFSRequest::create` and queued by its + // libuv completion; the arm consumes it. + $(task_tag::$tag => unsafe { bun_core::heap::take(cast_ptr!(fs_async::$ty)) }.run_from_js_thread()?,)* // SAFETY: outer arm guard proves one of the table tags matched. _ => unsafe { core::hint::unreachable_unchecked() }, }}; diff --git a/src/runtime/jsc_hooks.rs b/src/runtime/jsc_hooks.rs index 5eb8bfa11816..71d7736f62cd 100644 --- a/src/runtime/jsc_hooks.rs +++ b/src/runtime/jsc_hooks.rs @@ -1326,8 +1326,9 @@ unsafe fn create_node_fs(vm: *mut VirtualMachine) -> *mut c_void { // `.vm` is set only when standalone-module-graph is active // (it gates the embedded-file `Bun.file()` lookups inside `node:fs`). // SAFETY: per fn contract. - let vm_field = if unsafe { &*vm }.standalone_module_graph.is_some() { - core::ptr::NonNull::new(vm) + let vm_ref: &VirtualMachine = unsafe { &*vm }; + let vm_field = if vm_ref.standalone_module_graph.is_some() { + Some(bun_ptr::BackRef::new(vm_ref)) } else { None }; diff --git a/src/runtime/node/node_fs.rs b/src/runtime/node/node_fs.rs index 828858d06994..69319dc1fcd1 100644 --- a/src/runtime/node/node_fs.rs +++ b/src/runtime/node/node_fs.rs @@ -3,8 +3,7 @@ // The top-level functions assume the arguments are already validated use bun_paths::strings; -use core::ffi::{c_char, c_int, c_uint, c_void}; -use core::ptr::NonNull; +use core::ffi::{c_int, c_uint}; use core::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; use crate::api::bun::process::event_loop_handle_to_ctx; @@ -18,13 +17,12 @@ use bun_jsc::debugger::AsyncTaskTracker; use bun_jsc::virtual_machine::VirtualMachine; use bun_jsc::{ ArrayBuffer, EventLoopHandle, JSGlobalObject, JSValue, JsResult, PinnedArrayBuffer, - StringJsc as _, + StringJsc as _, Utf8WithStringJsc as _, }; use bun_paths::{self as paths, OSPathBuffer, OSPathChar, OSPathSliceZ, PathBuffer}; use bun_sys::FdExt as _; use bun_sys::{self as sys, E, Fd as FD, Maybe, Mode, SystemErrno}; -use bun_threading::UnboundedQueue; -use bun_threading::work_pool::{IntrusiveWorkTask as _, Task as WorkPoolTask, WorkPool}; +use bun_threading::work_pool::{Task as WorkPoolTask, WorkPool}; // ────────────────────────────────────────────────────────────────────────── // `Maybe(T)` shim — `crate::node::Maybe` is the same `Result` alias @@ -190,17 +188,6 @@ use super::util::validators; // - `bun_sys_jsc::ErrorJsc`→ `bun_sys::Error::to_js_with_async_stack()` use bun_sys_jsc::ErrorJsc as _; -/// `WorkPoolTask` (aka `bun_threading::thread_pool::Task`) does not derive -/// `Default` (its `callback` field has no sensible default). Build one with -/// the intrusive `node` zeroed and the supplied callback. -#[inline] -fn work_pool_task(callback: unsafe fn(*mut WorkPoolTask)) -> WorkPoolTask { - WorkPoolTask { - node: bun_threading::thread_pool::Node::default(), - callback, - } -} - pub use super::node_fs_constant as constants; // The `Watcher` / `StatWatcher` sibling modules are declared in // `node.rs`; re-export them under the names the `args::Watch` / `watch()` @@ -247,47 +234,6 @@ const IOV_MAX: usize = libc::IOV_MAX as usize; #[cfg(windows)] const IOV_MAX: usize = core::ffi::c_uint::MAX as usize; -/// In-place RAII wrapper for a libuv `fs_t` request. -/// -/// `scopeguard::guard(fs_t, |mut r| r.deinit())` is *wrong* here: its `Drop` -/// `ManuallyDrop::take`s the value into the closure parameter, relocating the -/// ~440-byte request to a new stack address before `uv_fs_req_cleanup` runs. -/// libuv stores self-referential pointers (`req->fs.info.bufs` may point at -/// `req->fs.info.bufsml`), so the request must not move between init and -/// cleanup. A real `Drop` impl runs in place at the original address. -#[cfg(windows)] -#[repr(transparent)] -struct UvFsReq(uv::fs_t); -#[cfg(windows)] -impl UvFsReq { - #[inline] - fn new() -> Self { - Self(uv::fs_t::uninitialized()) - } -} -#[cfg(windows)] -impl Drop for UvFsReq { - #[inline] - fn drop(&mut self) { - self.0.deinit(); - } -} -#[cfg(windows)] -impl core::ops::Deref for UvFsReq { - type Target = uv::fs_t; - #[inline] - fn deref(&self) -> &uv::fs_t { - &self.0 - } -} -#[cfg(windows)] -impl core::ops::DerefMut for UvFsReq { - #[inline] - fn deref_mut(&mut self) -> &mut uv::fs_t { - &mut self.0 - } -} - // ────────────────────────────────────────────────────────────────────────── // Local cross-crate shims // @@ -474,8 +420,7 @@ pub(crate) const DEFAULT_PERMISSION: Mode = 0; // `AbortSignalRef` (= `ExternalShared`) implements `Deref`, so // `signal.pending_activity_ref()` / `signal.aborted()` resolve directly to the -// `&AbortSignal` inherent methods — the former `AbortSignalRefExt` shim with -// per-call `unsafe { self.as_ref() }` is gone. `unref()` is handled by `Drop`. +// `&AbortSignal` inherent methods. `unref()` is handled by `Drop`. // ────────────────────────────────────────────────────────────────────────── // Async task type aliases @@ -536,8 +481,11 @@ mod _async_tasks { pub(crate) type Read = UVFSRequest; pub(crate) type Readdir = AsyncFSTask, { NodeFSFunctionEnum::Readdir }>; - pub(crate) type ReadFile = - AsyncFSTask, { NodeFSFunctionEnum::ReadFile }>; + pub(crate) type ReadFile = AsyncFSTask< + ret::ReadFileOffThread, + args::ReadFile<'static>, + { NodeFSFunctionEnum::ReadFile }, + >; pub(crate) type Readlink = AsyncFSTask, { NodeFSFunctionEnum::Readlink }>; pub(crate) type Readv = UVFSRequest; @@ -585,8 +533,7 @@ mod _async_tasks { /// Pool thread; `ticket` is this task's, for the callee to post its /// hop back through. pub(crate) completion: fn(*mut (), Maybe<()>, &bun_jsc::Ticket), - /// Memory is not owned by this struct - pub path: *const [u8], // BORROW: not owned + pub path: Box<[u8]>, pub(crate) ticket: bun_jsc::Ticket, pub task: WorkPoolTask, } @@ -605,11 +552,8 @@ mod _async_tasks { #[allow(clippy::boxed_local)] fn run_owned(self: Box) { let mut node_fs = NodeFS::default(); - // SAFETY: the scheduling caller keeps `path` alive until `completion` - // runs (it points into caller-owned state, not this box). - let path = unsafe { &*self.path }; let result = node_fs.mkdir_recursive(&args::Mkdir { - path: PathLike::borrowed(path), + path: PathLike::borrowed(&self.path), recursive: true, ..Default::default() }); @@ -639,315 +583,349 @@ mod _async_tasks { #[cfg(not(windows))] pub type UVFSRequest = AsyncFSTask; + /// One libuv fs request (Windows): boxed, lent to libuv by + /// [`bun_io::uv_fs`] while in flight, handed back on the loop thread, then + /// queued as a [`bun_jsc::Task`] whose dispatch arm runs + /// [`run_from_js_thread`](Self::run_from_js_thread). Dropping it releases + /// the keep-alive, the argument protection and the libuv request. #[cfg(windows)] pub struct UVFSRequest { pub(crate) promise: JSPromiseStrong, pub args: ThreadIsolated, pub(crate) global_object: bun_ptr::BackRef, - pub(crate) req: uv::fs_t, + pub(crate) req: uv::OwnedFsReq, pub(crate) result: Maybe, pub(crate) r#ref: KeepAlive, pub(crate) tracker: AsyncTaskTracker, } #[cfg(windows)] - impl UVFSRequest + impl Drop for UVFSRequest { + fn drop(&mut self) { + self.r#ref.unref(bun_io::js_vm_ctx()); + } + } + + // Queued as a box under its per-op tag; an unrun request is released by + // dropping it (promise handle, argument protection, keep-alive). + #[cfg(windows)] + bun_event_loop::boxed_taskable!( + [R: FsReturn + 'static, A: FsArgument + 'static, const F: NodeFSFunctionEnum] + UVFSRequest + where [Op<{ F }>: NodeFSDispatch + UvFsSubmit] + => F.task_tag() + ); + + /// How each libuv-backed op submits its boxed request (Windows). One impl + /// per `async_::*` alias that is a [`UVFSRequest`]. + #[cfg(windows)] + pub trait UvFsSubmit { + fn submit(task: Box>, binding: &Binding); + } + + #[cfg(windows)] + impl + bun_io::uv_fs::UvFsRequest for UVFSRequest where - Op<{ F }>: NodeFSDispatch, + Op<{ F }>: NodeFSDispatch + UvFsSubmit, + { + #[inline] + fn req(&mut self) -> &mut uv::OwnedFsReq { + &mut self.req + } + /// Loop (= JS) thread: libuv is done with the request. + fn on_complete(mut this: Box) { + let mut node_fs = NodeFS::default(); + let rc = this.req.result; + this.result = if F == NodeFSFunctionEnum::Statfs { + NodeFS::uv_dispatch_req::(&mut node_fs, &this.args, &this.req, rc) + } else { + NodeFS::uv_dispatch::(&mut node_fs, &this.args, rc) + }; + let global_object = this.global_object; + global_object + .bun_vm() + .event_loop_mut() + .enqueue_task(bun_jsc::Task::from_boxed(this)); + } + } + + /// The descriptor/buffers/position of a `read`/`write`/`readv`/`writev` + /// request. The JS buffers behind them are pinned and rooted + /// (`ThreadIsolated`) for the request's life. + #[cfg(windows)] + pub trait UvIoArgs { + fn io_fd(&self) -> uv::uv_file; + fn io_bufs(&self) -> bun_io::uv_fs::IoBufs<'_>; + fn io_position(&self) -> i64; + } + #[cfg(windows)] + impl UvIoArgs for args::Read { + fn io_fd(&self) -> uv::uv_file { + self.fd.uv() + } + /// One buffer, windowed by `offset`/`length`. + fn io_bufs(&self) -> bun_io::uv_fs::IoBufs<'_> { + let buf = self.buffer.slice(); + let off = buf.len().min(self.offset as usize); + let buf = &buf[off..]; + let buf = &buf[..buf.len().min(self.length as usize)]; + bun_io::uv_fs::IoBufs::One(uv::uv_buf_t::init(buf)) + } + fn io_position(&self) -> i64 { + self.position.map(|p| p as i64).unwrap_or(-1) + } + } + #[cfg(windows)] + impl UvIoArgs for args::Write<'_> { + fn io_fd(&self) -> uv::uv_file { + self.fd.uv() + } + /// One buffer, windowed by `offset`/`length`. + fn io_bufs(&self) -> bun_io::uv_fs::IoBufs<'_> { + let buf = self.buffer.slice(); + let off = buf.len().min(self.offset as usize); + let buf = &buf[off..]; + let buf = &buf[..buf.len().min(self.length as usize)]; + bun_io::uv_fs::IoBufs::One(uv::uv_buf_t::init(buf)) + } + fn io_position(&self) -> i64 { + self.position.map(|p| p as i64).unwrap_or(-1) + } + } + #[cfg(windows)] + impl UvIoArgs for args::FdVectorIo { + fn io_fd(&self) -> uv::uv_file { + self.fd.uv() + } + fn io_bufs(&self) -> bun_io::uv_fs::IoBufs<'_> { + bun_io::uv_fs::IoBufs::Many(&self.buffers.buffers) + } + fn io_position(&self) -> i64 { + self.position.map(|p| p as i64).unwrap_or(-1) + } + } + #[cfg(windows)] + impl + bun_io::uv_fs::UvFsIo for UVFSRequest + where + Op<{ F }>: NodeFSDispatch + UvFsSubmit, { - /// Deref the raw `global_object` pointer. - /// - /// Invariant: set from a live `&JSGlobalObject` in `create()` and never - /// null; the JSC global outlives every task (JSC_BORROW per LIFETIMES.tsv). #[inline] - pub(crate) fn global_object(&self) -> &JSGlobalObject { - self.global_object.get() + fn io_parts( + &mut self, + ) -> ( + &mut uv::OwnedFsReq, + uv::uv_file, + bun_io::uv_fs::IoBufs<'_>, + i64, + ) { + ( + &mut self.req, + self.args.io_fd(), + self.args.io_bufs(), + self.args.io_position(), + ) + } + } + + /// libuv reports bad arguments synchronously (and never calls back then); + /// the ops below pass arguments it cannot reject, so that is a bug here — + /// in release, settle the request with the error it reported. + #[cfg(windows)] + fn debug_assert_submitted( + r: Result<(), (Box, uv::ReturnCode)>, + ) { + if let Err((task, rc)) = r { + debug_assert!(false, "uv_fs submit failed synchronously: {}", rc.int()); + T::on_complete(task); + } + } + + /// Copy `path` into the binding's scratch buffer (with the Windows + /// long-path/cwd normalisation `slice_z` applies) and return its length; + /// the buffer then holds it NUL-terminated. Held only across the submit + /// (libuv copies the path) and never across a JS re-entry point. + #[cfg(windows)] + fn scratch_path_z(path: &PathLike, buf: &mut PathBuffer) -> usize { + path.slice_z_with_force_copy::(buf).len() + } + + #[cfg(windows)] + impl UvFsSubmit, { NodeFSFunctionEnum::Open }> + for Op<{ NodeFSFunctionEnum::Open }> + { + fn submit(task: Box, binding: &Binding) { + let mut flags: c_int = task.args.flags.as_int(); + flags = uv::O::from_bun_o(flags); + let mut mode: c_int = task.args.mode as c_int; + if mode == 0 { + mode = 0o644; + } + binding.node_fs.with_mut(|node_fs| { + let path = if strings::eql_comptime(task.args.path.slice(), b"/dev/null") { + ZStr::from_static(b"\\\\.\\NUL\0") + } else { + let len = scratch_path_z(&task.args.path, &mut node_fs.sync_error_buf); + ZStr::from_buf(&node_fs.sync_error_buf[..], len) + }; + sys::syslog!( + "uv open({}, {}, {}) = scheduled", + ::bstr::BStr::new(path.as_bytes()), + flags, + mode + ); + debug_assert_submitted(bun_io::uv_fs::open(task, path, flags, mode)); + }); + } + } + + #[cfg(windows)] + impl UvFsSubmit + for Op<{ NodeFSFunctionEnum::Close }> + { + fn submit(task: Box, _binding: &Binding) { + let fd = task.args.fd.uv(); + debug_assert_submitted(bun_io::uv_fs::close(task, fd)); + sys::syslog!("uv close({}) = scheduled", fd); + } + } + + #[cfg(windows)] + impl UvFsSubmit + for Op<{ NodeFSFunctionEnum::Read }> + { + fn submit(task: Box, _binding: &Binding) { + let fd = task.args.fd.uv(); + debug_assert_submitted(bun_io::uv_fs::read(task)); + sys::syslog!("uv read({}) = scheduled", fd); + } + } + + #[cfg(windows)] + impl UvFsSubmit, { NodeFSFunctionEnum::Write }> + for Op<{ NodeFSFunctionEnum::Write }> + { + fn submit(task: Box, _binding: &Binding) { + let fd = task.args.fd.uv(); + debug_assert_submitted(bun_io::uv_fs::write(task)); + sys::syslog!("uv write({}) = scheduled", fd); + } + } + + #[cfg(windows)] + impl UvFsSubmit + for Op<{ NodeFSFunctionEnum::Readv }> + { + fn submit(mut task: Box, _binding: &Binding) { + let fd = task.args.fd.uv(); + let nbufs = task.args.buffers.buffers.len(); + if nbufs == 0 { + task.result = Ok(ret::Read { bytes_read: 0 }); + let global_object = task.global_object; + global_object + .bun_vm() + .event_loop_mut() + .enqueue_task(bun_jsc::Task::from_boxed(task)); + return; + } + let pos = task.args.position.map(|p| p as i64).unwrap_or(-1); + let sum: u64 = (task.args.buffers.buffers.iter()) + .map(|b| b.slice().len() as u64) + .sum(); + debug_assert_submitted(bun_io::uv_fs::read(task)); + sys::syslog!( + "uv readv({}, {}, {}, {} total bytes) = scheduled", + fd, + nbufs, + pos, + sum + ); + } + } + + #[cfg(windows)] + impl UvFsSubmit + for Op<{ NodeFSFunctionEnum::Writev }> + { + fn submit(mut task: Box, _binding: &Binding) { + let fd = task.args.fd.uv(); + let nbufs = task.args.buffers.buffers.len(); + if nbufs == 0 { + task.result = Ok(ret::Write { bytes_written: 0 }); + let global_object = task.global_object; + global_object + .bun_vm() + .event_loop_mut() + .enqueue_task(bun_jsc::Task::from_boxed(task)); + return; + } + let pos = task.args.position.map(|p| p as i64).unwrap_or(-1); + let sum: u64 = (task.args.buffers.buffers.iter()) + .map(|b| b.slice().len() as u64) + .sum(); + debug_assert_submitted(bun_io::uv_fs::write(task)); + sys::syslog!( + "uv writev({}, {}, {}, {} total bytes) = scheduled", + fd, + nbufs, + pos, + sum + ); } + } + #[cfg(windows)] + impl UvFsSubmit, { NodeFSFunctionEnum::Statfs }> + for Op<{ NodeFSFunctionEnum::Statfs }> + { + fn submit(task: Box, binding: &Binding) { + binding.node_fs.with_mut(|node_fs| { + let len = scratch_path_z(&task.args.path, &mut node_fs.sync_error_buf); + let path = ZStr::from_buf(&node_fs.sync_error_buf[..], len); + sys::syslog!("uv statfs({}) = ~~", ::bstr::BStr::new(path.as_bytes())); + debug_assert_submitted(bun_io::uv_fs::statfs(task, path)); + }); + } + } + + #[cfg(windows)] + impl + UVFSRequest + where + Op<{ F }>: NodeFSDispatch + UvFsSubmit, + { pub(crate) fn create( global_object: &JSGlobalObject, binding: &Binding, task_args: ThreadIsolated, vm: &mut VirtualMachine, ) -> JSValue { - let task = Box::new(Self { + let mut task = Box::new(Self { promise: JSPromiseStrong::init(global_object), args: task_args, - // Sentinel — overwritten by `uv_callback` (or the early-return arms - // below) before any read on the JS thread. `Maybe` is + // Sentinel — overwritten by `on_complete` (or the empty-writev + // arm) before any read on the JS thread. `Maybe` is // `Result` and may be niche-optimised for arbitrary // `R`; never construct an all-zero `Result` value. result: Err(sys::Error::default()), global_object: bun_ptr::BackRef::new(global_object), - req: bun_core::ffi::zeroed(), + req: uv::OwnedFsReq::new(), r#ref: KeepAlive::default(), tracker: AsyncTaskTracker::init(vm), }); - // Transfer ownership to libuv: the box outlives the async request and is - // reclaimed in `destroy()` (run_from_js_thread → scopeguard). `heap::release` - // names that hand-off — it is `Box::leak` under the hood; the reclaim - // happens in `destroy()`, not in this scope. - let task: &mut Self = bun_core::heap::release(task); - // KeepAlive::ref_ now takes the type-erased aio EventLoopCtx; the JS - // event loop is the only one that owns AsyncFSTask/UVFSRequest. task.r#ref.ref_(bun_io::js_vm_ctx()); - let _ = vm; task.tracker.did_schedule(global_object); - - let loop_ = uv::Loop::get(); - task.req.data = core::ptr::from_mut::(task).cast::(); - - // The match resolves at compile time (`F` is a const generic), but - // each arm's body needs `A` re-asserted to its concrete `args::*` - // type — same identity-cast pattern as `NodeFS::dispatch` (per the - // `async_::*` aliases, `A == $Args` for the matched `F`). - macro_rules! args_as { - ($Args:ty) => {{ - debug_assert_eq!(core::mem::size_of::(), core::mem::size_of::<$Args>()); - // SAFETY: identity cast — `A == $Args` for this `F` (see `async_::*`). - // `ThreadIsolated` is `repr(transparent)`; deref through it for the inner `A`. - unsafe { &*(&*task.args as *const A as *const $Args) } - }}; - } - match F { - NodeFSFunctionEnum::Open => { - let args: &args::Open = args_as!(args::Open); - let path = if strings::eql_comptime(args.path.slice(), b"/dev/null") { - ZStr::from_static(b"\\\\.\\NUL\0") - } else { - // SAFETY (R-2): single-JS-thread `JsCell` projection of the - // scratch path buffer; the borrow is held only across the - // libuv enqueue below (which copies `path` internally) and - // never across a JS re-entry point. - args.path - .slice_z(unsafe { &mut binding.node_fs.get_mut().sync_error_buf }) - }; - let mut flags: c_int = args.flags.as_int(); - flags = uv::O::from_bun_o(flags); - let mut mode: c_int = args.mode as c_int; - if mode == 0 { - mode = 0o644; - } - // SAFETY: libuv async request; `task.req` and `path` outlive the - // call (path is copied internally by libuv before return). - let rc = unsafe { - uv::uv_fs_open( - loop_, - &mut task.req, - path.as_ptr(), - flags, - mode, - Some(Self::uv_callback), - ) - }; - debug_assert!(rc == uv::ReturnCode::ZERO); - sys::syslog!( - "uv open({}, {}, {}) = scheduled", - ::bstr::BStr::new(path.as_bytes()), - flags, - mode - ); - } - NodeFSFunctionEnum::Close => { - let args: &args::Close = args_as!(args::Close); - let fd = args.fd.uv(); - // SAFETY: libuv async request. - let rc = unsafe { - uv::uv_fs_close(loop_, &mut task.req, fd, Some(Self::uv_callback)) - }; - debug_assert!(rc == uv::ReturnCode::ZERO); - sys::syslog!("uv close({}) = scheduled", fd); - } - NodeFSFunctionEnum::Read => { - let args: &args::Read = args_as!(args::Read); - let fd = args.fd.uv(); - let buf = args.buffer.slice(); - let off = (buf.len()).min(args.offset as usize); - let buf = &buf[off..]; - let buf = &buf[..buf.len().min(args.length as usize)]; - let bufs = [uv::uv_buf_t::init(buf)]; - // SAFETY: libuv copies the iovec descriptor before return; the - // backing Buffer is pinned and rooted (`ReadBuffer::PinnedBuffer`). - let rc = unsafe { - uv::uv_fs_read( - loop_, - &mut task.req, - fd, - bufs.as_ptr(), - 1, - args.position.map(|p| p as i64).unwrap_or(-1), - Some(Self::uv_callback), - ) - }; - debug_assert!(rc == uv::ReturnCode::ZERO); - sys::syslog!("uv read({}) = scheduled", fd); - } - NodeFSFunctionEnum::Write => { - let args: &args::Write = args_as!(args::Write); - let fd = args.fd.uv(); - let buf = args.buffer.slice(); - let off = (buf.len()).min(args.offset as usize); - let buf = &buf[off..]; - let buf = &buf[..buf.len().min(args.length as usize)]; - let bufs = [uv::uv_buf_t::init(buf)]; - // SAFETY: see Read arm. - let rc = unsafe { - uv::uv_fs_write( - loop_, - &mut task.req, - fd, - bufs.as_ptr(), - 1, - args.position.map(|p| p as i64).unwrap_or(-1), - Some(Self::uv_callback), - ) - }; - debug_assert!(rc == uv::ReturnCode::ZERO); - sys::syslog!("uv write({}) = scheduled", fd); - } - NodeFSFunctionEnum::Readv => { - let args: &args::Readv = args_as!(args::Readv); - let fd = args.fd.uv(); - let bufs = &args.buffers.buffers; - let pos: i64 = args.position.map(|p| p as i64).unwrap_or(-1); - let sum: u64 = bufs.iter().map(|b| b.slice().len() as u64).sum(); - // SAFETY: `bufs` (Vec == Vec) lives in - // the leaked task; libuv copies the array before return. - let rc = unsafe { - uv::uv_fs_read( - loop_, - &mut task.req, - fd, - bufs.as_ptr().cast(), - c_uint::try_from(bufs.len()).expect("int cast"), - pos, - Some(Self::uv_callback), - ) - }; - debug_assert!(rc == uv::ReturnCode::ZERO); - sys::syslog!( - "uv readv({}, {:p}, {}, {}, {} total bytes) = scheduled", - fd, - bufs.as_ptr(), - bufs.len(), - pos, - sum - ); - } - NodeFSFunctionEnum::Writev => { - let args: &args::Writev = args_as!(args::Writev); - let fd = args.fd.uv(); - let bufs = &args.buffers.buffers; - if bufs.is_empty() { - // SAFETY: identity write — `R == ret::Writev == ret::Write` for this `F`. - unsafe { - core::ptr::write( - &mut task.result as *mut Maybe as *mut Maybe, - Ok(ret::Write { bytes_written: 0 }), - ) - }; - let task_ptr: *mut Self = task; - task.global_object() - .bun_vm() - .event_loop_mut() - .enqueue_task(bun_jsc::Task::init(task_ptr)); - return task.promise.value(); - } - let pos: i64 = args.position.map(|p| p as i64).unwrap_or(-1); - let sum: u64 = bufs.iter().map(|b| b.slice().len() as u64).sum(); - // SAFETY: see Readv arm. - let rc = unsafe { - uv::uv_fs_write( - loop_, - &mut task.req, - fd, - bufs.as_ptr().cast(), - c_uint::try_from(bufs.len()).expect("int cast"), - pos, - Some(Self::uv_callback), - ) - }; - debug_assert!(rc == uv::ReturnCode::ZERO); - sys::syslog!( - "uv writev({}, {:p}, {}, {}, {} total bytes) = scheduled", - fd, - bufs.as_ptr(), - bufs.len(), - pos, - sum - ); - } - NodeFSFunctionEnum::Statfs => { - let args: &args::StatFS = args_as!(args::StatFS); - // SAFETY (R-2): single-JS-thread `JsCell` projection; held only - // across the libuv enqueue (copies `path` internally). - let path = args - .path - .slice_z(unsafe { &mut binding.node_fs.get_mut().sync_error_buf }); - // SAFETY: libuv copies `path` internally before return. - let rc = unsafe { - uv::uv_fs_statfs( - loop_, - &mut task.req, - path.as_ptr(), - Some(Self::uv_callbackreq), - ) - }; - debug_assert!(rc == uv::ReturnCode::ZERO); - sys::syslog!("uv statfs({}) = ~~", ::bstr::BStr::new(path.as_bytes())); - } - _ => unreachable!("UVFSRequest type not implemented"), - } - - task.promise.value() - } - - extern "C" fn uv_callback(req: *mut uv::fs_t) { - // SAFETY: req points to a live uv::fs_t passed by libuv; cleanup is the documented pair - scopeguard::defer! { unsafe { uv::uv_fs_req_cleanup(req) } }; - // SAFETY: req.data was set to the Box::leak'd `*mut Self` in create() - let this: &mut Self = unsafe { bun_ptr::callback_ctx::((*req).data) }; - let mut node_fs = NodeFS::default(); - // `req` aliases `this.req` (see create(): `task.req.data = from_mut(task)`); once - // `this: &mut Self` is live, re-deriving through the raw `req` would create a - // second overlapping `&mut` (Stacked-Borrows UB). Go through `this.req` instead. - this.result = NodeFS::uv_dispatch::(&mut node_fs, &this.args, this.req.result); - let this_ptr: *mut Self = this; - this.global_object() - .bun_vm() - .event_loop_mut() - .enqueue_task(bun_jsc::Task::init(this_ptr)); - } - - extern "C" fn uv_callbackreq(req: *mut uv::fs_t) { - // Same as uv_callback but passes `req` through to the dispatch fn (statfs needs req.ptr). - // SAFETY: req points to a live uv::fs_t passed by libuv; cleanup is the documented pair - scopeguard::defer! { unsafe { uv::uv_fs_req_cleanup(req) } }; - // SAFETY: req.data was set to the Box::leak'd `*mut Self` in create() - let this: &mut Self = unsafe { bun_ptr::callback_ctx::((*req).data) }; - let mut node_fs = NodeFS::default(); - // `req` aliases `this.req`; once `this: &mut Self` is live, re-deriving `&mut *req` - // would overlap it (Stacked-Borrows UB). Go through `this.req` instead — disjoint-field - // borrow alongside `&this.args` / `this.result =`. Hoist the result read so it isn't - // evaluated after `&mut this.req` is formed in the same call expression. - let rc = this.req.result; - this.result = - NodeFS::uv_dispatch_req::(&mut node_fs, &this.args, &mut this.req, rc); - let this_ptr: *mut Self = this; - this.global_object() - .bun_vm() - .event_loop_mut() - .enqueue_task(bun_jsc::Task::init(this_ptr)); + let promise = task.promise.value(); + as UvFsSubmit>::submit(task, binding); + promise } - pub(crate) fn run_from_js_thread(&mut self) -> JsResult<()> { - // SAFETY: self was Box::leak'd in create(); destroy() runs exactly once on scope exit - let _deinit = - scopeguard::guard(core::ptr::from_mut(self), |p| unsafe { Self::destroy(p) }); - // Move `result` out so the `global_object()` `&self` borrow can coexist - // with consuming it below; the sentinel left behind is dropped in `destroy()`. + /// JS thread, from the task queue: settle the promise. `self` drops on + /// return. + pub(crate) fn run_from_js_thread(mut self: Box) -> JsResult<()> { let result = core::mem::replace(&mut self.result, Err(sys::Error::default())); - let global_object = self.global_object(); + let global_object: &JSGlobalObject = &self.global_object; let success = matches!(result, Ok(_)); let promise_value = self.promise.value(); let promise = self.promise.get(); @@ -976,15 +954,6 @@ mod _async_tasks { } Ok(()) } - - /// SAFETY: `this` must be the pointer Box::leak'd in `create()`; called exactly once. - pub(crate) unsafe fn destroy(this: *mut Self) { - // SAFETY: caller guarantees `this` is the live Box-leaked allocation; - // reclaim ownership (paired with the Box::leak in create()). - let mut task = unsafe { bun_core::heap::take(this) }; - // `bun_sys::Error` frees its path on Drop. - task.r#ref.unref(bun_io::js_vm_ctx()); - } } // ────────────────────────────────────────────────────────────────────────── @@ -1160,6 +1129,12 @@ mod _async_tasks { self.into_js(global) } } + impl FsReturn for StringOrBytes { + #[inline] + fn fs_to_js(self, global: &JSGlobalObject) -> JsResult { + self.into_js(global) + } + } impl FsReturn for ret::Read { #[inline] fn fs_to_js(self, global: &JSGlobalObject) -> JsResult { @@ -1191,35 +1166,13 @@ mod _async_tasks { } } - /// `Taskable` glue for the libuv-request ops (Windows), which complete on - /// the JS thread and re-enter through the task queue under a per-`F` tag. - #[cfg(windows)] - impl bun_event_loop::Taskable - for UVFSRequest - where - Op<{ F }>: NodeFSDispatch, - { - const TAG: bun_event_loop::TaskTag = F.task_tag(); - /// A libuv fs request that completed into the queue after the last - /// tick: destroy releases its promise handle and keep-alive. - unsafe fn release_unrun(this: *mut Self) { - // SAFETY: fn contract — `Box::leak`'d in `UVFSRequest::create`. - unsafe { Self::destroy(this) } - } - } - /// One `fs.promises.*` operation on the work pool. The arguments' JS-backed - /// buffers are pinned and rooted (`ThreadIsolated`) and read under the job's ticket. + /// buffers are pinned and rooted (`ThreadIsolated`) and read under the job's ticket; + /// the result is plain data (`R: Send`) turned into JS values in `then`. pub struct AsyncFSTask { pub args: ThreadIsolated, pub(crate) result: Maybe, } - // SAFETY: results are plain data / owned buffers / WTF strings built off - // thread for hand-off (`ret::*`); `ThreadIsolated` is Send by its contract. - unsafe impl Send - for AsyncFSTask - { - } /// The JS-thread half of an async fs operation. #[derive(bun_jsc::JsAffine)] @@ -1228,7 +1181,7 @@ mod _async_tasks { pub(crate) tracker: AsyncTaskTracker, } - impl + impl bun_jsc::JobContext for AsyncFSTask where Op<{ F }>: NodeFSDispatch, @@ -1289,7 +1242,7 @@ mod _async_tasks { } } - impl + impl AsyncFSTask where Op<{ F }>: NodeFSDispatch, @@ -1332,30 +1285,28 @@ mod _async_tasks { pub type AsyncCpTask = NewAsyncCpTask; pub type ShellAsyncCpTask = NewAsyncCpTask; - // The shell flattens builtins under `crate::shell::builtins::*`. The - // `cp_on_copy`/`cp_on_finish` hooks are inherent methods on that type - // (cp.rs), called directly below — no trait indirection. - pub(crate) type ShellCpTask = crate::shell::builtins::cp::ShellCpTask; + // The shell flattens builtins under `crate::shell::builtins::*`. Progress + // and completion are reported through the `ShellCpHandle` the builtin + // hands over (cp.rs) — no trait indirection. + pub(crate) use crate::shell::builtins::cp::ShellCpHandle; + /// One `fs.cp` / `fs.promises.cp` (or shell `cp`) operation. Shared through + /// [`CpTaskRef`] by the directory-scan task and every per-file subtask; + /// dropping the last reference posts it to its loop, which runs + /// [`run_from_js_thread`](Self::run_from_js_thread) (or + /// [`finish_shell`](Self::finish_shell)) and drops it. pub struct NewAsyncCpTask { pub(crate) promise: JSPromiseStrong, pub args: ThreadIsolated>, /// Owning-thread uses (global object, keep-alive context). pub(crate) evtloop: EventLoopHandle, - /// How the last subtask's thread delivers the completion (moved out - /// by it: the loop may free `self` once the completion is queued). For - /// a JS loop this is the ticket its VM waits for — the arguments may - /// point into JS buffers and the promise lives on the JS heap. - pub(crate) poster: core::cell::Cell>, - pub task: WorkPoolTask, - /// Written from any workpool thread (first `finish_concurrently` caller wins via - /// `has_result` CAS); read on the JS thread in `run_from_js_thread`. Wrapped in - /// `Cell` so concurrent subtasks can hold `&Self` and `.set()` without aliased - /// `&mut`. Cross-thread soundness is provided by the `has_result` CAS (single - /// writer) + `subtask_count` AcqRel fence (happens-before for the JS-thread - /// read), not by `Cell` itself — `Cell` is `repr(transparent)` over - /// `UnsafeCell` and `set()` is exactly the prior `*ptr = val` open-coded. - pub(crate) result: core::cell::Cell>, + /// How the last reference's thread delivers the completion. For a JS + /// loop this is the ticket its VM waits for — the arguments may point + /// into JS buffers and the promise lives on the JS heap. + pub(crate) poster: Option, + /// The first result any subtask reports (first writer wins); `None` + /// until then. Read on the owning thread once every subtask is done. + pub(crate) result: bun_threading::Guarded>>, /// If this task is called by the shell then we shouldn't call this as /// it is not threadsafe and is unnecessary as the process will be kept /// alive by the shell instance. @@ -1363,31 +1314,88 @@ mod _async_tasks { // `ref_()`/`unref()` (`KeepAlive::default()` is inert until ref'd). pub(crate) r#ref: KeepAlive, pub(crate) tracker: AsyncTaskTracker, - pub(crate) has_result: AtomicBool, - /// Number of in-flight references to `this`. Starts at 1 for the main - /// directory-scan task; incremented for each `SingleTask` spawned. Every - /// holder calls `onSubtaskDone` exactly once when finished (regardless of - /// success or error). `runFromJSThread` — which destroys `this` — is only - /// enqueued once the count reaches zero, so subtasks still running on the - /// thread pool never dereference a freed parent. - pub(crate) subtask_count: AtomicUsize, - /// BACKREF — `Some` iff `IS_SHELL`. The shell `ShellCpTask` owns and - /// outlives this task; `ParentRef` gives a safe `&ShellCpTask` projection - /// for `cp_on_copy` and round-trips the `*mut` for `cp_on_finish`. - pub(crate) shelltask: Option>, + /// `Some` iff `IS_SHELL`: the shell `cp` builtin this copy reports to. + pub(crate) shelltask: Option, + } + + impl Drop for NewAsyncCpTask { + fn drop(&mut self) { + if !IS_SHELL { + self.r#ref.unref(event_loop_handle_to_ctx(self.evtloop)); + } + } + } + + // Queued as a box by `on_all_done`; an unrun completion is released by + // dropping it (promise handle, protected arguments, keep-alive). + bun_event_loop::boxed_taskable!( + [const IS_SHELL: bool] NewAsyncCpTask + => if IS_SHELL { + bun_event_loop::task_tag::ShellAsyncCpTask + } else { + bun_event_loop::task_tag::AsyncCpTask + } + ); + + /// A share of an in-flight [`NewAsyncCpTask`]. The directory-scan task + /// holds one and each [`CpSingleTask`] holds one; whichever thread drops + /// the last hands the task back to its loop, so subtasks still running on + /// the pool never see a freed parent. + pub struct CpTaskRef(Option>>); + + impl CpTaskRef { + fn new(task: NewAsyncCpTask) -> Self { + // Shared with the pool by design (its carriers are `owned_task!`s). + // Pool threads touch only `args` (`ThreadIsolated`), `result` + // (locked) and `shelltask.on_copy` (locked inside); `promise`, + // `tracker`, `r#ref`, `evtloop` and `poster` are touched only by + // `on_all_done` (exclusive by then) and by `run_from_js_thread` / + // `finish_shell` / `Drop`, which run from the posted box on the + // owning thread. + #[allow(clippy::arc_with_non_send_sync)] + Self(Some(std::sync::Arc::new(task))) + } + } + impl Clone for CpTaskRef { + fn clone(&self) -> Self { + Self(self.0.clone()) + } + } + impl core::ops::Deref for CpTaskRef { + type Target = NewAsyncCpTask; + fn deref(&self) -> &Self::Target { + self.0.as_deref().expect("live cp task share") + } + } + impl Drop for CpTaskRef { + fn drop(&mut self) { + if let Some(task) = self.0.take().and_then(std::sync::Arc::into_inner) { + task.on_all_done(); + } + } + } + + /// The pool task that scans the source tree (and tries `clonefile`), + /// fanning out one [`CpSingleTask`] per file. + pub(super) struct CpDirTask { + cp_task: CpTaskRef, + task: WorkPoolTask, } - bun_threading::intrusive_work_task!([const IS_SHELL: bool] NewAsyncCpTask, task); + bun_threading::owned_task!([const IS_SHELL: bool] CpDirTask, task); + + impl CpDirTask { + #[allow(clippy::boxed_local)] + fn run_owned(self: Box) { + let mut node_fs = NodeFS::default(); + NewAsyncCpTask::cp_async(&mut node_fs, &self.cp_task); + } + } /// This task is used by `AsyncCpTask/fs.promises.cp` to copy a single file. /// When clonefile cannot be used, this task is started once per file. pub struct CpSingleTask { - /// BACKREF — the parent `NewAsyncCpTask` is `Box::leak`'d and outlives every - /// subtask via the `subtask_count` refcount (see `on_subtask_done`). Stored - /// as `ParentRef` (constructed from the `*mut` with `Box::leak` provenance) - /// so shared reads are safe-projected and `as_mut_ptr()` round-trips the - /// original write provenance for `on_subtask_done`'s `&mut` promotion. - pub(crate) cp_task: bun_ptr::ParentRef, bun_ptr::Mut>, + pub(crate) cp_task: CpTaskRef, /// Single owned allocation laid out as `\0\0`. Ownership is /// encoded directly as `Box<[OSPathChar]>` and /// the two NUL-terminated views are reconstructed via `src()` / `dest()`. @@ -1402,7 +1410,7 @@ mod _async_tasks { impl CpSingleTask { /// `path_buf` layout: `[src @ ..src_len][0][dest @ ..dest_len][0]`. pub(crate) fn create( - parent: *mut NewAsyncCpTask, + parent: CpTaskRef, path_buf: Box<[OSPathChar]>, src_len: usize, dest_len: usize, @@ -1411,11 +1419,7 @@ mod _async_tasks { debug_assert_eq!(path_buf[src_len], 0); debug_assert_eq!(path_buf[src_len + 1 + dest_len], 0); WorkPool::schedule_new(CpSingleTask { - // `parent` is the `Box::leak`'d task — never null; `NonNull → ParentRef` - // preserves the mutable provenance for `on_subtask_done`. - // SAFETY: `parent` is the live `Box::leak`'d task (write provenance), never null. - cp_task: unsafe { bun_ptr::ParentRef::from_nullable_mut(parent) } - .expect("cp parent"), + cp_task: parent, path_buf, src_len, dest_len, @@ -1436,15 +1440,10 @@ mod _async_tasks { OSPathSliceZ::from_buf(&self.path_buf[self.src_len + 1..], self.dest_len) } + /// Drops `self` — and with it possibly the last [`CpTaskRef`] — on return. + #[allow(clippy::boxed_local)] fn run_owned(self: Box) { - // `ParentRef` preserves the `Box::leak` mutable provenance so - // `on_subtask_done` may later promote it to `&mut` via `as_mut_ptr()` - // once the refcount reaches zero. - let cp_task = self.cp_task; - // Shared borrow only — other workpool threads (and the directory-scan - // thread) may hold `&Self` to the same parent concurrently; `ParentRef` - // invariant: the parent outlives all subtasks (subtask_count refcount). - let parent = cp_task.get(); + let parent: &NewAsyncCpTask = &self.cp_task; // TODO: error strings on node_fs will die let mut node_fs = NodeFS::default(); @@ -1475,26 +1474,6 @@ mod _async_tasks { } } } - - // `self: Box` drops here (frees the owned `path_buf`). - drop(self); - // Must be the very last use of the parent: when the count reaches - // zero, runFromJSThread is enqueued and may destroy the parent. - NewAsyncCpTask::on_subtask_done(cp_task.as_mut_ptr()); - } - } - - impl bun_event_loop::Taskable for NewAsyncCpTask { - const TAG: bun_event_loop::TaskTag = if IS_SHELL { - bun_event_loop::task_tag::ShellAsyncCpTask - } else { - bun_event_loop::task_tag::AsyncCpTask - }; - /// A finished fs.cp whose completion will not run: destroy releases - /// its promise handle, protected arguments and keep-alive. - unsafe fn release_unrun(this: *mut Self) { - // SAFETY: fn contract — posted by `on_subtask_done` with the count at zero. - unsafe { Self::destroy(this) } } } @@ -1507,12 +1486,12 @@ mod _async_tasks { if !IS_SHELL { return; } - // When IS_SHELL, `shelltask` is `Some` (ParentRef invariant: owner - // outlives this task). Shared borrow only — concurrent subtasks may - // call this in parallel; `cp_on_copy` serialises via its internal mutex. + // Concurrent subtasks may call this in parallel; `on_copy` + // serialises via the shell task's internal mutex. self.shelltask + .as_ref() .expect("IS_SHELL ⇒ shelltask") - .cp_on_copy(src.as_ref(), dest.as_ref()); + .on_copy(src.as_ref(), dest.as_ref()); } /// `fs.cp` / `fs.promises.cp` (JS thread): a promise, an async-stack @@ -1525,16 +1504,17 @@ mod _async_tasks { ) -> JSValue { let tracker = AsyncTaskTracker::init(vm); tracker.did_schedule(global_object); - let task = Self::schedule_new( - JSPromiseStrong::init(global_object), + let promise = JSPromiseStrong::init(global_object); + let value = promise.value(); + Self::schedule_new( + promise, cp_args, EventLoopHandle::init(vm.event_loop.cast()), bun_jsc::ConcurrentPoster::Js(vm.ticket()), tracker, - core::ptr::null_mut(), + None, ); - // SAFETY: `schedule_new` returns a Box::leak'd pointer; valid until destroy() - unsafe { &*task }.promise.value() + value } /// The shell's `cp` builtin, from its pool task (any thread): no VM or @@ -1544,16 +1524,16 @@ mod _async_tasks { cp_args: ThreadIsolated>, evtloop: EventLoopHandle, poster: bun_jsc::ConcurrentPoster, - shelltask: *mut ShellCpTask, - ) -> *mut Self { + shelltask: ShellCpHandle, + ) { Self::schedule_new( JSPromiseStrong::default(), cp_args, evtloop, poster, AsyncTaskTracker { id: 0 }, - shelltask, - ) + Some(shelltask), + ); } fn schedule_new( @@ -1562,211 +1542,119 @@ mod _async_tasks { evtloop: EventLoopHandle, poster: bun_jsc::ConcurrentPoster, tracker: AsyncTaskTracker, - shelltask: *mut ShellCpTask, - ) -> *mut Self { - let mut task = Box::new(Self { + shelltask: Option, + ) { + let mut task = Self { promise, args: cp_args, - has_result: AtomicBool::new(false), - // Sentinel — overwritten by `finish_concurrently` (gated by the - // `has_result` CAS) before any read on the JS thread. - result: core::cell::Cell::new(Ok(())), + result: bun_threading::Guarded::new(None), evtloop, - poster: core::cell::Cell::new(Some(poster)), - task: work_pool_task(Self::work_pool_callback), + poster: Some(poster), r#ref: KeepAlive::default(), tracker, - subtask_count: AtomicUsize::new(1), - // SAFETY: `shelltask` (when non-null) is the live heap-alloc'd `ShellCpTask` - // that owns and outlives this task; pointer carries write provenance. - shelltask: unsafe { bun_ptr::ParentRef::from_nullable_mut(shelltask) }, - }); + shelltask, + }; if !IS_SHELL { task.r#ref.ref_(event_loop_handle_to_ctx(task.evtloop)); } - - let raw = bun_core::heap::release(task); - WorkPool::schedule(&raw mut raw.task); - raw - } - - fn work_pool_callback(task: *mut WorkPoolTask) { - // SAFETY: task points to Self.task. Kept as a raw pointer — `cp_async` - // may spawn subtasks that hold `&Self` to the same allocation while - // this call is still on the stack, so we must not form `&mut Self` here. - let this = unsafe { Self::from_task_ptr(task) }; - let mut node_fs = NodeFS::default(); - Self::cp_async(&mut node_fs, this); + WorkPool::schedule_new(CpDirTask { + cp_task: CpTaskRef::new(task), + task: WorkPoolTask::default(), + }); } /// May be called from any thread (the subtasks). - /// Records the result (first caller wins). Does NOT schedule destruction — - /// `runFromJSThread` is only enqueued from `onSubtaskDone` once every - /// in-flight subtask has dropped its reference, so that subtasks still - /// running on the thread pool don't dereference a freed parent. + /// Records the result (first caller wins). Does NOT schedule completion — + /// that happens when the last [`CpTaskRef`] is dropped, so that subtasks + /// still running on the thread pool don't dereference a freed parent. fn finish_concurrently(&self, result: Maybe) { - if self - .has_result - .compare_exchange(false, true, Ordering::Relaxed, Ordering::Relaxed) - .is_err() - { - return; - } - // The CAS above guarantees exactly one thread reaches this write; the - // `subtask_count` AcqRel fence in `on_subtask_done` publishes it to the - // JS-thread reader. (`sys::Error::path` is already `Box<[u8]>`, so - // move-assign suffices.) - self.result.set(result); - } - - /// Called exactly once by the main directory-scan task and once by each - /// `SingleTask` when it is done touching `this`. The last caller (count - /// drops to zero) enqueues `runFromJSThread`, which resolves the promise - /// and destroys `this`. - /// - /// Takes a raw `*mut Self` (not `&self`) so the pointer retains the - /// mutable provenance from the original `Box::leak`; the JS-thread - /// callback later materializes `&mut *this`, which would be UB if the - /// pointer were derived from a shared reference. - fn on_subtask_done(this: *mut Self) { - // SAFETY: `this` is a live Box-leaked task; shared access only here — - // other workpool threads may concurrently hold `&Self` until the - // refcount reaches zero below. - let this_ref = unsafe { &*this }; - let old_count = this_ref.subtask_count.fetch_sub(1, Ordering::AcqRel); - debug_assert!(old_count > 0); - if old_count != 1 { - return; - } - - // All subtasks have finished. If none reported an error, the copy succeeded. - if !this_ref.has_result.load(Ordering::Relaxed) { - this_ref.has_result.store(true, Ordering::Relaxed); - // count reached zero ⇒ this thread now has exclusive access. - this_ref.result.set(Ok(())); + let mut slot = self.result.lock(); + if slot.is_none() { + *slot = Some(result); } + } - // Count reached zero ⇒ exclusive access. `this` carries mutable - // provenance from `Box::leak`, so the enqueued callback may safely - // form `&mut *this` on the JS thread. - let poster = this_ref + /// The thread that dropped the last [`CpTaskRef`]: hand the task to + /// its loop. If no subtask reported an error, the copy succeeded. + fn on_all_done(mut self) { + self.result.get_mut().get_or_insert(Ok(())); + let poster = self .poster .take() .expect("fs.cp in flight holds its poster"); if poster.is_js() { - poster.post_js(ConcurrentTask::ConcurrentTask::create(bun_jsc::Task::init( - this, - ))); + poster.post_js(ConcurrentTask::ConcurrentTask::create( + bun_jsc::Task::from_boxed(Box::new(self)), + )); } else { - let at = AnyTaskWithExtraContext::from_callback_auto_deinit( - this, - |p: *mut Self, ctx| { - // SAFETY: subtask count hit zero ⇒ exclusive access to the leaked task. - unsafe { (*p).run_from_js_thread_mini(ctx) } - }, - ); - // `from_callback_auto_deinit` heap-allocates; never null. - poster.post_mini(core::ptr::NonNull::new(at).expect("heap task")); + poster.post_mini(AnyTaskWithExtraContext::from_value(self, |this, _ctx| { + this.finish_shell() + })); } - // The pool side is done (`this` may already be freed by its loop). + // The pool side is done (the task may already be freed by its loop). drop(poster); } - pub(crate) fn run_from_js_thread_mini(&mut self, _: *mut c_void) { - let _ = self.run_from_js_thread(); // TODO: properly propagate exception upwards + /// The shell builtin's loop thread: report the result to the builtin, + /// which continues (and may free what `args` points into) in place. + pub(crate) fn finish_shell(mut self) { + debug_assert!(IS_SHELL); + let result = self.result.get_mut().take().unwrap_or(Ok(())); + let src = core::mem::take(&mut self.args.src); + let dest = core::mem::take(&mut self.args.dest); + let shelltask = self.shelltask.take().expect("IS_SHELL ⇒ shelltask"); + shelltask.finish(src, dest, result); + drop(self); } - pub(crate) fn run_from_js_thread(&mut self) -> JsResult<()> { + /// JS thread, from the task queue: settle the promise (or, for the + /// shell's copy, report to the builtin). `self` is dropped here. + pub(crate) fn run_from_js_thread( + mut self: Box, + global_object: &JSGlobalObject, + ) -> JsResult<()> { if IS_SHELL { - // SAFETY: shelltask is set by create_for_shell and outlives this task - // Move the result out — `Maybe` (= `Maybe<()>`) has a cheap - // `Ok(())` placeholder. - let result = core::mem::replace(self.result.get_mut(), Ok(())); - let src = core::mem::take(&mut self.args.src); - let dest = core::mem::take(&mut self.args.dest); - let shelltask = self.shelltask.expect("IS_SHELL ⇒ shelltask").as_mut_ptr(); - // SAFETY: shelltask is non-null in the IS_SHELL specialization and - // outlives this task; `cp_on_finish` enqueues it concurrently. - unsafe { ShellCpTask::cp_on_finish(shelltask, src, dest, result) }; - // SAFETY: self was Box::leak'd in create*(); destroyed exactly once here - unsafe { Self::destroy(std::ptr::from_mut::(self)) }; + (*self).finish_shell(); return Ok(()); } - let go_ptr = self.evtloop.global_object(); - if go_ptr.is_null() { - panic!( - "No global object, this indicates a bug in Bun. Please file a GitHub issue." - ); - } - // SAFETY: non-null erased *mut JSGlobalObject from the JS event loop vtable. - let global_object: &JSGlobalObject = unsafe { &*go_ptr.cast::() }; - let success = (*self.result.get_mut()).is_ok(); + let result = self.result.get_mut().take().unwrap_or(Ok(())); + let success = result.is_ok(); let promise_value = self.promise.value(); - // Captured as a raw pointer because `Self::destroy(self)` runs *before* the - // resolve/reject. The `JSPromise` itself lives on the JS heap - // and is kept alive past `destroy` by `promise_value.ensure_still_alive()`. - let promise: *mut bun_jsc::JSPromise = self.promise.get(); - let result = match core::mem::replace(self.result.get_mut(), Ok(())) { - // SAFETY: `promise` is the sole live reference to the heap `JSPromise`. - Err(err) => match err.to_js_with_async_stack(global_object, unsafe { &*promise }) { + let promise = self.promise.take(); + let tracker = self.tracker; + let result = match result { + Err(err) => match err.to_js_with_async_stack(global_object, promise.get()) { Ok(v) => v, Err(e) => { - // SAFETY: `promise` points at a GC-rooted JS heap cell; sole live - // reference on this thread (see comment above `let promise`). - return unsafe { &mut *promise }.reject(global_object, Err(e)); + return promise.get().reject(global_object, Err(e)); } }, Ok(res) => match FsReturn::fs_to_js(res, global_object) { Ok(v) => v, Err(e) => { - // SAFETY: `promise` points at a GC-rooted JS heap cell; sole live - // reference on this thread (see comment above `let promise`). - return unsafe { &mut *promise }.reject(global_object, Err(e)); + return promise.get().reject(global_object, Err(e)); } }, }; promise_value.ensure_still_alive(); - let _dispatch = self.tracker.dispatch(global_object); + let _dispatch = tracker.dispatch(global_object); - // SAFETY: self was Box::leak'd in create*(); destroyed exactly once here - unsafe { Self::destroy(std::ptr::from_mut::(self)) }; + drop(self); if success { - bun_jsc::JSPromise::opaque_mut(promise).resolve(global_object, result)?; + promise.get().resolve(global_object, result)?; } else { - bun_jsc::JSPromise::opaque_mut(promise).reject(global_object, Ok(result))?; + promise.get().reject(global_object, Ok(result))?; } Ok(()) } - /// SAFETY: `this` must be the pointer returned by Box::leak in - /// `schedule_new()`; called exactly once. - pub(crate) unsafe fn destroy(this: *mut Self) { - // SAFETY: caller guarantees `this` is the live Box-leaked allocation; - // reclaim ownership (paired with the Box::leak in - // schedule_new()). - let mut task = unsafe { bun_core::heap::take(this) }; - if !IS_SHELL { - let ctx = event_loop_handle_to_ctx(task.evtloop); - task.r#ref.unref(ctx); - } - } - /// Directory scanning + clonefile will block this thread, then each individual file copy (what the sync version /// calls "copy_single_file_sync") will be dispatched as a separate task. - pub(crate) fn cp_async(nodefs: &mut NodeFS, this: *mut Self) { - // The directory-scan task holds one reference in `subtask_count` - // (initialized to 1 in create*). Drop it on return. `runFromJSThread` - // (which destroys `this`) is only enqueued once this reference and - // every spawned SingleTask's reference have been dropped. - // `this` is the live Box-leaked task; on_subtask_done only enqueues destruction - // once every reference (including this one) has been dropped. - let _done = scopeguard::guard(this, Self::on_subtask_done); - // SAFETY: same pointer as above; valid for the duration of this fn. - // Shared borrow only — once `cp_async_directory` spawns `CpSingleTask`s, - // other workpool threads concurrently hold `&Self` to this same allocation. - let this = unsafe { &**_done }; - + /// `task` is the directory scan's share; every `CpSingleTask` it + /// spawns gets a clone, and the completion runs once all are dropped. + pub(crate) fn cp_async(nodefs: &mut NodeFS, task: &CpTaskRef) { + let this: &Self = task; let args = &this.args; let mut src_buf = bun_paths::os_path_buffer_pool::get(); let mut dest_buf = bun_paths::os_path_buffer_pool::get(); @@ -1793,8 +1681,7 @@ mod _async_tasks { #[cfg(windows)] { - // SAFETY: src is NUL-terminated (os_path); GetFileAttributesW is the Win32 FFI - let attributes = unsafe { bun_sys::c::GetFileAttributesW(src.as_ptr()) }; + let attributes = sys::windows::get_file_attributes(src); if attributes == bun_sys::c::INVALID_FILE_ATTRIBUTES { this.finish_concurrently(Err(sys::Error { errno: SystemErrno::ENOENT as _, @@ -1901,10 +1788,7 @@ mod _async_tasks { let _ = Self::cp_async_directory( nodefs, args.flags, - // Pass the raw `*mut Self` (Box::leak provenance) so spawned - // `CpSingleTask`s store a pointer that may later be promoted to - // `&mut` in `on_subtask_done`. - *_done, + task, &mut src_buf, src_len, &mut dest_buf, @@ -1916,24 +1800,16 @@ mod _async_tasks { fn cp_async_directory( nodefs: &mut NodeFS, args: args::CpFlags, - this: *mut Self, + this: &CpTaskRef, src_buf: &mut OSPathBuffer, src_dir_len: PathInt, dest_buf: &mut OSPathBuffer, dest_dir_len: PathInt, ) -> bool { - // SAFETY: `this` is the live Box-leaked task. Shared borrow only — spawned - // `CpSingleTask`s on other workpool threads may concurrently hold `&Self`. - // The raw `*mut` is threaded through (instead of `&Self`) so that the - // `cp_task` pointers stored in subtasks retain mutable provenance for - // `on_subtask_done`'s eventual `&mut` promotion. - let this_ref = unsafe { &*this }; - // SAFETY: callers NUL-terminate at src_dir_len/dest_dir_len before calling. - // Platform-generic — `OSPathBuffer` is `[u16;N]` on Windows, `[u8;N]` on POSIX, - // so reconstruct as `&OSPathSliceZ`. - let src = unsafe { OSPathSliceZ::from_raw(src_buf.as_ptr(), src_dir_len as usize) }; - // SAFETY: dest_buf[dest_dir_len] == 0 written by caller - let dest = unsafe { OSPathSliceZ::from_raw(dest_buf.as_ptr(), dest_dir_len as usize) }; + let this_ref: &Self = this; + // Callers NUL-terminate at `src_dir_len`/`dest_dir_len` before calling. + let src = OSPathSliceZ::from_buf(&src_buf[..], src_dir_len as usize); + let dest = OSPathSliceZ::from_buf(&dest_buf[..], dest_dir_len as usize); #[cfg(target_os = "macos")] { @@ -2017,9 +1893,9 @@ mod _async_tasks { loop { let current = match entry { Err(err) => { - this_ref.finish_concurrently(Err( - err.with_path(nodefs.os_path_into_sync_error_buf(src)) - )); + this_ref.finish_concurrently(Err(err.with_path( + nodefs.os_path_into_sync_error_buf(&src_buf[..src_dir_len as usize]), + ))); return false; } Ok(ent) => match ent { @@ -2071,7 +1947,6 @@ mod _async_tasks { } } _ => { - this_ref.subtask_count.fetch_add(1, Ordering::Relaxed); let sd = src_dir_len as usize; let dd = dest_dir_len as usize; let total = sd + 1 + cname.len() + 1 + dd + 1 + cname.len() + 1; @@ -2091,7 +1966,7 @@ mod _async_tasks { path_buf[dest_off + dd + 1 + cname.len()] = 0; CpSingleTask::::create( - this, + this.clone(), path_buf, sd + 1 + cname.len(), dd + 1 + cname.len(), @@ -2109,18 +1984,26 @@ mod _async_tasks { // AsyncReaddirRecursiveTask // ────────────────────────────────────────────────────────────────────────── - /// `readdir(.., { recursive: true })`: a scan fanned out over pool subtasks - /// that share this state (it is the job's off-thread part, so its address - /// is stable while any subtask runs). Subtasks touch only owned data here — - /// never the JS-backed `args` — since they run outside `run`. + /// `readdir(.., { recursive: true })`: the job's off-thread part. The scan + /// itself is [`ReaddirScan`], shared with the pool subtasks it fans out to; + /// those touch only owned data there — never the JS-backed `args`. pub struct AsyncReaddirRecursiveTask { /// Async-parsed arguments; their JS-backed path is not read off-thread - /// (`root_path` is the owned copy). + /// (`ReaddirScan::root_path` is the owned copy). pub args: ThreadIsolated>, + pub(crate) scan: std::sync::Arc, + } + + /// One recursive directory scan, shared by [`AsyncReaddirRecursiveTask`] + /// and every [`ReaddirSubtask`]. Each directory listed counts one in + /// `subtask_count`; whichever thread brings it to zero finishes the scan. + pub struct ReaddirScan { pub(crate) tag: ret::ReaddirTag, pub(crate) encoding: Encoding, - /// The completion token, finished by whichever subtask ends the scan. - pub(crate) done: Option>, + /// The completion token, parked by `run` and finished by whichever + /// thread ends the scan. + pub(crate) done: + bun_threading::Guarded>>, // It's not 100% clear this one is necessary pub(crate) has_result: AtomicBool, @@ -2134,35 +2017,68 @@ mod _async_tasks { /// that frontier 2^41 paths deep before the kernel reports ELOOP. pub(crate) has_error: AtomicBool, - /// The final result list - pub(crate) result_list: ResultListEntryValue, + /// The final result list, joined from `result_list_queue` at the end. + pub(crate) result_list: bun_threading::Guarded, /// When joining the result list, we use this to preallocate the joined array. pub(crate) result_list_count: AtomicUsize, - /// A lockless queue of result lists. + /// A lockless queue of result lists, one per directory listed. /// /// Using a lockless queue instead of mutex + joining the lists as we go was a meaningful performance improvement - pub(crate) result_list_queue: UnboundedQueue, + pub(crate) result_list_queue: bun_threading::BoxQueue, /// All the subtasks will use this fd to open files - pub(crate) root_fd: FD, + pub(crate) root_fd: SharedFd, /// This is used when joining the file paths for error messages. - /// Heap-owned, NUL-terminated (`[path.., 0]`); freed on drop. + /// Heap-owned, NUL-terminated (`[path.., 0]`). pub(crate) root_path: Box<[u8]>, - pub(crate) pending_err: Option, - pub(crate) pending_err_mutex: bun_threading::Mutex, + pub(crate) pending_err: bun_threading::Guarded>, + } + + /// A descriptor published once (by the thread that opens it, before any + /// other thread can look) and taken once (by the thread that closes it). + pub(crate) struct SharedFd(core::sync::atomic::AtomicU64); + impl SharedFd { + #[cfg(windows)] + fn encode(fd: FD) -> u64 { + fd.0 + } + #[cfg(windows)] + fn decode(v: u64) -> FD { + FD::from_native(v) + } + #[cfg(not(windows))] + fn encode(fd: FD) -> u64 { + u64::from(fd.0 as u32) + } + #[cfg(not(windows))] + fn decode(v: u64) -> FD { + FD::from_native(v as u32 as i32) + } + pub(crate) fn new(fd: FD) -> Self { + Self(core::sync::atomic::AtomicU64::new(Self::encode(fd))) + } + pub(crate) fn get(&self) -> FD { + Self::decode(self.0.load(Ordering::Acquire)) + } + pub(crate) fn set(&self, fd: FD) { + self.0.store(Self::encode(fd), Ordering::Release); + } + /// Swap in `FD::INVALID` and return what was there. + pub(crate) fn take(&self) -> FD { + Self::decode(self.0.swap(Self::encode(FD::INVALID), Ordering::AcqRel)) + } } - // SAFETY: shared by the pool subtasks through atomics / the lock-free - // queue / the mutex; `args` is Send by `ThreadIsolated`'s contract; results - // are owned buffers and WTF strings built off-thread for hand-off. - unsafe impl Send for AsyncReaddirRecursiveTask {} - impl Drop for AsyncReaddirRecursiveTask { + impl Drop for ReaddirScan { fn drop(&mut self) { - debug_assert!(self.root_fd == FD::INVALID, "scan still owns its root fd"); + debug_assert!( + self.root_fd.get() == FD::INVALID, + "scan still owns its root fd" + ); self.clear_result_list(); } } @@ -2175,31 +2091,25 @@ mod _async_tasks { this: &mut Self, done: bun_jsc::Completion, ) -> Option> { - this.done = Some(done); + *this.scan.done.lock() = Some(done); let mut buf = bun_paths::path_buffer_pool::get(); - let root_path_z = { - let bytes: &'static [u8] = - // SAFETY: `root_path` is a NUL-terminated `Box<[u8]>` fixed for the - // task's lifetime; `perform_work` mutates other fields only. - unsafe { bun_ptr::detach_lifetime(&this.root_path[..]) }; - ZStr::from_buf(bytes, bytes.len() - 1) - }; + // Subtasks reach the scan through their own `Arc`s; this thread's + // borrow of the job is not what they alias. + let scan = std::sync::Arc::clone(&this.scan); + let root_path_z = ZStr::from_buf(&scan.root_path[..], scan.root_path.len() - 1); // May finish synchronously (no subdirectories) or fan out; the last // subtask finishes the token. - this.perform_work(root_path_z, &mut buf, true); + scan.perform_work(root_path_z, &mut buf, true); None } - fn then( - mut this: Self, - js: AsyncFSJs, - cx: &bun_jsc::JsThread<'_>, - ) -> bun_jsc::JsResult<()> { + fn then(this: Self, js: AsyncFSJs, cx: &bun_jsc::JsThread<'_>) -> bun_jsc::JsResult<()> { let global_object = cx.global(); - let success = this.pending_err.is_none(); + let mut pending_err = this.scan.pending_err.lock().take(); + let success = pending_err.is_none(); let promise_value = js.promise.value(); let promise = js.promise.get(); - let result = if let Some(err) = &mut this.pending_err { + let result = if let Some(err) = &mut pending_err { match err.to_js_with_async_stack(global_object, promise) { Ok(v) => v, Err(e) => { @@ -2208,7 +2118,7 @@ mod _async_tasks { } } else { let res = match core::mem::replace( - &mut this.result_list, + &mut *this.scan.result_list.lock(), ResultListEntryValue::Files(Vec::new()), ) { ResultListEntryValue::WithFileTypes(v) => { @@ -2238,29 +2148,12 @@ mod _async_tasks { pub enum ResultListEntryValue { WithFileTypes(Vec), - Buffers(Vec), + Buffers(Vec>), Files(Vec), } - pub struct ResultListEntry { - pub(crate) next: bun_threading::Link, // INTRUSIVE: UnboundedQueue link - pub value: ResultListEntryValue, - } - - // SAFETY: all four accessors route through the same `next` field; the atomic - // variants reinterpret it in-place as `AtomicPtr` (identical layout/ - // alignment to `*mut Self`). `UnboundedQueue` only ever calls these with a - // live, properly aligned `*mut ResultListEntry` it previously had pushed. - unsafe impl bun_threading::Linked for ResultListEntry { - #[inline] - unsafe fn link(item: *mut Self) -> *const bun_threading::Link { - // SAFETY: `item` is valid and properly aligned per `UnboundedQueue` contract. - unsafe { core::ptr::addr_of!((*item).next) } - } - } - pub(super) struct ReaddirSubtask { - pub readdir_task: bun_ptr::ParentRef, + pub scan: std::sync::Arc, /// Heap-owned, NUL-terminated (`[basename.., 0]`); freed on drop. pub basename: Box<[u8]>, pub task: WorkPoolTask, @@ -2274,32 +2167,25 @@ mod _async_tasks { #[allow(clippy::boxed_local)] fn run_owned(self: Box) { let ReaddirSubtask { - readdir_task, + scan, basename, task: _, } = *self; - // `basename` is a NUL-terminated `Box<[u8]>` (`[bytes.., 0]`) from - // `enqueue()`; it frees on scope exit. - // SAFETY: `enqueue()` built `basename` with a trailing NUL at - // `[len]`, so `ZStr::from_buf` is valid. + // `enqueue()` built `basename` with a trailing NUL at `[len]`. let basename_z = ZStr::from_buf(&basename, basename.len() - 1); let mut buf = bun_paths::path_buffer_pool::get(); - // SAFETY: readdir_task (ParentRef) outlives subtask via subtask_count - // refcount. `from_raw_mut` was used at enqueue, so write provenance is - // present; this work-pool callback is the sole holder of `&mut` to the - // parent's per-result fields (it pushes to a lock-free queue). - unsafe { readdir_task.assume_mut() }.perform_work(basename_z, &mut buf, false); + scan.perform_work(basename_z, &mut buf, false); } } - impl AsyncReaddirRecursiveTask { - pub(crate) fn enqueue(&mut self, basename: &ZStr) { + impl ReaddirScan { + pub(crate) fn enqueue(self: &std::sync::Arc, basename: &ZStr) { if self.has_error.load(Ordering::Relaxed) { return; } // The subtask runs on another thread after the caller's `name_to_copy_z` // (which points into a per-iteration buffer) has been overwritten, so we - // must heap-own the bytes here. Freed in ReaddirSubtask::call's cleanup. + // must heap-own the bytes here. let mut owned = Vec::with_capacity(basename.len() + 1); owned.extend_from_slice(basename.as_bytes()); owned.push(0); @@ -2311,17 +2197,14 @@ mod _async_tasks { let prev = self.subtask_count.fetch_add(1, Ordering::Relaxed); debug_assert!(prev > 0); WorkPool::schedule_new(ReaddirSubtask { - // SAFETY: `self` is a `Box` (stable - // address) and outlives every subtask via the `subtask_count` - // refcount it just bumped. Write provenance from `&mut self`. - readdir_task: unsafe { - bun_ptr::ParentRef::from_raw_mut(core::ptr::from_mut(self)) - }, + scan: std::sync::Arc::clone(self), basename: basename_owned, task: WorkPoolTask::default(), }); } + } + impl AsyncReaddirRecursiveTask { pub(crate) fn create( global_object: &JSGlobalObject, args: ThreadIsolated>, @@ -2351,27 +2234,30 @@ mod _async_tasks { &global_object.js_thread(), AsyncReaddirRecursiveTask { args, - tag, - encoding, - done: None, - has_result: AtomicBool::new(false), - subtask_count: AtomicUsize::new(1), - has_error: AtomicBool::new(false), - root_path, - result_list, - result_list_count: AtomicUsize::new(0), - result_list_queue: UnboundedQueue::default(), - root_fd: FD::INVALID, - pending_err: None, - pending_err_mutex: bun_threading::Mutex::default(), + scan: std::sync::Arc::new(ReaddirScan { + tag, + encoding, + done: bun_threading::Guarded::new(None), + has_result: AtomicBool::new(false), + subtask_count: AtomicUsize::new(1), + has_error: AtomicBool::new(false), + root_path, + result_list: bun_threading::Guarded::new(result_list), + result_list_count: AtomicUsize::new(0), + result_list_queue: bun_threading::BoxQueue::default(), + root_fd: SharedFd::new(FD::INVALID), + pending_err: bun_threading::Guarded::new(None), + }), }, AsyncFSJs { promise, tracker }, ); value } + } + impl ReaddirScan { pub(crate) fn perform_work( - &mut self, + self: &std::sync::Arc, basename: &ZStr, buf: &mut PathBuffer, is_root: bool, @@ -2401,14 +2287,14 @@ mod _async_tasks { match res { Err(err) => { { - let _lock = self.pending_err_mutex.lock_guard(); - if self.pending_err.is_none() { + let mut pending_err = self.pending_err.lock(); + if pending_err.is_none() { let err_path: &[u8] = if !err.path.is_empty() { &err.path[..] } else { &self.root_path[..self.root_path.len() - 1] }; - self.pending_err = Some(err.with_path(err_path)); + *pending_err = Some(err.with_path(err_path)); } } self.has_error.store(true, Ordering::Relaxed); @@ -2423,11 +2309,11 @@ mod _async_tasks { match self.tag { ret::ReaddirTag::Files => impl_tag!(BunString, Files), ret::ReaddirTag::WithFileTypes => impl_tag!(Dirent, WithFileTypes), - ret::ReaddirTag::Buffers => impl_tag!(Buffer, Buffers), + ret::ReaddirTag::Buffers => impl_tag!(Box<[u8]>, Buffers), } } - pub(crate) fn write_results(&mut self, result: &mut Vec) { + pub(crate) fn write_results(&self, result: &mut Vec) { if !result.is_empty() { // `result` is already a heap `Vec`, so cloning would be a redundant // alloc+memcpy; just take ownership and trim the over-reservation @@ -2436,16 +2322,8 @@ mod _async_tasks { clone.shrink_to_fit(); self.result_list_count .fetch_add(clone.len(), Ordering::Relaxed); - // `IntoResultListEntry::into_variant` (trait dispatch on `T`). - let list = Box::new(ResultListEntry { - next: bun_threading::Link::new(), - value: ResultListEntryValue::from_vec(clone), - }); - // SAFETY: freshly boxed node; `into_raw` yields a valid owned non-null pointer. - unsafe { - self.result_list_queue - .push(NonNull::new_unchecked(bun_core::heap::into_raw(list))) - }; + let list = ResultListEntryValue::from_vec(clone); + self.result_list_queue.push(list); } self.on_subtask_done(); @@ -2454,14 +2332,14 @@ mod _async_tasks { /// Drops this subtask's `subtask_count` reference. The last one finishes /// the scan; `AcqRel` publishes every subtask's `pending_err` and queued /// results to it. - fn on_subtask_done(&mut self) { + fn on_subtask_done(&self) { if self.subtask_count.fetch_sub(1, Ordering::AcqRel) == 1 { self.finish_concurrently(); } } /// May be called from any thread (the subtasks) - pub(crate) fn finish_concurrently(&mut self) { + pub(crate) fn finish_concurrently(&self) { if self .has_result .compare_exchange(false, true, Ordering::Relaxed, Ordering::Relaxed) @@ -2471,72 +2349,34 @@ mod _async_tasks { } debug_assert!(self.subtask_count.load(Ordering::Relaxed) == 0); - let root_fd = self.root_fd; + let root_fd = self.root_fd.take(); if root_fd != FD::INVALID { use bun_sys::FdExt as _; - self.root_fd = FD::INVALID; root_fd.close(); } - if self.pending_err.is_some() { + if self.pending_err.lock().is_some() { self.clear_result_list(); } { - let list = self.result_list_queue.pop_batch(); - let mut iter = list.iterator(); - // we have to free only the previous one because the next value will - // be read by the iterator. - let mut to_destroy: Option<*mut ResultListEntry> = None; - - // `reserve_exact`/`append_from` dispatch on the runtime tag. + let mut result_list = self.result_list.lock(); let cap = self.result_list_count.swap(0, Ordering::Relaxed); - self.result_list.reserve_exact(cap); - loop { - let val = iter.next(); - if val.is_null() { - break; - } - if let Some(dest) = to_destroy { - // SAFETY: paired with heap::alloc in write_results() - unsafe { drop(bun_core::heap::take(dest)) }; - } - to_destroy = Some(val); - // SAFETY: `val` came from the queue and is live until heap::take above on the next iter - self.result_list - .append_from(&mut unsafe { &mut *val }.value); - } - if let Some(dest) = to_destroy { - // SAFETY: paired with heap::alloc in write_results() - unsafe { drop(bun_core::heap::take(dest)) }; + result_list.reserve_exact(cap); + for mut list in self.result_list_queue.drain() { + result_list.append_from(&mut list); } } // Hand the scan back to its VM (or, if that is gone, to the release // that frees this off-thread part). Last touch of `self` on this thread. - self.done.take().expect("scan finished twice").finish(); + let done = self.done.lock().take(); + done.expect("scan finished twice").finish(); } - fn clear_result_list(&mut self) { - self.result_list.clear(); - let batch = self.result_list_queue.pop_batch(); - let mut iter = batch.iterator(); - let mut to_destroy: Option<*mut ResultListEntry> = None; - loop { - let val = iter.next(); - if val.is_null() { - break; - } - if let Some(dest) = to_destroy { - // SAFETY: paired with heap::alloc in write_results() - unsafe { drop(bun_core::heap::take(dest)) }; - } - to_destroy = Some(val); - } - if let Some(dest) = to_destroy { - // SAFETY: paired with heap::alloc in write_results() - unsafe { drop(bun_core::heap::take(dest)) }; - } + fn clear_result_list(&self) { + self.result_list.lock().clear(); + drop(self.result_list_queue.drain()); self.result_list_count.store(0, Ordering::Relaxed); } } @@ -2553,7 +2393,7 @@ mod _async_tasks { ResultListEntryValue::WithFileTypes(v) } } - impl IntoResultListEntry for Buffer { + impl IntoResultListEntry for Box<[u8]> { fn into_variant(v: Vec) -> ResultListEntryValue { ResultListEntryValue::Buffers(v) } @@ -2592,10 +2432,12 @@ mod _async_tasks { } } } // mod _async_tasks +#[cfg(windows)] +pub use _async_tasks::UvFsSubmit; pub use _async_tasks::{ - AsyncCpTask, AsyncFSTask, AsyncReaddirRecursiveTask, CpSingleTask, FsArgument, FsReturn, - IntoResultListEntry, NewAsyncCpTask, ResultListEntry, ResultListEntryValue, ShellAsyncCpTask, - UVFSRequest, async_, + AsyncCpTask, AsyncFSTask, AsyncReaddirRecursiveTask, CpSingleTask, CpTaskRef, FsArgument, + FsReturn, IntoResultListEntry, NewAsyncCpTask, ReaddirScan, ResultListEntryValue, + ShellAsyncCpTask, UVFSRequest, async_, }; // ────────────────────────────────────────────────────────────────────────── @@ -3509,7 +3351,7 @@ pub mod args { /// If this method is invoked as its `util.promisify()` ed version, it returns /// a promise for an `Object` with `bytesWritten` and `buffer` properties. /// - /// It is unsafe to use `fs.write()` multiple times on the same file without waiting + /// It is not safe to use `fs.write()` multiple times on the same file without waiting /// for the callback. For this scenario, {@link createWriteStream} is /// recommended. /// @@ -4273,6 +4115,28 @@ impl StringOrUndefined { } } +/// A path or file-contents result built off-thread: an already-encoded +/// string, or bytes that become a node `Buffer` on the JS thread. +pub enum StringOrBytes { + String(Utf8WithString), + Bytes(Box<[u8]>), +} +impl StringOrBytes { + #[inline] + pub fn string(s: BunString) -> Self { + Self::String(Utf8WithString::js_only(s)) + } + pub fn into_js(self, global_object: &JSGlobalObject) -> JsResult { + match self { + StringOrBytes::String(s) => s.into_js(global_object), + StringOrBytes::Bytes(bytes) => { + bun_jsc::MarkedArrayBuffer::from_owned_bytes(bytes, bun_jsc::JSType::Uint8Array) + .to_node_buffer(global_object) + } + } + } +} + /// For use in `Return`'s definitions to act as `void` while returning `null` to JavaScript pub struct Null; @@ -4299,7 +4163,7 @@ pub mod ret { pub(crate) type Link = (); pub(crate) type Lstat = StatOrNotFound; pub(crate) type Mkdir = StringOrUndefined; - pub(crate) type Mkdtemp = StringOrBuffer<'static>; + pub(crate) type Mkdtemp = StringOrBytes; pub(crate) type Open = FD; pub(crate) type WriteFile = (); pub(crate) type Readv = Read; @@ -4333,7 +4197,8 @@ pub mod ret { pub enum Readdir { WithFileTypes(Box<[Dirent]>), - Buffers(Box<[Buffer]>), + /// Entry names as bytes; each becomes a node `Buffer` in `to_js`. + Buffers(Box<[Box<[u8]>]>), Files(Box<[BunString]>), } impl Readdir { @@ -4350,12 +4215,16 @@ pub mod ret { } Readdir::Buffers(mut items) => { // Node returns `Buffer[]` for `{ encoding: "buffer" }`, not - // `Uint8Array[]`. Ownership of every `Buffer`'s bytes + // `Uint8Array[]`. Ownership of every entry's bytes // transfers to JSC via `to_node_buffer`; the boxed slice // itself is freed when `items` drops. let array = JSValue::create_empty_array(global_object, items.len())?; for (i, item) in items.iter_mut().enumerate() { - let res = item.to_node_buffer(global_object)?; + let res = bun_jsc::MarkedArrayBuffer::from_owned_bytes( + core::mem::take(item), + bun_jsc::JSType::Uint8Array, + ) + .to_node_buffer(global_object)?; array.put_index(global_object, i as u32, res)?; } Ok(array) @@ -4371,16 +4240,23 @@ pub mod ret { } pub(crate) type ReadFile = StringOrBuffer<'static>; + /// What `fs.promises.readFile` carries back from the pool: never the + /// JSC-heap buffer the sync path can produce, so it is `Send`. + pub(crate) type ReadFileOffThread = StringOrBytes; pub(crate) enum ReadFileWithOptions { String(Box<[u8]>), TranscodedString(BunString), - Buffer(Buffer), + /// File contents to hand to JS as a `Buffer`. + Bytes(Box<[u8]>), + /// `Flavor::Sync` with a VM only: the contents already copied into a + /// JSC-heap buffer (kept alive by the caller's stack until returned). + JsBuffer(Buffer), NullTerminated(bun_core::ZBox), // [:0]const u8 owned } - pub(crate) type Readlink = StringOrBuffer<'static>; - pub(crate) type Realpath = StringOrBuffer<'static>; + pub(crate) type Readlink = StringOrBytes; + pub(crate) type Realpath = StringOrBytes; pub(crate) type Rename = (); pub(crate) type Rmdir = (); pub(crate) type Stat = StatOrNotFound; @@ -4404,7 +4280,9 @@ pub mod ret { pub struct NodeFS { /// Scratch for a temporary file path that might appear in a returned error message. pub(crate) sync_error_buf: bun_paths::path_buffer_pool::Guard, - pub(crate) vm: Option>, + /// The VM whose `fs` binding owns this (the sync path); `None` for the + /// pool's and other ad-hoc instances. + pub(crate) vm: Option>, } impl Default for NodeFS { @@ -4419,17 +4297,11 @@ impl Default for NodeFS { /// Encode a path returned by the OS (`mkdtemp`/`readlink`/`realpath`) using the /// caller's `encoding` option, matching Node.js: `"buffer"` yields a `Buffer` /// of the raw bytes, any other encoding is `Buffer.from(bytes).toString(enc)`. -fn encode_path_result(bytes: &[u8], encoding: Encoding) -> StringOrBuffer<'static> { +fn encode_path_result(bytes: &[u8], encoding: Encoding) -> StringOrBytes { match encoding { - Encoding::Buffer => { - StringOrBuffer::Buffer(Buffer::from_string(bytes).expect("unreachable")) - } - Encoding::Utf8 => { - StringOrBuffer::String(Utf8WithString::js_only(BunString::clone_utf8(bytes))) - } - enc => StringOrBuffer::String(Utf8WithString::js_only(webcore::encoding::to_bun_string( - bytes, enc, - ))), + Encoding::Buffer => StringOrBytes::Bytes(bytes.into()), + Encoding::Utf8 => StringOrBytes::string(BunString::clone_utf8(bytes)), + enc => StringOrBytes::string(webcore::encoding::to_bun_string(bytes, enc)), } } @@ -4527,33 +4399,25 @@ impl NodeFS { // window for the sequential read()s below. Best-effort. #[cfg(any(target_os = "linux", target_os = "android", target_os = "freebsd"))] { - // SAFETY: `src_fd` is a valid open fd; `posix_fadvise` only reads it. - let _ = - unsafe { libc::posix_fadvise(src_fd.native(), 0, 0, libc::POSIX_FADV_SEQUENTIAL) }; + let _ = sys::posix_fadvise(src_fd, 0, 0, libc::POSIX_FADV_SEQUENTIAL); } + // The slab stays uninitialised (write-only: `read` fills it from the + // kernel and hands back the filled prefix). Zero-filling it was a + // debug-build hot path. const STACK_BUF_LEN: usize = 64 * 1024; let mut stack_buf = bun_core::vec::UninitBuf::::uninit(); let mut buf_to_free: Vec = Vec::new(); - // SAFETY: `Syscall::read` is the only writer of `buf`; each iteration reads back only `buf[..amt]`. - let mut buf: &mut [u8] = unsafe { stack_buf.as_bytes_mut() }; + let mut buf: &mut [core::mem::MaybeUninit] = stack_buf.as_uninit_mut(); 'maybe_allocate_large_temp_buf: { if stat_size > STACK_BUF_LEN * 16 { // Don't allocate more than 8 MB at a time let clamped_size: usize = stat_size.min(8 * 1024 * 1024); - // The slab must stay uninitialised: `Vec::resize` here was a - // debug-build hot path (byte-by-byte `extend_with`). Use - // `expand_to_capacity` instead — the slab is write-only, - // `Syscall::read` fills it from the kernel. - use bun_collections::vec_ext::VecExt as _; if buf_to_free.try_reserve_exact(clamped_size).is_err() { break 'maybe_allocate_large_temp_buf; } - // SAFETY: `u8` has no validity invariant; the buffer is handed - // straight to the kernel which only stores into it. - unsafe { buf_to_free.expand_to_capacity() }; - buf = &mut buf_to_free[..]; + buf = buf_to_free.spare_capacity_mut(); } } // buf_to_free dropped at scope exit @@ -4566,7 +4430,7 @@ impl NodeFS { let mut broke = false; 'toplevel: while remain > 0 { let read_len = (buf.len() as u64).min(remain) as usize; - let amt = match Syscall::read(src_fd, &mut buf[..read_len]) { + let filled: &[u8] = match sys::read_uninit(src_fd, &mut buf[..read_len]) { Ok(result) => result, Err(err) => { return Err(if !src.is_empty() { @@ -4576,6 +4440,7 @@ impl NodeFS { }); } }; + let amt = filled.len(); // 0 == EOF if amt == 0 { broke = true; @@ -4584,7 +4449,7 @@ impl NodeFS { *wrote += amt as u64; remain = remain.saturating_sub(amt as u64); - let mut slice = &buf[..amt]; + let mut slice = filled; while !slice.is_empty() { let written = match Syscall::write(dest_fd, slice) { Ok(result) => result, @@ -4605,7 +4470,7 @@ impl NodeFS { } if !broke { 'outer: loop { - let amt = match Syscall::read(src_fd, buf) { + let filled: &[u8] = match sys::read_uninit(src_fd, buf) { Ok(result) => result, Err(err) => { return Err(if !src.is_empty() { @@ -4615,6 +4480,7 @@ impl NodeFS { }); } }; + let amt = filled.len(); // we don't know the size // so we just go forever until we get an EOF if amt == 0 { @@ -4622,7 +4488,7 @@ impl NodeFS { } *wrote += amt as u64; - let mut slice = &buf[..amt]; + let mut slice = filled; while !slice.is_empty() { let written = match Syscall::write(dest_fd, slice) { Ok(result) => result, @@ -4857,20 +4723,17 @@ impl NodeFS { // first; fall back to read/write on cross-device or unsupported // fd types. 'cfr: loop { - // SAFETY: src_fd/dest_fd are valid open fds; copy_file_range is the libc FFI. - // Null offsets so the kernel advances the file's seek position, keeping the + // No offsets so the kernel advances the file's seek position, keeping the // read/write fallback (which uses the seek position) coherent if we ever // break mid-loop. - let rc: isize = unsafe { - sys::freebsd::copy_file_range( - src_fd.native(), - core::ptr::null_mut(), - dest_fd.native(), - core::ptr::null_mut(), - (i32::MAX - 1) as usize, - 0, - ) - } as isize; + let rc: isize = sys::freebsd::copy_file_range_fd( + src_fd, + None, + dest_fd, + None, + (i32::MAX - 1) as usize, + 0, + ) as isize; match sys::get_errno(rc) { E::SUCCESS => { if rc == 0 { @@ -4999,17 +4862,14 @@ impl NodeFS { loop { // Linux Kernel 5.3 or later // Not supported in gVisor - // SAFETY: src_fd/dest_fd are valid open fds; copy_file_range is the libc FFI - let written = unsafe { - sys::linux::copy_file_range( - src_fd.native(), - &raw mut off_in_copy, - dest_fd.native(), - &raw mut off_out_copy, - sys::page_size(), - 0, - ) - }; + let written = sys::linux::copy_file_range_fd( + src_fd, + Some(&mut off_in_copy), + dest_fd, + Some(&mut off_out_copy), + sys::page_size(), + 0, + ); if let Some(err) = Maybe::::errno_sys_p( written, sys::Tag::copy_file_range, @@ -5037,17 +4897,14 @@ impl NodeFS { } } else { while size > 0 { - // SAFETY: src_fd/dest_fd are valid open fds; copy_file_range is the libc FFI - let written = unsafe { - sys::linux::copy_file_range( - src_fd.native(), - &raw mut off_in_copy, - dest_fd.native(), - &raw mut off_out_copy, - size, - 0, - ) - }; + let written = sys::linux::copy_file_range_fd( + src_fd, + Some(&mut off_in_copy), + dest_fd, + Some(&mut off_out_copy), + size, + 0, + ); if let Some(err) = Maybe::::errno_sys_p( written, sys::Tag::copy_file_range, @@ -5098,15 +4955,7 @@ impl NodeFS { args.src.slice(), ); let dest = strings::to_kernel32_path(&mut *dest_buf, args.dest.slice()); - // SAFETY: src/dest are NUL-terminated wide paths; CopyFileW is the Win32 FFI - if unsafe { - windows::CopyFileW( - src.as_ptr(), - dest.as_ptr(), - if args.mode.shouldnt_overwrite() { 1 } else { 0 }, - ) - } == windows::FALSE - { + if !windows::copy_file(src, dest, args.mode.shouldnt_overwrite()) { return Self::should_ignore_ebusy( &args.src, &args.dest, @@ -5204,15 +5053,8 @@ impl NodeFS { } #[cfg(not(windows))] { - // `fdatasync(int)` has no memory-safety preconditions (a bad fd just - // yields EBADF), so declare it `safe fn` for all unix instead of - // routing through `libc::fdatasync` (which is blanket-`unsafe`). - // `libc` also omits the Darwin binding (fdatasync exists since 10.7). - unsafe extern "C" { - safe fn fdatasync(fd: libc::c_int) -> libc::c_int; - } Maybe::::errno_sys_fd( - fdatasync(args.fd.native()), + sys::safe_libc::fdatasync(args.fd.native()), sys::Tag::fdatasync, args.fd, ) @@ -5241,13 +5083,7 @@ impl NodeFS { } #[cfg(not(windows))] { - // `fsync(int)` has no memory-safety preconditions (a bad fd just yields - // EBADF), so declare it `safe fn` instead of routing through - // `libc::fsync` (which is blanket-`unsafe`). Mirrors `fdatasync` above. - unsafe extern "C" { - safe fn fsync(fd: libc::c_int) -> libc::c_int; - } - Maybe::::errno_sys(fsync(args.fd.native()), sys::Tag::fsync) + Maybe::::errno_sys(sys::safe_libc::fsync(args.fd.native()), sys::Tag::fsync) .unwrap_or(Ok(())) } } @@ -5259,21 +5095,7 @@ impl NodeFS { pub(crate) fn futimes(&mut self, args: &args::Futimes, _: Flavor) -> Maybe { #[cfg(windows)] { - let mut req = UvFsReq::new(); - let rc = unsafe { - uv::uv_fs_futime( - uv::Loop::get(), - &mut *req, - args.fd.uv(), - args.atime, - args.mtime, - None, - ) - }; - if let Some(err) = rc.to_error(sys::Tag::futime) { - return Err(err.with_fd(args.fd)); - } - return Ok(()); + return sys::sys_uv::futime(args.fd, args.atime, args.mtime); } #[cfg(not(windows))] match Syscall::futimens( @@ -5331,22 +5153,10 @@ impl NodeFS { let mut to_buf = bun_paths::path_buffer_pool::get(); let from = args.old_path.slice_z(&mut self.sync_error_buf); let to = args.new_path.slice_z(&mut to_buf); - #[cfg(windows)] - { - return match Syscall::link(from, to) { - Err(err) => Err(err.with_path_dest(args.old_path.slice(), args.new_path.slice())), - Ok(result) => Ok(result), - }; + match Syscall::link(from, to) { + Err(err) => Err(err.with_path_dest(args.old_path.slice(), args.new_path.slice())), + Ok(result) => Ok(result), } - // SAFETY: `from`/`to` are NUL-terminated by `slice_z`; `link(2)` is the libc FFI. - #[cfg(not(windows))] - Maybe::::errno_sys_pd( - unsafe { libc::link(from.as_ptr().cast(), to.as_ptr().cast()) }, - sys::Tag::link, - args.old_path.slice(), - args.new_path.slice(), - ) - .unwrap_or(Ok(())) } pub(crate) fn lstat(&mut self, args: &args::Lstat, _: Flavor) -> Maybe { @@ -5520,25 +5330,8 @@ impl NodeFS { } } - // SAFETY: `sync_error_buf` is a pooled heap allocation, and every - // supported allocator aligns it to at least 8 bytes, so this address is - // ≥ `align_of::()`-aligned. On Windows - // `OSPathBuffer = [u16; PATH_MAX_WIDE]` (65 534 B) which fits inside - // `PathBuffer` (`MAX_PATH_BYTES` = 98 302 B); on POSIX it is the same - // type. The `assert!` below verifies the alignment at runtime. - // Keep the raw `*mut PathBuffer` so error-return paths can re-derive a fresh - // `&mut PathBuffer` without reborrowing `&mut self` (which would alias - // `working_mem` under stacked borrows). On every such path `working_mem` is - // not used afterward, so the re-derive is sound. - let sync_error_buf_ptr: *mut PathBuffer = &raw mut *self.sync_error_buf; - assert!( - sync_error_buf_ptr.cast::().is_aligned(), - "NodeFS.sync_error_buf misaligned for OSPathChar", - ); - // SAFETY: alignment asserted above; `OSPathBuffer` fits inside `PathBuffer` - // (same type on POSIX; smaller on Windows) and `self.sync_error_buf` is exclusive. - let working_mem: &mut OSPathBuffer = - unsafe { &mut *sync_error_buf_ptr.cast::() }; + let mut working_mem = paths::os_path_buffer_pool::get(); + let working_mem: &mut OSPathBuffer = &mut working_mem; working_mem[..len as usize].copy_from_slice(&(&path[..])[..len as usize]); let mut i: u16 = len - 1; @@ -5547,9 +5340,7 @@ impl NodeFS { while i > 0 { if bun_paths::is_sep_native_t::((&path[..])[i as usize]) { working_mem[i as usize] = 0; - // SAFETY: `working_mem[..i]` is initialized from `path` above and - // `working_mem[i]` was just set to NUL; the slice is in-bounds. - let parent = unsafe { OSPathSliceZ::from_raw(working_mem.as_ptr(), i as usize) }; + let parent = OSPathSliceZ::from_buf(&working_mem[..], i as usize); match mkdir_os_path(parent, mode) { Err(err) => { // The SEP-restore must NOT happen before the errno match: @@ -5567,16 +5358,14 @@ impl NodeFS { { // is a directory. break. if !res { - // SAFETY: `working_mem` is not used after this return; the - // re-derived &mut PathBuffer is scoped to the call. return Err(sys::Error { errno: E::ENOTDIR as _, syscall: sys::Tag::mkdir, - path: Self::os_path_into_buf( - unsafe { &mut *sync_error_buf_ptr }, - without_nt_prefix(&(&path[..])[..len as usize]), - ) - .into(), + path: self + .os_path_into_sync_error_buf(without_nt_prefix( + &(&path[..])[..len as usize], + )) + .into(), ..Default::default() }); } @@ -5592,24 +5381,11 @@ impl NodeFS { continue; } _ => { - #[cfg(windows)] - let p = { - // `parent` aliases `working_mem` (== sync_error_buf). Copy it - // out to a temp before re-deriving `&mut PathBuffer` so we - // never hold `&mut buf` and `&buf[..]` simultaneously. - let stripped = without_nt_prefix(&parent[..]); - let n = stripped.len(); - let mut tmp = paths::os_path_buffer_pool::get(); - tmp[..n].copy_from_slice(stripped); - // SAFETY: `working_mem`/`parent` are not used after this return. - Self::os_path_into_buf( - unsafe { &mut *sync_error_buf_ptr }, - &tmp[..n], - ) - }; - #[cfg(not(windows))] - let p = without_nt_prefix(&parent[..]); - return Err(err.with_path(p)); + return Err(err.with_path( + self.os_path_into_sync_error_buf(without_nt_prefix( + &parent[..], + )), + )); } } } @@ -5629,9 +5405,7 @@ impl NodeFS { while i < len { if bun_paths::is_sep_native_t::((&path[..])[i as usize]) { working_mem[i as usize] = 0; - // SAFETY: `working_mem[..i]` is initialized from `path` and - // `working_mem[i]` was just set to NUL; the slice is in-bounds. - let parent = unsafe { OSPathSliceZ::from_raw(working_mem.as_ptr(), i as usize) }; + let parent = OSPathSliceZ::from_buf(&working_mem[..], i as usize); match mkdir_os_path(parent, mode) { Err(err) => { working_mem[i as usize] = paths::SEP as OSPathChar; @@ -5640,12 +5414,9 @@ impl NodeFS { E::EEXIST => {} // NOENT shouldn't happen here _ => { - // SAFETY: `working_mem` is not used after this return; - // the re-derived &mut PathBuffer is scoped to the call. - return Err(err.with_path(Self::os_path_into_buf( - unsafe { &mut *sync_error_buf_ptr }, - without_nt_prefix(&path[..]), - ))); + return Err(err.with_path( + self.os_path_into_sync_error_buf(without_nt_prefix(&path[..])), + )); } } } @@ -5662,19 +5433,14 @@ impl NodeFS { // Our final directory will not have a trailing separator // so we have to create it once again - // SAFETY: `working_mem[..len]` is the full input path and `working_mem[len]` - // was just set to NUL; the slice is in-bounds. - let final_ = unsafe { OSPathSliceZ::from_raw(working_mem.as_ptr(), len as usize) }; + let final_ = OSPathSliceZ::from_buf(&working_mem[..], len as usize); match mkdir_os_path(final_, mode) { Err(err) => match err.get_errno() { E::EEXIST => {} _ => { - // SAFETY: `working_mem` is not used after this return; the - // re-derived &mut PathBuffer is scoped to the call. - return Err(err.with_path(Self::os_path_into_buf( - unsafe { &mut *sync_error_buf_ptr }, - without_nt_prefix(&path[..]), - ))); + return Err(err.with_path( + self.os_path_into_sync_error_buf(without_nt_prefix(&path[..])), + )); } }, Ok(_) => {} @@ -5707,50 +5473,15 @@ impl NodeFS { prefix_buf[len..len + 6].copy_from_slice(b"XXXXXX"); prefix_buf[len + 6] = 0; - // The mkdtemp() function returns a pointer to the modified template - // string on success, and NULL on failure, in which case errno is set to - // indicate the error - - #[cfg(windows)] - { - let mut req = UvFsReq::new(); - let rc = unsafe { - uv::uv_fs_mkdtemp( - bun_io::Loop::get(), - &mut *req, - prefix_buf.as_ptr().cast(), - None, - ) - }; - if let Some(err) = rc.to_error(sys::Tag::mkdtemp) { - return Err(err.with_path(&prefix_buf[..len + 6])); - } - // SAFETY: on success libuv populates `req.path` with a NUL-terminated - // UTF-8 string owned by the request; `UvFsReq::drop` runs - // `uv_fs_req_cleanup` in place after we've copied the bytes out. - let bytes = unsafe { bun_core::ffi::cstr(req.path) }.to_bytes(); - return Ok(encode_path_result(bytes, args.encoding)); - } - - #[cfg(not(windows))] - { - // SAFETY: `prefix_buf` is NUL-terminated and writable; mkdtemp(3) writes the - // generated name back into the buffer in-place. - let rc = unsafe { libc::mkdtemp(prefix_buf.as_mut_ptr().cast()) }; - if !rc.is_null() { - // SAFETY: `rc` is non-null and points back into `prefix_buf`, which is - // NUL-terminated and outlives this borrow. - let bytes = unsafe { bun_core::ffi::cstr(rc) }.to_bytes(); - return Ok(encode_path_result(bytes, args.encoding)); - } - - let errno = sys::last_errno(); - Err(sys::Error { - errno: errno as _, + // The created name is written back over the template in `prefix_buf`. + match sys::mkdtemp(&mut prefix_buf[..]) { + Ok(n) => Ok(encode_path_result(&prefix_buf[..n], args.encoding)), + Err(err) => Err(sys::Error { + errno: err.errno, syscall: sys::Tag::mkdtemp, path: prefix_buf[..len + 6].into(), ..Default::default() - }) + }), } } @@ -5779,20 +5510,16 @@ impl NodeFS { pub(crate) fn uv_statfs( &mut self, args: &args::StatFS, - req: &mut uv::fs_t, + req: &uv::fs_t, rc: uv::ReturnCodeI64, ) -> Maybe { if let Some(err) = rc.to_error(sys::Tag::statfs) { return Err(err.with_path(args.path.slice())); } - // libuv stores - // a `uv_statfs_t*` in `req.ptr` on success. The struct is unaligned in - // the request buffer, hence `read_unaligned`. - // SAFETY: `rc >= 0` ⇒ libuv populated `req.ptr` with a valid - // `uv_statfs_t` (= `RawStatFS` on Windows); we copy it out by value - // before `uv_fs_req_cleanup` releases the backing storage. + // `rc >= 0` ⇒ libuv populated `req.ptr` with a `uv_statfs_t` + // (= `RawStatFS` on Windows); copied out before cleanup frees it. let statfs_: super::statfs::RawStatFS = - unsafe { core::ptr::read_unaligned(req.ptr_as::()) }; + req.statfs_result().expect("uv_fs_statfs succeeded"); Ok(ret::StatFS::init(&statfs_, args.big_int)) } @@ -5992,19 +5719,8 @@ impl NodeFS { fn pwritev_inner(&mut self, args: &args::Writev) -> Maybe { let mut position = args.position.unwrap() as i64; - // `PlatformIoVec` - // and `PlatformIoVecConst` are layout-identical (`{ *void, usize }`); the - // kernel never writes through `iov_base` for pwritev(2). - // SAFETY: layout-compatible reinterpretation, asserted in `bun_sys`. - let vecs: &[sys::PlatformIoVecConst] = unsafe { - core::slice::from_raw_parts( - args.buffers - .buffers - .as_ptr() - .cast::(), - args.buffers.buffers.len(), - ) - }; + // The kernel never writes through `iov_base` for pwritev(2). + let vecs: &[sys::PlatformIoVecConst] = sys::iovecs_as_const(&args.buffers.buffers); // libuv `uv__fs_write_all`: loop IOV_MAX-sized batches until every // buffer is written; an error after the first batch returns the // accumulated total instead of the error. @@ -6074,7 +5790,7 @@ impl NodeFS { ); } let maybe = match args.tag() { - ret::ReaddirTag::Buffers => Self::readdir_inner::( + ret::ReaddirTag::Buffers => Self::readdir_inner::>( &mut self.sync_error_buf, args, args.recursive, @@ -6211,28 +5927,20 @@ impl NodeFS { pub(crate) fn readdir_with_entries_recursive_async( buf: &mut PathBuffer, - async_task: &mut AsyncReaddirRecursiveTask, + async_task: &std::sync::Arc, basename: &ZStr, entries: &mut Vec, is_root: bool, ) -> Maybe<()> { - // `root_path` is never mutated for the lifetime of the task, but - // borrowck can't see that across `async_task.enqueue(&mut self, …)`. Detach - // the slice via raw-pointer round-trip. - let root_basename: &[u8] = { - // `root_path` is NUL-terminated (`[path.., 0]`); the basename - // excludes the trailing NUL. - let path = &async_task.root_path; - // SAFETY: `async_task.root_path`'s backing storage is fixed at - // `create()` and outlives every `enqueue` call below. - unsafe { bun_ptr::detach_lifetime(&path[..path.len() - 1]) } - }; + // `root_path` is NUL-terminated (`[path.., 0]`); the basename + // excludes the trailing NUL. + let root_basename: &[u8] = &async_task.root_path[..async_task.root_path.len() - 1]; #[cfg(not(windows))] let flags = sys::O::DIRECTORY | sys::O::RDONLY; let atfd = if is_root { FD::cwd() } else { - async_task.root_fd + async_task.root_fd.get() }; #[cfg(not(windows))] let open_res = Syscall::openat(atfd, basename, flags, 0); @@ -6269,7 +5977,7 @@ impl NodeFS { }; if is_root { - async_task.root_fd = fd; + async_task.root_fd.set(fd); } let _close = scopeguard::guard((fd, is_root), |(fd, is_root)| { if !is_root { @@ -6305,21 +6013,16 @@ impl NodeFS { // The root subtask's basename *is* root_path; the caller passes // `is_root` explicitly. - let name_to_copy: &[u8] = if is_root { - utf8_name + let name_to_copy_z: &ZStr = if is_root { + current.name_assume_z() } else { paths::resolve_path::join_z_buf_spill::( &mut buf[..], &mut spill, &[basename.as_bytes(), utf8_name], ) - .as_bytes() }; - // SAFETY: both branches yield NUL-terminated storage — `utf8_name` is a - // slice over the iterator's NUL-terminated dirent name, and - // `join_z_buf_spill` writes a sentinel. - let name_to_copy_z = - unsafe { ZStr::from_raw(name_to_copy.as_ptr(), name_to_copy.len()) }; + let name_to_copy: &[u8] = name_to_copy_z.as_bytes(); // Track effective kind - may be resolved from .unknown via stat let mut effective_kind = current.kind; @@ -6724,39 +6427,62 @@ impl NodeFS { args: &args::ReadFile, flavor: Flavor, ) -> Maybe { - let result = self.read_file_with_options(args, flavor, ReadFileStringType::Default); + match self.read_file_with_options(args, flavor, ReadFileStringType::Default)? { + ret::ReadFileWithOptions::JsBuffer(buffer) => Ok(StringOrBuffer::Buffer(buffer)), + ret::ReadFileWithOptions::Bytes(bytes) => Ok(StringOrBuffer::Buffer( + bun_jsc::MarkedArrayBuffer::from_owned_bytes(bytes, bun_jsc::JSType::Uint8Array), + )), + string => Ok(StringOrBuffer::String(Utf8WithString::js_only( + Self::read_file_string(args, string)?, + ))), + } + } + + /// [`read_file`](Self::read_file) with a `Send` result: what + /// `fs.promises.readFile` runs on the pool (no VM there, so never the + /// JSC-heap buffer case). + pub(crate) fn read_file_off_thread( + &mut self, + args: &args::ReadFile, + flavor: Flavor, + ) -> Maybe { + match self.read_file_with_options(args, flavor, ReadFileStringType::Default)? { + ret::ReadFileWithOptions::Bytes(bytes) => Ok(StringOrBytes::Bytes(bytes)), + string => Ok(StringOrBytes::string(Self::read_file_string(args, string)?)), + } + } + + /// The string cases of a `ReadFileStringType::Default` read, encoded per + /// `args.encoding`. + fn read_file_string( + args: &args::ReadFile, + result: ret::ReadFileWithOptions, + ) -> Maybe { match result { - Err(err) => Err(err), - Ok(result) => match result { - ret::ReadFileWithOptions::Buffer(buffer) => Ok(StringOrBuffer::Buffer(buffer)), - ret::ReadFileWithOptions::TranscodedString(str) => { - if str.is_dead() { - return Err(with_path_like( - sys::Error::from_code(E::ENOMEM, sys::Tag::read), - &args.path, - )); - } - Ok(StringOrBuffer::String(Utf8WithString::js_only(str))) + ret::ReadFileWithOptions::TranscodedString(str) => { + if str.is_dead() { + return Err(with_path_like( + sys::Error::from_code(E::ENOMEM, sys::Tag::read), + &args.path, + )); } - ret::ReadFileWithOptions::String(s) => { - let str = if s.is_empty() { - bun_core::String::EMPTY - } else { - webcore::encoding::to_bun_string_from_owned_slice( - s.into_vec(), - args.encoding, - ) - }; - if str.is_dead() { - return Err(with_path_like( - sys::Error::from_code(E::ENOMEM, sys::Tag::read), - &args.path, - )); - } - Ok(StringOrBuffer::String(Utf8WithString::js_only(str))) + Ok(str) + } + ret::ReadFileWithOptions::String(s) => { + let str = if s.is_empty() { + BunString::EMPTY + } else { + webcore::encoding::to_bun_string_from_owned_slice(s.into_vec(), args.encoding) + }; + if str.is_dead() { + return Err(with_path_like( + sys::Error::from_code(E::ENOMEM, sys::Tag::read), + &args.path, + )); } - _ => unreachable!(), - }, + Ok(str) + } + _ => unreachable!(), } } @@ -6775,18 +6501,7 @@ impl NodeFS { if let Some(file) = graph.find_ref(path.as_bytes()) { let contents: &[u8] = file.utf8_contents(); return if args.encoding == Encoding::Buffer { - // PORTING.md §Forbidden bans `Vec::leak()`; round-trip through - // `into_boxed_slice()` so the allocation layout JSC frees with - // matches what we hand it (capacity == len). - let raw = - bun_core::heap::into_raw(contents.to_vec().into_boxed_slice()); - // SAFETY: ownership of the allocation is transferred to JSC; the - // ArrayBuffer finalizer reconstructs the Box and frees it - // (PORTING.md:348 — `heap::alloc`/`from_raw` across FFI). - Ok(ret::ReadFileWithOptions::Buffer(Buffer::from_bytes( - unsafe { &mut *raw }, - bun_jsc::JSType::Uint8Array, - ))) + Ok(ret::ReadFileWithOptions::Bytes(contents.into())) } else if string_type == ReadFileStringType::Default { Ok(ret::ReadFileWithOptions::String( contents.to_vec().into_boxed_slice(), @@ -6856,48 +6571,54 @@ impl NodeFS { // otherwise (async, no VM, or a read further up the stack holds it) // a heap buffer stands in. It stays uninitialised: it is write-only, // `Syscall::read` hands it straight to the kernel. - use bun_collections::vec_ext::VecExt as _; - let mut scratch = match self.vm { - // SAFETY: `self.vm` is the live owning `*mut VirtualMachine` (single-threaded VM), which outlives this call. - Some(vm) if flavor == Flavor::Sync => unsafe { - (*(*vm.as_ptr()).rare_data_ptr()).pipe_read_scratch.claim() - }, + let vm = self.vm; + let mut scratch = match &vm { + Some(vm) if flavor == Flavor::Sync => { + vm.get().as_mut().rare_data().pipe_read_scratch.claim() + } _ => None, }; let mut heap_buffer: Vec = Vec::new(); - if scratch.is_none() && heap_buffer.try_reserve_exact(256 * 1024).is_ok() { - // SAFETY: `u8` has no validity invariant; the buffer is handed - // straight to the kernel which only stores into it. Only the - // `[..total]` prefix actually filled by `read` is ever observed. - unsafe { heap_buffer.expand_to_capacity() }; - } - let pre_stat_buf: &mut [u8] = match scratch.as_mut() { - Some(scratch) => &mut scratch[..], - None => &mut heap_buffer[..], - }; - let pre_stat_len = pre_stat_buf - .len() - .min(args.max_size.map_or(usize::MAX, |v| v as usize)); - let pre_stat_buf = &mut pre_stat_buf[..pre_stat_len]; - let temporary_read_buffer_before_stat_call: &[u8] = { - let mut available: &mut [u8] = &mut pre_stat_buf[..]; - while !available.is_empty() { - let amt = Syscall::read(fd, available)?; - if amt == 0 { - did_succeed = true; - break; + if scratch.is_none() { + let _ = heap_buffer.try_reserve_exact(256 * 1024); + } + let max_len = args.max_size.map_or(usize::MAX, |v| v as usize); + let temporary_read_buffer_before_stat_call: &[u8] = match scratch.as_mut() { + Some(scratch) => { + let pre_stat_len = scratch.len().min(max_len); + let pre_stat_buf = &mut scratch[..pre_stat_len]; + let mut filled = 0usize; + while filled < pre_stat_buf.len() { + let amt = Syscall::read(fd, &mut pre_stat_buf[filled..])?; + if amt == 0 { + did_succeed = true; + break; + } + total += amt; + filled += amt; } - total += amt; - available = &mut available[amt..]; + &pre_stat_buf[..total] + } + None => { + let pre_stat_len = heap_buffer.capacity().min(max_len); + while heap_buffer.len() < pre_stat_len { + let want = pre_stat_len - heap_buffer.len(); + let amt = sys::read_into_vec(fd, &mut heap_buffer, want)?; + if amt == 0 { + did_succeed = true; + break; + } + total += amt; + } + &heap_buffer[..] } - &pre_stat_buf[..total] }; if did_succeed { return match args.encoding { Encoding::Buffer => { if flavor == Flavor::Sync && string_type == ReadFileStringType::Default { - if let Some(vm) = self.vm.map(bun_ptr::BackRef::from) { + if let Some(vm) = vm { // Attempt to create the buffer in JSC's heap. // This avoids creating a WastefulTypedArray. // `self.vm` is the live owning `VirtualMachine` (per-thread singleton) — `BackRef` invariant holds. @@ -6916,7 +6637,7 @@ impl NodeFS { }; array_buffer.ensure_still_alive(); return match array_buffer.as_array_buffer(global) { - Some(buffer) => Ok(ret::ReadFileWithOptions::Buffer( + Some(buffer) => Ok(ret::ReadFileWithOptions::JsBuffer( bun_jsc::MarkedArrayBuffer { buffer, owns_buffer: false, @@ -6930,17 +6651,9 @@ impl NodeFS { }; } } - let raw = bun_core::heap::into_raw( - temporary_read_buffer_before_stat_call - .to_vec() - .into_boxed_slice(), - ); - // SAFETY: ownership transferred to JSC; freed via ArrayBuffer finalizer - // (PORTING.md:348 — `heap::alloc`/`from_raw` across FFI). - Ok(ret::ReadFileWithOptions::Buffer(Buffer::from_bytes( - unsafe { &mut *raw }, - bun_jsc::JSType::Uint8Array, - ))) + Ok(ret::ReadFileWithOptions::Bytes( + temporary_read_buffer_before_stat_call.into(), + )) } _ => { if string_type == ReadFileStringType::Default { @@ -7007,14 +6720,10 @@ impl NodeFS { if !temporary_read_buffer_before_stat_call.is_empty() { buf.extend_from_slice(temporary_read_buffer_before_stat_call); } - // Read into the uninitialised tail. `Vec::resize(cap, 0)` is *not* equivalent in - // debug builds: it goes through `extend_with`'s byte-by-byte loop (no memset - // specialisation), which dominated `readFileSync` of large files. Use - // `VecExt::expand_to_capacity` (the tail is write-only — `Syscall::read` - // hands it straight to the kernel, which only stores into it). - // SAFETY: `u8` has no validity invariant; the buffer is handed straight - // to the kernel which only stores into it. - unsafe { buf.expand_to_capacity() }; + // Read into the uninitialised spare capacity (`buf.len() == total` + // throughout). `Vec::resize(cap, 0)` is *not* equivalent in debug + // builds: it goes through `extend_with`'s byte-by-byte loop (no memset + // specialisation), which dominated `readFileSync` of large files. // Two-phase read: first up to `size`, then keep going until EOF. // `phase == 0` is the size-bounded loop, `phase == 1` is the unbounded tail. @@ -7028,7 +6737,7 @@ impl NodeFS { // Do NOT pre-grow here; growth happens only in the `total > size && amt != 0 && // !has_max_size` arm below. let upper = (buf.capacity() as u64).min(max_size) as usize; - let amt = Syscall::read(fd, &mut buf[total..upper])?; + let amt = sys::read_into_vec(fd, &mut buf, upper.saturating_sub(total))?; total += amt; if args.limit_size_for_javascript { @@ -7043,20 +6752,15 @@ impl NodeFS { // There are cases where stat()'s size is wrong or out of date if (total as u64) > size && amt != 0 && !has_max_size { - // Reset len to the bytes actually read. `expand_to_capacity` left - // `len == capacity`, so without this `try_reserve(8192)` - // would reallocate every read and RawVec doubling grows - // the buffer exponentially (a >256 KB FIFO / proc file - // balloons to multi-GB RSS). - buf.truncate(total); + // `buf.len() == total` here, so this grows by (amortised) 8 KiB + // rather than doubling from a stale `len == capacity` (a >256 KB + // FIFO / proc file would otherwise balloon to multi-GB RSS). if buf.try_reserve(8192).is_err() { return Err(with_path_like( sys::Error::from_code(E::ENOMEM, sys::Tag::read), &args.path, )); } - // SAFETY: `u8` has no validity invariant; kernel only stores. - unsafe { buf.expand_to_capacity() }; continue; } @@ -7080,7 +6784,7 @@ impl NodeFS { if total == 0 { drop(buf); return match args.encoding { - Encoding::Buffer => Ok(ret::ReadFileWithOptions::Buffer(Buffer::EMPTY)), + Encoding::Buffer => Ok(ret::ReadFileWithOptions::Bytes(Box::default())), _ => { if string_type == ReadFileStringType::Default { Ok(ret::ReadFileWithOptions::String(Box::<[u8]>::default())) @@ -7097,13 +6801,7 @@ impl NodeFS { match args.encoding { Encoding::Buffer => { buf.truncate(final_len); - let raw = bun_core::heap::into_raw(buf.into_boxed_slice()); - // SAFETY: ownership transferred to JSC; freed via ArrayBuffer finalizer - // (PORTING.md:348 — `heap::alloc`/`from_raw` across FFI). - Ok(ret::ReadFileWithOptions::Buffer(Buffer::from_bytes( - unsafe { &mut *raw }, - bun_jsc::JSType::Uint8Array, - ))) + Ok(ret::ReadFileWithOptions::Bytes(buf.into_boxed_slice())) } _ => { if string_type == ReadFileStringType::Default { @@ -7240,7 +6938,7 @@ impl NodeFS { // Not all files are seekable (and thus, not all files can be truncated). #[cfg(windows)] { - let _ = unsafe { windows::SetEndOfFile(fd.native()) }; + let _ = windows::set_end_of_file(fd); } #[cfg(not(windows))] { @@ -7255,7 +6953,7 @@ impl NodeFS { if args.flush { #[cfg(windows)] { - let _ = unsafe { windows::kernel32::FlushFileBuffers(fd.native()) }; + let _ = windows::flush_file_buffers(fd); } #[cfg(not(windows))] { @@ -7289,7 +6987,7 @@ impl NodeFS { if args.encoding == Encoding::Utf8 { if let PathLike::String(s) = &args.path { if strings::eql_long(s.slice(), link_path, true) { - return Ok(StringOrBuffer::String(s.clone())); + return Ok(StringOrBytes::String(s.clone())); } } } @@ -7335,38 +7033,16 @@ impl NodeFS { ) -> Maybe { #[cfg(windows)] { - let mut req = UvFsReq::new(); - let rc = unsafe { - uv::uv_fs_realpath( - bun_io::Loop::get(), - &mut *req, - args.path.slice_z(&mut self.sync_error_buf).as_ptr(), - None, - ) + let mut outbuf = paths::path_buffer_pool::get(); + let mut buf: &[u8] = match sys::sys_uv::realpath( + args.path.slice_z(&mut self.sync_error_buf), + &mut outbuf[..], + ) { + Ok(resolved) => resolved, + Err(err) => return Err(err.with_path(args.path.slice())), }; - if let Some(err) = rc.to_error(sys::Tag::realpath) { - return Err(err.with_path(args.path.slice())); - } - // `fs_t.ptr` *is* the nullable C - // string pointer (libuv stores the realpath result directly), so - // `ptr_as::()` yields the value, not a pointer-to-Option. - // SAFETY: `rc` was not an error ⇒ libuv populated `req.ptr`. - let ptr: *const c_char = unsafe { req.ptr_as::() }; - if ptr.is_null() { - return Err(sys::Error { - errno: E::ENOENT as _, - syscall: sys::Tag::realpath, - path: args.path.slice().into(), - ..Default::default() - }); - } - let mut buf = unsafe { bun_core::ffi::cstr(ptr) }.to_bytes(); if variant == RealpathVariant::Emulated { // remove the trailing slash - // - // `buf` is an immutable view and every consumer below copies by - // length, so just shrink the slice — writing a NUL back through - // `ptr.cast_mut()` while `buf` is live would be Stacked-Borrows UB. if buf.last() == Some(&b'\\') { buf = &buf[..buf.len() - 1]; } @@ -7374,7 +7050,7 @@ impl NodeFS { if args.encoding == Encoding::Utf8 { if let PathLike::String(s) = &args.path { if strings::eql_long(s.slice(), buf, true) { - return Ok(StringOrBuffer::String(s.clone())); + return Ok(StringOrBytes::String(s.clone())); } } } @@ -7427,7 +7103,7 @@ impl NodeFS { if args.encoding == Encoding::Utf8 { if let PathLike::String(s) = &args.path { if strings::eql_long(s.slice(), buf, true) { - return Ok(StringOrBuffer::String(s.clone())); + return Ok(StringOrBytes::String(s.clone())); } } } @@ -7476,14 +7152,11 @@ impl NodeFS { Ok(result) => Ok(result), }; } - // SAFETY: path is NUL-terminated by slice_z; rmdir(2) is the libc FFI #[cfg(not(windows))] - Maybe::::errno_sys_p( - unsafe { libc::rmdir(args.path.slice_z(&mut self.sync_error_buf).as_ptr().cast()) }, - sys::Tag::rmdir, - args.path.slice(), - ) - .unwrap_or(Ok(())) + match sys::posix_rmdir(args.path.slice_z(&mut self.sync_error_buf)) { + Err(err) => Err(err.with_path(args.path.slice())), + Ok(()) => Ok(()), + } } pub(crate) fn rm(&mut self, args: &args::Rm, _: Flavor) -> Maybe { @@ -7726,18 +7399,10 @@ impl NodeFS { #[cfg(not(windows))] { let _ = flags; - // SAFETY: path is NUL-terminated by slice_z; truncate(2) is the libc FFI - Maybe::::errno_sys_p( - unsafe { - libc::truncate( - path.slice_z(&mut self.sync_error_buf).as_ptr().cast(), - len_i64, - ) - }, - sys::Tag::truncate, - path.slice(), - ) - .unwrap_or(Ok(())) + match sys::truncate(path.slice_z(&mut self.sync_error_buf), len_i64) { + Err(err) => Err(err.with_path(path.slice())), + Ok(()) => Ok(()), + } } } @@ -7759,14 +7424,11 @@ impl NodeFS { Ok(result) => Ok(result), }; } - // SAFETY: path is NUL-terminated by slice_z; unlink(2) is the libc FFI #[cfg(not(windows))] - Maybe::::errno_sys_p( - unsafe { libc::unlink(args.path.slice_z(&mut self.sync_error_buf).as_ptr().cast()) }, - sys::Tag::unlink, - args.path.slice(), - ) - .unwrap_or(Ok(())) + match sys::unlink(args.path.slice_z(&mut self.sync_error_buf)) { + Err(err) => Err(err.with_path(args.path.slice())), + Ok(()) => Ok(()), + } } pub(crate) fn watch_file( @@ -7807,21 +7469,14 @@ impl NodeFS { pub(crate) fn utimes(&mut self, args: &args::Utimes, _: Flavor) -> Maybe { #[cfg(windows)] { - let mut req = UvFsReq::new(); - let rc = unsafe { - uv::uv_fs_utime( - bun_io::Loop::get(), - &mut *req, - args.path.slice_z(&mut self.sync_error_buf).as_ptr(), - args.atime, - args.mtime, - None, - ) + return match sys::sys_uv::utime( + args.path.slice_z(&mut self.sync_error_buf), + args.atime, + args.mtime, + ) { + Err(err) => Err(err.with_path(args.path.slice())), + Ok(()) => Ok(()), }; - if let Some(err) = rc.to_error(sys::Tag::utime) { - return Err(err.with_path(args.path.slice())); - } - return Ok(()); } #[cfg(not(windows))] match Syscall::utimens( @@ -7838,21 +7493,14 @@ impl NodeFS { pub(crate) fn lutimes(&mut self, args: &args::Lutimes, _: Flavor) -> Maybe { #[cfg(windows)] { - let mut req = UvFsReq::new(); - let rc = unsafe { - uv::uv_fs_lutime( - bun_io::Loop::get(), - &mut *req, - args.path.slice_z(&mut self.sync_error_buf).as_ptr(), - args.atime, - args.mtime, - None, - ) + return match sys::sys_uv::lutime( + args.path.slice_z(&mut self.sync_error_buf), + args.atime, + args.mtime, + ) { + Err(err) => Err(err.with_path(args.path.slice())), + Ok(()) => Ok(()), }; - if let Some(err) = rc.to_error(sys::Tag::lutime) { - return Err(err.with_path(args.path.slice())); - } - return Ok(()); } #[cfg(not(windows))] match Syscall::lutimens( @@ -7905,14 +7553,7 @@ impl NodeFS { } pub(crate) fn os_path_into_sync_error_buf(&mut self, slice: &[OSPathChar]) -> &[u8] { - Self::os_path_into_buf(&mut self.sync_error_buf, slice) - } - - /// Free-function form of [`os_path_into_sync_error_buf`] that does not borrow - /// `&mut self`. Needed by `mkdir_recursive_os_path_impl`, which holds a long-lived - /// `&mut OSPathBuffer` reinterpreted from `sync_error_buf` and so must not reborrow - /// `&mut self` on its error-return paths (PORTING.md §Forbidden aliased `&mut`). - fn os_path_into_buf<'a>(buf: &'a mut PathBuffer, slice: &[OSPathChar]) -> &'a [u8] { + let buf = &mut self.sync_error_buf; #[cfg(windows)] { return strings::from_wpath(buf, slice); @@ -7937,15 +7578,12 @@ impl NodeFS { let dd = dest_dir_len as usize; src_buf[sd] = 0; dest_buf[dd] = 0; - // SAFETY: `src_buf[..sd]`/`dest_buf[..dd]` were written by the caller and the - // NUL sentinel at `[len]` was set just above; both ranges are in-bounds. - let src = unsafe { OSPathSliceZ::from_raw(src_buf.as_ptr(), sd) }; - // SAFETY: see `src` above. - let dest = unsafe { OSPathSliceZ::from_raw(dest_buf.as_ptr(), dd) }; + let src = OSPathSliceZ::from_buf(&src_buf[..], sd); + let dest = OSPathSliceZ::from_buf(&dest_buf[..], dd); #[cfg(windows)] { - let attributes = unsafe { sys::c::GetFileAttributesW(src.as_ptr()) }; + let attributes = sys::windows::get_file_attributes(src); if attributes == sys::c::INVALID_FILE_ATTRIBUTES { return Err(sys::Error { errno: SystemErrno::ENOENT as _, @@ -8448,17 +8086,14 @@ impl NodeFS { loop { // Linux Kernel 5.3 or later // Not supported in gVisor - // SAFETY: src_fd/dest_fd are valid open fds; copy_file_range is the libc FFI - let written = unsafe { - sys::linux::copy_file_range( - src_fd.native(), - &raw mut off_in_copy, - dest_fd.native(), - &raw mut off_out_copy, - sys::page_size(), - 0, - ) - }; + let written = sys::linux::copy_file_range_fd( + src_fd, + Some(&mut off_in_copy), + dest_fd, + Some(&mut off_out_copy), + sys::page_size(), + 0, + ); if let Some(err) = Maybe::::errno_sys_p( written, sys::Tag::copy_file_range, @@ -8491,17 +8126,14 @@ impl NodeFS { while size > 0 { // Linux Kernel 5.3 or later // Not supported in gVisor - // SAFETY: src_fd/dest_fd are valid open fds; copy_file_range is the libc FFI - let written = unsafe { - sys::linux::copy_file_range( - src_fd.native(), - &raw mut off_in_copy, - dest_fd.native(), - &raw mut off_out_copy, - size, - 0, - ) - }; + let written = sys::linux::copy_file_range_fd( + src_fd, + Some(&mut off_in_copy), + dest_fd, + Some(&mut off_out_copy), + size, + 0, + ); if let Some(err) = Maybe::::errno_sys_p( written, sys::Tag::copy_file_range, @@ -8620,17 +8252,14 @@ impl NodeFS { } else { size.saturating_sub(wrote.get() as usize) }; - // SAFETY: src_fd/dest_fd are valid open fds; copy_file_range is the libc FFI - let rc: isize = unsafe { - sys::freebsd::copy_file_range( - src_fd.native(), - &mut off_in, - dest_fd.native(), - &mut off_out, - want, - 0, - ) - } as isize; + let rc: isize = sys::freebsd::copy_file_range_fd( + src_fd, + Some(&mut off_in), + dest_fd, + Some(&mut off_out), + want, + 0, + ) as isize; match sys::get_errno(rc) { E::SUCCESS => { if rc == 0 { @@ -8679,7 +8308,7 @@ impl NodeFS { let stat_ = match reuse_stat { Some(a) => a, None => { - let a = unsafe { sys::c::GetFileAttributesW(src.as_ptr()) }; + let a = sys::windows::get_file_attributes(src); if a == sys::c::INVALID_FILE_ATTRIBUTES { return Err(sys::Error::from_win32( windows::Win32Error::get(), @@ -8691,28 +8320,14 @@ impl NodeFS { } }; if stat_ & sys::c::FILE_ATTRIBUTE_REPARSE_POINT == 0 { - if unsafe { - sys::c::CopyFileW( - src.as_ptr(), - dest.as_ptr(), - mode.shouldnt_overwrite() as i32, - ) - } == 0 - { + if !sys::windows::copy_file(src, dest, mode.shouldnt_overwrite()) { let mut err = windows::Win32Error::get(); if err == windows::Win32Error::PATH_NOT_FOUND { let _ = sys::make_path::make_path_u16( &sys::Dir::cwd(), paths::dirname_w(dest.as_slice()), ); - if unsafe { - sys::c::CopyFileW( - src.as_ptr(), - dest.as_ptr(), - mode.shouldnt_overwrite() as i32, - ) - } != 0 - { + if sys::windows::copy_file(src, dest, mode.shouldnt_overwrite()) { return Ok(()); } err = windows::Win32Error::get(); @@ -8732,14 +8347,7 @@ impl NodeFS { }; let _close = scopeguard::guard(handle, |fd| fd.close()); let mut wbuf = paths::os_path_buffer_pool::get(); - let len = unsafe { - windows::GetFinalPathNameByHandleW( - handle.native(), - wbuf.as_mut_ptr(), - wbuf.len() as u32, - 0, - ) - } as usize; + let len = windows::get_final_path_name_by_handle(handle, &mut wbuf[..], 0); if len == 0 || len >= wbuf.len() { let err = if len == 0 { sys::Error::from_win32(windows::Win32Error::get(), sys::Tag::copyfile) @@ -8868,15 +8476,15 @@ impl NodeFS { as NodeFSDispatch>::run_uv(self, args, rc) } - /// Variant of [`Self::uv_dispatch`] for `uv_callbackreq` — passes the live - /// `uv::fs_t` through so the handler can read `req.ptr` (only `statfs` + /// Variant of [`Self::uv_dispatch`] that passes the completed `uv::fs_t` + /// through so the handler can read its result payload (only `statfs` /// needs it). #[cfg(windows)] #[inline] pub(crate) fn uv_dispatch_req( &mut self, args: &A, - req: &mut uv::fs_t, + req: &uv::fs_t, rc: uv::ReturnCodeI64, ) -> Maybe where @@ -8905,7 +8513,7 @@ pub trait NodeFSDispatch { fn run_uv_req( _fs: &mut NodeFS, _args: &A, - _req: &mut uv::fs_t, + _req: &uv::fs_t, _rc: uv::ReturnCodeI64, ) -> Maybe { unreachable!("uv_dispatch_req: not a req-passing UVFSRequest variant") @@ -8934,7 +8542,7 @@ macro_rules! node_fs_ops { $( #[cfg(windows)] #[inline] - fn run_uv_req(fs: &mut NodeFS, args: &$Args, req: &mut uv::fs_t, rc: uv::ReturnCodeI64) -> Maybe<$Ret> { + fn run_uv_req(fs: &mut NodeFS, args: &$Args, req: &uv::fs_t, rc: uv::ReturnCodeI64) -> Maybe<$Ret> { fs.$uv_req_method(args, req, rc) } )? @@ -8987,6 +8595,20 @@ node_fs_ops! { Writev => writev, args::Writev, ret::Writev, uv = uv_writev; } +/// `fs.promises.readFile`: the pool runs the `Send`-result variant. +impl NodeFSDispatch> + for Op<{ NodeFSFunctionEnum::ReadFile }> +{ + #[inline] + fn run( + fs: &mut NodeFS, + args: &args::ReadFile<'static>, + flavor: Flavor, + ) -> Maybe { + fs.read_file_off_thread(args, flavor) + } +} + #[derive(Copy, Clone, PartialEq, Eq)] pub enum RealpathVariant { Native, @@ -9162,7 +8784,7 @@ impl ReaddirEntry for Dirent { }); } } -impl ReaddirEntry for Buffer { +impl ReaddirEntry for Box<[u8]> { const IS_DIRENT: bool = false; const IS_U16: bool = false; fn into_readdir(v: Vec) -> ret::Readdir { @@ -9175,7 +8797,7 @@ impl ReaddirEntry for Buffer { _kind: sys::FileKind, _encoding: Encoding, ) { - entries.push(Buffer::from_string(utf8_name).expect("oom")); + entries.push(utf8_name.into()); } fn append_entry_w( _: &mut Vec, @@ -9185,8 +8807,8 @@ impl ReaddirEntry for Buffer { _: Encoding, _: Option<&mut PathBuffer>, ) { - // Buffer never - // takes the u16 iterator (`IS_U16 = false`); the call site is gated on + // Byte entries never + // take the u16 iterator (`IS_U16 = false`); the call site is gated on // `T::IS_U16` so this arm is statically dead. unreachable!() } @@ -9199,7 +8821,7 @@ impl ReaddirEntry for Buffer { _encoding: Encoding, _apply_encoding: bool, ) { - entries.push(Buffer::from_string(without_nt_prefix::(name_to_copy)).expect("oom")); + entries.push(without_nt_prefix::(name_to_copy).into()); } } @@ -9256,7 +8878,7 @@ fn map_anyerror_to_errno_rm_tree(err: &crate::Error) -> E { // `rm` non-recursive unlink/rmdir fallback — narrower table; anything not // listed here falls through to EFAULT. // -// `bun_sys::unlink`/`libc::rmdir` yield a raw errno. Notably raw EPERM — +// `bun_sys::unlink`/`bun_sys::posix_rmdir` yield a raw errno. Notably raw EPERM — // like EISDIR/ENOTDIR/ENOTEMPTY — intentionally falls through to EFAULT here. fn map_rm_errno_narrow(e: E) -> E { match e { @@ -9266,22 +8888,15 @@ fn map_rm_errno_narrow(e: E) -> E { } } -/// # Safety -/// `path` must point to a valid NUL-terminated C string. -#[unsafe(no_mangle)] -pub(crate) unsafe extern "C" fn Bun__mkdirp( - global_this: &JSGlobalObject, - path: *const c_char, -) -> bool { - // SAFETY: caller passes a NUL-terminated C string - let path_bytes = unsafe { bun_core::ffi::cstr(path) }.to_bytes(); - // SAFETY: `bun_vm()` returns the live VM; `node_fs()` returns its cached - // `*NodeFS` (type-erased to `*mut c_void` in `bun_jsc` to break the dep cycle). - let node_fs: &mut NodeFS = - unsafe { &mut *global_this.bun_vm().as_mut().node_fs().cast::() }; +// HOST_EXPORT(Bun__mkdirp, c) +pub fn mkdirp(_global_this: &JSGlobalObject, path: Option<&core::ffi::CStr>) -> bool { + let Some(path) = path else { + return false; + }; + let mut node_fs = NodeFS::default(); node_fs .mkdir_recursive(&args::Mkdir { - path: PathLike::borrowed(path_bytes), + path: PathLike::borrowed(path.to_bytes()), recursive: true, ..Default::default() }) diff --git a/src/runtime/node/node_fs_binding.rs b/src/runtime/node/node_fs_binding.rs index 2c511397f5ca..594475db0dde 100644 --- a/src/runtime/node/node_fs_binding.rs +++ b/src/runtime/node/node_fs_binding.rs @@ -1,5 +1,3 @@ -use core::ptr::NonNull; - use bun_jsc::call_frame::ArgumentsSlice; use bun_jsc::virtual_machine::VirtualMachine; use bun_jsc::{CallFrame, JSGlobalObject, JSPromise, JSValue, JsCell, JsResult, SysErrorJsc as _}; @@ -328,10 +326,11 @@ impl Binding { pub(crate) fn create_binding(global: &JSGlobalObject) -> JSValue { let module = Binding::new(Binding::default()); - let vm = global.bun_vm_ptr(); // R-2: init-time write before the JS wrapper exists; `with_mut` here is // trivially un-aliased (sole owner of the fresh `Box`). - module.node_fs.with_mut(|nfs| nfs.vm = NonNull::new(vm)); + module + .node_fs + .with_mut(|nfs| nfs.vm = Some(bun_ptr::BackRef::new(global.bun_vm()))); // `module` was `Box::new`-allocated; ownership transfers to the GC // wrapper, which calls `Binding::finalize` to reclaim it. diff --git a/src/runtime/shell/builtin/cp.rs b/src/runtime/shell/builtin/cp.rs index f51257afdfcd..64614214472c 100644 --- a/src/runtime/shell/builtin/cp.rs +++ b/src/runtime/shell/builtin/cp.rs @@ -399,7 +399,7 @@ pub struct ShellCpTask { pub(crate) src: Vec, pub(crate) tgt: Vec, /// The absolute paths handed to the `ShellAsyncCpTask`, moved back by - /// [`cp_on_finish`](Self::cp_on_finish) for the EBUSY bookkeeping. + /// [`ShellCpHandle::finish`] for the EBUSY bookkeeping. pub(crate) src_absolute: Option>, pub(crate) tgt_absolute: Option>, pub(crate) cwd_path: Vec, @@ -475,26 +475,36 @@ impl ShellCpTask { self.on_copy_impl(src8, dest8); } } +} - /// Called when the node:fs - /// async cp completes (success or first error). Records the error (if any) - /// and re-queues this `ShellCpTask` onto the JS thread so the interpreter - /// can drain `verbose_output` / surface the error. - /// - /// # Safety - /// `this` is the live `heap::alloc`'d task originally passed to - /// [`schedule`](Self::schedule); not touched again on this thread after - /// return. - pub(crate) unsafe fn cp_on_finish( - this: *mut ShellCpTask, +/// The heap `ShellCpTask` an in-flight +/// [`ShellAsyncCpTask`](crate::node::fs::ShellAsyncCpTask) reports to: minted +/// only by [`ShellCpTask`]'s pool callback from the task it owns, which stays +/// alive (and is touched by no one else) until [`finish`](Self::finish) hands +/// it back to the interpreter. +pub struct ShellCpHandle(bun_ptr::ParentRef); + +impl ShellCpHandle { + /// Any pool thread, once per file copied (`cp -v` progress). + pub(crate) fn on_copy(&self, src: &[bun_paths::OSPathChar], dest: &[bun_paths::OSPathChar]) { + self.0.get().cp_on_copy(src, dest); + } + + /// The shell's loop thread: the copy finished (success or first error). + /// Takes back the absolute paths, records the error (if any) and continues + /// the interpreter in place, so it can drain `verbose_output` / surface + /// the error. + pub(crate) fn finish( + self, src: PathLike<'static>, dest: PathLike<'static>, result: bun_sys::Maybe<()>, ) { - // SAFETY: caller contract — JS thread, from the `ShellAsyncCpTask`'s - // completion; `this` is live and ours. The pool side finished (and - // dropped its poster) when it handed the copy to that task, so continue - // in place rather than bouncing through the concurrent queue again. + let this = self.0.as_mut_ptr(); + // SAFETY: type invariant — `this` is the live heap task, ours alone now + // that the copy is over. The pool side finished (and dropped its poster) + // when it handed the copy over, so continue in place rather than + // bouncing through the concurrent queue again. unsafe { (*this).src_absolute = Some(src.into_vec()); (*this).tgt_absolute = Some(dest.into_vec()); @@ -504,13 +514,15 @@ impl ShellCpTask { ShellTask::run_from_main_thread::(this); } } +} +impl ShellCpTask { /// Unlike most shell builtins this does NOT use the generic /// [`ShellTask::schedule`] trampoline (which auto-enqueues back to main /// on return): on the /// success path the [`ShellAsyncCpTask`](crate::node::fs::ShellAsyncCpTask) - /// owns the bounce-back via `cp_on_finish`, so an unconditional post would - /// double-enqueue. The embedded [`ShellTask`] is reused for its + /// owns the bounce-back via [`ShellCpHandle::finish`], so an unconditional + /// post would double-enqueue. The embedded [`ShellTask`] is reused for its /// `WorkPoolTask` / `concurrent_task` / `keep_alive` storage. /// /// # Safety @@ -531,7 +543,7 @@ impl ShellCpTask { /// Recover `*ShellCpTask` from the /// intrusive `*WorkPoolTask`, run the impl, and on error post back - /// immediately (success path defers the post to `cp_on_finish`). + /// immediately (success path defers the post to `ShellCpHandle::finish`). unsafe fn work_pool_callback(task: *mut crate::shell::interpreter::WorkPoolTask) { // SAFETY: `task` is the first `#[repr(C)]` field of `ShellTask`, which // is embedded in `ShellCpTask` at `TASK_OFFSET`. `this` is a live @@ -549,15 +561,25 @@ impl ShellCpTask { .poster .take() .expect("shell cp task on the pool is armed"); - if let Some(e) = (*this).run_from_thread_pool_impl(&poster) { - (*this).err = Some(e); - (*this).task.poster = Some(poster); - Self::enqueue_to_event_loop(this); - } else { - // The copy now belongs to a `ShellAsyncCpTask` (holding its - // own poster, completing on the JS thread via `cp_on_finish`); - // this task's pool part is over. - drop(poster); + match (*this).run_from_thread_pool_impl() { + Err(e) => { + (*this).err = Some(e); + (*this).task.poster = Some(poster); + Self::enqueue_to_event_loop(this); + } + Ok(args) => { + // Hand the copy to an fs.cp task bound to the loop and + // poster this shell task captured on its own thread; it + // completes on that thread via `ShellCpHandle::finish`. + // This task's pool part is over. + let event_loop = (*this).task.event_loop; + crate::node::fs::ShellAsyncCpTask::create_for_shell( + args, + event_loop, + poster, + ShellCpHandle(bun_ptr::ParentRef::from_raw_mut(this)), + ); + } } } } @@ -597,14 +619,14 @@ impl ShellCpTask { } } - /// Resolves src/tgt to absolute paths, classifies them per the three + /// Resolves src/tgt to absolute paths and classifies them per the three /// POSIX `cp` synopses - /// (), then hands off to - /// the node:fs async cp implementation. + /// () into the + /// arguments for the node:fs async cp implementation. + #[allow(clippy::result_large_err)] fn run_from_thread_pool_impl( &mut self, - poster: &bun_jsc::ConcurrentPoster, - ) -> Option { + ) -> Result>, ShellErr> { use resolve_path::{Platform, platform}; let mut buf2 = bun_paths::path_buffer_pool::get(); @@ -637,12 +659,12 @@ impl ShellCpTask { // need to create it. let src_is_dir = match Self::is_dir(src) { Ok(x) => x, - Err(e) => return Some(ShellErr::new_sys(&e)), + Err(e) => return Err(ShellErr::new_sys(&e)), }; // Any source directory without -R is an error. if src_is_dir && !self.opts.recursive { - return Some(ShellErr::Custom( + return Err(ShellErr::Custom( format!("{} is a directory (not copied)", bstr::BStr::new(&self.src)) .into_bytes() .into_boxed_slice(), @@ -650,7 +672,7 @@ impl ShellCpTask { } if !src_is_dir && src.as_bytes() == tgt.as_bytes() { - return Some(ShellErr::Custom( + return Err(ShellErr::Custom( format!( "{0} and {0} are identical (not copied)", bstr::BStr::new(&self.src) @@ -666,7 +688,7 @@ impl ShellCpTask { // If it has a trailing directory separator, it's a directory. (Self::has_trailing_sep(tgt.as_bytes()), false) } - Err(e) => return Some(ShellErr::new_sys(&e)), + Err(e) => return Err(ShellErr::new_sys(&e)), }; let mut _copying_many = false; @@ -685,7 +707,7 @@ impl ShellCpTask { } else if self.operands == 2 { // source_dir -> new_target_dir. } else { - return Some(ShellErr::Custom( + return Err(ShellErr::Custom( format!("directory {} does not exist", bstr::BStr::new(&self.tgt)) .into_bytes() .into_boxed_slice(), @@ -695,14 +717,14 @@ impl ShellCpTask { } else { // 3rd synopsis: source_files... -> target. if src_is_dir { - return Some(ShellErr::Custom( + return Err(ShellErr::Custom( format!("{} is a directory (not copied)", bstr::BStr::new(&self.src)) .into_bytes() .into_boxed_slice(), )); } if !tgt_exists || !tgt_is_dir { - return Some(ShellErr::Custom( + return Err(ShellErr::Custom( format!("{} is not a directory", bstr::BStr::new(&self.tgt)) .into_bytes() .into_boxed_slice(), @@ -726,16 +748,7 @@ impl ShellCpTask { }, ); - // Pool thread: hand the copy to an fs.cp task bound to the loop and - // poster this shell task captured on its own thread. - let _ = crate::node::fs::ShellAsyncCpTask::create_for_shell( - args, - self.task.event_loop, - poster.clone(), - std::ptr::from_mut::(self), - ); - - None + Ok(args) } /// # Safety @@ -767,7 +780,7 @@ impl crate::shell::interpreter::ShellTaskCtx for ShellCpTask { // Not reached: `ShellCpTask::schedule` installs `work_pool_callback` // directly (the generic trampoline auto-posts back, which would // double-enqueue when the `ShellAsyncCpTask` later calls - // `cp_on_finish`). + // `ShellCpHandle::finish`). debug_assert!( false, "ShellCpTask scheduled via ShellTask::schedule; use ShellCpTask::schedule" diff --git a/src/runtime/webcore/blob/copy_file.rs b/src/runtime/webcore/blob/copy_file.rs index d5a6edc8bafb..f134732376ff 100644 --- a/src/runtime/webcore/blob/copy_file.rs +++ b/src/runtime/webcore/blob/copy_file.rs @@ -1751,10 +1751,7 @@ impl<'a> CopyFileWindows<'a> { fn mkdirp(&mut self) { bun_sys::syslog!("mkdirp"); self.mkdirp_if_not_exists = false; - // Borrowck: compute the raw path slice pointer up-front so the - // immutable borrow of `self.destination_file_store` ends before we take - // `core::ptr::from_mut(self)` for `completion_ctx` below. - let path: *const [u8] = { + let path: Box<[u8]> = { let destination = &self.destination_file_store.data.as_file(); if !matches!(destination.pathlike, PathOrFileDescriptor::Path(_)) { self.throw(bun_sys::Error { @@ -1765,12 +1762,10 @@ impl<'a> CopyFileWindows<'a> { return; } let path_slice = destination.pathlike.path().slice(); - // BORROW: not owned — `destination_file_store` (and thus its path) is held in - // `self`, which outlives the workpool task (completion runs `copyfile`/`throw` - // on `self` before any `destroy`). bun_paths::dirname(path_slice) // this shouldn't happen - .unwrap_or(path_slice) as *const [u8] + .unwrap_or(path_slice) + .into() }; self.event_loop.ref_keep_alive(); diff --git a/src/runtime/webcore/blob/write_file.rs b/src/runtime/webcore/blob/write_file.rs index 9de9be8c72bf..b6261ae67752 100644 --- a/src/runtime/webcore/blob/write_file.rs +++ b/src/runtime/webcore/blob/write_file.rs @@ -899,12 +899,10 @@ mod windows_impl { crate::node::fs::async_::AsyncMkdirp::schedule(crate::node::fs::async_::AsyncMkdirp { completion: Self::on_mkdirp_complete_concurrent, completion_ctx: ctx, - // BORROW: AsyncMkdirp.path is `*const [u8]` (not owned); `path` - // points into `self.file_blob.store`, which outlives the mkdirp - // task (it's released only in `deinit()`). path: bun_core::dirname(path) // this shouldn't happen - .unwrap_or(path) as *const [u8], + .unwrap_or(path) + .into(), ticket: bun_jsc::virtual_machine::VirtualMachine::get().ticket(), task: Default::default(), }); diff --git a/src/sys/lib.rs b/src/sys/lib.rs index ecd5635ae1af..75b8a4aa58ba 100644 --- a/src/sys/lib.rs +++ b/src/sys/lib.rs @@ -1561,7 +1561,7 @@ pub(crate) const MAX_COUNT: usize = u32::MAX as usize; // them locally as `safe fn` (instead of routing through the `libc` crate's // `unsafe extern fn` items) drops the per-call-site `unsafe { }` block. #[cfg(unix)] -mod safe_libc { +pub mod safe_libc { use core::ffi::c_int; // `close` is a libc symbol std relies on; this is an FFI import (not a // competing definition) with the canonical signature. @@ -1571,7 +1571,9 @@ mod safe_libc { pub(crate) safe fn close(fd: c_int) -> c_int; pub(crate) safe fn dup2(old: c_int, new: c_int) -> c_int; pub(crate) safe fn isatty(fd: c_int) -> c_int; - pub(crate) safe fn fsync(fd: c_int) -> c_int; + pub safe fn fsync(fd: c_int) -> c_int; + // `libc` omits the Darwin binding (fdatasync exists since 10.7). + pub safe fn fdatasync(fd: c_int) -> c_int; pub(crate) safe fn fchdir(fd: c_int) -> c_int; pub(crate) safe fn umask(mode: libc::mode_t) -> libc::mode_t; pub(crate) safe fn fchmod(fd: c_int, mode: libc::mode_t) -> c_int; @@ -2587,6 +2589,54 @@ mod posix_impl { ); Ok(()) } + /// `link(2)`; no EINTR retry. The error carries both paths. + pub fn link(from: &ZStr, to: &ZStr) -> Maybe<()> { + // SAFETY: both `ZStr`s are valid NUL-terminated C strings. + let rc = unsafe { libc::link(from.as_ptr(), to.as_ptr()) }; + if rc < 0 { + return Err(Error::from_code_int(last_errno(), Tag::link) + .with_path_dest(from.as_bytes(), to.as_bytes())); + } + Ok(()) + } + /// `truncate(2)`; no EINTR retry. + pub fn truncate(path: &ZStr, len: i64) -> Maybe<()> { + // SAFETY: `path` is a valid NUL-terminated C string. + let rc = unsafe { libc::truncate(path.as_ptr(), len as libc::off_t) }; + if rc < 0 { + return Err( + Error::from_code_int(last_errno(), Tag::truncate).with_path(path.as_bytes()) + ); + } + Ok(()) + } + /// `mkdtemp(3)`: `template` holds a NUL-terminated `...XXXXXX` pattern that + /// is rewritten in place; returns the length of the resulting path (the + /// bytes before the NUL). The error carries no path. + pub fn mkdtemp(template: &mut [u8]) -> Maybe { + let nul = template + .iter() + .position(|&b| b == 0) + .expect("mkdtemp template must be NUL-terminated"); + // SAFETY: `template[..=nul]` is a writable NUL-terminated C string; + // mkdtemp(3) rewrites the `XXXXXX` suffix in place and never grows it. + let rc = unsafe { libc::mkdtemp(template.as_mut_ptr().cast::()) }; + if rc.is_null() { + return Err(Error::from_code_int(last_errno(), Tag::mkdtemp)); + } + Ok(nul) + } + /// `posix_fadvise(2)`; advisory, so the return code is handed back raw. + #[cfg(any(target_os = "linux", target_os = "android", target_os = "freebsd"))] + pub fn posix_fadvise( + fd: Fd, + offset: i64, + len: i64, + advice: core::ffi::c_int, + ) -> core::ffi::c_int { + // SAFETY: no memory arguments; the kernel validates `fd`. + unsafe { libc::posix_fadvise(fd.native(), offset as _, len as _, advice) } + } pub fn readlink(path: &ZStr, buf: &mut [u8]) -> Maybe { let n = check_p!( // SAFETY: `path` is NUL-terminated (`ZStr`); `buf` is a valid @@ -4258,6 +4308,11 @@ mod windows_impl { // negative LARGE_INTEGER never becomes ~18 EB after the i64→u64 cast. Ok(size.max(0) as u64) } + /// See [`sys_uv::mkdtemp`]; same shape as the POSIX `mkdtemp`. + #[inline] + pub fn mkdtemp(template: &mut [u8]) -> Maybe { + sys_uv::mkdtemp(template) + } pub fn realpath<'a>(path: &ZStr, buf: &'a mut bun_core::PathBuffer) -> Maybe<&'a [u8]> { // sys_uv.rs:216 — open + GetFinalPathNameByHandle (uv_fs_realpath edge cases). let fd = open(path, O::RDONLY, 0)?; @@ -4414,6 +4469,29 @@ fn read_fill_vec( } } +/// `read` into uninitialized storage; returns the prefix the kernel filled. +pub fn read_uninit(fd: Fd, buf: &mut [core::mem::MaybeUninit]) -> Maybe<&mut [u8]> { + // SAFETY: `u8` has no validity invariant and `read` only stores into the + // slice; only the kernel-written prefix `[..n]` is handed back as initialized. + let bytes: &mut [u8] = unsafe { &mut *(core::ptr::from_mut(buf) as *mut [u8]) }; + let n = read(fd, bytes)?; + Ok(&mut bytes[..n]) +} + +/// `read` into `vec`'s spare capacity (at most `max` bytes), growing its +/// length by the number of bytes read. Never reallocates. +pub fn read_into_vec(fd: Fd, vec: &mut Vec, max: usize) -> Maybe { + // SAFETY: `read` only stores into the spare slice; exactly the `n` bytes it + // wrote are committed. + unsafe { + let spare = bun_core::vec::spare_bytes_mut(vec); + let len = spare.len().min(max); + let n = read(fd, &mut spare[..len])?; + bun_core::vec::commit_spare(vec, n); + Ok(n) + } +} + // ────────────────────────────────────────────────────────────────────────── // `bun.PlatformIOVecConst` / `bun.platformIOVecConstCreate` — POSIX // `iovec_const` (= `struct iovec` with the writev contract that `base` is @@ -4438,6 +4516,20 @@ const _: () = assert!( && core::mem::align_of::() == core::mem::align_of::() ); +/// View mutable iovecs as the const form `pwritev`/`writev` take (identical +/// layout on every platform; the kernel never writes through `base` there). +#[inline] +pub fn iovecs_as_const(vecs: &[PlatformIoVec]) -> &[PlatformIoVecConst] { + const _: () = assert!( + core::mem::size_of::() == core::mem::size_of::() + && core::mem::align_of::() + == core::mem::align_of::() + ); + // SAFETY: layout identity asserted above; `{ *const u8, usize }` admits every + // `{ *mut u8, usize }` bit pattern. + unsafe { core::slice::from_raw_parts(vecs.as_ptr().cast::(), vecs.len()) } +} + #[cfg(unix)] #[inline] pub fn platform_iovec_const_create(buf: &[u8]) -> PlatformIoVecConst { @@ -5463,6 +5555,32 @@ pub mod linux { unsafe { super::linux_syscall::copy_file_range(in_, off_in, out, off_out, len, flags) } } + /// `copy_file_range(2)` with borrowed offsets (`None` = use and advance the + /// file position). Returns the raw result for `get_errno`. + #[inline] + pub fn copy_file_range_fd( + in_: super::Fd, + off_in: Option<&mut i64>, + out: super::Fd, + off_out: Option<&mut i64>, + len: usize, + flags: u32, + ) -> isize { + let off_in = off_in.map_or(core::ptr::null_mut(), core::ptr::from_mut); + let off_out = off_out.map_or(core::ptr::null_mut(), core::ptr::from_mut); + // SAFETY: the offset pointers are null or exclusive borrows live for the call. + unsafe { + super::linux_syscall::copy_file_range( + in_.native(), + off_in, + out.native(), + off_out, + len, + flags, + ) + } + } + // sendfile — use the existing `linux::sendfile` (libc // wrapper, isize return) defined above; `get_errno::` decodes it. @@ -8548,6 +8666,23 @@ pub mod freebsd { ) -> libc::ssize_t { unsafe { libc::copy_file_range(in_, off_in, out, off_out, len, flags) } } + + /// `copy_file_range(2)` with borrowed offsets (`None` = use and advance the + /// file position). Returns the raw result for `get_errno`. + #[inline] + pub fn copy_file_range_fd( + in_: super::Fd, + off_in: Option<&mut libc::off_t>, + out: super::Fd, + off_out: Option<&mut libc::off_t>, + len: usize, + flags: u32, + ) -> libc::ssize_t { + let off_in = off_in.map_or(core::ptr::null_mut(), core::ptr::from_mut); + let off_out = off_out.map_or(core::ptr::null_mut(), core::ptr::from_mut); + // SAFETY: the offset pointers are null or exclusive borrows live for the call. + unsafe { libc::copy_file_range(in_.native(), off_in, out.native(), off_out, len, flags) } + } } #[cfg(not(target_os = "freebsd"))] pub mod freebsd {} @@ -8789,6 +8924,17 @@ pub fn rmdir(to: &ZStr) -> Maybe<()> { rmdirat(Fd::cwd(), to) } +/// Plain `rmdir(2)` (tag `rmdir`, no EINTR retry) — what node:fs reports. +#[cfg(not(windows))] +pub fn posix_rmdir(to: &ZStr) -> Maybe<()> { + // SAFETY: `to` is a valid NUL-terminated C string. + let rc = unsafe { libc::rmdir(to.as_ptr()) }; + if rc < 0 { + return Err(Error::from_code_int(last_errno(), Tag::rmdir).with_path(to.as_bytes())); + } + Ok(()) +} + /// Type-style alias so callers can write `bun_sys::MakePath::make_path::(..)` /// (the `bun.MakePath` namespace re-export). pub use make_path as MakePath; diff --git a/src/sys/sys_uv.rs b/src/sys/sys_uv.rs index 9cf9c4ef936b..d777fced6a66 100644 --- a/src/sys/sys_uv.rs +++ b/src/sys/sys_uv.rs @@ -30,53 +30,8 @@ pub use crate::symlink; pub use crate::unlinkat; pub use crate::unlinkat_with_flags; -// Note: `req = undefined; req.deinit()` has a safety-check in a debug build - -/// RAII owner for a synchronous `uv_fs_t` request. -/// -/// `uv_fs_t` becomes **self-referential** after `uv_fs_read`/`uv_fs_write` with -/// `nbufs <= 4`: libuv points `req->fs.info.bufs` at the inline -/// `req->fs.info.bufsml[4]` array (vendor/libuv/src/win/fs.c:3291). If the -/// struct is bitwise-moved before `uv_fs_req_cleanup`, the cleanup check -/// `if (bufs != bufsml) uv__free(bufs);` (fs.c:3237) sees the *old* stack -/// address ≠ the *new* `bufsml` slot and frees a stack pointer — heap UB. -/// -/// `scopeguard::guard(fs_t, |mut r| r.deinit())` triggers exactly that move -/// (its `Drop` `ManuallyDrop::take`s the value into the closure arg), so we -/// instead give the request a real `Drop` impl: Rust calls `Drop::drop` *in -/// place* at the original address, so `bufs == bufsml` still holds and cleanup -/// is sound. Do **not** move an `FsReq` after passing it to libuv. -#[repr(transparent)] -struct FsReq(uv::fs_t); - -impl FsReq { - #[inline] - fn new() -> Self { - Self(uv::fs_t::uninitialized()) - } -} - -impl Drop for FsReq { - #[inline] - fn drop(&mut self) { - self.0.deinit(); - } -} - -impl core::ops::Deref for FsReq { - type Target = uv::fs_t; - #[inline] - fn deref(&self) -> &uv::fs_t { - &self.0 - } -} - -impl core::ops::DerefMut for FsReq { - #[inline] - fn deref_mut(&mut self) -> &mut uv::fs_t { - &mut self.0 - } -} +/// RAII owner for a synchronous `uv_fs_t` request (cleanup on drop). +type FsReq = uv::OwnedFsReq; pub fn open(file_path: &ZStr, c_flags: i32, perm_: Mode) -> Result { // libuv heap-allocates the WCHAR path copy @@ -203,14 +158,8 @@ pub fn statfs(file_path: &ZStr) -> Result { Result::Err(Error::new(errno, Tag::statfs).with_path(file_path.as_bytes())) } else { // On Windows `StatFS == uv_statfs_t`, so the libuv result *is* the - // public type. - // SAFETY: libuv guarantees `req.ptr` points to a valid `uv_statfs_t` - // on success; the pointer carries no alignment guarantee for `StatFS`, - // so read it by value via `read_unaligned`. The value is copied out - // *before* `FsReq::drop` runs `uv_fs_req_cleanup` and frees the - // backing allocation. - let p = unsafe { req.ptr_as::() }; - Result::Ok(unsafe { core::ptr::read_unaligned(p) }) + // public type; copied out before `FsReq::drop` frees it. + Result::Ok(req.statfs_result().expect("uv_fs_statfs succeeded")) } } @@ -374,6 +323,152 @@ pub(crate) fn readlink<'a>(file_path: &ZStr, buf: &'a mut [u8]) -> Result<&'a mu } } +/// `uv_fs_realpath`; the resolved path is copied into `buf` (no terminator). +/// A successful call that yields no path reports `ENOENT`. +pub fn realpath<'a>(file_path: &ZStr, buf: &'a mut [u8]) -> Result<&'a [u8]> { + let mut req = FsReq::new(); + // SAFETY: synchronous libuv fs call; `req` lives on the stack for the duration. + let rc = unsafe { uv::uv_fs_realpath(uv::Loop::get(), &mut *req, file_path.as_ptr(), None) }; + if let Some(errno) = rc.errno() { + log!( + "uv realpath({}) = {}", + BStr::new(file_path.as_bytes()), + rc.int() + ); + return Result::Err(Error { + errno, + syscall: Tag::realpath, + path: file_path.as_bytes().into(), + ..Default::default() + }); + } + let Some(resolved) = req.ptr_c_str() else { + return Result::Err(Error::new(E::NOENT, Tag::realpath).with_path(file_path.as_bytes())); + }; + let resolved = resolved.to_bytes(); + if resolved.len() > buf.len() { + return Result::Err( + Error::new(E::NAMETOOLONG, Tag::realpath).with_path(file_path.as_bytes()), + ); + } + log!( + "uv realpath({}) = {}", + BStr::new(file_path.as_bytes()), + BStr::new(resolved) + ); + let len = resolved.len(); + buf[..len].copy_from_slice(resolved); + Result::Ok(&buf[..len]) +} + +/// `uv_fs_mkdtemp`: `template` holds a NUL-terminated `...XXXXXX` pattern; the +/// created directory's path is written back over it and its length returned. +/// The error carries no path. +pub fn mkdtemp(template: &mut [u8]) -> Result { + assert!( + template.contains(&0), + "mkdtemp template must be NUL-terminated" + ); + let mut req = FsReq::new(); + // SAFETY: synchronous libuv fs call; `template` is NUL-terminated and libuv + // copies it before returning. + let rc = unsafe { + uv::uv_fs_mkdtemp( + uv::Loop::get(), + &mut *req, + template.as_ptr().cast::(), + None, + ) + }; + if let Some(errno) = rc.errno() { + return Result::Err(Error { + errno, + syscall: Tag::mkdtemp, + ..Default::default() + }); + } + let created = req.path_c_str().map(|p| p.to_bytes()).unwrap_or(&[]); + let len = created.len().min(template.len()); + template[..len].copy_from_slice(&created[..len]); + Result::Ok(len) +} + +/// `uv_fs_utime` (times in seconds). The error carries no path. +pub fn utime(file_path: &ZStr, atime: f64, mtime: f64) -> Result<()> { + let mut req = FsReq::new(); + // SAFETY: synchronous libuv fs call; req lives on the stack for the duration. + let rc = unsafe { + uv::uv_fs_utime( + uv::Loop::get(), + &mut *req, + file_path.as_ptr(), + atime, + mtime, + None, + ) + }; + log!( + "uv utime({}) = {}", + BStr::new(file_path.as_bytes()), + rc.int() + ); + match rc.errno() { + Some(errno) => Result::Err(Error { + errno, + syscall: Tag::utime, + ..Default::default() + }), + None => Result::Ok(()), + } +} + +/// `uv_fs_lutime` (times in seconds). The error carries no path. +pub fn lutime(file_path: &ZStr, atime: f64, mtime: f64) -> Result<()> { + let mut req = FsReq::new(); + // SAFETY: synchronous libuv fs call; req lives on the stack for the duration. + let rc = unsafe { + uv::uv_fs_lutime( + uv::Loop::get(), + &mut *req, + file_path.as_ptr(), + atime, + mtime, + None, + ) + }; + log!( + "uv lutime({}) = {}", + BStr::new(file_path.as_bytes()), + rc.int() + ); + match rc.errno() { + Some(errno) => Result::Err(Error { + errno, + syscall: Tag::lutime, + ..Default::default() + }), + None => Result::Ok(()), + } +} + +/// `uv_fs_futime` (times in seconds). +pub fn futime(fd: Fd, atime: f64, mtime: f64) -> Result<()> { + let uv_fd = fd.uv(); + let mut req = FsReq::new(); + // SAFETY: synchronous libuv fs call; req lives on the stack for the duration. + let rc = unsafe { uv::uv_fs_futime(uv::Loop::get(), &mut *req, uv_fd, atime, mtime, None) }; + log!("uv futime({}) = {}", uv_fd, rc.int()); + match rc.errno() { + Some(errno) => Result::Err(Error { + errno, + syscall: Tag::futime, + fd, + ..Default::default() + }), + None => Result::Ok(()), + } +} + pub fn rename(from: &ZStr, to: &ZStr) -> Result<()> { let mut req = FsReq::new(); // SAFETY: synchronous libuv fs call; req lives on the stack for the duration. diff --git a/src/sys/windows/mod.rs b/src/sys/windows/mod.rs index 594bfda0a2cc..a5f488e9aa4b 100644 --- a/src/sys/windows/mod.rs +++ b/src/sys/windows/mod.rs @@ -654,6 +654,45 @@ fn final_name_raw(h: HANDLE, flags: DWORD, buf: &mut [u16]) -> Option { } } +/// [`GetFinalPathNameByHandleW`] (this module's, with the AppContainer +/// fallback) over a slice: the length written, or the required length when +/// `buf` is too small, or 0 with the thread's last error set. +pub fn get_final_path_name_by_handle(fd: Fd, buf: &mut [u16], flags: DWORD) -> usize { + // SAFETY: `buf` is valid for writes of `buf.len()` u16s. + unsafe { + GetFinalPathNameByHandleW(fd.native(), buf.as_mut_ptr(), buf.len() as u32, flags) as usize + } +} + +/// `GetFileAttributesW`: the attribute mask, or `INVALID_FILE_ATTRIBUTES` with +/// the thread's last error set. +#[inline] +pub fn get_file_attributes(path: &bun_core::WStr) -> DWORD { + // SAFETY: `WStr` is NUL-terminated. + unsafe { externs::GetFileAttributesW(path.as_ptr()) } +} + +/// `CopyFileW`; `false` on failure with the thread's last error set. +#[inline] +pub fn copy_file(src: &bun_core::WStr, dest: &bun_core::WStr, fail_if_exists: bool) -> bool { + // SAFETY: both `WStr`s are NUL-terminated. + unsafe { externs::CopyFileW(src.as_ptr(), dest.as_ptr(), BOOL::from(fail_if_exists)) != 0 } +} + +/// `SetEndOfFile` at the handle's current position; `false` on failure. +#[inline] +pub fn set_end_of_file(fd: Fd) -> bool { + // SAFETY: by-value handle; the kernel validates it. + unsafe { externs::SetEndOfFile(fd.native()) != 0 } +} + +/// `FlushFileBuffers`; `false` on failure. +#[inline] +pub fn flush_file_buffers(fd: Fd) -> bool { + // SAFETY: by-value handle; the kernel validates it. + unsafe { bun_windows_sys::kernel32::FlushFileBuffers(fd.native()) != 0 } +} + /// Attribute-only `CreateFileW` (0 access): exempt from share-mode arbitration /// and the smallest ACL surface — don't add access bits. `pathz` must be /// NUL-terminated; `FILE_FLAG_BACKUP_SEMANTICS` covers directories, harmless on files. diff --git a/src/threading/lib.rs b/src/threading/lib.rs index b3af867af395..bc540d90cd9d 100644 --- a/src/threading/lib.rs +++ b/src/threading/lib.rs @@ -37,7 +37,7 @@ pub use rwlock::RwLock; pub use semaphore::Semaphore; pub use signal_ring::SignalRing; pub use thread_pool::ThreadPool; -pub use unbounded_queue::{Link, Linked, UnboundedQueue}; +pub use unbounded_queue::{BoxQueue, Link, Linked, UnboundedQueue}; pub use wait_group::WaitGroup; pub use work_pool::Task as WorkPoolTask; pub use work_pool::{IntrusiveWorkTask, OwnedTask, WorkPool}; diff --git a/src/threading/unbounded_queue.rs b/src/threading/unbounded_queue.rs index 7ad4a399b7ae..956e6e4844e3 100644 --- a/src/threading/unbounded_queue.rs +++ b/src/threading/unbounded_queue.rs @@ -333,3 +333,83 @@ impl UnboundedQueue { self.back.0.load(Ordering::Acquire).is_null() } } + +/// A lock-free MPSC queue of owned values: [`UnboundedQueue`] over heap nodes +/// this type allocates on [`push`](Self::push) and frees on +/// [`drain`](Self::drain), so callers never see a raw node. +pub struct BoxQueue { + inner: UnboundedQueue>, +} + +/// The heap node behind [`BoxQueue`]. +pub struct BoxNode { + next: Link>, + value: V, +} + +// SAFETY: `link` projects to the embedded `next` field of a live node. +unsafe impl Linked for BoxNode { + #[inline] + unsafe fn link(item: *mut Self) -> *const Link { + // SAFETY: `item` is valid per the `UnboundedQueue` contract; no + // intermediate reference is formed. + unsafe { ptr::addr_of!((*item).next) } + } +} + +impl Default for BoxQueue { + fn default() -> Self { + Self { + inner: UnboundedQueue::default(), + } + } +} + +impl BoxQueue { + /// Any thread. + pub fn push(&self, value: V) { + let node = Box::new(BoxNode { + next: Link::new(), + value, + }); + self.inner.push(NonNull::from(Box::leak(node))); + } + + /// The consumer: take everything pushed so far, in push order. + pub fn drain(&self) -> BoxQueueDrain { + BoxQueueDrain { + iter: self.inner.pop_batch().iterator(), + } + } +} + +impl Drop for BoxQueue { + fn drop(&mut self) { + self.drain().for_each(drop); + } +} + +/// Owned values out of a [`BoxQueue`]; dropping it frees what was not yielded. +pub struct BoxQueueDrain { + iter: BatchIterator>, +} + +impl Iterator for BoxQueueDrain { + type Item = V; + fn next(&mut self) -> Option { + let node = self.iter.next(); + if node.is_null() { + return None; + } + // SAFETY: every node in the queue was leaked from a `Box` by `push`, + // and the iterator has already advanced past it. + let node = unsafe { Box::from_raw(node) }; + Some(node.value) + } +} + +impl Drop for BoxQueueDrain { + fn drop(&mut self) { + self.for_each(drop); + } +} From 7db816ebf8b1392bbe287df140475b7270ce2721 Mon Sep 17 00:00:00 2001 From: Jarred Sumner Date: Sun, 23 Aug 2026 13:47:27 +0000 Subject: [PATCH 2/8] mkdtemp: NUL scan through bun_core::strings; test readv/writev with no buffers --- src/sys/lib.rs | 4 +--- src/sys/sys_uv.rs | 2 +- test/js/node/fs/fs.test.ts | 8 ++++++++ 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/sys/lib.rs b/src/sys/lib.rs index 75b8a4aa58ba..4802042a3392 100644 --- a/src/sys/lib.rs +++ b/src/sys/lib.rs @@ -2614,9 +2614,7 @@ mod posix_impl { /// is rewritten in place; returns the length of the resulting path (the /// bytes before the NUL). The error carries no path. pub fn mkdtemp(template: &mut [u8]) -> Maybe { - let nul = template - .iter() - .position(|&b| b == 0) + let nul = bun_core::strings::index_of_char_usize(template, 0) .expect("mkdtemp template must be NUL-terminated"); // SAFETY: `template[..=nul]` is a writable NUL-terminated C string; // mkdtemp(3) rewrites the `XXXXXX` suffix in place and never grows it. diff --git a/src/sys/sys_uv.rs b/src/sys/sys_uv.rs index d777fced6a66..419ca3397c24 100644 --- a/src/sys/sys_uv.rs +++ b/src/sys/sys_uv.rs @@ -366,7 +366,7 @@ pub fn realpath<'a>(file_path: &ZStr, buf: &'a mut [u8]) -> Result<&'a [u8]> { /// The error carries no path. pub fn mkdtemp(template: &mut [u8]) -> Result { assert!( - template.contains(&0), + bun_core::strings::contains_char(template, 0), "mkdtemp template must be NUL-terminated" ); let mut req = FsReq::new(); diff --git a/test/js/node/fs/fs.test.ts b/test/js/node/fs/fs.test.ts index 55663f10f4e3..3e7a230d9571 100644 --- a/test/js/node/fs/fs.test.ts +++ b/test/js/node/fs/fs.test.ts @@ -322,6 +322,14 @@ describe("FileHandle", () => { expect(await fd.readv(buffers, 0)).toEqual({ bytesRead: 20, buffers }); }); + it("FileHandle#readv / #writev with no buffers resolve with 0 bytes", async () => { + await using fd = await fs.promises.open(import.meta.path, "r"); + expect(await fd.readv([], 0)).toEqual({ bytesRead: 0, buffers: [] }); + const out = join(tmpdirSync(), "writev-empty.txt"); + await using wfd = await fs.promises.open(out, "w"); + expect(await wfd.writev([], 0)).toEqual({ bytesWritten: 0, buffers: [] }); + }); + it("FileHandle#write throws EBADF when closed", async () => { let handle: FileHandle; let spy; From d2e634cb2c11478e6f2f74cda64035d88b9093bc Mon Sep 17 00:00:00 2001 From: Jarred Sumner Date: Sun, 23 Aug 2026 21:26:20 +0000 Subject: [PATCH 3/8] map_rm_errno_narrow: comment names its one remaining caller --- src/runtime/node/node_fs.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/runtime/node/node_fs.rs b/src/runtime/node/node_fs.rs index 69319dc1fcd1..7aa3c427e080 100644 --- a/src/runtime/node/node_fs.rs +++ b/src/runtime/node/node_fs.rs @@ -8875,11 +8875,11 @@ fn map_anyerror_to_errno_rm_tree(err: &crate::Error) -> E { } } -// `rm` non-recursive unlink/rmdir fallback — narrower table; anything not -// listed here falls through to EFAULT. +// `rm` non-recursive unlink — narrower table; anything not listed here falls +// through to EFAULT. // -// `bun_sys::unlink`/`bun_sys::posix_rmdir` yield a raw errno. Notably raw EPERM — -// like EISDIR/ENOTDIR/ENOTEMPTY — intentionally falls through to EFAULT here. +// `bun_sys::unlink` yields a raw errno. Notably raw EPERM — like +// EISDIR/ENOTDIR/ENOTEMPTY — intentionally falls through to EFAULT here. fn map_rm_errno_narrow(e: E) -> E { match e { E::EACCES => E::EACCES, From ab82dc33d8b304d8419eba6c0b45bcebc5d0c5d8 Mon Sep 17 00:00:00 2001 From: Jarred Sumner Date: Sat, 29 Aug 2026 09:27:52 +0000 Subject: [PATCH 4/8] sys_uv: realpath/mkdtemp/utime/lutime/futime report errors through ReturnCodeExt::to_error --- src/sys/sys_uv.rs | 44 ++++++++------------------------------------ 1 file changed, 8 insertions(+), 36 deletions(-) diff --git a/src/sys/sys_uv.rs b/src/sys/sys_uv.rs index 419ca3397c24..396c9e6f869f 100644 --- a/src/sys/sys_uv.rs +++ b/src/sys/sys_uv.rs @@ -329,18 +329,13 @@ pub fn realpath<'a>(file_path: &ZStr, buf: &'a mut [u8]) -> Result<&'a [u8]> { let mut req = FsReq::new(); // SAFETY: synchronous libuv fs call; `req` lives on the stack for the duration. let rc = unsafe { uv::uv_fs_realpath(uv::Loop::get(), &mut *req, file_path.as_ptr(), None) }; - if let Some(errno) = rc.errno() { + if let Some(err) = rc.to_error(Tag::realpath) { log!( "uv realpath({}) = {}", BStr::new(file_path.as_bytes()), rc.int() ); - return Result::Err(Error { - errno, - syscall: Tag::realpath, - path: file_path.as_bytes().into(), - ..Default::default() - }); + return Result::Err(err.with_path(file_path.as_bytes())); } let Some(resolved) = req.ptr_c_str() else { return Result::Err(Error::new(E::NOENT, Tag::realpath).with_path(file_path.as_bytes())); @@ -380,12 +375,8 @@ pub fn mkdtemp(template: &mut [u8]) -> Result { None, ) }; - if let Some(errno) = rc.errno() { - return Result::Err(Error { - errno, - syscall: Tag::mkdtemp, - ..Default::default() - }); + if let Some(err) = rc.to_error(Tag::mkdtemp) { + return Result::Err(err); } let created = req.path_c_str().map(|p| p.to_bytes()).unwrap_or(&[]); let len = created.len().min(template.len()); @@ -412,14 +403,7 @@ pub fn utime(file_path: &ZStr, atime: f64, mtime: f64) -> Result<()> { BStr::new(file_path.as_bytes()), rc.int() ); - match rc.errno() { - Some(errno) => Result::Err(Error { - errno, - syscall: Tag::utime, - ..Default::default() - }), - None => Result::Ok(()), - } + rc.to_result(Tag::utime) } /// `uv_fs_lutime` (times in seconds). The error carries no path. @@ -441,14 +425,7 @@ pub fn lutime(file_path: &ZStr, atime: f64, mtime: f64) -> Result<()> { BStr::new(file_path.as_bytes()), rc.int() ); - match rc.errno() { - Some(errno) => Result::Err(Error { - errno, - syscall: Tag::lutime, - ..Default::default() - }), - None => Result::Ok(()), - } + rc.to_result(Tag::lutime) } /// `uv_fs_futime` (times in seconds). @@ -458,13 +435,8 @@ pub fn futime(fd: Fd, atime: f64, mtime: f64) -> Result<()> { // SAFETY: synchronous libuv fs call; req lives on the stack for the duration. let rc = unsafe { uv::uv_fs_futime(uv::Loop::get(), &mut *req, uv_fd, atime, mtime, None) }; log!("uv futime({}) = {}", uv_fd, rc.int()); - match rc.errno() { - Some(errno) => Result::Err(Error { - errno, - syscall: Tag::futime, - fd, - ..Default::default() - }), + match rc.to_error(Tag::futime) { + Some(err) => Result::Err(err.with_fd(fd)), None => Result::Ok(()), } } From 95e28ebbb51cd46d6091a9af87115693ddcd3644 Mon Sep 17 00:00:00 2001 From: Jarred Sumner Date: Sat, 29 Aug 2026 10:23:19 +0000 Subject: [PATCH 5/8] BoxQueue: Send/Sync iff V: Send (PhantomData> + Sync for V: Send) --- src/threading/unbounded_queue.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/threading/unbounded_queue.rs b/src/threading/unbounded_queue.rs index 956e6e4844e3..8087d4c84189 100644 --- a/src/threading/unbounded_queue.rs +++ b/src/threading/unbounded_queue.rs @@ -339,8 +339,15 @@ impl UnboundedQueue { /// [`drain`](Self::drain), so callers never see a raw node. pub struct BoxQueue { inner: UnboundedQueue>, + /// Owns the queued `V`s: auto-`Send` only if `V: Send`. + _values: core::marker::PhantomData>, } +// SAFETY: channel semantics — `push`/`drain` are the internally synchronized +// MPSC operations and only ever move a `V` from the pushing thread to the +// draining one; no `&V` is shared, so `V: Send` suffices. +unsafe impl Sync for BoxQueue {} + /// The heap node behind [`BoxQueue`]. pub struct BoxNode { next: Link>, @@ -361,6 +368,7 @@ impl Default for BoxQueue { fn default() -> Self { Self { inner: UnboundedQueue::default(), + _values: core::marker::PhantomData, } } } From 26838af4bee6b8f88ac4dea8d03b229431c997ea Mon Sep 17 00:00:00 2001 From: Jarred Sumner Date: Sat, 29 Aug 2026 10:34:33 +0000 Subject: [PATCH 6/8] empty typed arrays of any kind get a zero-length view with nothing to free; name the fs_t poison sentinel --- src/jsc/array_buffer.rs | 14 +++++++++++++- src/libuv_sys/libuv.rs | 10 ++++++---- 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/src/jsc/array_buffer.rs b/src/jsc/array_buffer.rs index 8c1c9e6328f1..86f91f99064c 100644 --- a/src/jsc/array_buffer.rs +++ b/src/jsc/array_buffer.rs @@ -439,7 +439,19 @@ impl ArrayBuffer { return Self::create::<{ JSType::Uint8Array }>(ctx, b""); } - // TODO: others + // Other kinds: a zero-length view with no backing store to free + // (`ptr` may be dangling here, e.g. `EMPTY`). + // SAFETY: null/0 with no deallocator is the documented empty case. + return unsafe { + make_typed_array_with_bytes_no_copy( + ctx, + self.typed_array_type.to_typed_array_type(), + core::ptr::null_mut(), + 0, + None, + core::ptr::null_mut(), + ) + }; } if self.typed_array_type == JSType::ArrayBuffer { diff --git a/src/libuv_sys/libuv.rs b/src/libuv_sys/libuv.rs index e943fdad09e3..7c53270f20fd 100644 --- a/src/libuv_sys/libuv.rs +++ b/src/libuv_sys/libuv.rs @@ -1966,6 +1966,8 @@ impl core::ops::DerefMut for OwnedFsReq { impl fs_t { #[cfg(debug_assertions)] const UV_FS_CLEANEDUP: c_int = 0x0010; + /// Poison `loop_` value marking a request never handed to libuv. + const UNINIT_LOOP_SENTINEL: usize = 0xAAAA_AAAA_AAAA_0000; /// Debug sentinel: `loop_` is poisoned so `deinit()` can assert that libuv /// actually wrote the request before we try to clean it up. @@ -1976,7 +1978,7 @@ impl fs_t { #[inline(always)] pub fn uninitialized() -> fs_t { let mut v: fs_t = bun_core::ffi::zeroed(); - v.loop_ = 0xAAAA_AAAA_AAAA_0000usize as *mut Loop; + v.loop_ = Self::UNINIT_LOOP_SENTINEL as *mut Loop; v } @@ -1990,7 +1992,7 @@ impl fs_t { #[inline] fn assert_initialized(&self) { #[cfg(debug_assertions)] - if self.loop_ as usize == 0xAAAA_AAAA_AAAA_0000usize { + if self.loop_ as usize == Self::UNINIT_LOOP_SENTINEL { panic!("uv_fs_t was not initialized"); } } @@ -1998,7 +2000,7 @@ impl fs_t { pub fn assert_cleaned_up(&self) { #[cfg(debug_assertions)] { - if self.loop_ as usize == 0xAAAA_AAAA_AAAA_0000usize { + if self.loop_ as usize == Self::UNINIT_LOOP_SENTINEL { return; } if (self.flags & Self::UV_FS_CLEANEDUP) != 0 { @@ -2018,7 +2020,7 @@ impl fs_t { /// [`uninitialized`](Self::uninitialized) sentinel). #[inline] pub fn is_initialized(&self) -> bool { - self.loop_ as usize != 0xAAAA_AAAA_AAAA_0000usize + self.loop_ as usize != Self::UNINIT_LOOP_SENTINEL } /// The `uv_statfs_t` a successful `uv_fs_statfs` left in `req.ptr`, copied /// out (`None` before completion, on error, or after cleanup nulled it). From 3e489894a6d301dda71c37e12573d2ca7ca091d8 Mon Sep 17 00:00:00 2001 From: Jarred Sumner Date: Sat, 29 Aug 2026 10:44:14 +0000 Subject: [PATCH 7/8] array_buffer: build empty typed arrays of other kinds by copy so the view has a real backing store (a null no-copy buffer is detached and JSC throws) --- src/jsc/array_buffer.rs | 42 +++++++++++++++++------------------------ 1 file changed, 17 insertions(+), 25 deletions(-) diff --git a/src/jsc/array_buffer.rs b/src/jsc/array_buffer.rs index 86f91f99064c..ec4308e82b07 100644 --- a/src/jsc/array_buffer.rs +++ b/src/jsc/array_buffer.rs @@ -439,19 +439,7 @@ impl ArrayBuffer { return Self::create::<{ JSType::Uint8Array }>(ctx, b""); } - // Other kinds: a zero-length view with no backing store to free - // (`ptr` may be dangling here, e.g. `EMPTY`). - // SAFETY: null/0 with no deallocator is the documented empty case. - return unsafe { - make_typed_array_with_bytes_no_copy( - ctx, - self.typed_array_type.to_typed_array_type(), - core::ptr::null_mut(), - 0, - None, - core::ptr::null_mut(), - ) - }; + return create_typed_array_copy(ctx, self.typed_array_type.to_typed_array_type(), b""); } if self.typed_array_type == JSType::ArrayBuffer { @@ -861,18 +849,7 @@ impl BinaryType { | BinaryType::Float64Array | BinaryType::BigInt64Array | BinaryType::BigUint64Array => { - crate::host_fn::from_js_host_call(global, || { - // SAFETY: `global` is a live opaque ZST handle; `bytes` is a - // valid slice whose pointer/len are only read (copied) by C++. - unsafe { - Bun__createTypedArrayForCopy( - global, - self.to_typed_array_type(), - bytes.as_ptr().cast(), - bytes.len(), - ) - } - }) + create_typed_array_copy(global, self.to_typed_array_type(), bytes) } } } @@ -1070,6 +1047,21 @@ pub(crate) unsafe fn make_array_buffer_with_bytes_no_copy( }) } +/// A new typed array of `array_type` over a fresh JSC-owned copy of `bytes` +/// (a real 1-byte backing store when empty, so the result is never detached). +pub(crate) fn create_typed_array_copy( + global: &JSGlobalObject, + array_type: TypedArrayType, + bytes: &[u8], +) -> JsResult { + crate::host_fn::from_js_host_call(global, || { + // SAFETY: `bytes` is a live slice; C++ only reads `len` bytes from it. + unsafe { + Bun__createTypedArrayForCopy(global, array_type, bytes.as_ptr().cast(), bytes.len()) + } + }) +} + /// Wrap caller-provided bytes in a JS typed array of `array_type` without /// copying. JSC adopts `ptr..ptr+len` as the backing store of the returned /// object and calls `deallocator(ptr, deallocator_context)` on the JS thread From 5f1af6ec8de3d7c461c350ab936ad2f817f874f8 Mon Sep 17 00:00:00 2001 From: Jarred Sumner Date: Sat, 29 Aug 2026 10:46:27 +0000 Subject: [PATCH 8/8] =?UTF-8?q?libuv=5Fsys:=20fs=5Ft::path=5Fc=5Fstr=20is?= =?UTF-8?q?=20unsafe=20=E2=80=94=20synchronous=20requests=20borrow=20the?= =?UTF-8?q?=20caller's=20path=20instead=20of=20copying=20it?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/libuv_sys/libuv.rs | 10 +++++++--- src/sys/sys_uv.rs | 5 ++++- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/src/libuv_sys/libuv.rs b/src/libuv_sys/libuv.rs index 7c53270f20fd..82138ad9c6d3 100644 --- a/src/libuv_sys/libuv.rs +++ b/src/libuv_sys/libuv.rs @@ -2050,13 +2050,17 @@ impl fs_t { } /// `req.path` — the request's (UTF-8, NUL-terminated) path; for /// `uv_fs_mkdtemp`/`uv_fs_mkstemp` libuv rewrites it to the created name. + /// + /// # Safety + /// The path passed to the originating `uv_fs_*` call must still be alive: + /// synchronous requests (`cb == NULL`) borrow it instead of copying. #[inline] - pub fn path_c_str(&self) -> Option<&core::ffi::CStr> { + pub unsafe fn path_c_str(&self) -> Option<&core::ffi::CStr> { if !self.is_initialized() || self.path.is_null() { return None; } - // SAFETY: libuv keeps `path` pointing at a NUL-terminated copy it owns - // until `uv_fs_req_cleanup` nulls it. + // SAFETY: NUL-terminated; either libuv's owned copy (until + // `uv_fs_req_cleanup` nulls it) or the caller's input per the contract. Some(unsafe { core::ffi::CStr::from_ptr(self.path) }) } /// `req.file.fd` union arm. The union is private diff --git a/src/sys/sys_uv.rs b/src/sys/sys_uv.rs index 396c9e6f869f..bd1ad09e4d9d 100644 --- a/src/sys/sys_uv.rs +++ b/src/sys/sys_uv.rs @@ -378,7 +378,10 @@ pub fn mkdtemp(template: &mut [u8]) -> Result { if let Some(err) = rc.to_error(Tag::mkdtemp) { return Result::Err(err); } - let created = req.path_c_str().map(|p| p.to_bytes()).unwrap_or(&[]); + // SAFETY: `template` (the path passed to `uv_fs_mkdtemp`) is still alive. + let created = unsafe { req.path_c_str() } + .map(|p| p.to_bytes()) + .unwrap_or(&[]); let len = created.len().min(template.len()); template[..len].copy_from_slice(&created[..len]); Result::Ok(len)