From 894cd0711eaf7180f1eac2e2d46b26f782578989 Mon Sep 17 00:00:00 2001 From: Jarred Sumner Date: Sun, 23 Aug 2026 16:56:27 +0000 Subject: [PATCH 1/6] shell: remove unsafe from the interpreter, state nodes, IOWriter/IOReader and Builtin State nodes live in a chunked RefCell arena addressed by NodeId; the shell env and captured output buffers are shared through Rc handles; IOWriter and IOReader are intrusively refcounted (RefPtr/ThisPtr) with cell fields; the parsed AST is owned by bun_shell_parser::ParsedScript and reached through BackRef; thread-pool tasks are boxed and bounce back through the node they embed (EventLoopTask::arm_boxed), so the round trip allocates nothing. Builtins no longer hold their Cmd node borrowed while an IOWriter runs (Builtin::write_out*), Stdio::Capture carries no buffer pointer, and Body::Value::with_request_or_response scopes the body borrow. --- Cargo.lock | 1 + src/bun_core/lib.rs | 17 +- src/event_loop/AnyEventLoop.rs | 36 + src/event_loop/AnyTaskWithExtraContext.rs | 62 +- src/event_loop/ConcurrentTask.rs | 27 + src/event_loop/MiniEventLoop.rs | 8 +- src/event_loop/lib.rs | 2 +- src/io/PipeWriter.rs | 109 +- src/jsc/VmHandle.rs | 9 + src/ptr/lib.rs | 29 + src/ptr/ref_count.rs | 15 + src/runtime/api/bun/spawn/stdio.rs | 25 +- src/runtime/api/bun/subprocess/Readable.rs | 2 +- src/runtime/api/bun/subprocess/Writable.rs | 4 +- src/runtime/cli/exec_command.rs | 2 +- src/runtime/cli/run_command.rs | 4 +- src/runtime/dispatch.rs | 48 +- src/runtime/node/node_fs.rs | 2 +- src/runtime/shell/Builtin.rs | 447 +++-- src/runtime/shell/IO.rs | 65 +- src/runtime/shell/IOReader.rs | 356 ++-- src/runtime/shell/IOWriter.rs | 839 +++++---- src/runtime/shell/ParsedShellScript.rs | 56 +- src/runtime/shell/Yield.rs | 4 +- src/runtime/shell/builtin/basename.rs | 18 +- src/runtime/shell/builtin/cat.rs | 142 +- src/runtime/shell/builtin/cd.rs | 34 +- src/runtime/shell/builtin/cp.rs | 356 ++-- src/runtime/shell/builtin/dirname.rs | 9 +- src/runtime/shell/builtin/echo.rs | 8 +- src/runtime/shell/builtin/exit.rs | 25 +- src/runtime/shell/builtin/export.rs | 18 +- src/runtime/shell/builtin/ls.rs | 246 ++- src/runtime/shell/builtin/mkdir.rs | 195 +- src/runtime/shell/builtin/mv.rs | 256 ++- src/runtime/shell/builtin/pwd.rs | 23 +- src/runtime/shell/builtin/rm.rs | 190 +- src/runtime/shell/builtin/seq.rs | 70 +- src/runtime/shell/builtin/touch.rs | 177 +- src/runtime/shell/builtin/which.rs | 60 +- src/runtime/shell/builtin/yes.rs | 37 +- src/runtime/shell/dispatch_tasks.rs | 136 +- src/runtime/shell/interpreter.rs | 1593 +++++++---------- src/runtime/shell/mod.rs | 14 +- src/runtime/shell/shell_body.rs | 44 +- src/runtime/shell/states/Assigns.rs | 40 +- src/runtime/shell/states/Async.rs | 134 +- src/runtime/shell/states/Base.rs | 37 +- src/runtime/shell/states/Binary.rs | 19 +- src/runtime/shell/states/Cmd.rs | 357 ++-- src/runtime/shell/states/CondExpr.rs | 112 +- src/runtime/shell/states/Expansion.rs | 155 +- src/runtime/shell/states/If.rs | 33 +- src/runtime/shell/states/Pipeline.rs | 89 +- src/runtime/shell/states/Script.rs | 72 +- src/runtime/shell/states/Stmt.rs | 35 +- src/runtime/shell/states/Subshell.rs | 55 +- src/runtime/shell/subproc.rs | 93 +- src/runtime/webcore/Body.rs | 14 +- src/shell_parser/Cargo.toml | 1 + src/shell_parser/lib.rs | 4 +- src/shell_parser/parse.rs | 128 +- src/threading/work_pool.rs | 26 +- .../vm-thread-door.inventory.json | 21 +- test/js/bun/shell/commands/rm.test.ts | 22 + 65 files changed, 3444 insertions(+), 3823 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 20012ce3d0cf..1381950e6486 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1384,6 +1384,7 @@ dependencies = [ "bstr", "bun_alloc", "bun_core", + "bun_ptr", "libc", "smallvec", "strum", diff --git a/src/bun_core/lib.rs b/src/bun_core/lib.rs index aa42dd2f9f2d..63437ea01714 100644 --- a/src/bun_core/lib.rs +++ b/src/bun_core/lib.rs @@ -950,6 +950,7 @@ pub unsafe trait IntrusiveField: Sized { /// /// ```ignore /// bun_core::intrusive_field!(ShellCpTask, task: ShellTask); +/// bun_core::intrusive_field!(ShellCpTask, task.task: WorkPoolTask); // nested field /// bun_core::intrusive_field!([T: Send] Wrapper, inner: Mixin>); /// ``` #[macro_export] @@ -957,14 +958,22 @@ macro_rules! intrusive_field { // Bracketed-generics arm MUST come first: the bare `$T:ty` arm below would // otherwise try to parse `['a]` as a slice type and hard-error on the // lifetime before backtracking to this arm. - ([$($gen:tt)*] $T:ty, $field:ident : $F:ty) => { + ([$($gen:tt)*] $T:ty, $($field:ident).+ : $F:ty) => { unsafe impl<$($gen)*> $crate::IntrusiveField<$F> for $T { - const OFFSET: usize = ::core::mem::offset_of!($T, $field); + const OFFSET: usize = { + // The named field must actually be an `$F`. + let _ = |s: &$T| -> *const $F { &raw const s.$($field).+ }; + ::core::mem::offset_of!($T, $($field).+) + }; } }; - ($T:ty, $field:ident : $F:ty) => { + ($T:ty, $($field:ident).+ : $F:ty) => { unsafe impl $crate::IntrusiveField<$F> for $T { - const OFFSET: usize = ::core::mem::offset_of!($T, $field); + const OFFSET: usize = { + // The named field must actually be an `$F`. + let _ = |s: &$T| -> *const $F { &raw const s.$($field).+ }; + ::core::mem::offset_of!($T, $($field).+) + }; } }; } diff --git a/src/event_loop/AnyEventLoop.rs b/src/event_loop/AnyEventLoop.rs index 6ec41178355d..a5786fbb7311 100644 --- a/src/event_loop/AnyEventLoop.rs +++ b/src/event_loop/AnyEventLoop.rs @@ -252,6 +252,14 @@ pub enum EventLoopTask { Mini(AnyTaskWithExtraContext), } +/// A leaked box's embedded node, armed and ready to post to its loop +/// ([`EventLoopTask::arm_boxed`]). +#[derive(Clone, Copy)] +pub enum ArmedLoopTask { + Js(NonNull), + Mini(NonNull), +} + impl EventLoopTask { pub fn from_event_loop(loop_: EventLoopHandle) -> EventLoopTask { match loop_ { @@ -259,6 +267,27 @@ impl EventLoopTask { EventLoopHandle::Mini(_) => EventLoopTask::Mini(AnyTaskWithExtraContext::default()), } } + + /// Leak `owner` into the node it embeds (`node(owner)`). The JS arm queues + /// `Task { T::TAG, owner }` for `bun_runtime::dispatch` to rebox; the mini + /// arm hands the box to `R::run_from_loop_thread`. No allocation. + pub fn arm_boxed(owner: Box, node: fn(&mut T) -> &mut EventLoopTask) -> ArmedLoopTask + where + T: crate::Taskable, + R: crate::AnyTaskWithExtraContext::BoxedMiniTaskRunner, + { + let raw: *mut T = bun_core::heap::into_raw(owner); + // SAFETY: `raw` was just leaked; this thread owns it exclusively until + // the node is queued. + match node(unsafe { &mut *raw }) { + EventLoopTask::Js(ct) => ArmedLoopTask::Js(NonNull::from( + ct.from(raw, crate::ConcurrentTask::AutoDeinit::ManualDeinit), + )), + EventLoopTask::Mini(at) => { + ArmedLoopTask::Mini(AnyTaskWithExtraContext::arm_at::(at, raw)) + } + } + } } /// RAII pairing for [`EventLoopHandle::enter`] / [`EventLoopHandle::exit`]. @@ -508,6 +537,13 @@ impl EventLoopHandle { f(unsafe { (*env).get(key) }) } + /// `f(loader)` on the loop's dotenv loader; the borrow ends with `f` (the + /// map is mutable at runtime). + pub fn with_env(self, f: impl FnOnce(&DotEnvLoader) -> R) -> R { + // SAFETY: as `with_env_var`. + f(unsafe { &*self.env() }) + } + pub fn top_level_dir(self) -> &'static [u8] { match self { // SAFETY: slice borrowed for VM lifetime. diff --git a/src/event_loop/AnyTaskWithExtraContext.rs b/src/event_loop/AnyTaskWithExtraContext.rs index e033e5cb6e5b..a1c8855c0c30 100644 --- a/src/event_loop/AnyTaskWithExtraContext.rs +++ b/src/event_loop/AnyTaskWithExtraContext.rs @@ -68,6 +68,27 @@ impl AnyTaskWithExtraContext { } } + /// Leak `owner` and arm the node it embeds (`node(owner)`) so the mini + /// loop hands the box to `R::run_from_loop_thread`. No allocation. + pub fn arm_boxed>( + owner: Box, + node: fn(&mut T) -> &mut AnyTaskWithExtraContext, + ) -> NonNull { + let raw: *mut T = bun_core::heap::into_raw(owner); + // SAFETY: `raw` was just leaked; this thread owns it exclusively until + // the node is queued. + Self::arm_at::(node(unsafe { &mut *raw }), raw) + } + + /// Arm `at` (a node inside the leaked box `raw`) for `R`. + pub(crate) fn arm_at>( + at: &mut AnyTaskWithExtraContext, + raw: *mut T, + ) -> NonNull { + *at = New::::init(raw, mini_boxed_trampoline::); + NonNull::from(at) + } + /// 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. @@ -77,14 +98,45 @@ impl AnyTaskWithExtraContext { std::ptr::from_mut::(self) } - pub(crate) fn run(&mut self, extra: *mut c_void) { - let callback = self.callback; - let ctx = self.ctx; - // SAFETY: caller contract — `ctx` was set by `init`/`from*` to a live pointer. - callback(ctx.expect("ctx is non-null").as_ptr(), extra.cast::<()>()); + /// Copy the node's two words out so running it borrows nothing from the + /// allocation it lives in (the callback may free that allocation). + /// + /// # Safety + /// `this` is a queued node, live at the time of the call. + pub(crate) unsafe fn load(this: *const Self) -> Runnable { + // SAFETY: fn contract. + let (callback, ctx) = unsafe { ((*this).callback, (*this).ctx) }; + Runnable { callback, ctx } } } +/// A dequeued [`AnyTaskWithExtraContext`], detached from its storage. +pub(crate) struct Runnable { + callback: fn(*mut (), *mut ()), + ctx: Option>, +} + +impl Runnable { + pub(crate) fn run(self, extra: *mut c_void) { + (self.callback)( + self.ctx.expect("ctx is non-null").as_ptr(), + extra.cast::<()>(), + ); + } +} + +/// Receives a boxed payload back on the mini loop's thread +/// ([`AnyTaskWithExtraContext::arm_boxed`]). +pub trait BoxedMiniTaskRunner { + fn run_from_loop_thread(owner: Box); +} + +fn mini_boxed_trampoline>(this: *mut T, _extra: *mut ()) { + // SAFETY: `this` is the box `arm_boxed` leaked into its own node; the loop + // copied the node out (`load`) and fires it once. + R::run_from_loop_thread(unsafe { bun_core::heap::take(this) }); +} + /// Stable Rust cannot take a fn value as a const generic, so `Callback` moves to /// a runtime argument on `init` and is type-erased (ABI-identical: both forms /// are thin fn pointers taking two thin data pointers). diff --git a/src/event_loop/ConcurrentTask.rs b/src/event_loop/ConcurrentTask.rs index 245c9500ea3c..d87603433eb4 100644 --- a/src/event_loop/ConcurrentTask.rs +++ b/src/event_loop/ConcurrentTask.rs @@ -164,6 +164,33 @@ pub trait Taskable { unsafe fn release_unrun(this: *mut Self); } +/// [`Taskable`] for a type that is only ever queued as a leaked `Box` +/// (`heap::into_raw` / `Box::into_raw` at every post site, `Box::from_raw` in +/// its `bun_runtime::dispatch` arm). Released unrun by reclaiming the box and +/// handing it to `|this| $release` (default: drop it). +/// +/// ```ignore +/// bun_event_loop::boxed_taskable!(ShellGlobTask, ShellGlobTask, |this| this.task.unref_unrun()); +/// ``` +#[macro_export] +macro_rules! boxed_taskable { + ($ty:ty, $tag:ident) => { + $crate::boxed_taskable!($ty, $tag, |this| ()); + }; + ($ty:ty, $tag:ident, |$this:ident| $release:expr) => { + impl $crate::Taskable for $ty { + const TAG: $crate::TaskTag = $crate::task_tag::$tag; + unsafe fn release_unrun(this: *mut Self) { + // SAFETY: `release_unrun` contract — `this` is the queued + // `Task::ptr` under `TAG`, which for this type is a leaked `Box`. + #[allow(unused_mut)] + let mut $this: ::std::boxed::Box = unsafe { ::bun_core::heap::take(this) }; + $release; + } + } + }; +} + impl TaskTag { /// The tag's identifier, for diagnostics. pub fn name(self) -> &'static str { diff --git a/src/event_loop/MiniEventLoop.rs b/src/event_loop/MiniEventLoop.rs index 6219edb62805..40f26a45deda 100644 --- a/src/event_loop/MiniEventLoop.rs +++ b/src/event_loop/MiniEventLoop.rs @@ -315,8 +315,9 @@ impl MiniEventLoop { } while let Some(task) = self.tasks.read_item() { - // SAFETY: tasks are pushed by enqueue_task* and remain valid until run() consumes them. - unsafe { (*task).run(context) }; + // SAFETY: tasks are pushed by enqueue_task* and remain valid until loaded here. + let task = unsafe { AnyTaskWithExtraContext::load(task) }; + task.run(context); } } @@ -325,7 +326,8 @@ impl MiniEventLoop { let _ = self.tick_concurrent_with_count(); while let Some(task) = self.tasks.read_item() { // SAFETY: see tick_once. - unsafe { (*task).run(context) }; + let task = unsafe { AnyTaskWithExtraContext::load(task) }; + task.run(context); } // SAFETY: see `loop_ptr()` invariant. diff --git a/src/event_loop/lib.rs b/src/event_loop/lib.rs index 9a0be447363f..4fb4d605a7c2 100644 --- a/src/event_loop/lib.rs +++ b/src/event_loop/lib.rs @@ -36,7 +36,7 @@ pub use ConcurrentTask::{Task, TaskTag, Taskable, task_tag}; pub use DeferredTaskQueue as deferred_task_queue; pub use any_event_loop::{ - AnyEventLoop, EventLoopHandle, EventLoopTask, JsPoster, JsPosterVTable, Posted, + AnyEventLoop, ArmedLoopTask, EventLoopHandle, EventLoopTask, JsPoster, JsPosterVTable, Posted, }; pub use bun_io::PipeReadScratch; diff --git a/src/io/PipeWriter.rs b/src/io/PipeWriter.rs index 31adcfb8f93f..5e07c1e56ea4 100644 --- a/src/io/PipeWriter.rs +++ b/src/io/PipeWriter.rs @@ -2595,14 +2595,18 @@ pub type StreamingWriter

= WindowsStreamingWriter

; // parents that may drop their last ref mid-callback). // // Accessor args use closure-literal syntax (`|this| expr`) purely as a binder -// for the macro — no actual closure is created. For `mut`/`shared`/`ptr`, -// `expr` is pasted into an `unsafe` block with `this: *mut Self` in scope; -// for `this`, `expr` is pasted as-is with `this: ThisPtr` in scope. +// for the macro — no actual closure is created. For `mut`/`ptr`, `expr` is +// pasted into an `unsafe` block with `this: *mut Self` in scope; for `this`, +// `impl_streaming_writer_parent!` pastes `expr` as-is with `this: ThisPtr` +// in scope. `impl_buffered_writer_parent!` binds `this: &Self` (pasted as-is) +// for its read-only accessors under `shared`/`this`, and under `this` maps +// the writer's `ref_`/`deref` hooks to the parent's intrusive refcount. /// Re-exports for `$crate::`-qualified use inside the macro bodies so callers /// need no extra `use` items. #[doc(hidden)] pub mod __parent_macro { + pub use ::bun_ptr::AnyRefCounted; pub use ::bun_ptr::ThisPtr; pub use ::bun_sys::Error as SysError; #[cfg(windows)] @@ -2758,6 +2762,41 @@ macro_rules! impl_streaming_writer_parent { /// `WindowsBufferedWriterParent` for a parent type. See module comment above. #[macro_export] macro_rules! impl_buffered_writer_parent { + // Internal: dispatch a callback off the raw-ptr backref per `borrow` mode. + (@call mut $p:expr; $m:ident($($a:tt)*)) => { (&mut *$p).$m($($a)*) }; + (@call shared $p:expr; $m:ident($($a:tt)*)) => { (&*$p).$m($($a)*) }; + (@call ptr $p:expr; $m:ident($($a:tt)*)) => { ::$m($p, $($a)*) }; + (@call this $p:expr; $m:ident($($a:tt)*)) => { + ::$m($crate::pipe_writer::__parent_macro::ThisPtr::::new($p), $($a)*) + }; + + // Internal: evaluate an accessor body with `$id` bound per `borrow` mode. + // `shared`/`this` bind `&Self` (accessors only read), so accessor bodies + // are plain safe expressions; `mut`/`ptr` keep the raw `*mut Self`. + (@acc mut $id:ident = $p:ident; $e:expr) => {{ + let $id = $p; + #[allow(unused_unsafe)] + unsafe { $e } + }}; + (@acc ptr $id:ident = $p:ident; $e:expr) => { + $crate::impl_buffered_writer_parent!(@acc mut $id = $p; $e) + }; + (@acc $borrow:tt $id:ident = $p:ident; $e:expr) => {{ + // SAFETY: `$p` is the BACKREF set via `set_parent` — the live parent + // for the duration of this read-only accessor. + let $id: &Self = unsafe { &*$p }; + $e + }}; + // Internal: refcount hooks. `this` hands the body the raw root pointer + // (the body may free `*$p`); other modes bind as `@acc` does. + (@rc this $id:ident = $p:ident; $e:expr) => {{ + let $id: *mut Self = $p; + $e + }}; + (@rc $borrow:tt $id:ident = $p:ident; $e:expr) => { + $crate::impl_buffered_writer_parent!(@acc $borrow $id = $p; $e) + }; + (@emit [$($gen:tt)*] $Ty:ty; poll_tag = $poll_tag:expr, @@ -2777,31 +2816,30 @@ macro_rules! impl_buffered_writer_parent { #[inline] unsafe fn on_write(this: *mut Self, amount: usize, status: $crate::WriteStatus) { // SAFETY: `this` is the BACKREF set via `set_parent`; the - // BufferedWriter never materializes `&mut Parent`. The handler - // is dispatched per the `borrow` mode (`mut`/`shared`/`ptr`/`this` — - // see the module comment). - unsafe { $crate::impl_streaming_writer_parent!(@call $borrow this; $on_write(amount, status)) }; + // BufferedWriter never materializes `&mut Parent`, so this is + // the unique access path for the callback's duration. + unsafe { $crate::impl_buffered_writer_parent!(@call $borrow this; $on_write(amount, status)) }; } #[inline] unsafe fn on_error(this: *mut Self, err: $crate::pipe_writer::__parent_macro::SysError) { // SAFETY: see on_write. - unsafe { $crate::impl_streaming_writer_parent!(@call $borrow this; $on_error(&err)) }; + unsafe { $crate::impl_buffered_writer_parent!(@call $borrow this; $on_error(&err)) }; } const HAS_ON_CLOSE: bool = true; #[inline] unsafe fn on_close(this: *mut Self) { // SAFETY: see on_write. - unsafe { $crate::impl_streaming_writer_parent!(@call $borrow this; $on_close()) }; + unsafe { $crate::impl_buffered_writer_parent!(@call $borrow this; $on_close()) }; } #[inline] unsafe fn get_buffer<'a>(this: *mut Self) -> &'a [u8] { // SAFETY: see on_write. Shared-only borrow of the buffer storage. - $crate::impl_streaming_writer_parent!(@acc $borrow $gb_this = this; $gb) + $crate::impl_buffered_writer_parent!(@acc $borrow $gb_this = this; $gb) } #[inline] unsafe fn event_loop(this: *mut Self) -> $crate::EventLoopHandle { // SAFETY: see on_write. - $crate::impl_streaming_writer_parent!(@acc $borrow $el_this = this; $el) + $crate::impl_buffered_writer_parent!(@acc $borrow $el_this = this; $el) } } @@ -2810,17 +2848,17 @@ macro_rules! impl_buffered_writer_parent { #[inline] unsafe fn loop_(this: *mut Self) -> *mut $crate::pipe_writer::__parent_macro::UvLoop { // SAFETY: BACKREF set via `set_parent`; shared-only read. - $crate::impl_streaming_writer_parent!(@acc $borrow $uv_this = this; $uv) + $crate::impl_buffered_writer_parent!(@acc $borrow $uv_this = this; $uv) } #[inline] unsafe fn ref_(this: *mut Self) { // SAFETY: see loop_. Intrusive refcount bump. - $crate::impl_streaming_writer_parent!(@acc $borrow $ref_this = this; $ref_) + $crate::impl_buffered_writer_parent!(@rc $borrow $ref_this = this; $ref_) } #[inline] unsafe fn deref(this: *mut Self) { // SAFETY: see loop_. May free `this`. - $crate::impl_streaming_writer_parent!(@acc $borrow $deref_this = this; $deref) + $crate::impl_buffered_writer_parent!(@rc $borrow $deref_this = this; $deref) } } @@ -2829,28 +2867,63 @@ macro_rules! impl_buffered_writer_parent { #[inline] unsafe fn on_write(this: *mut Self, amount: usize, status: $crate::WriteStatus) { // SAFETY: BACKREF set via `set_parent`; see borrow-mode note. - unsafe { $crate::impl_streaming_writer_parent!(@call $borrow this; $on_write(amount, status)) }; + unsafe { $crate::impl_buffered_writer_parent!(@call $borrow this; $on_write(amount, status)) }; } #[inline] unsafe fn on_error(this: *mut Self, err: $crate::pipe_writer::__parent_macro::SysError) { // SAFETY: see on_write. - unsafe { $crate::impl_streaming_writer_parent!(@call $borrow this; $on_error(&err)) }; + unsafe { $crate::impl_buffered_writer_parent!(@call $borrow this; $on_error(&err)) }; } const HAS_ON_CLOSE: bool = true; #[inline] unsafe fn on_close(this: *mut Self) { // SAFETY: see on_write. - unsafe { $crate::impl_streaming_writer_parent!(@call $borrow this; $on_close()) }; + unsafe { $crate::impl_buffered_writer_parent!(@call $borrow this; $on_close()) }; } #[inline] unsafe fn get_buffer<'a>(this: *mut Self) -> &'a [u8] { // SAFETY: see on_write. - $crate::impl_streaming_writer_parent!(@acc $borrow $gb_this = this; $gb) + $crate::impl_buffered_writer_parent!(@acc $borrow $gb_this = this; $gb) } const HAS_ON_WRITABLE: bool = false; } }; + // Public entry — `borrow = this`, intrusively refcounted parent: the + // writer's `ref_`/`deref` hooks are the parent's own refcount. + ( + $Ty:ty; + poll_tag = $poll_tag:expr, + borrow = this, + on_write = $on_write:ident, + on_error = $on_error:ident, + on_close = $on_close:ident, + get_buffer = |$gb_this:ident| $gb:expr, + event_loop = |$el_this:ident| $el:expr, + uv_loop = |$uv_this:ident| $uv:expr, + ) => { + $crate::impl_buffered_writer_parent! { + @emit [] $Ty; + poll_tag = $poll_tag, + borrow = this, + on_write = $on_write, + on_error = $on_error, + on_close = $on_close, + get_buffer = |$gb_this| $gb, + event_loop = |$el_this| $el, + uv_loop = |$uv_this| $uv, + // SAFETY: `this_` is the live parent's root pointer. + ref_ = |this_| unsafe { + <$Ty as $crate::pipe_writer::__parent_macro::AnyRefCounted>::rc_ref(this_) + }, + // SAFETY: releases the ref the writer took through `ref_` above; + // `this_` is the parent's root pointer. + deref = |this_| unsafe { + <$Ty as $crate::pipe_writer::__parent_macro::AnyRefCounted>::rc_deref(this_) + }, + } + }; + // Public entry — generic parent. ( for<$($gp:ident $(: $b0:path)?),+> $Ty:ty; diff --git a/src/jsc/VmHandle.rs b/src/jsc/VmHandle.rs index 8b767f90b8a7..8672501b3901 100644 --- a/src/jsc/VmHandle.rs +++ b/src/jsc/VmHandle.rs @@ -817,6 +817,15 @@ impl ConcurrentPoster { matches!(self, ConcurrentPoster::Js(..)) } + /// Post a node armed by `EventLoopTask::arm_boxed` to whichever loop this + /// poster targets. + pub fn post(&self, task: bun_event_loop::ArmedLoopTask) { + match task { + bun_event_loop::ArmedLoopTask::Js(t) => self.post_js(t), + bun_event_loop::ArmedLoopTask::Mini(t) => self.post_mini(t), + } + } + /// Post a JS-loop `ConcurrentTask`. Panics (debug) if this poster is `Mini`. pub fn post_js(&self, task: NonNull) { match self { diff --git a/src/ptr/lib.rs b/src/ptr/lib.rs index 77e16c478440..0374b89ae610 100644 --- a/src/ptr/lib.rs +++ b/src/ptr/lib.rs @@ -162,6 +162,35 @@ impl BackRef { } } +/// The root pointer of a value built by [`RefPtr::new_cyclic`], stored inside +/// that value. It has no accessors of its own: the only way to use it is +/// [`SelfRoot::this_ptr`], which takes the enclosing `&T` — so it cannot be +/// followed before the value exists or after it is gone. +#[repr(transparent)] +pub struct SelfRoot(pub(crate) core::ptr::NonNull); + +impl SelfRoot { + /// The enclosing value as a [`ThisPtr`]. `owner` must be the value this + /// token is stored in (checked in debug builds). + #[inline] + pub fn this_ptr(&self, owner: &T) -> ThisPtr { + debug_assert!( + core::ptr::eq(self.0.as_ptr().cast_const(), owner), + "SelfRoot used from a value it does not belong to" + ); + let _ = owner; + // SAFETY: `owner: &T` proves the value is constructed and live; `self.0` + // is its allocation root (minted by `new_cyclic`). + unsafe { ThisPtr::new(self.0.as_ptr()) } + } + + /// The enclosing value as a root back-reference. + #[inline] + pub fn backref(&self, owner: &T) -> BackRef { + self.this_ptr(owner).into() + } +} + impl BackRef { #[inline] pub const fn dangling() -> Self { diff --git a/src/ptr/ref_count.rs b/src/ptr/ref_count.rs index 646a5f68866e..0c36a6cd9245 100644 --- a/src/ptr/ref_count.rs +++ b/src/ptr/ref_count.rs @@ -555,6 +555,21 @@ impl RefPtr { } } + /// [`new`](Self::new) for a `T` that stores its own root pointer (to hand + /// out [`ThisPtr`](crate::ThisPtr)s from `&self` entry points). `init` + /// receives a [`SelfRoot`](crate::SelfRoot) to store in the value; the + /// token cannot be dereferenced on its own, only through the `&T` that + /// exists once construction is done. + pub fn new_cyclic(init: impl FnOnce(crate::SelfRoot) -> T) -> Self { + let raw: NonNull = bun_core::heap::into_raw_nn(Box::::new_uninit()).cast::(); + let value = init(crate::SelfRoot(raw)); + // SAFETY: `raw` is a live, uninitialized, properly aligned `T` slot. + unsafe { raw.as_ptr().write(value) }; + // SAFETY: freshly written, so live. + debug_assert!(unsafe { T::rc_has_one_ref(raw.as_ptr()) }); + Self(raw) + } + /// Take a new ref on the pointee of a [`ThisPtr`](crate::ThisPtr). Safe: /// the `ThisPtr` invariant is that its pointee is live. This is the guard /// to hold across a re-entrant call that may otherwise drop the last ref. diff --git a/src/runtime/api/bun/spawn/stdio.rs b/src/runtime/api/bun/spawn/stdio.rs index 787aec56fc7b..776fdf266f81 100644 --- a/src/runtime/api/bun/spawn/stdio.rs +++ b/src/runtime/api/bun/spawn/stdio.rs @@ -1,5 +1,3 @@ -#[cfg(any(target_os = "linux", target_os = "android"))] -use bun_collections::VecExt; use bun_jsc::{self as jsc, JSGlobalObject, JSValue, JsResult}; #[cfg(windows)] use bun_sys::windows::libuv as uv; @@ -30,16 +28,6 @@ use sys::Stdio as FdStdio; // `const log = bun.sys.syslog;` bun_output::define_scoped_log!(log, SYS, visible); -/// Payload of `Stdio::Capture`. -#[derive(Clone, Copy)] -pub struct Capture { - // BACKREF: raw pointer to a capture buffer owned by the shell interpreter. - // The shell keeps the buffer alive for the lifetime - // of the spawned process; this struct never frees it. - #[cfg(any(target_os = "linux", target_os = "android"))] - pub(crate) buf: *mut Vec, -} - /// Payload of `Stdio::Dup2`. #[derive(Clone, Copy)] pub struct Dup2 { @@ -52,7 +40,9 @@ pub struct Dup2 { #[allow(clippy::large_enum_variant)] pub enum Stdio { Inherit, - Capture(Capture), + /// The shell tees the child's output into its own buffer through the + /// `PipeReader`. + Capture, Ignore, Fd(Fd), Dup2(Dup2), @@ -104,10 +94,7 @@ impl Stdio { #[cfg(any(target_os = "linux", target_os = "android"))] pub(crate) fn byte_slice(&self) -> &[u8] { match self { - // SAFETY: `buf` is a live backref owned by the caller (shell); the - // returned slice borrows `self` and the caller guarantees the - // Vec outlives this Stdio. - Self::Capture(c) => unsafe { (*c.buf).slice() }, + Self::Capture => &[], Self::Blob(blob) => blob.slice(), _ => &[], } @@ -284,7 +271,7 @@ impl Stdio { out: d.out, to: d.to, }), - Self::Capture(_) | Self::Pipe | Self::ReadableStream(_) => buffer(), + Self::Capture | Self::Pipe | Self::ReadableStream(_) => buffer(), #[cfg(not(windows))] Self::SocketFd => SpawnOptionsStdio::SocketFd, // Windows extra-stdio is a libuv pipe handle (no raw-fd ownership @@ -308,7 +295,7 @@ impl Stdio { pub(crate) fn is_piped(&self) -> bool { match self { - Self::Capture(_) | Self::Blob(_) | Self::Pipe | Self::ReadableStream(_) => true, + Self::Capture | Self::Blob(_) | Self::Pipe | Self::ReadableStream(_) => true, Self::Ipc => cfg!(windows), _ => false, } diff --git a/src/runtime/api/bun/subprocess/Readable.rs b/src/runtime/api/bun/subprocess/Readable.rs index fc8d8eb81a4a..0421fe792a13 100644 --- a/src/runtime/api/bun/subprocess/Readable.rs +++ b/src/runtime/api/bun/subprocess/Readable.rs @@ -144,7 +144,7 @@ impl Readable { Readable::Pipe(PipeReader::create(event_loop, process, result, max_size)) } Stdio::Blob(..) => panic!("TODO: implement Blob support in Stdio readable"), - Stdio::Capture(..) => panic!("TODO: implement capture support in Stdio readable"), + Stdio::Capture => panic!("TODO: implement capture support in Stdio readable"), // ReadableStream is handled separately Stdio::ReadableStream(..) => Readable::Ignore, // Rejected at i < 3 in Stdio::extract(); stdout/stderr never see this. diff --git a/src/runtime/api/bun/subprocess/Writable.rs b/src/runtime/api/bun/subprocess/Writable.rs index 8d85512e39c0..273720c9ced2 100644 --- a/src/runtime/api/bun/subprocess/Writable.rs +++ b/src/runtime/api/bun/subprocess/Writable.rs @@ -195,7 +195,7 @@ impl<'a> Writable<'a> { Stdio::Memfd(_) | Stdio::Path(_) | Stdio::Ignore => { return Ok(Writable::Ignore); } - Stdio::Ipc | Stdio::Capture(_) => { + Stdio::Ipc | Stdio::Capture => { return Ok(Writable::Ignore); } // Rejected at i < 3 in Stdio::extract(); stdin never sees this. @@ -296,7 +296,7 @@ impl<'a> Writable<'a> { Stdio::Fd(_) => Ok(Writable::Fd(result.unwrap())), Stdio::Inherit => Ok(Writable::Inherit), Stdio::Path(_) | Stdio::Ignore => Ok(Writable::Ignore), - Stdio::Ipc | Stdio::Capture(_) => Ok(Writable::Ignore), + Stdio::Ipc | Stdio::Capture => Ok(Writable::Ignore), // Rejected at i < 3 in Stdio::extract(); stdin never sees this. Stdio::SocketFd => unreachable!("SocketFd at stdin"), } diff --git a/src/runtime/cli/exec_command.rs b/src/runtime/cli/exec_command.rs index 39dfdadc801d..411377c90746 100644 --- a/src/runtime/cli/exec_command.rs +++ b/src/runtime/cli/exec_command.rs @@ -73,7 +73,7 @@ impl ExecCommand { // other live `&mut` to the same `MiniEventLoop` on this thread). let mini_ref = unsafe { &mut *mini }; let code = match Interpreter::init_and_run_from_source( - ctx, + &*ctx, mini_ref, script_path, &script, diff --git a/src/runtime/cli/run_command.rs b/src/runtime/cli/run_command.rs index ffa4802bb9d0..20e314543a22 100644 --- a/src/runtime/cli/run_command.rs +++ b/src/runtime/cli/run_command.rs @@ -333,7 +333,7 @@ Full documentation is available at https://bun.com/docs/cli/run // no aliasing `&mut` exists across this call. let mini = unsafe { &mut *mini }; let code = match crate::shell::Interpreter::init_and_run_from_source( - ctx, + &*ctx, mini, name, ©_script, @@ -918,7 +918,7 @@ Full documentation is available at https://bun.com/docs/cli/run let path_z = ZStr::from_buf(&path_buf[..], entry_path.len()); let src = sys::File::read_from(Fd::cwd(), path_z)?; - crate::shell::Interpreter::init_and_run_from_file(ctx, mini, entry_path, &src) + crate::shell::Interpreter::init_and_run_from_file(&*ctx, mini, entry_path, &src) } /// `VirtualMachine::init`, diff --git a/src/runtime/dispatch.rs b/src/runtime/dispatch.rs index afae984518da..7dfb8d203424 100644 --- a/src/runtime/dispatch.rs +++ b/src/runtime/dispatch.rs @@ -78,7 +78,6 @@ use crate::shell::dispatch_tasks::{ShellCondExprStatTask, ShellGlobTask, ShellRm use crate::shell::interpreter::ShellTask; #[cfg(not(windows))] use crate::shell::io_writer::Poll as ShellBufferedWriterPoll; -use crate::shell::states::r#async::Async as ShellAsync; use crate::webcore::fetch::fetch_tasklet::FetchTasklet; use crate::webcore::file_sink::FlushPendingTask as FlushPendingFileSinkTask; @@ -509,47 +508,30 @@ fn run_task_cold(task: Task) { task.ptr.cast::<$ty>() }; } - /// Shell builtin tasks: route through `ShellTask::run_from_main_thread` - /// so the keep-alive ref taken in `ShellTask::schedule` is unref'd before - /// the per-builtin body runs. - /// The wrapper recovers `&mut Interpreter` from the embedded - /// `ShellTask.interp` back-ref. + /// Shell builtin tasks: `ShellTask::on_finish` posts a leaked `Box<$ty>`; + /// rebox it and route through `ShellTask::run_from_main_thread` so the + /// keep-alive ref taken in `ShellTask::schedule` is unref'd before the + /// per-builtin body runs. macro_rules! shell_dispatch { ($ty:ty) => {{ - // SAFETY: §Dispatch — `t` is a live heap-allocated shell task; - // `interp` was set at schedule time and outlives the task. - unsafe { ShellTask::run_from_main_thread::<$ty>(cast_ptr!($ty)) }; - }}; - // Cond-expr wraps an inner `task: ShellTask`-embedding struct one - // level deeper. The type *does* implement `ShellTaskCtx` - // (with a two-hop `TASK_OFFSET`, needed for `ShellTask::schedule`), - // so this arm is behaviorally identical to the plain arm; the unref + - // interp-recovery are inlined here only to keep the `.task.task` - // shape explicit at the dispatch site. - (nested $ty:ty) => {{ - let t = cast_ptr!($ty); - // SAFETY: see above; `task.task` is the embedded ShellTask. - unsafe { - let st = &raw mut (*t).task.task; - (*st).keep_alive.unref((*st).event_loop.as_event_loop_ctx()); - let interp = &*(*st).interp; - <$ty>::run_from_main_thread(t, interp); - } + // SAFETY: §Dispatch — tag identifies pointee: the `Box<$ty>` + // `ShellTask::on_finish` leaked when posting. + let t = unsafe { bun_core::heap::take(cast_ptr!($ty)) }; + ShellTask::run_from_main_thread::<$ty>(t); }}; } match task.tag { // ── shell interpreter ──────────────────────────────────────────── task_tag::ShellAsync => { - // SAFETY: §Dispatch — tag identifies pointee. - let t = unsafe { &mut *cast_ptr!(crate::shell::dispatch_tasks::ShellAsyncTask) }; - // SAFETY: `interp` set at enqueue; outlives task. - let interp = unsafe { &*t.interp }; - ShellAsync::run_from_main_thread(interp, t.node); - } - task_tag::ShellCondExprStatTask => { - shell_dispatch!(nested ShellCondExprStatTask); + // SAFETY: §Dispatch — tag identifies pointee: the box + // `Async::enqueue_self` leaked when posting. + let t = unsafe { + bun_core::heap::take(cast_ptr!(crate::shell::dispatch_tasks::ShellAsyncTask)) + }; + t.run(); } + task_tag::ShellCondExprStatTask => shell_dispatch!(ShellCondExprStatTask), task_tag::ShellCpTask => shell_dispatch!(ShellCpTask), task_tag::ShellTouchTask => shell_dispatch!(ShellTouchTask), task_tag::ShellMkdirTask => shell_dispatch!(ShellMkdirTask), diff --git a/src/runtime/node/node_fs.rs b/src/runtime/node/node_fs.rs index 828858d06994..36f8b2081040 100644 --- a/src/runtime/node/node_fs.rs +++ b/src/runtime/node/node_fs.rs @@ -1686,7 +1686,7 @@ mod _async_tasks { 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. + // outlives this task; `cp_on_finish` reclaims it. 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)) }; diff --git a/src/runtime/shell/Builtin.rs b/src/runtime/shell/Builtin.rs index 3504a9a802ea..7eb5eb0bbbf8 100644 --- a/src/runtime/shell/Builtin.rs +++ b/src/runtime/shell/Builtin.rs @@ -3,30 +3,30 @@ use bun_collections::VecExt; use bun_jsc::PinnedArrayBuffer; -use core::ffi::c_char; +use core::cell::{Ref, RefMut}; +use std::rc::Rc; use std::sync::Arc; use crate::shell::ExitCode; use crate::shell::ast; use crate::shell::interpreter::{ - Interpreter, NodeId, OutputNeedsIOSafeGuard, ParseError, is_pollable_from_mode, shell_openat, + CapturedBuf, Interpreter, NodeId, OutputNeedsIOSafeGuard, ParseError, is_pollable_from_mode, + shell_openat, }; use crate::shell::io::{InKind, OutFd, OutKind}; -use crate::shell::io_reader::IOReader; -use crate::shell::io_writer::{self, IOWriter}; +use crate::shell::io_reader::{IOReader, IOReaderRef}; +use crate::shell::io_writer::{self, IOWriter, IOWriterRef}; use crate::shell::states::cmd::{Cmd, CmdState}; use crate::shell::yield_::Yield; pub struct Builtin { pub(crate) kind: Kind, - /// argv[1..] as NUL-terminated strings (argv[0] is the builtin name). - /// Points into the Cmd's `args` storage. - pub args: Vec<*const c_char>, + /// argv[1..], each NUL-terminated (argv[0], the builtin name, stays on + /// the Cmd). Moved out of the Cmd's `args` by `init`. + pub args: Vec>, pub(crate) stdin: BuiltinInput, pub(crate) stdout: BuiltinIO, pub(crate) stderr: BuiltinIO, - /// Scratch for `fmt_error_arena`. One outstanding error string at a time. - pub(crate) err_buf: Vec, pub(crate) impl_: Impl, } @@ -59,10 +59,15 @@ pub(crate) trait BuiltinState: Sized { /// Project `&mut Impl` → `&mut Self`. `unreachable!` on variant mismatch. fn extract(impl_: &mut Impl) -> &mut Self; + /// Borrow this builtin's state out of the Cmd node. A `RefMut` into the + /// node: keep it short and never hold it across a call that touches the + /// same Cmd (`Builtin::of*`, `write_no_io`, `done`, ...). #[inline] #[track_caller] - fn state_mut(interp: &Interpreter, cmd: NodeId) -> &mut Self { - Self::extract(&mut Builtin::of_mut(interp, cmd).impl_) + fn state_mut(interp: &Interpreter, cmd: NodeId) -> RefMut<'_, Self> { + RefMut::map(Builtin::of_mut(interp, cmd), |b| { + Self::extract(&mut b.impl_) + }) } } @@ -256,7 +261,7 @@ pub enum BuiltinIO { /// Input stream of a builtin. pub enum BuiltinInput { - Fd(Arc), + Fd(IOReaderRef), ArrayBuf { buf: PinnedArrayBuffer, i: u32 }, Blob(Arc), Ignore, @@ -268,14 +273,14 @@ pub struct BuiltinBlob { pub(crate) blob: crate::webcore::Blob, } // `BuiltinBlob` is auto-`Send + Sync`: its sole field is `webcore::Blob`, -// which already asserts `Send + Sync`. No `unsafe impl` needed. +// which already asserts `Send + Sync`. const _: fn() = || { fn assert() {} assert::(); }; impl BuiltinIO { - /// From the Cmd's IO::OutKind. `Arc::clone` (via `OutFd: Clone`) bumps + /// From the Cmd's IO::OutKind. `Rc::clone` (via `OutFd: Clone`) bumps /// the `IOWriter` refcount; `Drop` decrements it symmetrically. `target` /// is the shell-env bytelist this stream flushes to (Stdout or Stderr). fn from_out_kind(ok: &OutKind, target: IoKind) -> BuiltinIO { @@ -314,15 +319,11 @@ impl BuiltinIO { /// Body of [`Builtin::write_no_io`] with the Cmd split-borrow already /// performed by the caller. Exists so builtins whose payload lives in /// `Builtin.impl_` (disjoint from `stdout`/`stderr`) can write a borrowed - /// slice without an intermediate heap clone. - /// - /// # Safety - /// `shell` must point to the live `ShellExecEnv` owning this builtin - /// (i.e. `cmd.base.shell`); only dereferenced for the [`BuiltinIO::Buf`] - /// arm. - pub(crate) unsafe fn write_no_io_to( + /// slice without an intermediate heap clone. `shell` is the Cmd's env + /// (`cmd.base.shell`); only used for the [`BuiltinIO::Buf`] arm. + pub(crate) fn write_no_io_to( &mut self, - shell: *mut crate::shell::interpreter::ShellExecEnv, + shell: &crate::shell::interpreter::ShellExecEnv, buf: &[u8], ) -> bun_sys::Result { if buf.is_empty() { @@ -337,16 +338,7 @@ impl BuiltinIO { // `target` is the destination identity, fixed at construction // and preserved across `dup_ref` so `2>&1` lands in stdout's // bytelist. - // SAFETY: caller contract — shell env outlives the Cmd node - // (single-threaded); `captured` points into a live - // `ShellExecEnv` Bufio. - unsafe { - let captured = match *target { - IoKind::Stdout => (*shell).buffered_stdout(), - IoKind::Stderr => (*shell).buffered_stderr(), - }; - (*captured).append_slice(buf) - }; + let _ = shell.captured(*target).borrow_mut().append_slice(buf); Ok(buf.len()) } BuiltinIO::ArrayBuf { buf: arraybuf, i } => { @@ -371,44 +363,81 @@ impl BuiltinIO { } } - /// Queue `buf` on this stream's IOWriter and arrange for `child`'s - /// `on_io_writer_chunk` to fire when the chunk completes. Delegates to - /// `fd.writer.enqueue` passing `fd.captured` as the tee bytelist. - /// - /// `_safeguard` proves the caller checked `needs_io()`. - pub(crate) fn enqueue( - &mut self, + /// The writer and tee buffer, for callers holding the + /// [`OutputNeedsIOSafeGuard`] that proves this is `Fd`. + fn out_fd(&self, _safeguard: OutputNeedsIOSafeGuard) -> (IOWriterRef, Option) { + match self { + BuiltinIO::Fd(fd) => (fd.writer.clone(), fd.captured.clone()), + _ => unreachable!("non-fd output; caller must check needs_io()"), + } + } +} + +impl Builtin { + fn out_fd( + interp: &Interpreter, + cmd: NodeId, + to: IoKind, + safeguard: OutputNeedsIOSafeGuard, + ) -> (IOWriterRef, Option) { + let me = Self::of(interp, cmd); + match to { + IoKind::Stdout => me.stdout.out_fd(safeguard), + IoKind::Stderr => me.stderr.out_fd(safeguard), + } + } + + /// Queue `buf` on `to`'s IOWriter; `child`'s `on_io_writer_chunk` fires + /// when the chunk completes. The Cmd node is not borrowed while the writer + /// runs (it may call other children back, or this one on a synchronous + /// failure). + pub(crate) fn write_out( + interp: &Interpreter, + cmd: NodeId, + to: IoKind, child: io_writer::ChildPtr, buf: &[u8], - _safeguard: OutputNeedsIOSafeGuard, + safeguard: OutputNeedsIOSafeGuard, ) -> Yield { - match self { - BuiltinIO::Fd(fd) => fd.writer.enqueue(child, fd.captured, buf), - _ => unreachable!("enqueue() on non-fd output; caller must check needs_io()"), - } + let (writer, captured) = Self::out_fd(interp, cmd, to, safeguard); + writer.enqueue(child, captured, buf) } - /// Format with the optional `"{kind}: "` prefix and enqueue on the - /// underlying IOWriter. - pub(crate) fn enqueue_fmt( - &mut self, + /// [`write_out`](Self::write_out) with the bytes appended by `fill` (which + /// may borrow the node; the borrow ends before the writer runs). + pub(crate) fn write_out_with( + interp: &Interpreter, + cmd: NodeId, + to: IoKind, + child: io_writer::ChildPtr, + safeguard: OutputNeedsIOSafeGuard, + fill: impl FnOnce(&mut Vec), + ) -> Yield { + let (writer, captured) = Self::out_fd(interp, cmd, to, safeguard); + writer.enqueue_with(child, captured, fill) + } + + /// [`write_out`](Self::write_out), formatted with the optional + /// `"{kind}: "` prefix. + pub(crate) fn write_out_fmt( + interp: &Interpreter, + cmd: NodeId, + to: IoKind, child: io_writer::ChildPtr, kind: Option, args: core::fmt::Arguments<'_>, - _safeguard: OutputNeedsIOSafeGuard, + safeguard: OutputNeedsIOSafeGuard, ) -> Yield { - match self { - BuiltinIO::Fd(fd) => fd.writer.enqueue_fmt_bltn(child, fd.captured, kind, args), - _ => unreachable!("enqueue_fmt() on non-fd output; caller must check needs_io()"), - } + let (writer, captured) = Self::out_fd(interp, cmd, to, safeguard); + writer.enqueue_fmt_bltn(child, captured, kind, args) } } impl BuiltinInput { fn from_in_kind(ik: &InKind) -> BuiltinInput { match ik { - // `Arc::clone` bumps the IOReader refcount. - InKind::Fd(r) => BuiltinInput::Fd(Arc::clone(r)), + // `Rc::clone` bumps the IOReader refcount. + InKind::Fd(r) => BuiltinInput::Fd(r.clone()), InKind::Ignore => BuiltinInput::Ignore, } } @@ -421,10 +450,30 @@ impl BuiltinInput { impl Builtin { #[inline] - pub(crate) fn args_slice(&self) -> &[*const c_char] { + pub(crate) fn args_slice(&self) -> &[Vec] { &self.args } + /// [`parse_flags`](crate::shell::interpreter::parse_flags) over this + /// builtin's argv. Returns the index of the first non-flag argument + /// (`None`: there were no arguments at all). + pub(crate) fn parse_flags( + interp: &Interpreter, + cmd: NodeId, + opts: &mut O, + ) -> Result, ParseError> { + let me = Self::of(interp, cmd); + let args = me.args_slice(); + crate::shell::interpreter::parse_flags(opts, args) + .map(|rest| rest.map(|rest| args.len() - rest.len())) + } + + /// `argv[1..].len()`. + #[inline] + pub(crate) fn argc(interp: &Interpreter, cmd: NodeId) -> usize { + Self::of(interp, cmd).args.len() + } + /// `PinnedArrayBuffer::drop`'s unpin would write to a `JSC::ArrayBuffer` /// impl the heap sweep already deleted; see /// `ShellSubprocess::defuse_array_buffer_unpins`. VM-shutdown finalizer @@ -442,40 +491,27 @@ impl Builtin { } } + /// `arg` (a NUL-terminated argv entry) up to its first NUL — what + /// `execve` would see. + #[inline] + pub(crate) fn arg_bytes_of(arg: &[u8]) -> &[u8] { + bun_core::slice_to_nul(arg) + } + /// Borrow `argv[1..][idx]` as `&[u8]` (NUL excluded). - /// - /// Every entry in `self.args` borrows into the owning `Cmd`'s - /// `args: Vec>`, NUL-terminated by `Cmd::transition_to_exec` and - /// outliving this `Builtin` (the `Cmd` slot is freed only after - /// `Builtin::done`). Localises the per-callsite - /// `unsafe { CStr::from_ptr(...) }` that previously appeared at every - /// builtin's flag/operand parser. - /// - /// The returned slice's lifetime is intentionally **decoupled from - /// `&self`**: the raw `*const c_char` is copied out of `self.args` first, - /// so the borrow of `self` ends before `CStr::from_ptr`. This lets callers - /// hold the result across an `interp.as_cmd_mut(...)` reborrow (cat/ls/mv - /// flag loops). Soundness rests on the architectural invariant above — - /// argv storage is a separate heap allocation that is not freed or - /// reallocated while the `Builtin` is live — not on `'a`. #[inline] - pub(crate) fn arg_bytes<'a>(&self, idx: usize) -> &'a [u8] { - let p: *const c_char = self.args[idx]; - // SAFETY: see doc comment — `p` is a valid NUL-terminated pointer - // into the Cmd's argv storage, live for the Builtin's lifetime. - unsafe { core::ffi::CStr::from_ptr(p) }.to_bytes() + pub(crate) fn arg_bytes(&self, idx: usize) -> &[u8] { + Self::arg_bytes_of(&self.args[idx]) } - /// Borrow `argv[1..][idx]` as `&ZStr` (NUL-terminated view). - /// - /// Same invariant and lifetime decoupling as [`arg_bytes`]; for callers - /// that need to pass the argument to a `&ZStr`-taking syscall wrapper - /// without re-copying. + /// Borrow `argv[1..][idx]` as `&ZStr` (NUL-terminated view), for callers + /// that pass the argument to a `&ZStr`-taking syscall wrapper without + /// re-copying. #[inline] - pub(crate) fn arg_zstr<'a>(&self, idx: usize) -> &'a bun_core::ZStr { - let p: *const c_char = self.args[idx]; - // SAFETY: see `arg_bytes` — valid NUL-terminated argv pointer. - bun_core::ZStr::from_cstr(unsafe { core::ffi::CStr::from_ptr(p) }) + pub(crate) fn arg_zstr(&self, idx: usize) -> &bun_core::ZStr { + let arg = &self.args[idx]; + debug_assert_eq!(arg.last(), Some(&0)); + bun_core::ZStr::from_buf(arg, arg.len() - 1) } /// Construct a `Builtin` for `kind`, install it into the owning Cmd's @@ -486,35 +522,28 @@ impl Builtin { pub(crate) fn init(interp: &Interpreter, cmd: NodeId, kind: Kind) -> Option { use crate::shell::states::cmd::Exec; - // Borrow argv[1..] as `*const c_char` into the Cmd's `args` storage. - // The Cmd's `args: Vec>` are NUL-terminated by - // `Cmd::transition_to_exec` before this is called. - let (args, stdin, stdout, stderr) = { - let me = interp.as_cmd(cmd); - let mut argv: Vec<*const c_char> = Vec::with_capacity(me.args.len().saturating_sub(1)); - for a in me.args.iter().skip(1) { - argv.push(a.as_ptr().cast::()); - } - // `Arc::clone` (inside `OutFd: Clone` / `InKind: Clone`) bumps + // Take argv[1..] from the Cmd's `args` (NUL-terminated by + // `Cmd::transition_to_exec` before this is called); argv[0] stays. + { + let mut me = interp.as_cmd_mut(cmd); + let args = me.args.split_off(1); + // `Rc::clone` (inside `OutFd: Clone` / `InKind: Clone`) bumps // the `IOWriter`/`IOReader` refcount; the builtin's `Drop` // decrements it symmetrically. No double-deref. - ( - argv, + let (stdin, stdout, stderr) = ( BuiltinInput::from_in_kind(&me.io.stdin), BuiltinIO::from_out_kind(&me.io.stdout, IoKind::Stdout), BuiltinIO::from_out_kind(&me.io.stderr, IoKind::Stderr), - ) - }; - - interp.as_cmd_mut(cmd).exec = Exec::Builtin(Box::new(Builtin { - kind, - args, - stdin, - stdout, - stderr, - err_buf: Vec::new(), - impl_: Self::make_impl(kind), - })); + ); + me.exec = Exec::Builtin(Box::new(Builtin { + kind, + args, + stdin, + stdout, + stderr, + impl_: Self::make_impl(kind), + })); + } Self::init_redirections(interp, cmd, kind) } @@ -523,7 +552,8 @@ impl Builtin { /// `2>&1` (`duplicate_out`). fn init_redirections(interp: &Interpreter, cmd: NodeId, kind: Kind) -> Option { // `node` points into the AST arena which outlives every state node (see Cmd::next). - let node: &ast::Cmd = &*interp.as_cmd(cmd).node; + let node = interp.as_cmd(cmd).node; + let node: &ast::Cmd = node.get(); let redirect = node.redirect; match &node.redirect_file { @@ -543,7 +573,8 @@ impl Builtin { // `&mut interp` open call below doesn't overlap a borrow into // the Cmd node. let path_buf: Vec = { - let raw = &interp.as_cmd(cmd).redirection_file; + let me = interp.as_cmd(cmd); + let raw = &me.redirection_file; let len = raw.len().saturating_sub(1); let mut v = raw[..len].to_vec(); v.push(0); @@ -634,10 +665,9 @@ impl Builtin { } }; - let interp_ptr: *mut Interpreter = interp.as_ctx_ptr(); if redirect.stdin() { let r = IOReader::init(redirfd, evtloop); - r.set_interp(interp_ptr); + r.set_interp(interp); Self::of_mut(interp, cmd).stdin = BuiltinInput::Fd(r); } @@ -658,17 +688,17 @@ impl Builtin { }, evtloop, ); - redirect_writer.set_interp(interp_ptr); + redirect_writer.set_interp(interp); if redirect.stdout() { - let me = Self::of_mut(interp, cmd); + let mut me = Self::of_mut(interp, cmd); me.stdout = BuiltinIO::Fd(OutFd { - writer: Arc::clone(&redirect_writer), + writer: redirect_writer.clone(), captured: None, }); } if redirect.stderr() { - let me = Self::of_mut(interp, cmd); + let mut me = Self::of_mut(interp, cmd); me.stderr = BuiltinIO::Fd(OutFd { writer: redirect_writer, captured: None, @@ -678,8 +708,6 @@ impl Builtin { Some(ast::Redirect::JsBuf(jsbuf)) => { // ── JS object redirect (`> ${arraybuf}` / `> ${blob}`). let idx = jsbuf.idx as usize; - // Safe accessor — single `unsafe` deref lives in - // `Interpreter::global_this_ref`. let Some(global) = interp .global_this_ref() .filter(|_| idx < interp.jsobjs.len()) @@ -702,7 +730,7 @@ impl Builtin { } buf }; - let me = Self::of_mut(interp, cmd); + let mut me = Self::of_mut(interp, cmd); if redirect.stdin() { let Some(buf) = root() else { return Some(Yield::failed()); @@ -721,21 +749,22 @@ impl Builtin { }; me.stderr = BuiltinIO::ArrayBuf { buf, i: 0 }; } - } else if let Some(body) = - crate::webcore::body::Value::from_request_or_response(jsval) + } else if let Some(taken) = + crate::webcore::body::Value::with_request_or_response(jsval, |body| { + let is_file_blob = matches!(body, crate::webcore::body::Value::Blob(b) + if !b.needs_to_read_file()); + if (redirect.stdout() || redirect.stderr()) && !is_file_blob { + return None; + } + Some(body.use_()) + }) { - // SAFETY: returned a live JSC-owned `*mut Value` borrowed - // from a Response/Request wrapper. - let body = unsafe { &mut *body }; - let is_file_blob = matches!(body, crate::webcore::body::Value::Blob(b) - if !b.needs_to_read_file()); - if (redirect.stdout() || redirect.stderr()) && !is_file_blob { + let Some(original_blob) = taken else { let _ = global.throw(format_args!( "Cannot redirect stdout/stderr to an immutable blob. Expected a file" )); return Some(Yield::failed()); - } - let original_blob = body.use_(); + }; if !redirect.stdin() && !redirect.stdout() && !redirect.stderr() { drop(original_blob); return None; @@ -744,7 +773,7 @@ impl Builtin { blob: original_blob.dupe(), }); drop(original_blob); - let me = Self::of_mut(interp, cmd); + let mut me = Self::of_mut(interp, cmd); if redirect.stdin() { me.stdin = BuiltinInput::Blob(Arc::clone(&blob)); } @@ -764,7 +793,7 @@ impl Builtin { let theblob = Arc::new(BuiltinBlob { blob: blob_ref.dupe(), }); - let me = Self::of_mut(interp, cmd); + let mut me = Self::of_mut(interp, cmd); if redirect.stdin() { me.stdin = BuiltinInput::Blob(theblob); } else if redirect.stdout() { @@ -783,7 +812,8 @@ impl Builtin { None if redirect.duplicate_out() => { // `2>&1` (stderr=true,dup_out=true) → stderr := stdout // `1>&2` (stdout=true,dup_out=true) → stdout := stderr - let me = Self::of_mut(interp, cmd); + let mut me = Self::of_mut(interp, cmd); + let me = &mut *me; if redirect.stdout() { me.stderr = me.stdout.dup_ref(); } @@ -810,29 +840,23 @@ impl Builtin { use std::io::Write as _; let mut buf = Vec::new(); let _ = buf.write_fmt(args); - if let Some(_safeguard) = interp.as_cmd(cmd).io.stderr.needs_io() { + let stderr = interp.as_cmd(cmd).io.stderr.clone(); + if let OutKind::Fd(fd) = stderr { // Only the `Fd` arm transitions state. interp.as_cmd_mut(cmd).state = CmdState::WaitingWriteErr; let child = io_writer::ChildPtr::new(cmd, io_writer::WriterTag::Cmd); - // SAFETY: `OutKind::Fd` guaranteed by `needs_io()`. - if let OutKind::Fd(fd) = &interp.as_cmd(cmd).io.stderr { - return fd.writer.enqueue(child, fd.captured, &buf); - } - unreachable!() + return fd.writer.enqueue(child, fd.captured, &buf); } // No-IO path: append to the shell env's captured stderr and finish // synchronously with exit 1 (Cmd::on_io_writer_chunk's behaviour). - if let OutKind::Pipe = &interp.as_cmd(cmd).io.stderr { - // SAFETY: single trampoline frame; no other borrow of the env's - // (or its parent's) stderr buffer is live. - let stderr = unsafe { - interp - .as_cmd_mut(cmd) - .base - .shell_mut() - .buffered_stderr_mut() - }; - stderr.append_slice(&buf); + if let OutKind::Pipe = stderr { + let _ = interp + .as_cmd(cmd) + .base + .shell() + .buffered_stderr + .borrow_mut() + .append_slice(&buf); } let parent = interp.as_cmd(cmd).base.parent; interp.child_done(parent, cmd, 1) @@ -845,23 +869,25 @@ impl Builtin { Cmd::on_exec_done(interp, cmd, exit_code) } - /// Look up the Builtin inside a Cmd's `exec` slot. + /// Look up the Builtin inside a Cmd's `exec` slot. A `Ref` into the Cmd + /// node: it must be released before anything borrows that node mutably. #[inline] #[track_caller] - pub(crate) fn of<'a>(interp: &'a Interpreter, cmd: NodeId) -> &'a Builtin { - match &interp.as_cmd(cmd).exec { - crate::shell::states::cmd::Exec::Builtin(b) => b, + pub(crate) fn of(interp: &Interpreter, cmd: NodeId) -> Ref<'_, Builtin> { + Ref::map(interp.as_cmd(cmd), |c| match &c.exec { + crate::shell::states::cmd::Exec::Builtin(b) => &**b, _ => panic!("Cmd {} is not running a builtin", cmd), - } + }) } + /// [`of`](Self::of), mutably. #[inline] #[track_caller] - pub(crate) fn of_mut<'a>(interp: &'a Interpreter, cmd: NodeId) -> &'a mut Builtin { - match &mut interp.as_cmd_mut(cmd).exec { - crate::shell::states::cmd::Exec::Builtin(b) => b, + pub(crate) fn of_mut(interp: &Interpreter, cmd: NodeId) -> RefMut<'_, Builtin> { + RefMut::map(interp.as_cmd_mut(cmd), |c| match &mut c.exec { + crate::shell::states::cmd::Exec::Builtin(b) => &mut **b, _ => panic!("Cmd {} is not running a builtin", cmd), - } + }) } #[inline] @@ -871,8 +897,8 @@ impl Builtin { /// Returns the bytes available on stdin when it is *not* an async fd /// (arraybuf / piped buf / blob). - pub(crate) fn read_stdin_no_io<'a>(interp: &'a Interpreter, cmd: NodeId) -> &'a [u8] { - match &Self::of(interp, cmd).stdin { + pub(crate) fn read_stdin_no_io(&self) -> &[u8] { + match &self.stdin { BuiltinInput::ArrayBuf { buf, .. } => buf.slice(), BuiltinInput::Blob(b) => b.blob.shared_view(), BuiltinInput::Fd(_) | BuiltinInput::Ignore => b"", @@ -895,8 +921,9 @@ impl Builtin { } // Split-borrow the Cmd so `shell` // and the builtin's stdout/stderr are accessible simultaneously. - let cmd_node = interp.as_cmd_mut(cmd); - let shell = cmd_node.base.shell; + let mut cmd_node = interp.as_cmd_mut(cmd); + let cmd_node = &mut *cmd_node; + let shell = cmd_node.base.shell.borrow(); let crate::shell::states::cmd::Exec::Builtin(me) = &mut cmd_node.exec else { panic!("Cmd {} is not running a builtin", cmd); }; @@ -904,17 +931,14 @@ impl Builtin { IoKind::Stdout => &mut me.stdout, IoKind::Stderr => &mut me.stderr, }; - // SAFETY: `shell` is `cmd_node.base.shell`, live for the Cmd's lifetime. - unsafe { out.write_no_io_to(shell, buf) } + out.write_no_io_to(&shell, buf) } - /// Shell exec env of the owning Cmd. + /// Shell exec env of the owning Cmd (a clone of the handle, so the Cmd + /// node is not borrowed while it is used). #[inline] - pub fn shell<'a>( - interp: &'a Interpreter, - cmd: NodeId, - ) -> &'a crate::shell::interpreter::ShellExecEnv { - interp.as_cmd(cmd).base.shell() + pub fn shell(interp: &Interpreter, cmd: NodeId) -> crate::shell::interpreter::EnvRc { + Rc::clone(&interp.as_cmd(cmd).base.shell) } /// Event loop handle (forwarded from the interpreter). @@ -929,53 +953,34 @@ impl Builtin { /// Cwd fd of the owning Cmd's shell env. #[inline] pub(crate) fn cwd(interp: &Interpreter, cmd: NodeId) -> bun_sys::Fd { - Self::shell(interp, cmd).cwd_fd + interp.as_cmd(cmd).base.shell().cwd_fd } /// Format `"{kind}: {fmt}"` into a fresh heap buffer. - /// - /// Stored on the `Builtin` so the returned `&[u8]` borrow stays valid - /// across the immediate `write_no_io` / `enqueue` call. - pub(crate) fn fmt_error_arena<'a>( - interp: &'a Interpreter, - cmd: NodeId, - kind: Option, - args: core::fmt::Arguments<'_>, - ) -> &'a [u8] { + pub(crate) fn fmt_error_arena(kind: Option, args: core::fmt::Arguments<'_>) -> Vec { use std::io::Write as _; let mut buf = Vec::new(); if let Some(k) = kind { let _ = write!(&mut buf, "{}: ", k.as_str()); } let _ = buf.write_fmt(args); - let me = Self::of_mut(interp, cmd); - me.err_buf = buf; - &me.err_buf + buf } /// Error messages formatted to match bash. Dispatches on the variant; /// `Sys` recurses into the system-error formatter. - pub(crate) fn shell_err_to_string<'a>( - interp: &'a Interpreter, - cmd: NodeId, - kind: Kind, - err: &crate::shell::ShellErr, - ) -> &'a [u8] { + pub(crate) fn shell_err_to_string(kind: Kind, err: &crate::shell::ShellErr) -> Vec { use crate::shell::ShellErr; match err { ShellErr::Sys(sys) => { // `"{message}\n"` or `"{message}: {path}\n"`. if sys.path.is_empty() { Self::fmt_error_arena( - interp, - cmd, Some(kind), format_args!("{}\n", bstr::BStr::new(sys.message.byte_slice())), ) } else { Self::fmt_error_arena( - interp, - cmd, Some(kind), format_args!( "{}: {}\n", @@ -985,12 +990,9 @@ impl Builtin { ) } } - ShellErr::Custom(s) => Self::fmt_error_arena( - interp, - cmd, - Some(kind), - format_args!("{}\n", bstr::BStr::new(s)), - ), + ShellErr::Custom(s) => { + Self::fmt_error_arena(Some(kind), format_args!("{}\n", bstr::BStr::new(s))) + } } } @@ -998,36 +1000,19 @@ impl Builtin { /// `bun_sys::coreutils_error_map` so output matches GNU coreutils /// (e.g. `ENOENT` → "No such file or directory"); falls back to /// `"unknown error {errno}"` when unmapped. - pub(crate) fn task_error_to_string<'a>( - interp: &'a Interpreter, - cmd: NodeId, - kind: Kind, - err: &bun_sys::Error, - ) -> &'a [u8] { + pub(crate) fn task_error_to_string(kind: Kind, err: &bun_sys::Error) -> Vec { if let Some((_code, sys_errno)) = err.get_error_code_tag_name() { if let Some(message) = bun_sys::coreutils_error_map::get(sys_errno) { if !err.path.is_empty() { return Self::fmt_error_arena( - interp, - cmd, Some(kind), format_args!("{}: {}\n", bstr::BStr::new(&err.path[..]), message), ); } - return Self::fmt_error_arena( - interp, - cmd, - Some(kind), - format_args!("{}\n", message), - ); + return Self::fmt_error_arena(Some(kind), format_args!("{}\n", message)); } } - Self::fmt_error_arena( - interp, - cmd, - Some(kind), - format_args!("unknown error {}\n", err.errno), - ) + Self::fmt_error_arena(Some(kind), format_args!("unknown error {}\n", err.errno)) } /// Shared failure path for builtins whose option parser returns @@ -1044,23 +1029,17 @@ impl Builtin { ) -> Yield { let buf: Vec = match e { ParseError::IllegalOption(_) => Self::fmt_error_arena( - interp, - cmd, Some(kind), format_args!("illegal option -- {}\n", bstr::BStr::new(e.opt())), - ) - .to_vec(), + ), ParseError::ShowUsage => kind.usage_string().to_vec(), ParseError::Unsupported(_) => Self::fmt_error_arena( - interp, - cmd, Some(kind), format_args!( "unsupported option, please open a GitHub issue -- {}\n", bstr::BStr::new(e.opt()) ), - ) - .to_vec(), + ), }; set_wait_err(); Self::write_failing_error(interp, cmd, &buf, 1) @@ -1075,14 +1054,10 @@ impl Builtin { buf: &[u8], exit_code: crate::shell::ExitCode, ) -> Yield { - if let Some(safeguard) = Self::of(interp, cmd).stderr.needs_io() { + let needs_io = Self::of(interp, cmd).stderr.needs_io(); + if let Some(safeguard) = needs_io { let child = io_writer::ChildPtr::new(cmd, io_writer::WriterTag::Builtin); - // Clone buf so the &mut on - // `stderr` doesn't overlap a borrow into `err_buf`. - let owned = buf.to_vec(); - return Self::of_mut(interp, cmd) - .stderr - .enqueue(child, &owned, safeguard); + return Self::write_out(interp, cmd, IoKind::Stderr, child, buf, safeguard); } let _ = Self::write_no_io(interp, cmd, IoKind::Stderr, buf); Self::done(interp, cmd, exit_code) @@ -1090,7 +1065,7 @@ impl Builtin { } // Cleanup: every `Impl` variant owns its state via `Box`/`Vec`/`Arc`, and -// `BuiltinIO`/`BuiltinInput` hold `Arc` / `Arc` / +// `BuiltinIO`/`BuiltinInput` hold `Rc` / `Rc` / // `PinnedArrayBuffer` / `Arc` whose `Drop` already decrements // the refcount. So cleanup is fully covered by `Drop` on `Box` // (called from `Cmd::deinit`). No explicit deinit needed. diff --git a/src/runtime/shell/IO.rs b/src/runtime/shell/IO.rs index f5254fde114a..58c7b08faeb2 100644 --- a/src/runtime/shell/IO.rs +++ b/src/runtime/shell/IO.rs @@ -1,13 +1,13 @@ //! Carries stdin/stdout/stderr for a state node. //! -//! `IO` is a plain `Clone` value; `IOReader`/`IOWriter` are `Arc`-refcounted. +//! `IO` is a plain `Clone` value; `IOReader`/`IOWriter` are `Rc`-refcounted. use bun_collections::VecExt; -use crate::api::bun_spawn::stdio::{Capture, Stdio}; -use crate::shell::interpreter::OutputNeedsIOSafeGuard; -use crate::shell::io_reader::IOReader; -use crate::shell::io_writer::IOWriter; +use crate::api::bun_spawn::stdio::Stdio; +use crate::shell::interpreter::{CapturedBuf, OutputNeedsIOSafeGuard}; +use crate::shell::io_reader::IOReaderRef; +use crate::shell::io_writer::IOWriterRef; use crate::shell::shell_body::subproc::ShellIO; #[derive(Clone, Default)] @@ -29,7 +29,7 @@ impl IO { /// Maps the state-node IO triple onto /// `subproc::Stdio` for [`ShellSubprocess::spawn_async`], and stashes the - /// owning `IOWriter` Arcs on `shellio` so [`PipeReader`]'s captured-writer + /// owning `IOWriter` `Rc`s on `shellio` so [`PipeReader`]'s captured-writer /// path can tee subprocess output back into the JS-side buffers. pub(crate) fn to_subproc_stdio(&self, stdio: &mut [Stdio; 3], shellio: &mut ShellIO) { stdio[0] = self.stdin.to_subproc_stdio(); @@ -40,7 +40,7 @@ impl IO { #[derive(Clone, Default)] pub enum InKind { - Fd(std::sync::Arc), + Fd(IOReaderRef), #[default] Ignore, } @@ -55,36 +55,12 @@ pub enum OutKind { Ignore, } -// Clone: bitwise OK for `captured` — it is a non-owning backref into -// `ShellExecEnv::_buffered_{stdout,stderr}`; the env owns the Vec. `writer` -// is `Arc` so it ref-counts on clone. #[derive(Clone)] pub struct OutFd { - pub(crate) writer: std::sync::Arc, + pub(crate) writer: IOWriterRef, /// If set, also append every chunk to this buffer (the JS-side captured - /// stdout/stderr). Points into `ShellExecEnv::_buffered_{stdout,stderr}`. - pub(crate) captured: Option<*mut Vec>, -} - -impl OutFd { - /// Mutably borrow the JS-side captured stdout/stderr buffer if configured. - /// - /// `captured` is a non-owning backref into `ShellExecEnv::_buffered_*` - /// (see field doc); the owning `ShellExecEnv` outlives every `Cmd`/builtin - /// that holds an `OutFd`. Localises the per-callsite raw deref so callers - /// can `if let Some(buf) = fd.captured_mut() { buf.extend_from_slice(...) }`. - /// - /// # Safety - /// Caller must ensure no other `&`/`&mut` to the target `Vec` is live - /// (including via the parent `ShellExecEnv`) for the returned borrow's - /// lifetime. The `(&self) -> &mut T` shape cannot encode this, hence - /// `unsafe fn`. - #[inline] - #[allow(clippy::mut_from_ref)] - pub(crate) unsafe fn captured_mut(&self) -> Option<&mut Vec> { - // SAFETY: caller contract — single-threaded shell, env outlives `self`. - self.captured.map(|p| unsafe { &mut *p }) - } + /// stdout/stderr, shared with `ShellExecEnv::buffered_{stdout,stderr}`). + pub(crate) captured: Option, } impl InKind { @@ -106,10 +82,8 @@ impl InKind { impl OutFd { fn memory_cost(&self) -> usize { let mut cost = self.writer.memory_cost(); - if let Some(captured) = self.captured { - // SAFETY: `captured` points into a live `ShellExecEnv` buffer; - // the env outlives the IO that borrows it. - cost += unsafe { (*captured).memory_cost() }; + if let Some(captured) = &self.captured { + cost += captured.borrow().memory_cost(); } cost } @@ -133,20 +107,15 @@ impl OutKind { } } - /// Retains the `IOWriter` Arc on + /// Retains the `IOWriter` on /// `shellio` so the subprocess's `PipeReader::captured_writer` can drain /// captured bytes into it after the spawn returns. - fn to_subproc_stdio(&self, shellio: &mut Option>) -> Stdio { + fn to_subproc_stdio(&self, shellio: &mut Option) -> Stdio { match self { OutKind::Fd(val) => { - *shellio = Some(std::sync::Arc::clone(&val.writer)); - if let Some(cap) = val.captured { - #[cfg(not(any(target_os = "linux", target_os = "android")))] - let _ = cap; - Stdio::Capture(Capture { - #[cfg(any(target_os = "linux", target_os = "android"))] - buf: cap, - }) + *shellio = Some(val.writer.clone()); + if val.captured.is_some() { + Stdio::Capture } else { // `IOWriter::fd()` (IOWriter.rs) returns `Fd::INVALID` // once the fd has been handed off to libuv, so the diff --git a/src/runtime/shell/IOReader.rs b/src/runtime/shell/IOReader.rs index 0484e6ed5c2b..e207a3f37a83 100644 --- a/src/runtime/shell/IOReader.rs +++ b/src/runtime/shell/IOReader.rs @@ -1,8 +1,10 @@ //! Similar to `IOWriter` but for reading. //! -//! *NOTE* This type is reference counted via `Arc`; see the `Drop` impl note. +//! *NOTE* This type is reference counted via `Rc`; see the `Drop` impl note. -use core::cell::UnsafeCell; +use bun_jsc::JsCell; +use bun_ptr::{CellRefCounted as _, RefPtr, ThisPtr}; +use core::cell::{Cell, RefCell}; #[cfg(not(windows))] use core::ffi::c_void; @@ -39,87 +41,39 @@ type Readers = Vec; pub(crate) type ReaderImpl = bun_io::BufferedReader; -struct State { +/// Read callbacks re-enter the reader (`add_reader`/`remove_reader` from a +/// child's chunk handler), so every field is interior-mutable behind `&self` +/// and no borrow is held across a callback. +#[derive(bun_ptr::CellRefCounted)] +pub struct IOReader { + ref_count: Cell, + self_root: bun_ptr::SelfRoot, + /// The io-layer reader. Its callbacks arrive while the read loop holds a + /// `&mut` to it (see the `BufferedReaderParent` aliasing contract), so it + /// sits in its own cell that the callbacks never touch. + reader: JsCell, fd: Fd, - buf: Vec, - readers: Readers, + buf: RefCell>, + readers: RefCell, /// The raw `sys::Error`. `SystemError` is not `Clone` /// in the Rust port yet, so we keep the source error to re-derive a fresh /// `SystemError` per callee in `on_reader_done_cb`. - raw_err: Option, + raw_err: RefCell>, evtloop: EventLoopHandle, #[cfg(windows)] - is_reading: bool, - /// Weak self-ref so `keepalive()` can bump the strong count from `&self` - /// without unsafe Arc-pointer reconstruction. Set via `Arc::new_cyclic` in - /// `init()` (the sole constructor). - self_weak: std::sync::Weak, - read_guards: Vec>, + is_reading: Cell, /// Backref so async read callbacks can drive `Yield::run`. See /// `IOWriter::interp`. - interp: Option>, -} - -pub struct IOReader { - /// Split out of `State` so `state()`'s `&mut State` never overlaps the - /// `&mut ReaderImpl` the read-loop caller holds while invoking vtable - /// callbacks (see `BufferedReaderParent` aliasing contract). Both cells - /// root at SharedReadWrite; callbacks touch only `state` fields. - reader: UnsafeCell, - state: UnsafeCell, + interp: Cell>>, } -// SAFETY: shell is single-threaded; `Arc` is used purely for refcounting. -unsafe impl Send for IOReader {} -// SAFETY: shell is single-threaded; `Arc` is used purely for refcounting. -unsafe impl Sync for IOReader {} - impl IOReader { #[inline] - #[allow(clippy::mut_from_ref)] // interior mutability via UnsafeCell; single-threaded - fn state(&self) -> &mut State { - // SAFETY: shell is single-threaded; no overlapping borrow of `state` - // escapes a callback (see struct doc comment). - unsafe { &mut *self.state.get() } + fn this_ptr(&self) -> ThisPtr { + self.self_root.this_ptr(self) } - #[inline] - #[allow(clippy::mut_from_ref)] // interior mutability via UnsafeCell; single-threaded - fn reader(&self) -> &mut ReaderImpl { - // SAFETY: single-threaded. Split into its own cell so a `&mut ReaderImpl` - // held by the bun_io read loop never overlaps a `&mut State` derived in a - // vtable callback (see struct doc comment). - // - // MUST NOT be invoked from within a `BufferedReaderParent` vtable - // callback (`on_read_chunk_cb`/`on_reader_done_cb`/`on_reader_error`): - // the read loop already holds a live `&mut ReaderImpl` on its stack - // while the callback runs (PipeReader.rs aliasing contract), so - // re-deriving here would create two simultaneous `&mut` to the same - // BufferedReader = Stacked-Borrows UB. - unsafe { &mut *self.reader.get() } - } - - /// Bump our own Arc strong count. Held across re-entrant `run_yield` calls - /// whose child callback may drop the last external ref and free us - /// mid-method. - #[inline] - fn keepalive(&self) -> std::sync::Arc { - self.state() - .self_weak - .upgrade() - .expect("IOReader::keepalive after last Arc dropped") - } - - fn push_read_guard(&self) { - let guard = self.keepalive(); - self.state().read_guards.push(guard); - } - - fn pop_read_guard(&self) -> Option> { - self.state().read_guards.pop() - } - - pub(crate) fn init(fd: Fd, evtloop: EventLoopHandle) -> std::sync::Arc { + pub(crate) fn init(fd: Fd, evtloop: EventLoopHandle) -> IOReaderRef { let mut reader = ReaderImpl::init::(); #[cfg(not(windows))] { @@ -131,56 +85,42 @@ impl IOReader { { reader.set_source(bun_io::Source::File(bun_io::Source::open_file(fd))); } - let this = std::sync::Arc::new_cyclic(|w| IOReader { - reader: UnsafeCell::new(reader), - state: UnsafeCell::new(State { - fd, - buf: Vec::new(), - readers: Readers::new(), - raw_err: None, - evtloop, - #[cfg(windows)] - is_reading: false, - self_weak: std::sync::Weak::clone(w), - read_guards: Vec::new(), - interp: None, - }), + let this = RefPtr::new_cyclic(|self_root| IOReader { + ref_count: Cell::new(1), + self_root, + reader: JsCell::new(reader), + fd, + buf: RefCell::new(Vec::new()), + readers: RefCell::new(Readers::new()), + raw_err: RefCell::new(None), + evtloop, + #[cfg(windows)] + is_reading: Cell::new(false), + interp: Cell::new(None), }); - // NOTE: set the parent backref after Arc allocation so the - // address is stable. - let parent: *const IOReader = std::sync::Arc::as_ptr(&this); - // SAFETY: `Arc::as_ptr` yields `*const IOReader`, but every field of - // `IOReader` is `UnsafeCell`, so all mutation flows through interior - // mutability (SharedReadWrite). The `*mut` cast exists solely to satisfy - // `set_parent`'s `*mut` signature for the vtable backref; the - // `BufferedReaderParent` callbacks only ever reborrow it as `&Self` to - // call `&self` methods — no `&mut IOReader` is materialized from it. - unsafe { (*this.reader.get()).set_parent(parent.cast_mut().cast()) }; + let parent: *mut IOReader = this.as_ptr(); + this.reader.with_mut(|r| r.set_parent(parent.cast())); crate::shell_log!("IOReader(0x{:x}, fd={}) create", parent as usize, fd); - this + IOReaderRef(this) } - /// # Safety - /// `interp` must be null or point to the live owning `Interpreter` (it - /// owns the IO struct that holds this `Arc`) for the lifetime of this - /// reader; single-threaded. + /// Stash the interpreter backref so async read callbacks can drive + /// `Yield::run`. `interp` owns (through its IO structs) every handle to + /// this reader. #[inline] - #[allow(clippy::not_unsafe_ptr_arg_deref)] - pub(crate) fn set_interp(&self, interp: *mut Interpreter) { - // SAFETY: precondition above. - self.state().interp = unsafe { bun_ptr::ParentRef::from_nullable(interp) }; + pub(crate) fn set_interp(&self, interp: &Interpreter) { + self.interp.set(Some(bun_ptr::ParentRef::new(interp))); } #[inline] pub(crate) fn fd(&self) -> Fd { - self.state().fd + self.fd } pub(crate) fn memory_cost(&self) -> usize { - let s = self.state(); core::mem::size_of::() - + s.buf.capacity() - + s.readers.capacity() * core::mem::size_of::() + + self.buf.borrow().capacity() + + self.readers.borrow().capacity() * core::mem::size_of::() } /// `bun_io::EventLoopHandle` is an opaque `*mut c_void` that the io-layer @@ -189,9 +129,7 @@ impl IOReader { /// vtable can recover it. #[inline] fn io_evtloop(&self) -> bun_io::EventLoopHandle { - // SAFETY: `bun_io::EventLoopHandle` stores `*mut c_void` purely for - // type-erasure; vtable consumers treat the pointee as read-only - self.state().evtloop.as_event_loop_ctx() + self.evtloop.as_event_loop_ctx() } /// Only does things on windows. @@ -199,37 +137,43 @@ impl IOReader { fn set_reading(&self, reading: bool) { #[cfg(windows)] { - self.state().is_reading = reading; + self.is_reading.set(reading); } let _ = reading; } /// Idempotent function to start the reading. + /// + /// Not called from within a `BufferedReaderParent` callback: the read + /// loop already holds the reader mutably there. pub(crate) fn start(&self) -> Yield { #[cfg(not(windows))] { - let r = self.reader(); - let need_start = match &r.handle { - bun_io::pipes::PollOrFd::Closed => true, - bun_io::pipes::PollOrFd::Poll(p) => !p.is_registered(), - bun_io::pipes::PollOrFd::Fd(_) => true, - }; - if need_start { - let fd = self.state().fd; - if let Err(e) = r.start(fd, true) { - self.on_reader_error(&e); + let fd = self.fd; + let res = self.reader.with_mut(|r| { + let need_start = match &r.handle { + bun_io::pipes::PollOrFd::Closed => true, + bun_io::pipes::PollOrFd::Poll(p) => !p.is_registered(), + bun_io::pipes::PollOrFd::Fd(_) => true, + }; + if need_start { + r.start(fd, true) + } else { + Ok(()) } + }); + if let Err(e) = res { + self.on_reader_error(&e); } return Yield::suspended(); } #[cfg(windows)] { - let s = self.state(); - if s.is_reading { + if self.is_reading.get() { return Yield::suspended(); } - s.is_reading = true; - if let Err(e) = self.reader().start_with_current_pipe() { + self.is_reading.set(true); + if let Err(e) = self.reader.with_mut(|r| r.start_with_current_pipe()) { self.on_reader_error(&e); return Yield::failed(); } @@ -239,54 +183,52 @@ impl IOReader { /// Only adds if not already present. pub(crate) fn add_reader(&self, reader: ChildPtr) { - let s = self.state(); - if !s.readers.contains(&reader) { - s.readers.push(reader); + let mut readers = self.readers.borrow_mut(); + if !readers.contains(&reader) { + readers.push(reader); } } /// Unregister a listener; no-op if it was never added. pub(crate) fn remove_reader(&self, reader: ChildPtr) { - let s = self.state(); - if let Some(idx) = s.readers.iter().position(|r| *r == reader) { - s.readers.swap_remove(idx); + let mut readers = self.readers.borrow_mut(); + if let Some(idx) = readers.iter().position(|r| *r == reader) { + readers.swap_remove(idx); } } /// The `BufferedReader.onReadChunk` hook. fn on_read_chunk_cb(&self, chunk: &[u8], has_more: bun_io::ReadState) -> bool { // `dispatch_read_chunk` → `Cat::on_io_reader_chunk` may drop the last - // external Arc; hold one across the whole body so the trailing - // `state()` accesses (and `run_yield`'s re-read of `interp`) see live - // memory. - let _keepalive = self.keepalive(); + // external ref; hold one across the whole body so the trailing field + // accesses (and `run_yield`'s re-read of `interp`) see live memory. + let _keepalive = self.this_ptr().ref_guard(); self.set_reading(false); - // NOTE: reshaped for borrowck — `dispatch_read_chunk`/`run_yield` - // both re-derive `state()` (and the interpreter callback may re-enter - // `add_reader`/`remove_reader`), so we must NOT hold a long-lived - // `&mut State` across the dispatch. Re-derive `state()` per access - // instead. + // The interpreter callback may re-enter `add_reader`/`remove_reader`, + // so `readers` is re-borrowed per access rather than held across the + // dispatch. let mut i = 0usize; - while i < self.state().readers.len() { - let r = self.state().readers[i]; - let interp = self.state().interp; + loop { + let Some(r) = self.readers.borrow().get(i).copied() else { + break; + }; + let interp = self.interp.get(); let mut remove = false; self.run_yield(dispatch_read_chunk(r, chunk, &mut remove, interp)); if remove { - self.state().readers.swap_remove(i); + self.readers.borrow_mut().swap_remove(i); } else { i += 1; } } let should_continue = has_more != bun_io::ReadState::Eof; - if should_continue && !self.state().readers.is_empty() { + if should_continue && !self.readers.borrow().is_empty() { self.set_reading(true); // NOTE: no explicit re-arm (`registerPoll()` on posix / - // `startWithCurrentPipe()` on windows) here: that would re-derive - // a second `&mut ReaderImpl` while the bun_io read loop still - // holds one on its stack (PipeReader.rs aliasing contract) — - // Stacked-Borrows UB. + // `startWithCurrentPipe()` on windows) here: that would touch the + // reader while the bun_io read loop still holds it mutably on its + // stack (PipeReader.rs aliasing contract). // On posix the re-arm is redundant: the read loop re-registers // itself after the callback returns based on the `bool` we return // (PipeReader.rs:731/755/846/920/986). On Windows the re-arm is @@ -302,15 +244,14 @@ impl IOReader { } fn on_reader_error(&self, err: &sys::Error) { - // `dispatch_reader_done` may drop the last external Arc; keep `self` + // `dispatch_reader_done` may drop the last external ref; keep `self` // alive across the loop. - let _keepalive = self.keepalive(); + let _keepalive = self.this_ptr().ref_guard(); self.set_reading(false); - let s = self.state(); - s.raw_err = Some(err.clone()); - // NOTE: reshaped for borrowck — copy out before dispatching. - let readers: Vec = s.readers.clone(); - let interp = s.interp; + *self.raw_err.borrow_mut() = Some(err.clone()); + // Copy out before dispatching (callbacks may re-enter `remove_reader`). + let readers: Vec = self.readers.borrow().clone(); + let interp = self.interp.get(); for r in readers { // Re-derive a fresh SystemError per callee (see // IOWriter.on_error note). @@ -321,18 +262,16 @@ impl IOReader { fn on_reader_done_cb(&self) { // `dispatch_reader_done` → `Cat::on_io_reader_done` drops Cat's - // `Arc`; if that was the last external ref, `self` is freed - // mid-loop and `run_yield`'s `state().interp` reads 0xdfdf poison. - // Hold a strong ref across the body. - let _keepalive = self.keepalive(); + // `Rc`; if that was the last external ref, `self` would be + // freed mid-loop. Hold a strong ref across the body. + let _keepalive = self.this_ptr().ref_guard(); self.set_reading(false); - let s = self.state(); - let readers: Vec = s.readers.clone(); - let interp = s.interp; + let readers: Vec = self.readers.borrow().clone(); + let interp = self.interp.get(); // `SystemError` isn't `Clone` yet, so we keep the source `sys::Error` // (which IS `Clone`) and re-derive a fresh `SystemError` per callee — // same approach as `on_reader_error`. - let raw_err = s.raw_err.clone(); + let raw_err = self.raw_err.borrow().clone(); for r in readers { let ee = raw_err.as_ref().map(|e| e.to_shell_system_error()); self.run_yield(dispatch_reader_done(r, ee, interp)); @@ -340,7 +279,7 @@ impl IOReader { } fn run_yield(&self, y: Yield) { - let Some(interp) = self.state().interp else { + let Some(interp) = self.interp.get() else { debug_assert!( matches!(y, Yield::Done | Yield::Suspended), "IOReader async callback fired without interp backref" @@ -348,7 +287,7 @@ impl IOReader { return; }; // `ParentRef: Deref` — the interpreter owns the IO - // struct holding this Arc and outlives every IOReader. Single-threaded. + // struct holding this reader and outlives every IOReader. Single-threaded. y.run(&interp); } } @@ -357,21 +296,43 @@ impl IOReader { // BufferedReaderParent — wires the bun_io BufferedReader vtable // ────────────────────────────────────────────────────────────────────────── -// Derefs `this` only to call `&self` inherent methods (autoref → `&*this`); -// no `&mut IOReader` is materialized, satisfying the init() *const→*mut -// invariant. Aliasing with the caller's live `&mut ReaderImpl` is handled by -// the state/reader UnsafeCell split — callbacks touch only `state`, never -// `reader()`. +// Every hook views `this` as `&Self` (via `ThisPtr`); no `&mut IOReader` is +// materialized. Aliasing with the caller's live `&mut ReaderImpl` is handled +// by the `reader` cell split — callbacks touch only the other fields. bun_io::impl_buffered_reader_parent! { ShellIoReader for IOReader; + borrow = this; has_on_read_chunk = true; - on_read_chunk = |this, chunk, has_more| (*this).on_read_chunk_cb(&chunk, has_more); - on_reader_done = |this| (*this).on_reader_done_cb(); - on_reader_error = |this, err| (*this).on_reader_error(&err); - loop_ = |this| (*this).io_evtloop().native_loop(); - event_loop = |this| (*this).io_evtloop(); - ref_ = |this| (*this).push_read_guard(); - deref = |this| drop((*this).pop_read_guard()); + on_read_chunk = |this, chunk, has_more| this.on_read_chunk_cb(&chunk, has_more); + on_reader_done = |this| this.on_reader_done_cb(); + on_reader_error = |this, err| this.on_reader_error(&err); + loop_ = |this| this.io_evtloop().native_loop(); + event_loop = |this| this.io_evtloop(); + ref_ = |this| this.ref_(); + deref = |this| IOReader::deref_nn(this.into()); +} + +/// One owned ref on an [`IOReader`]; clone takes another, drop releases it. +pub struct IOReaderRef(RefPtr); + +impl Clone for IOReaderRef { + fn clone(&self) -> Self { + IOReaderRef(self.0.dupe_ref()) + } +} + +impl Drop for IOReaderRef { + fn drop(&mut self) { + self.0.deref(); + } +} + +impl core::ops::Deref for IOReaderRef { + type Target = IOReader; + #[inline] + fn deref(&self) -> &IOReader { + self.0.data() + } } // ────────────────────────────────────────────────────────────────────────── @@ -381,30 +342,31 @@ bun_io::impl_buffered_reader_parent! { impl Drop for IOReader { fn drop(&mut self) { // The bun_io read loop brackets every event-loop entry with the - // parent `ref_`/`deref` hooks (`read_guards`), so the last ref never + // parent `ref_`/`deref` hooks (our refcount), so the last ref never // drops while BufferedReader is still iterating. - let s = self.state.get_mut(); - let r = self.reader.get_mut(); - if s.fd != Fd::INVALID { - #[cfg(windows)] - { - // windows reader closes the file descriptor - if r.source.is_some() && !r.source.as_ref().is_some_and(|src| src.is_closed()) { - r.close_impl::(); + let fd = self.fd; + self.reader.with_mut(|r| { + if fd != Fd::INVALID { + #[cfg(windows)] + { + // windows reader closes the file descriptor + if r.source.is_some() && !r.source.as_ref().is_some_and(|src| src.is_closed()) { + r.close_impl::(); + } } - } - #[cfg(not(windows))] - { - // We cleared CLOSE_HANDLE in init(), so reader Drop will not - // return the FilePoll to its pool. Do it explicitly (without - // closing the fd — we own that and close it ourselves below). - if matches!(r.handle, bun_io::pipes::PollOrFd::Poll(_)) { - r.handle.close_impl(None, None::, false); + #[cfg(not(windows))] + { + // We cleared CLOSE_HANDLE in init(), so reader Drop will not + // return the FilePoll to its pool. Do it explicitly (without + // closing the fd — we own that and close it ourselves below). + if matches!(r.handle, bun_io::pipes::PollOrFd::Poll(_)) { + r.handle.close_impl(None, None::, false); + } + let _ = sys::close(fd); } - let _ = sys::close(s.fd); } - } - r.disable_keeping_process_alive(()); + r.disable_keeping_process_alive(()); + }); // `reader` Drop handles its own deinit. } } diff --git a/src/runtime/shell/IOWriter.rs b/src/runtime/shell/IOWriter.rs index 4c871bcdaf88..9ae50ef2dd79 100644 --- a/src/runtime/shell/IOWriter.rs +++ b/src/runtime/shell/IOWriter.rs @@ -9,11 +9,13 @@ //! //! So `IOWriter` is essentially a writer queue to a file descriptor. //! -//! We also make `IOWriter` reference counted (via `Arc` in the Rust port), +//! We also make `IOWriter` reference counted (via `Rc` in the Rust port), //! this simplifies management of the file descriptor. use bun_collections::VecExt; -use core::cell::UnsafeCell; +use bun_jsc::JsCell; +use bun_ptr::{RefPtr, ThisPtr}; +use core::cell::{Cell, RefCell}; #[cfg(not(windows))] use core::ffi::c_void; @@ -21,7 +23,8 @@ use core::ffi::c_void; use bun_io::pipe_writer::BaseWindowsPipeWriter as _; use bun_sys::{self as sys, E, Fd}; -use crate::shell::interpreter::{EventLoopHandle, Interpreter, NodeId}; +use crate::shell::interpreter::{CapturedBuf, EventLoopHandle, Interpreter, NodeId}; +use crate::shell::subproc::PipeReader; use crate::shell::yield_::Yield; // ────────────────────────────────────────────────────────────────────────── @@ -33,24 +36,24 @@ use crate::shell::yield_::Yield; /// impl to dispatch to. /// /// The one tag that does **not** live in the NodeId arena is -/// `WriterTag::Subproc` (the `subproc::CapturedWriter` embedded inside a -/// heap-allocated `PipeReader`); for that variant the dispatch target is -/// carried in `raw` instead of `node`. +/// `WriterTag::Subproc` (the captured-output tee of a subprocess +/// [`PipeReader`]); for that variant the dispatch target is carried in +/// `subproc` instead of `node`. #[derive(Clone, Copy, PartialEq, Eq, Debug)] pub struct ChildPtr { pub node: NodeId, pub(crate) tag: WriterTag, - /// Only meaningful when `tag == Subproc` — `*mut subproc::PipeReader`. - /// `core::ptr::null_mut()` otherwise. Stored untyped to keep this header - /// free of a `subproc` dependency. - pub(crate) raw: *mut core::ffi::c_void, + /// Only set when `tag == Subproc`. The reader is kept alive by the + /// `Readable::Pipe` ref on its `ShellSubprocess` until every chunk it + /// queued has completed or been `cancel_chunks`ed. + pub(crate) subproc: Option>, } impl ChildPtr { const NULL: ChildPtr = ChildPtr { node: NodeId::NONE, tag: WriterTag::Cmd, - raw: core::ptr::null_mut(), + subproc: None, }; #[inline] @@ -58,24 +61,24 @@ impl ChildPtr { ChildPtr { node, tag, - raw: core::ptr::null_mut(), + subproc: None, } } /// Construct a `ChildPtr` targeting a `subproc::PipeReader`'s captured /// writer (lives outside the NodeId arena). #[inline] - pub(crate) fn subproc_capture(cw: *mut core::ffi::c_void) -> ChildPtr { + pub(crate) fn subproc_capture(pipe: bun_ptr::ThisPtr) -> ChildPtr { ChildPtr { node: NodeId::NONE, tag: WriterTag::Subproc, - raw: cw, + subproc: Some(pipe.into()), } } #[inline] fn is_null(&self) -> bool { - self.node == NodeId::NONE && self.raw.is_null() + self.node == NodeId::NONE && self.subproc.is_none() } } @@ -87,8 +90,8 @@ pub enum WriterTag { Cmd, CondExpr, Pipeline, - /// `subproc::PipeReader::CapturedWriter` — heap-allocated, addressed via - /// `ChildPtr::raw` rather than `node`. + /// `subproc::PipeReader`'s captured-output tee — heap-allocated, addressed via + /// `ChildPtr::subproc` rather than `node`. Subproc, } @@ -105,13 +108,13 @@ pub struct Flags { } /// One queued chunk: which child enqueued it, how many bytes (in `buf`), how -/// many of those have been written so far, and an optional `Vec` to tee +/// many of those have been written so far, and an optional buffer to tee /// into. struct Writer { ptr: ChildPtr, len: usize, written: usize, - bytelist: Option<*mut Vec>, + bytelist: Option, } impl Writer { @@ -129,16 +132,10 @@ impl Writer { self.ptr = ChildPtr::NULL; } /// Tee `chunk` into the optional capture buffer. - /// - /// `bytelist` (when set) points into a live `ShellExecEnv` `Bufio` - /// (`OutFd::captured` — see its doc); the env outlives every queued - /// `Writer`. Localises the per-callsite raw deref in - /// `do_file_write` / `on_write_pollable`. #[inline] fn tee(&self, chunk: &[u8]) { - if let Some(bl) = self.bytelist { - // SAFETY: see doc comment. - let _ = unsafe { (*bl).append_slice(chunk) }; + if let Some(bl) = &self.bytelist { + let _ = bl.borrow_mut().append_slice(chunk); } } } @@ -163,7 +160,7 @@ pub(crate) type WriterImpl = bun_io::pipe_writer::WindowsBufferedWriter`-shared callers can -/// mutate via `&self` (single-threaded shell). -struct State { - writer: WriterImpl, - fd: Fd, - writers: Writers, - buf: Vec, +/// Multiple state nodes share one writer and its chunk callbacks re-enter it +/// (`enqueue` from inside `on_io_writer_chunk`), so every field is +/// interior-mutable behind `&self` and no borrow is held across a callback. +/// Intrusively refcounted: holders own an [`IOWriterRef`]; the io layer's +/// per-write `ref_`/`deref` hooks and the keep-alive brackets below use the +/// same count. +#[derive(bun_ptr::CellRefCounted)] +pub struct IOWriter { + ref_count: Cell, + self_root: bun_ptr::SelfRoot, + /// The io-layer writer. Its poll callbacks arrive while a `&mut` to it is + /// live on the io layer's stack (see the `BufferedWriterParent` aliasing + /// contract), so it sits in its own cell and is only touched through + /// short `with_mut` scopes. + writer: JsCell, + fd: Cell, + writers: RefCell, + /// The bytes being written. In its own cell because the io layer borrows + /// it (`get_buffer`) for the duration of a write syscall. + buf: JsCell>, /// quick hack to get windows working; ideally this should be removed. #[cfg(windows)] - winbuf: Vec, - writer_idx: usize, - total_bytes_written: usize, + winbuf: JsCell>, + writer_idx: Cell, + total_bytes_written: Cell, /// Set (and never cleared) by `fail_pending_writers`. A writer with a /// stored error is dead: `enqueue`/`enqueue_fmt_bltn` must reject new /// chunks with this error instead of queueing them (see /// `handle_dead_writer`). The syscall error is kept (not the derived /// `SystemError`) so each rejected chunk gets its own freshly-derived /// `SystemError`. - err: Option, + err: RefCell>, evtloop: EventLoopHandle, - is_writing: bool, - started: bool, - flags: Flags, - /// Weak self-ref so `keepalive()` can bump the strong count from `&self` - /// without unsafe Arc-pointer reconstruction. Set via `Arc::new_cyclic` in - /// `init()` (the sole constructor). - self_weak: std::sync::Weak, + is_writing: Cell, + started: Cell, + flags: Cell, /// Backref to the owning interpreter for async-poll callbacks (which must - /// drive `Yield::run`). Set by the first `enqueue`/`set_interp`; `None` - /// until then. - interp: Option>, + /// drive `Yield::run`). Set by `set_interp`; `None` until then. + interp: Cell>>, } -pub struct IOWriter { - state: UnsafeCell, -} - -// SAFETY: shell is single-threaded; `Arc` is used purely for refcounting. -// No cross-thread access. -unsafe impl Send for IOWriter {} -// SAFETY: see `Send` — single-threaded, `Arc` is used only for refcounting; no -// concurrent `&IOWriter` access occurs. -unsafe impl Sync for IOWriter {} - impl IOWriter { - /// SAFETY: single-threaded; no overlapping `&mut State` may be live across - /// a re-entrant `enqueue` from a child callback (the `Yield` trampoline - /// runs child callbacks after the borrow is dropped). #[inline] - #[allow(clippy::mut_from_ref)] - fn state(&self) -> &mut State { - // SAFETY: single-threaded; callers uphold the no-overlapping-`&mut State` - // invariant documented on this fn (re-derive across re-entrant calls). - unsafe { &mut *self.state.get() } + fn this_ptr(&self) -> ThisPtr { + self.self_root.this_ptr(self) } - /// Bump our own Arc strong count. Held across re-entrant `run_yield` calls - /// whose child callback may drop the last external ref and free us - /// mid-method; the stack-held strong ref prevents that. #[inline] - fn keepalive(&self) -> std::sync::Arc { - self.state() - .self_weak - .upgrade() - .expect("IOWriter::keepalive after last Arc dropped") + fn update_flags(&self, f: impl FnOnce(&mut Flags)) { + let mut v = self.flags.get(); + f(&mut v); + self.flags.set(v); } /// Read-only accessor for the `is_socket` flag (used by @@ -250,10 +233,10 @@ impl IOWriter { #[inline] #[cfg(not(windows))] pub(crate) fn is_socket(&self) -> bool { - self.state().flags.is_socket + self.flags.get().is_socket } - pub(crate) fn init(fd: Fd, flags: Flags, evtloop: EventLoopHandle) -> std::sync::Arc { + pub(crate) fn init(fd: Fd, flags: Flags, evtloop: EventLoopHandle) -> IOWriterRef { let mut writer = WriterImpl::default(); // Tell the PipeWriter impl to *not* close the file descriptor. #[cfg(not(windows))] @@ -264,75 +247,58 @@ impl IOWriter { { writer.owns_fd = false; } - let this = std::sync::Arc::new_cyclic(|w| IOWriter { - state: UnsafeCell::new(State { - writer, - fd, - writers: Writers::new(), - buf: Vec::new(), - #[cfg(windows)] - winbuf: Vec::new(), - writer_idx: 0, - total_bytes_written: 0, - err: None, - evtloop, - is_writing: false, - started: false, - flags, - self_weak: std::sync::Weak::clone(w), - interp: None, - }), + let this = RefPtr::new_cyclic(|self_root| IOWriter { + ref_count: Cell::new(1), + self_root, + writer: JsCell::new(writer), + fd: Cell::new(fd), + writers: RefCell::new(Writers::new()), + buf: JsCell::new(Vec::new()), + #[cfg(windows)] + winbuf: JsCell::new(Vec::new()), + writer_idx: Cell::new(0), + total_bytes_written: Cell::new(0), + err: RefCell::new(None), + evtloop, + is_writing: Cell::new(false), + started: Cell::new(false), + flags: Cell::new(flags), + interp: Cell::new(None), }); - // Set the parent backref after Arc allocation so the address is stable. - // SAFETY: `Arc::as_ptr` yields `*const IOWriter`; cast to `*mut` only - // because the `BufferedWriterParent` callback ABI is `*mut Self`. The - // pointer is never used to materialize `&mut IOWriter` — every callback - // (`on_write`/`on_error`/`get_buffer`/…) re-enters via `&*this` and - // mutates solely through `UnsafeCell` (`state()`), which carries - // its own write provenance. No const→mut UB. - let parent: *mut IOWriter = std::sync::Arc::as_ptr(&this).cast_mut(); - this.state().writer.set_parent(parent); + let parent: *mut IOWriter = this.as_ptr(); + this.writer.with_mut(|w| w.set_parent(parent)); crate::shell_log!("IOWriter(0x{:x}, fd={}) init", parent as usize, fd); - this + IOWriterRef(this) } /// Stash the interpreter backref so async poll callbacks can drive - /// `Yield::run`. Idempotent. - /// - /// # Safety - /// `interp` must be null or point to the live owning `Interpreter` (which - /// owns the IO struct holding this `Arc`) and outlive it; single-threaded. - // Forwards `interp` to `ParentRef::from_nullable` (shared provenance) - // without dereferencing it here; not_unsafe_ptr_arg_deref is a false - // positive on opaque-token forwarding. - #[allow(clippy::not_unsafe_ptr_arg_deref)] + /// `Yield::run`. `interp` owns (through its IO structs) every handle to + /// this writer. Idempotent. #[inline] - pub(crate) fn set_interp(&self, interp: *mut Interpreter) { - // SAFETY: caller contract above. - self.state().interp = unsafe { bun_ptr::ParentRef::from_nullable(interp) }; + pub(crate) fn set_interp(&self, interp: &Interpreter) { + self.interp.set(Some(bun_ptr::ParentRef::new(interp))); } #[inline] pub(crate) fn fd(&self) -> Fd { - self.state().fd + self.fd.get() } #[inline] #[cfg(windows)] pub(crate) fn evtloop(&self) -> EventLoopHandle { - self.state().evtloop + self.evtloop } pub(crate) fn memory_cost(&self) -> usize { - let s = self.state(); let mut cost = core::mem::size_of::(); - cost += s.buf.capacity(); + cost += self.buf.get().capacity(); #[cfg(windows)] { - cost += s.winbuf.capacity(); + cost += self.winbuf.get().capacity(); } - cost += s.writers.capacity() * core::mem::size_of::(); - cost += s.writer.memory_cost(); + cost += self.writers.borrow().capacity() * core::mem::size_of::(); + cost += self.writer.get().memory_cost(); cost } @@ -343,17 +309,16 @@ impl IOWriter { #[cfg(not(windows))] #[inline] fn io_evtloop(&self) -> bun_io::EventLoopHandle { - // SAFETY: `bun_io::EventLoopHandle` stores `*mut c_void` purely for - // type-erasure; vtable consumers treat the pointee as read-only - self.state().evtloop.as_event_loop_ctx() + self.evtloop.as_event_loop_ctx() } // ── start ──────────────────────────────────────────────────────────── fn __start(&self) -> sys::Result<()> { - let s = self.state(); - crate::shell_log!("IOWriter(fd={}) __start()", s.fd); - if let Err(e) = s.writer.start(s.fd, s.flags.pollable) { + let fd = self.fd.get(); + crate::shell_log!("IOWriter(fd={}) __start()", fd); + let pollable = self.flags.get().pollable; + if let Err(e) = self.writer.with_mut(|w| w.start(fd, pollable)) { #[cfg(not(windows))] { // We get this if we pass in a file descriptor that is not @@ -365,10 +330,8 @@ impl IOWriter { // same file descriptor. The shell code here makes sure to // _not_ run into that case, but it is possible. if e.get_errno() == E::EINVAL { - crate::shell_log!("IOWriter(fd={}) got EINVAL", s.fd); - s.flags.pollable = false; - s.flags.nonblock = false; - s.flags.is_socket = false; + crate::shell_log!("IOWriter(fd={}) got EINVAL", fd); + self.disable_polling(); return self.__start(); } #[cfg(any(target_os = "linux", target_os = "android"))] @@ -376,9 +339,7 @@ impl IOWriter { // On linux regular files are not pollable and return EPERM, // so restart if that's the case with polling disabled. if e.get_errno() == E::EPERM { - s.flags.pollable = false; - s.flags.nonblock = false; - s.flags.is_socket = false; + self.disable_polling(); return self.__start(); } } @@ -391,10 +352,12 @@ impl IOWriter { // uv_tty_init, but this returns EBADF. As a workaround, // we'll try opening the file descriptor as a file. if e.get_errno() == E::EBADF { - s.flags.pollable = false; - s.flags.nonblock = false; - s.flags.is_socket = false; - return s.writer.start_with_file(s.fd); + self.update_flags(|f| { + f.pollable = false; + f.nonblock = false; + f.is_socket = false; + }); + return self.writer.with_mut(|w| w.start_with_file(fd)); } } return Err(e); @@ -404,44 +367,54 @@ impl IOWriter { // When `Source::open` produced a uv pipe/tty, libuv has TAKEN // OWNERSHIP of the underlying HANDLE // (`uv_pipe_open`/`uv_tty_init`) and `uv_close` (issued by - // `s.writer.close()` in Drop) will close it. + // `writer.close()` in Drop) will close it. // `BaseWindowsPipeWriter::start` does not invalidate the stored // fd (TODO at PipeWriter.rs:1277), so disarm the Drop close here // instead. The `Source::File`/`SyncFile` case (incl. the // EBADF→`start_with_file` fallback above, which `return`s early) - // keeps `s.fd` valid: with `owns_fd=false` PipeWriter does NOT + // keeps `fd` valid: with `owns_fd=false` PipeWriter does NOT // close it there, so Drop must. if matches!( - s.writer.source, + self.writer.get().source, Some(bun_io::Source::Pipe(_) | bun_io::Source::Tty(_)) ) { - s.fd = Fd::INVALID; + self.fd.set(Fd::INVALID); } } #[cfg(not(windows))] { use bun_io::FilePollFlag; - // NOTE: re-derive `state()` — the EINVAL/EPERM fallback paths - // above re-enter `__start()` and mutate `writer.handle`, which - // invalidates `s` under Stacked Borrows. - let s = self.state(); - if let Some(poll) = s.writer.get_poll() { - if s.flags.nonblock { - poll.set_flag(FilePollFlag::Nonblocking); - } - // On macOS `sendto` with MSG_DONTWAIT can still block, so - // only mark as socket there if the fd is already O_NONBLOCK. - let sendto_msg_nowait_blocks = cfg!(target_os = "macos"); - if s.flags.is_socket && (!sendto_msg_nowait_blocks || s.flags.nonblock) { - poll.set_flag(FilePollFlag::Socket); - } else if s.flags.pollable { - poll.set_flag(FilePollFlag::Fifo); + let flags = self.flags.get(); + self.writer.with_mut(|w| { + if let Some(poll) = w.get_poll() { + if flags.nonblock { + poll.set_flag(FilePollFlag::Nonblocking); + } + // On macOS `sendto` with MSG_DONTWAIT can still block, so + // only mark as socket there if the fd is already O_NONBLOCK. + let sendto_msg_nowait_blocks = cfg!(target_os = "macos"); + if flags.is_socket && (!sendto_msg_nowait_blocks || flags.nonblock) { + poll.set_flag(FilePollFlag::Socket); + } else if flags.pollable { + poll.set_flag(FilePollFlag::Fifo); + } } - } + }); } Ok(()) } + /// EINVAL/EPERM fallback: this fd cannot be polled, so drop the poll (if + /// one was registered) and continue on the synchronous file path. + #[cfg(not(windows))] + fn disable_polling(&self) { + self.update_flags(|f| { + f.pollable = false; + f.nonblock = false; + f.is_socket = false; + }); + } + /// Idempotent write call. /// /// Failures are *returned* (`WriteOutcome::Failed`), never dispatched from @@ -449,28 +422,23 @@ impl IOWriter { /// error completion has to bounce off it (`on_sync_error`) instead of /// re-entering `Yield::run` (see `DbgDepthGuard`). fn write(&self) -> WriteOutcome { - let s = self.state(); #[cfg(not(windows))] - debug_assert!(s.flags.pollable); + debug_assert!(self.flags.get().pollable); - if !s.started { - crate::shell_log!("IOWriter(fd={}) starting", s.fd); + if !self.started.get() { + crate::shell_log!("IOWriter(fd={}) starting", self.fd.get()); // Set before the fallible `__start` so a later enqueue does not // retry it. - s.started = true; + self.started.set(true); if let Err(e) = self.__start() { return WriteOutcome::Failed(e); } #[cfg(not(windows))] { - // NOTE: `__start()` re-derives `state()` (and may mutate - // `writer.handle` on the EINVAL/EPERM fallback paths), which - // invalidates the `s` borrow under Stacked Borrows. Re-derive. - let s = self.state(); // if `handle == .fd` it means it's a file which does not // support polling for writeability and we should just write to it - if matches!(s.writer.handle, bun_io::pipes::PollOrFd::Fd(_)) { - debug_assert!(!s.flags.pollable); + if matches!(self.writer.get().handle, bun_io::pipes::PollOrFd::Fd(_)) { + debug_assert!(!self.flags.get().pollable); return WriteOutcome::IsActuallyFile; } return WriteOutcome::Suspended; @@ -481,12 +449,16 @@ impl IOWriter { #[cfg(windows)] { - crate::shell_log!("IOWriter(fd={}) write() is_writing={}", s.fd, s.is_writing); - if s.is_writing { + crate::shell_log!( + "IOWriter(fd={}) write() is_writing={}", + self.fd.get(), + self.is_writing.get() + ); + if self.is_writing.get() { return WriteOutcome::Suspended; } - s.is_writing = true; - if let Err(e) = s.writer.start_with_current_pipe() { + self.is_writing.set(true); + if let Err(e) = self.writer.with_mut(|w| w.start_with_current_pipe()) { return WriteOutcome::Failed(e); } return WriteOutcome::Suspended; @@ -494,18 +466,23 @@ impl IOWriter { #[cfg(not(windows))] { - debug_assert!(matches!(s.writer.handle, bun_io::pipes::PollOrFd::Poll(_))); - if let Some(poll) = s.writer.get_poll() { - // `is_watching()` = `is_registered() && !needs_rearm`. - // NOT `is_registered()`: after a one-shot fire that drains - // everything (no `register_poll()`), `PollWritable` stays set - // but `NeedsRearm` is set → `is_registered()` would return - // Suspended without re-arming and stall the queue forever. - if poll.is_watching() { - return WriteOutcome::Suspended; - } + debug_assert!(matches!( + self.writer.get().handle, + bun_io::pipes::PollOrFd::Poll(_) + )); + // `is_watching()` = `is_registered() && !needs_rearm`. + // NOT `is_registered()`: after a one-shot fire that drains + // everything (no `register_poll()`), `PollWritable` stays set + // but `NeedsRearm` is set → `is_registered()` would return + // Suspended without re-arming and stall the queue forever. + let watching = self + .writer + .with_mut(|w| w.get_poll().is_some_and(|poll| poll.is_watching())); + if watching { + return WriteOutcome::Suspended; } - if let Err(e) = s.writer.start(s.fd, s.flags.pollable) { + let (fd, pollable) = (self.fd.get(), self.flags.get().pollable); + if let Err(e) = self.writer.with_mut(|w| w.start(fd, pollable)) { return WriteOutcome::Failed(e); } WriteOutcome::Suspended @@ -516,15 +493,15 @@ impl IOWriter { /// Cancel the chunks enqueued by the given child by marking them as dead. pub(crate) fn cancel_chunks(&self, ptr: ChildPtr) { - let s = self.state(); - if s.writers.is_empty() { + let mut writers = self.writers.borrow_mut(); + if writers.is_empty() { return; } - let idx = s.writer_idx; - if idx >= s.writers.len() { + let idx = self.writer_idx.get(); + if idx >= writers.len() { return; } - for w in &mut s.writers[idx..] { + for w in &mut writers[idx..] { if w.ptr == ptr { w.set_dead(); } @@ -534,12 +511,13 @@ impl IOWriter { /// Skips over dead children and increments `total_bytes_written` by the /// amount they would have written so the buf is skipped as well. fn skip_dead(&self) { - let s = self.state(); - while s.writer_idx < s.writers.len() { - let w = &s.writers[s.writer_idx]; + let writers = self.writers.borrow(); + while self.writer_idx.get() < writers.len() { + let w = &writers[self.writer_idx.get()]; if w.is_dead() { - s.total_bytes_written += w.len - w.written; - s.writer_idx += 1; + self.total_bytes_written + .set(self.total_bytes_written.get() + (w.len - w.written)); + self.writer_idx.set(self.writer_idx.get() + 1); continue; } return; @@ -547,8 +525,7 @@ impl IOWriter { } fn wrote_everything(&self) -> bool { - let s = self.state(); - s.total_bytes_written >= s.buf.len() + self.total_bytes_written.get() >= self.buf.get().len() } /// Only does things on windows. @@ -556,7 +533,7 @@ impl IOWriter { fn set_writing(&self, writing: bool) { #[cfg(windows)] { - self.state().is_writing = writing; + self.is_writing.set(writing); } let _ = writing; } @@ -569,44 +546,41 @@ impl IOWriter { let result = self.get_buffer_impl(); #[cfg(windows)] { - let s = self.state(); - s.winbuf.clear(); - s.winbuf.extend_from_slice(result); - // `state()` ties `s` to `&self`, so the slice borrow already has - // the `'self` lifetime the signature wants — no raw-parts needed. - return s.winbuf.as_slice(); + self.winbuf.with_mut(|winbuf| { + winbuf.clear(); + winbuf.extend_from_slice(result); + }); + return self.winbuf.get().as_slice(); } #[cfg(not(windows))] result } fn get_buffer_impl(&self) -> &[u8] { - // NOTE: reshaped for borrowck — re-derive `state()` after - // `skip_dead()` instead of holding one `&mut State` across it. - { - let s = self.state(); - if s.writer_idx >= s.writers.len() { - return &[]; - } - if s.writers[s.writer_idx].is_dead() { - let _ = s; - self.skip_dead(); + let current_is_dead = { + let writers = self.writers.borrow(); + match writers.get(self.writer_idx.get()) { + None => return &[], + Some(w) => w.is_dead(), } + }; + if current_is_dead { + self.skip_dead(); } - let s = self.state(); - if s.writer_idx >= s.writers.len() { + let writers = self.writers.borrow(); + let idx = self.writer_idx.get(); + if idx >= writers.len() { return &[]; } let remaining = { - let writer = &s.writers[s.writer_idx]; + let writer = &writers[idx]; debug_assert!(writer.len != writer.written); writer.len - writer.written }; - // `state()` already ties `s` to `&self`, so a plain slice borrow has - // the right lifetime. `buf` is not reallocated until after the - // caller's write syscall completes. - let start = s.total_bytes_written; - &s.buf[start..start + remaining] + // `buf` is not reallocated until after the caller's write syscall + // completes. + let start = self.total_bytes_written.get(); + &self.buf.get()[start..start + remaining] } // ── bump (chunk completed) ────────────────────────────────────────── @@ -614,37 +588,36 @@ impl IOWriter { /// Advance past `current_writer`, shrinking `buf` if appropriate, and /// return the `Yield` for the child's `on_io_writer_chunk` callback. fn bump(&self, current_idx: usize) -> Yield { - // NOTE: reshaped for borrowck — `skip_dead()` re-derives `state()`, - // so we must drop `s` before calling it and re-derive after, otherwise - // two `&mut State` are live simultaneously (UB under Stacked Borrows). - let (is_dead, written, child_ptr) = { - let s = self.state(); - let w = &s.writers[current_idx]; - (w.is_dead(), w.written, w.ptr) + let (is_dead, written, len, child_ptr) = { + let writers = self.writers.borrow(); + let w = &writers[current_idx]; + (w.is_dead(), w.written, w.len, w.ptr) }; if is_dead { self.skip_dead(); } else { - let s = self.state(); - debug_assert!(s.writers[current_idx].written == s.writers[current_idx].len); - s.writer_idx += 1; + debug_assert!(written == len); + self.writer_idx.set(self.writer_idx.get() + 1); } - let s = self.state(); - if s.writer_idx >= s.writers.len() { - s.buf.clear(); - s.writer_idx = 0; - s.writers.clear(); - s.total_bytes_written = 0; - } else if s.total_bytes_written >= SHRINK_THRESHOLD { - s.buf.drain_front(s.total_bytes_written); - s.total_bytes_written = 0; - // Drop the *prefix* of the writers queue: Vec::drain(..idx). - s.writers.drain(..s.writer_idx); - s.writer_idx = 0; - if cfg!(debug_assertions) && !s.writers.is_empty() { - debug_assert!(s.buf.len() >= s.writers[0].len); + { + let mut writers = self.writers.borrow_mut(); + if self.writer_idx.get() >= writers.len() { + self.buf.with_mut(|b| b.clear()); + self.writer_idx.set(0); + writers.clear(); + self.total_bytes_written.set(0); + } else if self.total_bytes_written.get() >= SHRINK_THRESHOLD { + let total = self.total_bytes_written.get(); + self.buf.with_mut(|b| b.drain_front(total)); + self.total_bytes_written.set(0); + // Drop the *prefix* of the writers queue: Vec::drain(..idx). + writers.drain(..self.writer_idx.get()); + self.writer_idx.set(0); + if cfg!(debug_assertions) && !writers.is_empty() { + debug_assert!(self.buf.get().len() >= writers[0].len); + } } } @@ -662,36 +635,30 @@ impl IOWriter { /// Tee `amt` bytes from the current buffer position into `writers[idx]`'s /// capture and advance its `written` / `total_bytes_written` counters. - #[cfg(not(windows))] fn record_write_progress(&self, idx: usize, amt: usize) { - let s = self.state(); - let lo = s.total_bytes_written; - s.writers[idx].tee(&s.buf[lo..lo + amt]); - s.total_bytes_written += amt; - s.writers[idx].written += amt; + let mut writers = self.writers.borrow_mut(); + let lo = self.total_bytes_written.get(); + writers[idx].tee(&self.buf.get()[lo..lo + amt]); + self.total_bytes_written.set(lo + amt); + writers[idx].written += amt; } /// POSIX-only. `child` is the writer being enqueued (see `on_sync_error`). #[cfg(not(windows))] fn do_file_write(&self, child: ChildPtr) -> Yield { - { - let s = self.state(); - debug_assert!(!s.flags.pollable); - debug_assert!(s.writer_idx < s.writers.len()); - } + debug_assert!(!self.flags.get().pollable); + debug_assert!(self.writer_idx.get() < self.writers.borrow().len()); scopeguard::defer! { self.set_writing(false); } self.skip_dead(); - let idx = self.state().writer_idx; - debug_assert!(!self.state().writers[idx].is_dead()); + let idx = self.writer_idx.get(); + debug_assert!(!self.writers.borrow()[idx].is_dead()); let buf = self.get_buffer(); debug_assert!(!buf.is_empty()); let result = drain_buffered_data(self, buf, u32::MAX as usize); - // NOTE: re-derive `state()` after `drain_buffered_data` instead of - // holding a stale `&mut`. let amt = match result { bun_io::WriteResult::Done(amt) | bun_io::WriteResult::Wrote(amt) => amt, bun_io::WriteResult::Pending(amt) => { @@ -699,10 +666,11 @@ impl IOWriter { // FIFO or chardev opened by path with O_NONBLOCK). Record the // partial write and restart this writer on the pollable path. self.record_write_progress(idx, amt); - let s = self.state(); - s.flags.pollable = true; - s.flags.nonblock = true; - s.started = false; + self.update_flags(|f| { + f.pollable = true; + f.nonblock = true; + }); + self.started.set(false); return match self.write() { WriteOutcome::Suspended => Yield::suspended(), WriteOutcome::IsActuallyFile => self @@ -715,7 +683,7 @@ impl IOWriter { bun_io::WriteResult::Err(e) => return self.on_sync_error(child, &e), }; self.record_write_progress(idx, amt); - if !self.state().writers[idx].wrote_everything() { + if !self.writers.borrow()[idx].wrote_everything() { // The only case where we get partial writes is when an error is // encountered, which returns above. unreachable!( @@ -729,78 +697,75 @@ impl IOWriter { /// The `BufferedWriter.onWrite` hook. Runs on the event loop when the fd /// is writable. - fn on_write_pollable(&self, amount: usize, status: bun_io::WriteStatus) { - // NOTE: `set_writing` re-derives `state()` on Windows, which would - // invalidate `s` under Stacked Borrows; do it before binding `s` - // (matches the ordering in `on_error`). + fn on_write_pollable(this: ThisPtr, amount: usize, status: bun_io::WriteStatus) { + let _keepalive = this.ref_guard(); + let me: &Self = &this; + me.on_write_pollable_impl(amount, status); + } + + fn on_write_pollable_impl(&self, amount: usize, status: bun_io::WriteStatus) { self.set_writing(false); - let s = self.state(); #[cfg(not(windows))] - debug_assert!(s.flags.pollable); + debug_assert!(self.flags.get().pollable); - if s.writer_idx >= s.writers.len() { - return; - } - let idx = s.writer_idx; - if s.writers[idx].is_dead() { + let idx = self.writer_idx.get(); + let (is_dead, queue_len) = { + let writers = self.writers.borrow(); + if idx >= writers.len() { + return; + } + (writers[idx].is_dead(), writers.len()) + }; + if is_dead { self.run_yield(self.bump(idx)); } else { - let lo = s.total_bytes_written; - s.writers[idx].tee(&s.buf[lo..lo + amount]); - s.total_bytes_written += amount; - s.writers[idx].written += amount; + self.record_write_progress(idx, amount); + let (written, len) = { + let writers = self.writers.borrow(); + (writers[idx].written, writers[idx].len) + }; if status == bun_io::WriteStatus::EndOfFile { - // NOTE: inline `is_last_idx` instead of calling - // `self.is_last_idx(idx)` — that re-derives `state()` while `s` - // is still live, which is two simultaneous `&mut State` (UB). - let last = idx == s.writers.len().saturating_sub(1); - let not_fully_written = if last { - true - } else { - s.writers[idx].written < s.writers[idx].len - }; + let last = idx == queue_len.saturating_sub(1); + let not_fully_written = if last { true } else { written < len }; if !not_fully_written { return; } // Other end of the socket/pipe closed and we got EPIPE // (e.g. `ls | echo`). Quick hack: have all writers see an // error. - s.flags.broken_pipe = true; + self.update_flags(|f| f.broken_pipe = true); self.broken_pipe_for_writers(); return; } - if s.writers[idx].written >= s.writers[idx].len { + if written >= len { self.run_yield(self.bump(idx)); } } let wrote_everything = self.wrote_everything(); - let s = self.state(); - if !wrote_everything && s.writer_idx < s.writers.len() { + if !wrote_everything && self.writer_idx.get() < self.writers.borrow().len() { #[cfg(windows)] { - // NOTE: inline `set_writing(true)` instead of calling the - // helper — the helper re-derives `state()` while `s` is live, - // which is two simultaneous `&mut State` (UB under Stacked - // Borrows). Same discipline as the top of this fn. - s.is_writing = true; - s.writer.write(); + self.is_writing.set(true); + self.writer.with_mut(|w| w.write()); } #[cfg(not(windows))] { - debug_assert!(matches!(s.writer.handle, bun_io::pipes::PollOrFd::Poll(_))); - s.writer.register_poll(); + debug_assert!(matches!( + self.writer.get().handle, + bun_io::pipes::PollOrFd::Poll(_) + )); + self.writer.with_mut(|w| w.register_poll()); } } } fn broken_pipe_for_writers(&self) { - let s = self.state(); - debug_assert!(s.flags.broken_pipe); - // NOTE: reshaped for borrowck — collect targets first so we don't - // hold `&mut s.writers` across `cancel_chunks`/`run_yield`. + debug_assert!(self.flags.get().broken_pipe); + // Collect targets first so `writers` is not borrowed across + // `cancel_chunks`/`run_yield`. let mut targets: Vec = Vec::new(); - for w in &s.writers[s.writer_idx..] { + for w in &self.writers.borrow()[self.writer_idx.get()..] { if w.is_dead() { continue; } @@ -817,11 +782,10 @@ impl IOWriter { }); self.cancel_chunks(ptr); } - let s = self.state(); - s.total_bytes_written = 0; - s.writers.clear(); - s.buf.clear(); - s.writer_idx = 0; + self.total_bytes_written.set(0); + self.writers.borrow_mut().clear(); + self.buf.with_mut(|b| b.clear()); + self.writer_idx.set(0); } /// Shared failure bookkeeping: mark broken pipes, reset the queue, and @@ -830,39 +794,44 @@ impl IOWriter { /// re-enqueueing from its callback is not wiped afterwards. fn fail_pending_writers(&self, err: &sys::Error) -> Vec { self.set_writing(false); - let s = self.state(); if err.get_errno() == E::EPIPE { - s.flags.broken_pipe = true; + self.update_flags(|f| f.broken_pipe = true); } // Mark the writer dead before any completion below runs: a child that // enqueues from its callback (the next statement, the RHS of `&&`, ...) // must be rejected by `handle_dead_writer`, not queued onto a writer // whose handle the error path is tearing down. - s.err = Some(err.clone()); + *self.err.borrow_mut() = Some(err.clone()); // Writers before writer_idx have already had their callback fired and // may have been freed; only notify the still-pending ones, dedup'd. let mut pending: Vec = Vec::new(); - for w in &s.writers[s.writer_idx..] { + let mut writers = self.writers.borrow_mut(); + for w in &writers[self.writer_idx.get()..] { if !w.is_dead() && !pending.contains(&w.ptr) { pending.push(w.ptr); } } - s.total_bytes_written = 0; - s.writer_idx = 0; - s.buf.clear(); - s.writers.clear(); + self.total_bytes_written.set(0); + self.writer_idx.set(0); + self.buf.with_mut(|b| b.clear()); + writers.clear(); pending } /// Write failure reported by the `bun_io` writer callbacks. Each pending /// child's error completion is driven through its own `Yield::run`; on /// POSIX these callbacks only fire from the event loop, with no trampoline - /// on the stack. On Windows uv can also deliver a synchronous submission - /// failure from under `write()` (`start_with_current_pipe` returns `Ok` - /// unconditionally), a re-entry `write()` cannot turn into a - /// `WriteOutcome::Failed`. - fn on_error(&self, err: &sys::Error) { - let _keepalive = self.keepalive(); + /// on the stack. On Windows uv can also report one from under the + /// submitting call (`start_with_current_pipe` returns `Ok` regardless), so + /// the enqueuing child may be called back from inside its own `enqueue`; + /// callers therefore hold no node borrow across `enqueue`. + fn on_error(this: ThisPtr, err: &sys::Error) { + let _keepalive = this.ref_guard(); + let me: &Self = &this; + me.on_error_impl(err); + } + + fn on_error_impl(&self, err: &sys::Error) { for ptr in self.fail_pending_writers(err) { // `SystemError` owns `bun_core::String`s by value (no shared // refcount yet), so re-derive a fresh one per callee instead of @@ -886,7 +855,7 @@ impl IOWriter { /// re-registration fails while other children are still queued, those are /// dispatched the way the async path dispatches them. fn on_sync_error(&self, child: ChildPtr, err: &sys::Error) -> Yield { - let _keepalive = self.keepalive(); + let _keepalive = self.this_ptr().ref_guard(); let mut completion = None; for ptr in self.fail_pending_writers(err) { // `SystemError` owns `bun_core::String`s by value (no shared @@ -908,24 +877,22 @@ impl IOWriter { completion.unwrap_or_else(Yield::done) } - fn on_close(&self) { - self.set_writing(false); + fn on_close(this: ThisPtr) { + this.set_writing(false); } /// Drive a `Yield` from inside an async poll callback. Requires `interp` /// to have been set; if not, the chunk-complete is dropped (debug-asserts). fn run_yield(&self, y: Yield) { - let Some(interp) = self.state().interp else { + let Some(interp) = self.interp.get() else { debug_assert!( matches!(y, Yield::Done), "IOWriter async callback fired without interp backref" ); return; }; - // SAFETY: interp outlives every IOWriter (it owns the IO struct that - // holds the Arc). Single-threaded; R-2: `Interpreter::run` takes - // `&self` now — `ParentRef: Deref` yields the - // shared borrow without `assume_mut()`. + // The interpreter owns the IO structs that hold this writer and + // outlives it. Single-threaded. y.run(&interp); } @@ -940,8 +907,7 @@ impl IOWriter { /// flavor of the same thing. Report the error to the child instead of /// queueing the chunk. fn handle_dead_writer(&self, ptr: ChildPtr) -> Option { - let s = self.state(); - if s.flags.broken_pipe { + if self.flags.get().broken_pipe { let err = sys::Error::from_code(E::EPIPE, sys::Tag::write).to_system_error(); return Some(Yield::OnIoWriterChunk { child: ptr, @@ -949,7 +915,7 @@ impl IOWriter { err: Some(err), }); } - if let Some(err) = &s.err { + if let Some(err) = &*self.err.borrow() { return Some(Yield::OnIoWriterChunk { child: ptr, written: 0, @@ -963,13 +929,12 @@ impl IOWriter { #[cfg(not(windows))] fn enqueue_file(&self, child: ChildPtr) -> Yield { - let s = self.state(); - if s.is_writing { + if self.is_writing.get() { return Yield::suspended(); } // The pollable path sets `started` in write(); the non-pollable file // path bypasses write() entirely, so set it here. - s.started = true; + self.started.set(true); self.set_writing(true); self.do_file_write(child) } @@ -977,10 +942,10 @@ impl IOWriter { /// You MUST have already added the data to `self.buf`! /// `child` is the writer that was just pushed (see `on_sync_error`). fn enqueue_internal(&self, child: ChildPtr) -> Yield { - debug_assert!(!self.state().flags.broken_pipe); - debug_assert!(self.state().err.is_none()); + debug_assert!(!self.flags.get().broken_pipe); + debug_assert!(self.err.borrow().is_none()); #[cfg(not(windows))] - if !self.state().flags.pollable { + if !self.flags.get().pollable { return self.enqueue_file(child); } match self.write() { @@ -996,7 +961,7 @@ impl IOWriter { pub(crate) fn enqueue( &self, child: ChildPtr, - bytelist: Option<*mut Vec>, + bytelist: Option, buf: &[u8], ) -> Yield { if let Some(y) = self.handle_dead_writer(child) { @@ -1009,9 +974,8 @@ impl IOWriter { err: None, }; } - let s = self.state(); - s.buf.extend_from_slice(buf); - s.writers.push(Writer { + self.buf.with_mut(|b| b.extend_from_slice(buf)); + self.writers.borrow_mut().push(Writer { ptr: child, len: buf.len(), written: 0, @@ -1020,44 +984,63 @@ impl IOWriter { self.enqueue_internal(child) } + /// [`enqueue`](Self::enqueue) with the bytes produced by `fill`, which + /// appends them straight into the write buffer (any borrow it needs ends + /// with it, before the writer can call anyone back). + pub(crate) fn enqueue_with( + &self, + child: ChildPtr, + bytelist: Option, + fill: impl FnOnce(&mut Vec), + ) -> Yield { + if let Some(y) = self.handle_dead_writer(child) { + return y; + } + let len = self.buf.with_mut(|b| { + let start = b.len(); + fill(b); + b.len() - start + }); + if len == 0 { + return Yield::OnIoWriterChunk { + child, + written: 0, + err: None, + }; + } + self.writers.borrow_mut().push(Writer { + ptr: child, + len, + written: 0, + bytelist, + }); + self.enqueue_internal(child) + } + /// Prefix `"{kind}: "` then format. pub(crate) fn enqueue_fmt_bltn( &self, child: ChildPtr, - bytelist: Option<*mut Vec>, + bytelist: Option, kind: Option, args: core::fmt::Arguments<'_>, ) -> Yield { use std::io::Write as _; - let s = self.state(); - let start = s.buf.len(); - if let Some(k) = kind { - let _ = write!(&mut s.buf, "{}: ", k.as_str()); - } - let _ = s.buf.write_fmt(args); + let (start, end) = self.buf.with_mut(|buf| { + let start = buf.len(); + if let Some(k) = kind { + let _ = write!(buf, "{}: ", k.as_str()); + } + let _ = buf.write_fmt(args); + (start, buf.len()) + }); // `buf` is written *before* the dead-writer checks (the bytes are dead // on the error path but no `Writer` references them, and an errored // writer never drains again). - // NOTE: inline `handle_dead_writer` instead of calling the helper — - // the helper re-derives `state()` while `s` is still live, which is two - // simultaneous `&mut State` (UB under Stacked Borrows). - if s.flags.broken_pipe { - let err = sys::Error::from_code(E::EPIPE, sys::Tag::write).to_system_error(); - return Yield::OnIoWriterChunk { - child, - written: 0, - err: Some(err), - }; - } - if let Some(err) = &s.err { - return Yield::OnIoWriterChunk { - child, - written: 0, - err: Some(err.to_shell_system_error()), - }; + if let Some(y) = self.handle_dead_writer(child) { + return y; } - let end = s.buf.len(); - s.writers.push(Writer { + self.writers.borrow_mut().push(Writer { ptr: child, len: end - start, written: 0, @@ -1083,18 +1066,38 @@ enum WriteOutcome { bun_io::impl_buffered_writer_parent! { IOWriter; poll_tag = bun_io::posix_event_loop::poll_tag::SHELL_BUFFERED_WRITER, - // UnsafeCell aliasing model — child callbacks may re-enter `enqueue(&self)`. - borrow = shared, + // Child callbacks may re-enter `enqueue(&self)` and drop holder refs, so + // every hook gets a `ThisPtr` and takes a ref guard first. + borrow = this, on_write = on_write_pollable, on_error = on_error, on_close = on_close, - get_buffer = |this| (*this).get_buffer(), - event_loop = |this| (*this).io_evtloop(), - uv_loop = |this| (*(*this).evtloop().loop_()).uv_loop, - // INVARIANT: `this` is `Arc::as_ptr` stashed via `writer.set_parent` in - // `IOWriter::init` (sole constructor); passing a non-Arc ptr is UB. - ref_ = |this| std::sync::Arc::increment_strong_count(this as *const Self), - deref = |this| std::sync::Arc::decrement_strong_count(this as *const Self), + get_buffer = |this| this.get_buffer(), + event_loop = |this| this.io_evtloop(), + uv_loop = |this| this.evtloop().uv_loop(), +} + +/// One owned ref on an [`IOWriter`]; clone takes another, drop releases it. +pub struct IOWriterRef(RefPtr); + +impl Clone for IOWriterRef { + fn clone(&self) -> Self { + IOWriterRef(self.0.dupe_ref()) + } +} + +impl Drop for IOWriterRef { + fn drop(&mut self) { + self.0.deref(); + } +} + +impl core::ops::Deref for IOWriterRef { + type Target = IOWriter; + #[inline] + fn deref(&self) -> &IOWriter { + self.0.data() + } } // ────────────────────────────────────────────────────────────────────────── @@ -1143,7 +1146,7 @@ fn drain_buffered_data( }; let mut drained: usize = 0; while drained < trimmed.len() { - match try_write_with_write_fn(parent.state().fd, buf, sys::write) { + match try_write_with_write_fn(parent.fd(), buf, sys::write) { bun_io::WriteResult::Pending(pending) => { drained += pending; return bun_io::WriteResult::Pending(drained); @@ -1172,31 +1175,31 @@ fn drain_buffered_data( impl Drop for IOWriter { fn drop(&mut self) { - // With `Arc` the last ref drops *after* the callback returns, so the + // With `Rc` the last ref drops *after* the callback returns, so the // synchronous path is safe (PipeWriter cannot touch us after free). // TODO: if a PipeWriter callback is on the stack when the last - // Arc drops (possible via re-entrant child deinit), we need the async + // ref drops (possible via re-entrant child deinit), we need the async // hop. Revisit once `bun_event_loop::EventLoopTask` is wired to the // shell's `EventLoopHandle` shim. - let s = self.state.get_mut(); - crate::shell_log!("IOWriter(fd={}) deinit", s.fd); - #[cfg(not(windows))] - { - if matches!(s.writer.handle, bun_io::pipes::PollOrFd::Poll(_)) { - s.writer - .handle - .close_impl(None, None::, false); + let fd = self.fd.get(); + crate::shell_log!("IOWriter(fd={}) deinit", fd); + let evtloop = self.evtloop; + self.writer.with_mut(|w| { + #[cfg(not(windows))] + { + if matches!(w.handle, bun_io::pipes::PollOrFd::Poll(_)) { + w.handle.close_impl(None, None::, false); + } } - } - #[cfg(windows)] - { - s.writer.close(); - } - if s.fd != Fd::INVALID { - let _ = sys::close(s.fd); - } - s.writer - .disable_keeping_process_alive(s.evtloop.as_event_loop_ctx()); + #[cfg(windows)] + { + w.close(); + } + if fd != Fd::INVALID { + let _ = sys::close(fd); + } + w.disable_keeping_process_alive(evtloop.as_event_loop_ctx()); + }); } } @@ -1224,21 +1227,15 @@ pub(crate) fn on_io_writer_chunk( WriterTag::Pipeline => { pipeline::Pipeline::on_io_writer_chunk(interp, child.node, written, err) } - // The target is the subprocess PipeReader's `CapturedWriter`; it + // The target is the subprocess PipeReader's captured-output tee; it // lives outside the NodeId arena (heap-allocated PipeReader), so it - // is carried in `child.raw` instead of `child.node`. + // is carried in `child.subproc` instead of `child.node`. WriterTag::Subproc => { let _ = interp; - debug_assert!(!child.raw.is_null()); - // SAFETY: `raw` is the `PipeReader` root set in - // `PipeReader::captured_child_ptr`; the reader is kept alive by - // the `Readable::Pipe` ref on the owning ShellSubprocess until - // `on_close_io` runs, which only happens after the writer has - // finished draining (or `cancel_chunks` removed this entry). - let pipe = unsafe { - bun_ptr::ThisPtr::new(child.raw.cast::()) - }; - crate::shell::subproc::PipeReader::on_captured_iowriter_chunk(pipe, written, err) + let pipe = child + .subproc + .expect("WriterTag::Subproc carries its PipeReader"); + PipeReader::on_captured_iowriter_chunk(pipe.this_ptr(), written, err) } } } diff --git a/src/runtime/shell/ParsedShellScript.rs b/src/runtime/shell/ParsedShellScript.rs index 7381674211b1..1bb01b72d947 100644 --- a/src/runtime/shell/ParsedShellScript.rs +++ b/src/runtime/shell/ParsedShellScript.rs @@ -8,10 +8,10 @@ use bun_jsc::{ JsRef, JsResult, MarkedArgumentBuffer, StringJsc as _, }; +use super::EnvStr; use super::env_map::EnvMap; use super::interpreter::ShellArgs; use super::shell_body::shell_cmd_from_js; -use super::{EnvStr, Interpreter}; // NOTE: `pub const js = jsc.Codegen.JSParsedShellScript;` and the // `toJS`/`fromJS`/`fromJSDirect` re-exports are provided by the @@ -233,48 +233,20 @@ fn create_parsed_shell_script_impl( marked_argument_buffer, )?; - // Reshaped for borrowck — `out_parser`/`out_lex_result` borrow - // `shargs.__arena`, so they're scoped to a block that ends before - // `shargs.script_ast = script` below. The arena reference is taken via raw - // pointer so the `&shargs` borrow doesn't outlive the call (the returned - // `ast::Script` is lifetime-erased). - let arena_ptr: *const bun_alloc::Arena = shargs.arena(); - let script_ast = { - // SAFETY: `shargs` lives on this stack frame for the whole block; arena - // is not moved/dropped while `out_parser`/`out_lex_result` borrow it. - let arena = unsafe { &*arena_ptr }; - let mut out_parser: Option> = None; - let mut out_lex_result: Option> = None; - match Interpreter::parse( - arena, - &script[..], - &mut jsobjs[..], - &jsstrings[..], - &mut out_parser, - &mut out_lex_result, - ) { - Ok(ast) => ast, - Err(err) => { - // `out_lex_result.is_some()` ⇔ `err == ParseError::Lex` — `Interpreter::parse` - // only populates `out_lex_result` on the Lex error path. - if let Some(lex) = out_lex_result.as_ref() { - debug_assert!(!lex.errors.is_empty()); - let str = lex.combine_errors(arena); - return Err(global.throw(format_args!("{}", bstr::BStr::new(str)))); - } - - if let Some(p) = out_parser.as_mut() { - debug_assert!(!p.errors.is_empty()); - let errstr = p.combine_errors(); - return Err(global.throw(format_args!("{}", bstr::BStr::new(errstr)))); - } - - return Err(global.throw_error(err, "failed to lex/parse shell")); + if let Err(err) = shargs.parse(&script[..], &jsstrings[..], jsobjs.len() as u32) { + use bun_shell_parser::ParseFailure; + return Err(match err { + ParseFailure::Diagnostic(msg) => { + global.throw(format_args!("{}", bstr::BStr::new(&msg))) } - } - }; - - shargs.set_script_ast(script_ast); + ParseFailure::Lexer(e) => { + global.throw_error(crate::Error::from(e), "failed to lex/parse shell") + } + ParseFailure::Other(e) => { + global.throw_error(crate::Error::from(e), "failed to lex/parse shell") + } + }); + } let mut parsed_shell_script = Box::new(ParsedShellScript { args: JsCell::new(Some(shargs)), diff --git a/src/runtime/shell/Yield.rs b/src/runtime/shell/Yield.rs index 351032832410..ebce5b3962c7 100644 --- a/src/runtime/shell/Yield.rs +++ b/src/runtime/shell/Yield.rs @@ -2,8 +2,8 @@ //! //! See the doc comment on `Yield` for the design. The Rust port carries //! `NodeId`s (indices into `Interpreter::nodes`) instead of `&mut State` -//! borrows — the only `&mut` is the `&Interpreter` threaded through -//! `run`. +//! borrows; state is reached through the `&Interpreter` threaded through +//! `run` (per-node `RefCell`s). use core::cell::Cell; diff --git a/src/runtime/shell/builtin/basename.rs b/src/runtime/shell/builtin/basename.rs index 43bc4dcb7423..5239bc36b310 100644 --- a/src/runtime/shell/builtin/basename.rs +++ b/src/runtime/shell/builtin/basename.rs @@ -9,7 +9,7 @@ pub struct Basename { buf: Vec, } -#[derive(Default)] +#[derive(Default, Clone, Copy)] enum State { #[default] Idle, @@ -19,12 +19,12 @@ enum State { impl Basename { pub(crate) fn start(interp: &Interpreter, cmd: NodeId) -> Yield { + if Builtin::argc(interp, cmd) == 0 { + return Self::fail(interp, cmd, Kind::Basename.usage_string()); + } let buf = { let bltn = Builtin::of(interp, cmd); let argc = bltn.args_slice().len(); - if argc == 0 { - return Self::fail(interp, cmd, Kind::Basename.usage_string()); - } let mut buf = Vec::new(); for i in 0..argc { buf.extend_from_slice(bun_paths::resolve_path::basename(bltn.arg_bytes(i))); @@ -34,13 +34,12 @@ impl Basename { }; Self::state_mut(interp, cmd).state = State::Done; - if let Some(safeguard) = Builtin::of(interp, cmd).stdout.needs_io() { + let stdout_needs_io = Builtin::of(interp, cmd).stdout.needs_io(); + if let Some(safeguard) = stdout_needs_io { Self::state_mut(interp, cmd).buf = buf; let owned = Self::state_mut(interp, cmd).buf.clone(); let child = ChildPtr::new(cmd, WriterTag::Builtin); - return Builtin::of_mut(interp, cmd) - .stdout - .enqueue(child, &owned, safeguard); + return Builtin::write_out(interp, cmd, IoKind::Stdout, child, &owned, safeguard); } let _ = Builtin::write_no_io(interp, cmd, IoKind::Stdout, &buf); Builtin::done(interp, cmd, 0) @@ -61,7 +60,8 @@ impl Basename { Self::state_mut(interp, cmd).state = State::Err; return Builtin::done(interp, cmd, 1); } - match Self::state_mut(interp, cmd).state { + let state = Self::state_mut(interp, cmd).state; + match state { State::Done => Builtin::done(interp, cmd, 0), State::Err => Builtin::done(interp, cmd, 1), State::Idle => unreachable!("Basename.onIOWriterChunk: idle"), diff --git a/src/runtime/shell/builtin/cat.rs b/src/runtime/shell/builtin/cat.rs index ef09d1f66a76..d2360b6955ae 100644 --- a/src/runtime/shell/builtin/cat.rs +++ b/src/runtime/shell/builtin/cat.rs @@ -1,11 +1,9 @@ -use std::sync::Arc; - use crate::shell::ExitCode; use crate::shell::builtin::{Builtin, BuiltinIO, BuiltinInput, BuiltinState, IoKind, Kind}; use crate::shell::interpreter::{ - FlagParser, Interpreter, NodeId, ParseFlagResult, parse_flags, shell_openat, unsupported_flag, + FlagParser, Interpreter, NodeId, ParseFlagResult, shell_openat, unsupported_flag, }; -use crate::shell::io_reader::{ChildPtr as ReaderChildPtr, IOReader, ReaderTag}; +use crate::shell::io_reader::{ChildPtr as ReaderChildPtr, IOReader, IOReaderRef, ReaderTag}; use crate::shell::io_writer::{ChildPtr, WriterTag}; use crate::shell::yield_::Yield; @@ -30,7 +28,7 @@ pub enum CatState { /// Current index into the filepath args. idx: usize, /// Per-file reader. - reader: Option>, + reader: Option, chunks_queued: usize, chunks_done: usize, out_done: bool, @@ -59,16 +57,12 @@ impl Step { impl Cat { pub(crate) fn start(interp: &Interpreter, cmd: NodeId) -> Yield { let mut opts = Opts::default(); - let filepath_start = { - let args = Builtin::of(interp, cmd).args_slice(); - match parse_flags(&mut opts, args) { - Ok(Some(rest)) => Some(args.len() - rest.len()), - Ok(None) => None, - Err(e) => { - return Builtin::fail_parse(interp, cmd, Kind::Cat, &e, || { - Self::state_mut(interp, cmd).state = CatState::WaitingWriteErr - }); - } + let filepath_start = match Builtin::parse_flags(interp, cmd, &mut opts) { + Ok(start) => start, + Err(e) => { + return Builtin::fail_parse(interp, cmd, Kind::Cat, &e, || { + Self::state_mut(interp, cmd).state = CatState::WaitingWriteErr + }); } }; @@ -103,12 +97,11 @@ impl Cat { buf: &[u8], exit_code: ExitCode, ) -> Yield { - if let Some(safeguard) = Builtin::of(interp, cmd).stderr.needs_io() { + let stderr_needs_io = Builtin::of(interp, cmd).stderr.needs_io(); + if let Some(safeguard) = stderr_needs_io { Self::state_mut(interp, cmd).state = CatState::WaitingWriteErr; let child = ChildPtr::new(cmd, WriterTag::Builtin); - return Builtin::of_mut(interp, cmd) - .stderr - .enqueue(child, buf, safeguard); + return Builtin::write_out(interp, cmd, IoKind::Stderr, child, buf, safeguard); } let _ = Builtin::write_no_io(interp, cmd, IoKind::Stderr, buf); Builtin::done(interp, cmd, exit_code) @@ -145,25 +138,30 @@ impl Cat { } // Copy stdin bytes so the &mut on `stdout`/`write_no_io` // doesn't overlap a borrow of `stdin`. - let buf = Builtin::read_stdin_no_io(interp, cmd).to_vec(); - if let Some(safeguard) = Builtin::of(interp, cmd).stdout.needs_io() { + let buf = Builtin::of(interp, cmd).read_stdin_no_io().to_vec(); + let stdout_needs_io = Builtin::of(interp, cmd).stdout.needs_io(); + if let Some(safeguard) = stdout_needs_io { let child = ChildPtr::new(cmd, WriterTag::Builtin); - return Builtin::of_mut(interp, cmd) - .stdout - .enqueue(child, &buf, safeguard); + return Builtin::write_out( + interp, + cmd, + IoKind::Stdout, + child, + &buf, + safeguard, + ); } let _ = Builtin::write_no_io(interp, cmd, IoKind::Stdout, &buf); return Builtin::done(interp, cmd, 0); } - // Clone the `Arc` + // Clone the `Rc` // out of `stdin` so we hold no borrow of `interp` across - // `start()` (which may re-enter via the raw interp backref). - let interp_ptr: *mut Interpreter = interp.as_ctx_ptr(); + // `start()` (which may re-enter via the interp backref). let reader = match &Builtin::of(interp, cmd).stdin { - BuiltinInput::Fd(r) => Arc::clone(r), + BuiltinInput::Fd(r) => r.clone(), _ => unreachable!("needs_io() returned true"), }; - reader.set_interp(interp_ptr); + reader.set_interp(interp); reader.add_reader(ReaderChildPtr { node: cmd, tag: ReaderTag::Cat, @@ -188,8 +186,6 @@ impl Cat { *reader = None; } - let path = Builtin::of(interp, cmd).arg_zstr(args_start + idx); - if let CatState::ExecFilepathArgs { idx: i, .. } = &mut Self::state_mut(interp, cmd).state { @@ -197,20 +193,23 @@ impl Cat { } let dir = Builtin::cwd(interp, cmd); - let fd = match shell_openat(dir, path, bun_sys::O::RDONLY, 0) { + let opened = { + let bltn = Builtin::of(interp, cmd); + let path = bltn.arg_zstr(args_start + idx); + shell_openat(dir, path, bun_sys::O::RDONLY, 0) + }; + let fd = match opened { Ok(fd) => fd, Err(e) => { - let buf = - Builtin::task_error_to_string(interp, cmd, Kind::Cat, &e).to_vec(); + let buf = Builtin::task_error_to_string(Kind::Cat, &e); // The reader was already taken to `None` above. return Self::write_failing_error(interp, cmd, &buf, 1); } }; let evtloop = Builtin::event_loop(interp, cmd); - let interp_ptr: *mut Interpreter = interp.as_ctx_ptr(); let reader = IOReader::init(fd, evtloop); - reader.set_interp(interp_ptr); + reader.set_interp(interp); if let CatState::ExecFilepathArgs { reader: slot, chunks_done, @@ -224,7 +223,7 @@ impl Cat { *chunks_queued = 0; *in_done = false; *out_done = false; - *slot = Some(Arc::clone(&reader)); + *slot = Some(reader.clone()); } reader.add_reader(ReaderChildPtr { node: cmd, @@ -249,29 +248,34 @@ impl Cat { tag: ReaderTag::Cat, }; // Writing to stdout errored: cancel everything and finish. - // Pull the reader `Arc` out of + // Pull the reader out of // state before calling `remove_reader`, then drop it. - match &mut Self::state_mut(interp, cmd).state { - CatState::ExecStdin { - in_done, - errno: st_errno, - .. - } => { - *st_errno = errno; - let was_done = core::mem::replace(in_done, true); - if !was_done { - if let BuiltinInput::Fd(r) = &Builtin::of(interp, cmd).stdin { - r.remove_reader(rchild); + { + let mut bltn = Builtin::of_mut(interp, cmd); + let bltn = &mut *bltn; + let stdin = &bltn.stdin; + match &mut Self::extract(&mut bltn.impl_).state { + CatState::ExecStdin { + in_done, + errno: st_errno, + .. + } => { + *st_errno = errno; + let was_done = core::mem::replace(in_done, true); + if !was_done { + if let BuiltinInput::Fd(r) = stdin { + r.remove_reader(rchild); + } } } - } - CatState::ExecFilepathArgs { reader, .. } => { - if let Some(r) = reader.take() { - r.remove_reader(rchild); + CatState::ExecFilepathArgs { reader, .. } => { + if let Some(r) = reader.take() { + r.remove_reader(rchild); + } } + CatState::WaitingWriteErr => {} + _ => panic!("Invalid state"), } - CatState::WaitingWriteErr => {} - _ => panic!("Invalid state"), } return Builtin::done(interp, cmd, errno); } @@ -321,18 +325,20 @@ impl Cat { ) -> Yield { *remove = false; let stdout_needs_io = Builtin::of(interp, cmd).stdout.needs_io(); - match &mut Self::state_mut(interp, cmd).state { - CatState::ExecStdin { chunks_queued, .. } - | CatState::ExecFilepathArgs { chunks_queued, .. } => { - if let Some(safeguard) = stdout_needs_io { - *chunks_queued += 1; - let child = ChildPtr::new(cmd, WriterTag::Builtin); - return Builtin::of_mut(interp, cmd) - .stdout - .enqueue(child, chunk, safeguard); - } + if let Some(safeguard) = stdout_needs_io { + match &mut Self::state_mut(interp, cmd).state { + CatState::ExecStdin { chunks_queued, .. } + | CatState::ExecFilepathArgs { chunks_queued, .. } => *chunks_queued += 1, + _ => panic!("Invalid state"), } - _ => panic!("Invalid state"), + let child = ChildPtr::new(cmd, WriterTag::Builtin); + return Builtin::write_out(interp, cmd, IoKind::Stdout, child, chunk, safeguard); + } + if !matches!( + Self::state_mut(interp, cmd).state, + CatState::ExecStdin { .. } | CatState::ExecFilepathArgs { .. } + ) { + panic!("Invalid state"); } let _ = Builtin::write_no_io(interp, cmd, IoKind::Stdout, chunk); Yield::done() @@ -421,9 +427,7 @@ impl FlagParser for Opts { b't' => Some(ParseFlagResult::Unsupported(unsupported_flag(b"-t"))), b'u' => Some(ParseFlagResult::Unsupported(unsupported_flag(b"-u"))), b'v' => Some(ParseFlagResult::Unsupported(unsupported_flag(b"-v"))), - _ => Some(ParseFlagResult::IllegalOption( - &raw const smallflags[1 + i..], - )), + _ => Some(ParseFlagResult::IllegalOption(smallflags[1 + i..].into())), } } } diff --git a/src/runtime/shell/builtin/cd.rs b/src/runtime/shell/builtin/cd.rs index bd000954d397..b5907e213906 100644 --- a/src/runtime/shell/builtin/cd.rs +++ b/src/runtime/shell/builtin/cd.rs @@ -22,8 +22,8 @@ enum State { impl Cd { pub(crate) fn start(interp: &Interpreter, cmd: NodeId) -> Yield { - let args = Builtin::of(interp, cmd).args_slice(); - if args.len() > 1 { + let argc = Builtin::of(interp, cmd).args_slice().len(); + if argc > 1 { return Self::write_stderr_non_blocking( interp, cmd, @@ -31,8 +31,9 @@ impl Cd { ); } - if args.is_empty() { - let home_str = Builtin::shell(interp, cmd).get_homedir(); + let shell = Builtin::shell(interp, cmd); + if argc == 0 { + let home_str = shell.borrow().get_homedir(); let home = home_str.slice().to_vec(); home_str.deref(); if home.is_empty() { @@ -42,21 +43,24 @@ impl Cd { format_args!("HOME not set\n"), ); } - if let Err(err) = interp.as_cmd_mut(cmd).base.shell_mut().change_cwd(&home) { + let res = shell.borrow_mut().change_cwd(&home); + if let Err(err) = res { return Self::handle_change_cwd_err(interp, cmd, &err, &home); } return Builtin::done(interp, cmd, 0); } - let first_arg = Builtin::of(interp, cmd).arg_bytes(0); + let first_arg = Builtin::of(interp, cmd).arg_bytes(0).to_vec(); if first_arg == b"-" { - let prev = Builtin::shell(interp, cmd).prev_cwd().to_vec(); - if let Err(err) = interp.as_cmd_mut(cmd).base.shell_mut().change_prev_cwd() { + let prev = shell.borrow().prev_cwd().to_vec(); + let res = shell.borrow_mut().change_prev_cwd(); + if let Err(err) = res { return Self::handle_change_cwd_err(interp, cmd, &err, &prev); } } else { - let target = first_arg.to_vec(); - if let Err(err) = interp.as_cmd_mut(cmd).base.shell_mut().change_cwd(&target) { + let target = first_arg; + let res = shell.borrow_mut().change_cwd(&target); + if let Err(err) = res { return Self::handle_change_cwd_err(interp, cmd, &err, &target); } } @@ -102,16 +106,20 @@ impl Cd { args: core::fmt::Arguments<'_>, ) -> Yield { Self::state_mut(interp, cmd).state = State::WaitingIo; - if let Some(safeguard) = Builtin::of(interp, cmd).stderr.needs_io() { + let stderr_needs_io = Builtin::of(interp, cmd).stderr.needs_io(); + if let Some(safeguard) = stderr_needs_io { let child = ChildPtr::new(cmd, WriterTag::Builtin); - return Builtin::of_mut(interp, cmd).stderr.enqueue_fmt( + return Builtin::write_out_fmt( + interp, + cmd, + IoKind::Stderr, child, Some(Kind::Cd), args, safeguard, ); } - let buf = Builtin::fmt_error_arena(interp, cmd, Some(Kind::Cd), args).to_vec(); + let buf = Builtin::fmt_error_arena(Some(Kind::Cd), args); let _ = Builtin::write_no_io(interp, cmd, IoKind::Stderr, &buf); Self::state_mut(interp, cmd).state = State::Done; Builtin::done(interp, cmd, 1) diff --git a/src/runtime/shell/builtin/cp.rs b/src/runtime/shell/builtin/cp.rs index f51257afdfcd..228850be08cb 100644 --- a/src/runtime/shell/builtin/cp.rs +++ b/src/runtime/shell/builtin/cp.rs @@ -3,8 +3,8 @@ use bun_paths::resolve_path; use crate::node::PathLike; use crate::shell::builtin::{Builtin, BuiltinState, IoKind, Kind}; use crate::shell::interpreter::{ - EventLoopHandle, FlagParser, Interpreter, NodeId, OutputSrc, OutputTask, OutputTaskVTable, - ParseFlagResult, ShellTask, parse_flags, unsupported_flag, + FlagParser, Interpreter, NodeId, OutputSrc, OutputTask, OutputTaskVTable, OutputWrite, + ParseFlagResult, ShellTask, unsupported_flag, }; use crate::shell::io_writer::{ChildPtr, WriterTag}; use crate::shell::yield_::Yield; @@ -14,12 +14,11 @@ use crate::shell::{ExitCode, ShellErr}; pub struct Cp { pub(crate) opts: Opts, pub(crate) state: State, - /// FIFO of in-flight OutputTask pointers awaiting an IOWriter chunk - /// completion. Stopgap until `WriterTag` can carry the `*mut OutputTask` - /// directly (see mkdir.rs `Exec::output_queue`). Lives on `Cp` (not - /// `ExecState`) because `print_shell_cp_task` is also driven from - /// `State::Ebusy` on Windows; both states must be able to stash/pop. - pub(crate) output_queue: std::collections::VecDeque<*mut OutputTask>, + /// FIFO of in-flight OutputTasks awaiting an IOWriter chunk completion + /// (see mkdir.rs `Exec::output_queue`). Lives on `Cp` (not `ExecState`) + /// because `print_shell_cp_task` is also driven from `State::Ebusy` on + /// Windows; both states must be able to park/pop. + pub(crate) output_queue: std::collections::VecDeque>>, } #[derive(Default)] @@ -55,7 +54,9 @@ pub struct ExecState { #[cfg(windows)] #[derive(Default)] pub struct EbusyState { - pub(crate) tasks: Vec<*mut ShellCpTask>, + /// Deferred EBUSY tasks; `None` once `ignore_ebusy_error_if_possible` has + /// consumed the slot. + pub(crate) tasks: Vec>>, pub(crate) idx: usize, pub(crate) main_exit_code: ExitCode, /// Absolute target paths that some task copied successfully — used to @@ -68,12 +69,9 @@ impl Cp { pub(crate) fn start(interp: &Interpreter, cmd: NodeId) -> Yield { let mut opts = Opts::default(); let (sources_start, target_idx) = { - let args = Builtin::of(interp, cmd).args_slice(); - match parse_flags(&mut opts, args) { - Ok(Some(rest)) if rest.len() > 1 => { - let start = args.len() - rest.len(); - (start, args.len() - 1) - } + let argc = Builtin::argc(interp, cmd); + match Builtin::parse_flags(interp, cmd, &mut opts) { + Ok(Some(start)) if argc - start > 1 => (start, argc - 1), Ok(_) => { Self::state_mut(interp, cmd).state = State::WaitingWriteErr; return Builtin::write_failing_error(interp, cmd, Kind::Cp.usage_string(), 1); @@ -109,6 +107,11 @@ impl Cp { }, #[cfg(windows)] Ebusy(ExitCode), + #[cfg(windows)] + IgnoreEbusy, + Suspend, + Failed, + AlreadyDone, } let action = match &mut Self::state_mut(interp, cmd).state { State::Idle => panic!( @@ -130,7 +133,7 @@ impl Cp { let act = Action::Done(exit_code); act } else { - return Yield::suspended(); + Action::Suspend } } else { exec.started = true; @@ -143,47 +146,51 @@ impl Cp { } } #[cfg(windows)] - State::Ebusy(_) => return Self::ignore_ebusy_error_if_possible(interp, cmd), - State::WaitingWriteErr => return Yield::failed(), - State::Done => return Builtin::done(interp, cmd, 0), + State::Ebusy(_) => Action::IgnoreEbusy, + State::WaitingWriteErr => Action::Failed, + State::Done => Action::AlreadyDone, }; match action { + Action::Suspend => Yield::suspended(), + Action::Failed => Yield::failed(), + Action::AlreadyDone => Builtin::done(interp, cmd, 0), + #[cfg(windows)] + Action::IgnoreEbusy => Self::ignore_ebusy_error_if_possible(interp, cmd), Action::Done(code) => { Self::state_mut(interp, cmd).state = State::Done; Builtin::done(interp, cmd, code) } #[cfg(windows)] Action::Ebusy(exit_code) => { - let State::Exec(exec) = &mut Self::state_mut(interp, cmd).state else { - unreachable!() - }; - let mut ebusy = core::mem::take(&mut exec.ebusy); - ebusy.idx = 0; - ebusy.main_exit_code = exit_code; - Self::state_mut(interp, cmd).state = State::Ebusy(ebusy); + { + let mut me = Self::state_mut(interp, cmd); + let State::Exec(exec) = &mut me.state else { + unreachable!() + }; + let mut ebusy = core::mem::take(&mut exec.ebusy); + ebusy.idx = 0; + ebusy.main_exit_code = exit_code; + me.state = State::Ebusy(ebusy); + } Self::ignore_ebusy_error_if_possible(interp, cmd) } Action::Schedule { start, target } => { - let cwd = Builtin::shell(interp, cmd).cwd().to_vec(); + let cwd = Builtin::shell(interp, cmd).borrow().cwd().to_vec(); let opts = Self::state_mut(interp, cmd).opts; - let evtloop = Builtin::event_loop(interp, cmd); let tgt = Builtin::of(interp, cmd).arg_bytes(target).to_vec(); let operands = 1 + (target - start); - let interp_ptr = interp.as_ctx_ptr(); for i in start..target { let src = Builtin::of(interp, cmd).arg_bytes(i).to_vec(); let task = ShellCpTask::create( cmd, - evtloop, opts, operands, src, tgt.clone(), cwd.clone(), - interp_ptr, + interp, ); - // SAFETY: freshly heap-allocated. - unsafe { ShellCpTask::schedule(task) }; + ShellTask::schedule(task); } Yield::suspended() } @@ -199,10 +206,9 @@ impl Cp { if matches!(Self::state_mut(interp, cmd).state, State::WaitingWriteErr) { return Builtin::done(interp, cmd, 1); } - if let Some(task) = Self::state_mut(interp, cmd).output_queue.pop_front() { - // SAFETY: `task` was heap-allocated in `OutputTask::new` and - // pushed by `write_err`/`write_out`; not yet freed. - return unsafe { OutputTask::::on_io_writer_chunk(task, interp, written, e) }; + let pending = Self::state_mut(interp, cmd).output_queue.pop_front(); + if let Some(task) = pending { + return OutputTask::::on_io_writer_chunk(task, interp, written, e); } Self::next(interp, cmd) } @@ -221,17 +227,13 @@ impl Cp { unreachable!() }; if eb.idx < eb.tasks.len() { - let t = eb.tasks[eb.idx]; + let t = eb.tasks[eb.idx].take().expect("ebusy task consumed once"); eb.idx += 1; - // SAFETY: `t` is a live heap-allocated task stashed in - // `on_shell_cp_task_done`; not yet freed. - let tref = unsafe { &*t }; - let ignorable = tref + let ignorable = t .tgt_absolute .as_ref() .map_or(false, |p| eb.absolute_targets.contains(p)) - || tref - .src_absolute + || t.src_absolute .as_ref() .map_or(false, |p| eb.absolute_srcs.contains(p)); Some((t, ignorable)) @@ -240,40 +242,35 @@ impl Cp { } }; match next { - Some((t, true)) => { - // SAFETY: paired with `heap::alloc` in `create()`. - drop(unsafe { bun_core::heap::take(t) }); - } - // SAFETY: `t` is a live heap task stashed in - // `on_shell_cp_task_done`; reclaim ownership. - Some((t, false)) => { - return Self::print_shell_cp_task(interp, cmd, unsafe { - bun_core::heap::take(t) - }); - } + Some((t, true)) => drop(t), + Some((t, false)) => return Self::print_shell_cp_task(interp, cmd, t), None => break, } } - let State::Ebusy(eb) = &mut Self::state_mut(interp, cmd).state else { - unreachable!() + let exit_code = { + let me = Self::state_mut(interp, cmd); + let State::Ebusy(eb) = &me.state else { + unreachable!() + }; + eb.main_exit_code }; - let exit_code = eb.main_exit_code; // `Drop` frees the ebusy sets/vec here. Self::state_mut(interp, cmd).state = State::Done; Builtin::done(interp, cmd, exit_code) } - fn on_shell_cp_task_done(interp: &Interpreter, cmd: NodeId, task: *mut ShellCpTask) { + fn on_shell_cp_task_done( + interp: &Interpreter, + cmd: NodeId, + #[cfg_attr(not(windows), allow(unused_mut))] mut task: Box, + ) { if let State::Exec(exec) = &mut Self::state_mut(interp, cmd).state { exec.tasks_count -= 1; } - // SAFETY: `task` was heap-allocated in `create()`; ownership transfers - // to this completion callback. Re-leaked below only on the EBUSY-defer path. - #[cfg_attr(not(windows), allow(unused_mut))] - let mut task = unsafe { bun_core::heap::take(task) }; #[cfg(windows)] { - if let State::Exec(exec) = &mut Self::state_mut(interp, cmd).state { + let mut me = Self::state_mut(interp, cmd); + if let State::Exec(exec) = &mut me.state { if let Some(err) = &task.err { // Defer the task to the ebusy phase. Note the precedence: // `(is_sys && errno==EBUSY && tgt_match) || src_match` @@ -287,7 +284,8 @@ impl Cp { || task.src_absolute.as_deref() .map_or(false, |p| sys.path.eql_utf8(p))); if is_ebusy { - exec.ebusy.tasks.push(bun_core::heap::into_raw(task)); + exec.ebusy.tasks.push(Some(task)); + drop(me); return Self::next(interp, cmd).run(interp); } } else { @@ -301,6 +299,7 @@ impl Cp { } } } + drop(me); } Self::print_shell_cp_task(interp, cmd, task).run(interp); } @@ -316,7 +315,7 @@ impl Cp { let output_task = OutputTask::::new(cmd, OutputSrc::Arrlist(output)); let errstr: Option> = task.err.take().map(|e| { - let s = Builtin::shell_err_to_string(interp, cmd, Kind::Cp, &e).to_vec(); + let s = Builtin::shell_err_to_string(Kind::Cp, &e); if let State::Exec(exec) = &mut Self::state_mut(interp, cmd).state { exec.err = Some(e); } @@ -331,25 +330,29 @@ impl OutputTaskVTable for Cp { fn write_err( interp: &Interpreter, cmd: NodeId, - child: *mut OutputTask, + child: Box>, errbuf: &[u8], - ) -> Option { + ) -> OutputWrite { if let State::Exec(exec) = &mut Self::state_mut(interp, cmd).state { exec.output_waiting += 1; } - if let Some(safeguard) = Builtin::of(interp, cmd).stderr.needs_io() { - // Stash so on_io_writer_chunk can route to the OutputTask state - // machine and reclaim the box (stopgap for missing WriterTag). + let stderr_needs_io = Builtin::of(interp, cmd).stderr.needs_io(); + if let Some(safeguard) = stderr_needs_io { + // Park it so on_io_writer_chunk can route the completion back to + // the OutputTask state machine (there is no WriterTag for it). Self::state_mut(interp, cmd).output_queue.push_back(child); let childptr = ChildPtr::new(cmd, WriterTag::Builtin); - return Some( - Builtin::of_mut(interp, cmd) - .stderr - .enqueue(childptr, errbuf, safeguard), - ); + return OutputWrite::Enqueued(Builtin::write_out( + interp, + cmd, + IoKind::Stderr, + childptr, + errbuf, + safeguard, + )); } let _ = Builtin::write_no_io(interp, cmd, IoKind::Stderr, errbuf); - None + OutputWrite::Done(child) } fn on_write_err(interp: &Interpreter, cmd: NodeId) { if let State::Exec(exec) = &mut Self::state_mut(interp, cmd).state { @@ -359,25 +362,28 @@ impl OutputTaskVTable for Cp { fn write_out( interp: &Interpreter, cmd: NodeId, - child: *mut OutputTask, - output: &mut OutputSrc, - ) -> Option { + child: Box>, + ) -> OutputWrite { if let State::Exec(exec) = &mut Self::state_mut(interp, cmd).state { exec.output_waiting += 1; } - if let Some(safeguard) = Builtin::of(interp, cmd).stdout.needs_io() { - Self::state_mut(interp, cmd).output_queue.push_back(child); + let stdout_needs_io = Builtin::of(interp, cmd).stdout.needs_io(); + if let Some(safeguard) = stdout_needs_io { let childptr = ChildPtr::new(cmd, WriterTag::Builtin); - let buf = output.slice().to_vec(); - return Some( - Builtin::of_mut(interp, cmd) - .stdout - .enqueue(childptr, &buf, safeguard), - ); + return OutputWrite::Enqueued(Builtin::write_out_with( + interp, + cmd, + IoKind::Stdout, + childptr, + safeguard, + |buf| { + buf.extend_from_slice(child.output.slice()); + Self::state_mut(interp, cmd).output_queue.push_back(child); + }, + )); } - let buf = output.slice().to_vec(); - let _ = Builtin::write_no_io(interp, cmd, IoKind::Stdout, &buf); - None + let _ = Builtin::write_no_io(interp, cmd, IoKind::Stdout, child.output.slice()); + OutputWrite::Done(child) } fn on_write_out(interp: &Interpreter, cmd: NodeId) { if let State::Exec(exec) = &mut Self::state_mut(interp, cmd).state { @@ -411,18 +417,23 @@ pub struct ShellCpTask { pub task: ShellTask, } +// The pool-side body is `run_on_pool`, not `ShellTask::run_owned`: on the +// success path the copy is handed to a `ShellAsyncCpTask` that posts the +// bounce-back itself (`cp_on_finish`), so an unconditional post would +// double-enqueue. +crate::shell_task!(ShellCpTask, run = ShellCpTask::run_on_pool); + impl ShellCpTask { fn create( cmd: NodeId, - evtloop: EventLoopHandle, opts: Opts, operands: usize, src: Vec, tgt: Vec, cwd_path: Vec, - interp: *mut Interpreter, - ) -> *mut ShellCpTask { - let mut task = Box::new(ShellCpTask { + interp: &Interpreter, + ) -> Box { + Box::new(ShellCpTask { cmd, opts, operands, @@ -433,12 +444,8 @@ impl ShellCpTask { cwd_path, verbose_output: bun_threading::Guarded::new(Vec::new()), err: None, - task: ShellTask::new(evtloop), - }); - // Back-ref so `ShellTask::run_from_main_thread::` (the - // dispatch.rs bounce-back) can recover `&Interpreter`. - task.task.interp = interp; - bun_core::heap::into_raw(task) + task: ShellTask::new(interp), + }) } /// Appends `"{src} -> {dest}\n"` to the verbose @@ -476,15 +483,13 @@ impl ShellCpTask { } } - /// 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. + /// Called on the JS thread when the node:fs async cp completes (success or + /// first error). Records the error (if any) and finishes this `ShellCpTask` + /// in place so the interpreter can drain `verbose_output` / surface it. /// /// # Safety - /// `this` is the live `heap::alloc`'d task originally passed to - /// [`schedule`](Self::schedule); not touched again on this thread after - /// return. + /// `this` is the live task `run_on_pool` released to the + /// `ShellAsyncCpTask`; reclaimed here, not touched by the caller after. pub(crate) unsafe fn cp_on_finish( this: *mut ShellCpTask, src: PathLike<'static>, @@ -492,58 +497,30 @@ impl ShellCpTask { 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. - unsafe { - (*this).src_absolute = Some(src.into_vec()); - (*this).tgt_absolute = Some(dest.into_vec()); - if let Err(e) = result { - (*this).err = Some(ShellErr::new_sys(&e)); - } - ShellTask::run_from_main_thread::(this); - } - } - - /// 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 - /// `WorkPoolTask` / `concurrent_task` / `keep_alive` storage. - /// - /// # Safety - /// `this` must be a fresh `heap::alloc`'d task (see [`create`]). - unsafe fn schedule(this: *mut ShellCpTask) { - use bun_threading::work_pool::WorkPool; - // SAFETY: `this` is live; `task` is the embedded `ShellTask`. Stay on - // raw pointers — once `WorkPool::schedule` returns the worker thread - // may already be running. - unsafe { - let st = &raw mut (*this).task; - (*st).task.callback = Self::work_pool_callback; - (*st).keep_alive.ref_((*st).event_loop.as_event_loop_ctx()); - (*st).arm(); - WorkPool::schedule(&raw mut (*st).task); + // completion; `this` is live and ours (the box `run_on_pool` released + // to it). 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 mut this = unsafe { bun_core::heap::take(this) }; + this.src_absolute = Some(src.into_vec()); + this.tgt_absolute = Some(dest.into_vec()); + if let Err(e) = result { + this.err = Some(ShellErr::new_sys(&e)); } + ShellTask::run_from_main_thread::(this); } - /// Recover `*ShellCpTask` from the - /// intrusive `*WorkPoolTask`, run the impl, and on error post back - /// immediately (success path defers the post to `cp_on_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 - // heap-allocated task; the worker thread has exclusive access until - // the bounce-back is posted. + /// Pool-side body: run the impl, and on error post back immediately; + /// the success path hands the allocation to a `ShellAsyncCpTask`, whose + /// completion (`cp_on_finish`) reclaims it. + fn run_on_pool(this: Box) { + let this = bun_core::heap::into_raw(this); + // SAFETY: `this` is the box just released; the worker thread has + // exclusive access until it is handed off. Raw because on success the + // `ShellAsyncCpTask` it now belongs to may free it from another thread + // at once. unsafe { - let this = bun_ptr::container_of::( - task, - ::TASK_OFFSET, - ); - // Moved out first: on success the copy is handed to a - // `ShellAsyncCpTask` whose completion may free `*this` at once. + // Moved out first: see above. let poster = (*this) .task .poster @@ -552,7 +529,7 @@ impl ShellCpTask { 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); + ShellTask::on_finish::(bun_core::heap::take(this)); } else { // The copy now belongs to a `ShellAsyncCpTask` (holding its // own poster, completing on the JS thread via `cp_on_finish`); @@ -562,18 +539,6 @@ impl ShellCpTask { } } - /// Post this task to the main-thread - /// concurrent queue; routed by `dispatch.rs` → [`run_from_main_thread`]. - /// - /// # Safety - /// `this` is the live `heap::alloc`'d task; not touched again on this - /// thread after return. - unsafe fn enqueue_to_event_loop(this: *mut ShellCpTask) { - // Reuse the generic `ShellTask` post-back. - // SAFETY: caller contract. - unsafe { ShellTask::on_finish::(this) }; - } - fn has_trailing_sep(path: &[u8]) -> bool { path.last() .is_some_and(|&c| resolve_path::Platform::AUTO.is_separator(c)) @@ -737,46 +702,27 @@ impl ShellCpTask { None } - - /// # Safety - /// `this` must be a live `heap::alloc`'d task (see [`create`](Self::create)); - /// ownership is consumed via [`Cp::on_shell_cp_task_done`]. - fn run_from_main_thread(this: *mut ShellCpTask, interp: &Interpreter) { - // SAFETY: `this` is a live heap-allocated task per the caller's contract. - let cmd = unsafe { (*this).cmd }; - Cp::on_shell_cp_task_done(interp, cmd, this); - } } -impl bun_event_loop::Taskable for ShellCpTask { - const TAG: bun_event_loop::TaskTag = bun_event_loop::task_tag::ShellCpTask; - /// A pool completion that will not run: drop the keep-alive and the box - /// (nothing else frees an unrun one). - unsafe fn release_unrun(this: *mut Self) { - // SAFETY: fn contract — the box the builtin scheduled. - unsafe { - (*this).task.unref_unrun(); - drop(bun_core::heap::take(this)); - } - } -} +// `runtime::dispatch::run_task`'s `task_tag::ShellCpTask` arm reboxes the +// pointer `ShellTask::on_finish` posted; a completion that will not run drops +// the keep-alive and the box. impl crate::shell::interpreter::ShellTaskCtx for ShellCpTask { - const TASK_OFFSET: usize = core::mem::offset_of!(Self, task); - fn run_from_thread_pool(_this: &mut Self) { - // 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`). - debug_assert!( - false, - "ShellCpTask scheduled via ShellTask::schedule; use ShellCpTask::schedule" - ); + fn shell_task(&self) -> &ShellTask { + &self.task + } + fn shell_task_mut(&mut self) -> &mut ShellTask { + &mut self.task + } + fn run_from_thread_pool(&mut self) { + // Not reached: the pool-side entry is `run_on_pool` (see the + // `owned_task!` above), never `ShellTask::run_owned`. + debug_assert!(false, "ShellCpTask runs run_on_pool on the pool"); } - fn run_from_main_thread(this: *mut Self, interp: &Interpreter) { - // SAFETY: `ShellTask::run_from_main_thread` dispatch contract — `this` - // is the live heap-allocated task posted via `ShellTask::schedule`. - Self::run_from_main_thread(this, interp) + fn run_from_main_thread(self: Box, interp: &Interpreter) { + let cmd = self.cmd; + Cp::on_shell_cp_task_done(interp, cmd, self); } } @@ -810,7 +756,7 @@ impl FlagParser for Opts { Some(ParseFlagResult::ContinueParsing) } b'n' => Some(ParseFlagResult::ContinueParsing), - _ => Some(ParseFlagResult::IllegalOption(&raw const smallflags[i..])), + _ => Some(ParseFlagResult::IllegalOption(smallflags[i..].into())), } } } diff --git a/src/runtime/shell/builtin/dirname.rs b/src/runtime/shell/builtin/dirname.rs index 0b7dd1fe5b57..704077f6e114 100644 --- a/src/runtime/shell/builtin/dirname.rs +++ b/src/runtime/shell/builtin/dirname.rs @@ -19,12 +19,12 @@ enum State { impl Dirname { pub(crate) fn start(interp: &Interpreter, cmd: NodeId) -> Yield { - let bltn = Builtin::of(interp, cmd); - let argc = bltn.args_slice().len(); + let argc = Builtin::argc(interp, cmd); if argc == 0 { return Self::fail(interp, cmd, b"usage: dirname string\n"); } + let bltn = Builtin::of(interp, cmd); let stdout_needs_io = bltn.stdout.needs_io(); let mut buf = Vec::new(); for i in 0..argc { @@ -34,15 +34,14 @@ impl Dirname { buf.extend_from_slice(dir); buf.push(b'\n'); } + drop(bltn); Self::state_mut(interp, cmd).state = State::Done; if let Some(safeguard) = stdout_needs_io { Self::state_mut(interp, cmd).buf = buf; let owned = Self::state_mut(interp, cmd).buf.clone(); let child = ChildPtr::new(cmd, WriterTag::Builtin); - return Builtin::of_mut(interp, cmd) - .stdout - .enqueue(child, &owned, safeguard); + return Builtin::write_out(interp, cmd, IoKind::Stdout, child, &owned, safeguard); } let _ = Builtin::write_no_io(interp, cmd, IoKind::Stdout, &buf); Builtin::done(interp, cmd, 0) diff --git a/src/runtime/shell/builtin/echo.rs b/src/runtime/shell/builtin/echo.rs index 625863a9b7af..4e13212e8c3e 100644 --- a/src/runtime/shell/builtin/echo.rs +++ b/src/runtime/shell/builtin/echo.rs @@ -89,13 +89,13 @@ impl Echo { }; Self::state_mut(interp, cmd).output = output; - if let Some(safeguard) = Builtin::of(interp, cmd).stdout.needs_io() { + let stdout_needs_io = Builtin::of(interp, cmd).stdout.needs_io(); + + if let Some(safeguard) = stdout_needs_io { Self::state_mut(interp, cmd).state = State::WaitingIo; let buf = Self::state_mut(interp, cmd).output.clone(); let child = ChildPtr::new(cmd, WriterTag::Builtin); - return Builtin::of_mut(interp, cmd) - .stdout - .enqueue(child, &buf, safeguard); + return Builtin::write_out(interp, cmd, IoKind::Stdout, child, &buf, safeguard); } let buf = Self::state_mut(interp, cmd).output.clone(); let _ = Builtin::write_no_io(interp, cmd, IoKind::Stdout, &buf); diff --git a/src/runtime/shell/builtin/exit.rs b/src/runtime/shell/builtin/exit.rs index 767cbe4b2bd7..e350edef8a3b 100644 --- a/src/runtime/shell/builtin/exit.rs +++ b/src/runtime/shell/builtin/exit.rs @@ -17,22 +17,19 @@ enum State { impl Exit { pub(crate) fn start(interp: &Interpreter, cmd: NodeId) -> Yield { - let bltn = Builtin::of(interp, cmd); - let code: crate::shell::ExitCode = match bltn.args_slice().len() { - 0 => 0, - 1 => { - let s = bltn.arg_bytes(0); - match parse_exit_code(s) { - Some(c) => c, - None => { - return Self::fail(interp, cmd, b"exit: numeric argument required\n"); - } - } - } - _ => { - return Self::fail(interp, cmd, b"exit: too many arguments\n"); + let code: Result = { + let bltn = Builtin::of(interp, cmd); + match bltn.args_slice().len() { + 0 => Ok(0), + 1 => parse_exit_code(bltn.arg_bytes(0)) + .ok_or(b"exit: numeric argument required\n".as_slice()), + _ => Err(b"exit: too many arguments\n".as_slice()), } }; + let code = match code { + Ok(c) => c, + Err(msg) => return Self::fail(interp, cmd, msg), + }; // Intentional divergence from bash: this completes only the current // Cmd rather than unwinding the whole script. Builtin::done(interp, cmd, code) diff --git a/src/runtime/shell/builtin/export.rs b/src/runtime/shell/builtin/export.rs index 29d6b0e5c71b..78ca23836cc4 100644 --- a/src/runtime/shell/builtin/export.rs +++ b/src/runtime/shell/builtin/export.rs @@ -25,8 +25,10 @@ impl Export { // No args: print all exported vars. return Self::print_all(interp, cmd); } + let shell = Builtin::shell(interp, cmd); for i in 0..argc { - let s = Builtin::of(interp, cmd).arg_bytes(i); + let bltn = Builtin::of(interp, cmd); + let s = bltn.arg_bytes(i); if s.is_empty() { continue; } @@ -39,9 +41,8 @@ impl Export { // `init_slice` here would leave dangling EnvStr in `export_env`. let label = EnvStr::dupe_ref_counted(name); let val = EnvStr::dupe_ref_counted(value); - let shell = interp.as_cmd(cmd).base.shell; - // SAFETY: shell env outlives the Cmd node. - unsafe { (*shell).export_env.insert(label, val) }; + drop(bltn); + shell.borrow_mut().export_env.insert(label, val); label.deref(); val.deref(); } @@ -50,6 +51,7 @@ impl Export { fn print_all(interp: &Interpreter, cmd: NodeId) -> Yield { let mut entries: Vec<(EnvStr, EnvStr)> = Builtin::shell(interp, cmd) + .borrow() .export_env .iter() .map(|(k, v)| (*k, *v)) @@ -64,12 +66,12 @@ impl Export { buf.push(b'\n'); } - if let Some(safeguard) = Builtin::of(interp, cmd).stdout.needs_io() { + let stdout_needs_io = Builtin::of(interp, cmd).stdout.needs_io(); + + if let Some(safeguard) = stdout_needs_io { Self::state_mut(interp, cmd).state = State::WaitingIo; let child = ChildPtr::new(cmd, WriterTag::Builtin); - return Builtin::of_mut(interp, cmd) - .stdout - .enqueue(child, &buf, safeguard); + return Builtin::write_out(interp, cmd, IoKind::Stdout, child, &buf, safeguard); } let _ = Builtin::write_no_io(interp, cmd, IoKind::Stdout, &buf); Builtin::done(interp, cmd, 0) diff --git a/src/runtime/shell/builtin/ls.rs b/src/runtime/shell/builtin/ls.rs index 082c052687a8..59ba629117e8 100644 --- a/src/runtime/shell/builtin/ls.rs +++ b/src/runtime/shell/builtin/ls.rs @@ -1,4 +1,4 @@ -use core::ptr::NonNull; +use core::cell::RefMut; use core::sync::atomic::{AtomicUsize, Ordering}; use std::io::Write as _; @@ -8,8 +8,8 @@ use bun_sys::{E, FdExt, O, S, dir_iterator}; use crate::shell::ExitCode; use crate::shell::builtin::{Builtin, IoKind, Kind}; use crate::shell::interpreter::{ - EventLoopHandle, Interpreter, NodeId, OutputSrc, OutputTask, OutputTaskVTable, ShellTask, - shell_lstatat, shell_openat, shell_statat, + EventLoopHandle, Interpreter, NodeId, OutputSrc, OutputTask, OutputTaskVTable, OutputWrite, + ShellTask, shell_lstatat, shell_openat, shell_statat, }; use crate::shell::io_writer::{ChildPtr, WriterTag}; use crate::shell::yield_::Yield; @@ -35,10 +35,9 @@ pub struct ExecState { pub(crate) tasks_done: usize, pub(crate) output_waiting: usize, pub(crate) output_done: usize, - /// FIFO of in-flight OutputTask pointers awaiting an IOWriter chunk - /// completion. Stopgap until `WriterTag` can carry the `*mut OutputTask` - /// directly — see mkdir.rs `Exec::output_queue` for rationale. - pub(crate) output_queue: std::collections::VecDeque<*mut OutputTask>, + /// FIFO of in-flight OutputTasks awaiting an IOWriter chunk completion — + /// see mkdir.rs `Exec::output_queue` for rationale. + pub(crate) output_queue: std::collections::VecDeque>>, } enum ParseFlag { @@ -74,18 +73,15 @@ impl Ls { Ok(p) => p, Err(opt) => { let buf: Vec = Builtin::fmt_error_arena( - interp, - cmd, Some(Kind::Ls), format_args!("illegal option -- {}\n", bstr::BStr::new(&opt[..])), - ) - .to_vec(); + ); Self::state_mut(interp, cmd).state = State::WaitingWriteErr; return Builtin::write_failing_error(interp, cmd, &buf, 1); } }; - let argc = Builtin::of(interp, cmd).args_slice().len(); + let argc = Builtin::argc(interp, cmd); let task_count = match paths_start { Some(start) => argc - start, None => 1, @@ -101,35 +97,32 @@ impl Ls { // Stable address: `Ls` lives in `Box` (Builtin::Impl::Ls), // and the `Exec` variant is held until all tasks finish. - let task_count_ptr: *const AtomicUsize = { - let State::Exec(exec) = &Self::state_mut(interp, cmd).state else { + let task_count_ptr: bun_ptr::BackRef = { + let me = Self::state_mut(interp, cmd); + let State::Exec(exec) = &me.state else { unreachable!() }; - &raw const exec.task_count + bun_ptr::BackRef::new(&exec.task_count) }; let cwd = Builtin::cwd(interp, cmd); let opts = Self::state_mut(interp, cmd).opts; let evtloop = Builtin::event_loop(interp, cmd); - let interp_ptr = interp.as_ctx_ptr(); if let Some(start) = paths_start { let print_directory = task_count > 1; for i in start..argc { - let path = Builtin::of(interp, cmd).arg_bytes(i); - let task = ShellLsTask::create( + let path = ZBox::from_bytes(Builtin::of(interp, cmd).arg_bytes(i)); + let mut task = ShellLsTask::create( cmd, opts, task_count_ptr, cwd, - ZBox::from_bytes(path), + path, evtloop, - interp_ptr, + interp, ); - // SAFETY: freshly heap-allocated. - unsafe { - (*task).print_directory = print_directory; - ShellTask::schedule_no_ref::(task); - } + task.print_directory = print_directory; + ShellTask::schedule_no_ref(task); } } else { let task = ShellLsTask::create( @@ -139,10 +132,9 @@ impl Ls { cwd, ZBox::from_bytes(b"."), evtloop, - interp_ptr, + interp, ); - // SAFETY: freshly heap-allocated. - unsafe { ShellTask::schedule_no_ref::(task) }; + ShellTask::schedule_no_ref(task); } Yield::suspended() } @@ -188,19 +180,12 @@ impl Ls { None }; if let Some(task) = pending { - // SAFETY: `task` was heap-allocated in `OutputTask::new` and - // pushed by `write_err`/`write_out`; not yet freed. - return unsafe { OutputTask::::on_io_writer_chunk(task, interp, written, e) }; + return OutputTask::::on_io_writer_chunk(task, interp, written, e); } Self::next(interp, cmd) } - /// # Safety - /// `task` must be a live heap allocation produced by - /// [`ShellLsTask::create`]; ownership is reclaimed here. - fn on_shell_ls_task_done(interp: &Interpreter, cmd: NodeId, task: NonNull) { - // SAFETY: precondition. - let mut task = unsafe { bun_core::heap::take(task.as_ptr()) }; + fn on_shell_ls_task_done(interp: &Interpreter, cmd: NodeId, mut task: ShellLsTask) { if let State::Exec(exec) = &mut Self::state_mut(interp, cmd).state { exec.tasks_done += 1; } @@ -208,7 +193,7 @@ impl Ls { let output_task = OutputTask::::new(cmd, OutputSrc::Arrlist(output)); let errstr: Option> = task.err.take().map(|e| { - let s = Builtin::task_error_to_string(interp, cmd, Kind::Ls, &e).to_vec(); + let s = Builtin::task_error_to_string(Kind::Ls, &e); if let State::Exec(exec) = &mut Self::state_mut(interp, cmd).state { if exec.err.is_none() { exec.err = Some(e); @@ -222,21 +207,24 @@ impl Ls { /// Returns the index of the first non-flag arg, or `None` if there are no /// positional args. `Err` carries the offending flag byte. fn parse_opts(interp: &Interpreter, cmd: NodeId) -> Result, Box<[u8]>> { - let argc = Builtin::of(interp, cmd).args_slice().len(); + let argc = Builtin::argc(interp, cmd); if argc == 0 { return Ok(None); } - let mut idx = 0usize; - while idx < argc { - let flag = Builtin::of(interp, cmd).arg_bytes(idx); - match Self::parse_flag(&mut Self::state_mut(interp, cmd).opts, flag) { - ParseFlag::Done => return Ok(Some(idx)), - ParseFlag::ContinueParsing => {} - ParseFlag::IllegalOption(s) => return Err(s), + let mut opts = Self::state_mut(interp, cmd).opts; + let result = 'parsed: { + let bltn = Builtin::of(interp, cmd); + for idx in 0..argc { + match Self::parse_flag(&mut opts, bltn.arg_bytes(idx)) { + ParseFlag::Done => break 'parsed Ok(Some(idx)), + ParseFlag::ContinueParsing => {} + ParseFlag::IllegalOption(s) => break 'parsed Err(s), + } } - idx += 1; - } - Ok(None) + Ok(None) + }; + Self::state_mut(interp, cmd).opts = opts; + result } fn parse_flag(opts: &mut Opts, flag: &[u8]) -> ParseFlag { @@ -266,11 +254,11 @@ impl Ls { } #[inline] - fn state_mut(interp: &Interpreter, cmd: NodeId) -> &mut Ls { - match &mut Builtin::of_mut(interp, cmd).impl_ { + fn state_mut(interp: &Interpreter, cmd: NodeId) -> RefMut<'_, Ls> { + RefMut::map(Builtin::of_mut(interp, cmd), |b| match &mut b.impl_ { crate::shell::builtin::Impl::Ls(l) => &mut **l, _ => unreachable!(), - } + }) } } @@ -278,27 +266,31 @@ impl OutputTaskVTable for Ls { fn write_err( interp: &Interpreter, cmd: NodeId, - child: *mut OutputTask, + child: Box>, errbuf: &[u8], - ) -> Option { + ) -> OutputWrite { if let State::Exec(exec) = &mut Self::state_mut(interp, cmd).state { exec.output_waiting += 1; } - if let Some(safeguard) = Builtin::of(interp, cmd).stderr.needs_io() { - // Stash so on_io_writer_chunk can route to the OutputTask state - // machine and reclaim the box (stopgap for missing WriterTag). + let stderr_needs_io = Builtin::of(interp, cmd).stderr.needs_io(); + if let Some(safeguard) = stderr_needs_io { + // Park it so on_io_writer_chunk can route the completion back to + // the OutputTask state machine (there is no WriterTag for it). if let State::Exec(exec) = &mut Self::state_mut(interp, cmd).state { exec.output_queue.push_back(child); } let childptr = ChildPtr::new(cmd, WriterTag::Builtin); - return Some( - Builtin::of_mut(interp, cmd) - .stderr - .enqueue(childptr, errbuf, safeguard), - ); + return OutputWrite::Enqueued(Builtin::write_out( + interp, + cmd, + IoKind::Stderr, + childptr, + errbuf, + safeguard, + )); } let _ = Builtin::write_no_io(interp, cmd, IoKind::Stderr, errbuf); - None + OutputWrite::Done(child) } fn on_write_err(interp: &Interpreter, cmd: NodeId) { if let State::Exec(exec) = &mut Self::state_mut(interp, cmd).state { @@ -308,27 +300,30 @@ impl OutputTaskVTable for Ls { fn write_out( interp: &Interpreter, cmd: NodeId, - child: *mut OutputTask, - output: &mut OutputSrc, - ) -> Option { + child: Box>, + ) -> OutputWrite { if let State::Exec(exec) = &mut Self::state_mut(interp, cmd).state { exec.output_waiting += 1; } - if let Some(safeguard) = Builtin::of(interp, cmd).stdout.needs_io() { - if let State::Exec(exec) = &mut Self::state_mut(interp, cmd).state { - exec.output_queue.push_back(child); - } + let stdout_needs_io = Builtin::of(interp, cmd).stdout.needs_io(); + if let Some(safeguard) = stdout_needs_io { let childptr = ChildPtr::new(cmd, WriterTag::Builtin); - let buf = output.slice().to_vec(); - return Some( - Builtin::of_mut(interp, cmd) - .stdout - .enqueue(childptr, &buf, safeguard), - ); + return OutputWrite::Enqueued(Builtin::write_out_with( + interp, + cmd, + IoKind::Stdout, + childptr, + safeguard, + |buf| { + buf.extend_from_slice(child.output.slice()); + if let State::Exec(exec) = &mut Self::state_mut(interp, cmd).state { + exec.output_queue.push_back(child); + } + }, + )); } - let buf = output.slice().to_vec(); - let _ = Builtin::write_no_io(interp, cmd, IoKind::Stdout, &buf); - None + let _ = Builtin::write_no_io(interp, cmd, IoKind::Stdout, child.output.slice()); + OutputWrite::Done(child) } fn on_write_out(interp: &Interpreter, cmd: NodeId) { if let State::Exec(exec) = &mut Self::state_mut(interp, cmd).state { @@ -347,8 +342,9 @@ pub(crate) struct ShellLsTask { pub opts: Opts, pub print_directory: bool, /// Shared atomic counter (lives in `ExecState` inside `Box`; address - /// is stable for the lifetime of the Exec state). - pub task_count: *const AtomicUsize, + /// is stable for the lifetime of the Exec state, which outlives every + /// in-flight task). + pub task_count: bun_ptr::BackRef, pub cwd: bun_sys::Fd, pub path: ZBox, pub output: Vec, @@ -358,23 +354,22 @@ pub(crate) struct ShellLsTask { /// Cached once per task to avoid repeated syscalls. now_secs: u64, pub event_loop: EventLoopHandle, - /// Back-ref so recursive `enqueue` can populate the subtask's - /// `task.interp` (needed by [`ShellTask::run_from_main_thread`]). - pub interp: *mut Interpreter, pub task: ShellTask, } +crate::shell_task!(ShellLsTask); + impl ShellLsTask { fn create( cmd: NodeId, opts: Opts, - task_count: *const AtomicUsize, + task_count: bun_ptr::BackRef, cwd: bun_sys::Fd, path: ZBox, event_loop: EventLoopHandle, - interp: *mut Interpreter, - ) -> *mut ShellLsTask { - let mut task = Box::new(ShellLsTask { + interp: &Interpreter, + ) -> Box { + Box::new(ShellLsTask { cmd, opts, print_directory: false, @@ -386,11 +381,8 @@ impl ShellLsTask { err: None, now_secs: 0, event_loop, - interp, - task: ShellTask::new(event_loop), - }); - task.task.interp = interp; - bun_core::heap::into_raw(task) + task: ShellTask::new(interp), + }) } /// Spawns a subtask for a recursively @@ -398,11 +390,13 @@ impl ShellLsTask { fn enqueue(&mut self, name: &[u8]) { let new_path = self.join(name); // Pool thread: the subtask inherits our poster rather than deriving - // one from the VM (`ShellTask::new` is JS-thread only). - let subtask = bun_core::heap::into_raw(Box::new(ShellLsTask { + // one from the VM (`ShellTask::new` is JS-thread only). No + // keep-alive ref — it runs on a worker thread with no JS-VM + // thread-local. + let subtask = Box::new(ShellLsTask { cmd: self.cmd, opts: self.opts, - print_directory: false, + print_directory: true, task_count: self.task_count, cwd: self.cwd, path: new_path, @@ -411,21 +405,10 @@ impl ShellLsTask { err: None, now_secs: 0, event_loop: self.event_loop, - interp: self.interp, task: ShellTask::new_child(&self.task), - })); - // SAFETY: freshly allocated above. - unsafe { (*subtask).task.interp = self.interp }; - // SAFETY: `task_count` points into the `Box` ExecState which - // outlives every in-flight task (see `next`). `subtask` is freshly - // heap-allocated and scheduled via raw `WorkPool::schedule` (no - // keep-alive ref) — it runs on a worker thread with no JS-VM - // thread-local. - unsafe { - (*self.task_count).fetch_add(1, Ordering::Relaxed); - (*subtask).print_directory = true; - ShellTask::schedule_no_ref::(subtask); - } + }); + self.task_count.fetch_add(1, Ordering::Relaxed); + ShellTask::schedule_no_ref(subtask); } fn join(&self, child: &[u8]) -> ZBox { @@ -622,16 +605,6 @@ impl ShellLsTask { fn error_with_path(&self, err: &bun_sys::Error) -> bun_sys::Error { err.with_path(self.path.as_bytes()) } - - /// # Safety - /// `this` must be a live heap allocation produced by - /// [`ShellLsTask::create`]; ownership is reclaimed via - /// [`Ls::on_shell_ls_task_done`]. - fn run_from_main_thread(this: NonNull, interp: &Interpreter) { - // SAFETY: precondition. - let cmd = unsafe { this.as_ref() }.cmd; - Ls::on_shell_ls_task_done(interp, cmd, this); - } } fn get_file_type_char(mode: u32) -> u8 { @@ -769,28 +742,23 @@ fn civil_from_days(z: i64) -> (i32, u8, u8) { ((y + (m <= 2) as i64) as i32, m, d) } -impl bun_event_loop::Taskable for ShellLsTask { - const TAG: bun_event_loop::TaskTag = bun_event_loop::task_tag::ShellLsTask; - /// A pool completion that will not run: drop the keep-alive and the box - /// (nothing else frees an unrun one). - unsafe fn release_unrun(this: *mut Self) { - // SAFETY: fn contract — the box the builtin scheduled. - unsafe { - (*this).task.unref_unrun(); - drop(bun_core::heap::take(this)); - } - } -} +// `runtime::dispatch::run_task`'s `task_tag::ShellLsTask` arm reboxes the +// pointer `ShellTask::on_finish` posted; a completion that will not run drops +// the keep-alive and the box. impl crate::shell::interpreter::ShellTaskCtx for ShellLsTask { - const TASK_OFFSET: usize = core::mem::offset_of!(Self, task); - fn run_from_thread_pool(this: &mut Self) { - Self::run_from_thread_pool(this) + fn shell_task(&self) -> &ShellTask { + &self.task + } + fn shell_task_mut(&mut self) -> &mut ShellTask { + &mut self.task + } + fn run_from_thread_pool(&mut self) { + Self::run_from_thread_pool(self) } - fn run_from_main_thread(this: *mut Self, interp: &Interpreter) { - // The `ShellTask` trampoline hands back the live, non-null heap - // allocation produced by `ShellLsTask::create`. - Self::run_from_main_thread(NonNull::new(this).unwrap(), interp) + fn run_from_main_thread(self: Box, interp: &Interpreter) { + let cmd = self.cmd; + Ls::on_shell_ls_task_done(interp, cmd, *self); } } diff --git a/src/runtime/shell/builtin/mkdir.rs b/src/runtime/shell/builtin/mkdir.rs index df2515693382..218d0f1159a0 100644 --- a/src/runtime/shell/builtin/mkdir.rs +++ b/src/runtime/shell/builtin/mkdir.rs @@ -3,12 +3,11 @@ use crate::node::types::PathLike; use crate::shell::ExitCode; use crate::shell::builtin::{Builtin, BuiltinState, IoKind, Kind}; use crate::shell::interpreter::{ - EventLoopHandle, FlagParser, Interpreter, NodeId, OutputSrc, OutputTask, OutputTaskVTable, - ParseFlagResult, ShellTask, parse_flags, unsupported_flag, + FlagParser, Interpreter, NodeId, OutputSrc, OutputTask, OutputTaskVTable, OutputWrite, + ParseFlagResult, ShellTask, unsupported_flag, }; use crate::shell::io_writer::{ChildPtr, WriterTag}; use crate::shell::yield_::Yield; -use core::ptr::NonNull; #[derive(Default)] pub struct Mkdir { @@ -36,25 +35,20 @@ pub struct Exec { /// self-reference). pub(crate) args_start: usize, pub(crate) err: Option, - /// FIFO of in-flight OutputTask pointers awaiting an IOWriter chunk - /// completion. Stopgap until `WriterTag` can carry the `*mut OutputTask` - /// directly (IOWriter.rs is out of scope here): `write_err`/`write_out` + /// FIFO of in-flight OutputTasks awaiting an IOWriter chunk completion + /// (`WriterTag` cannot name an OutputTask): `write_err`/`write_out` /// push, `on_io_writer_chunk` pops and forwards to - /// `OutputTask::on_io_writer_chunk` so the box is reclaimed and the - /// writeErr→writeOut→onDone state machine runs. - pub(crate) output_queue: std::collections::VecDeque<*mut OutputTask>, + /// `OutputTask::on_io_writer_chunk` so the writeErr→writeOut→onDone + /// state machine runs. + pub(crate) output_queue: std::collections::VecDeque>>, } impl Mkdir { pub(crate) fn start(interp: &Interpreter, cmd: NodeId) -> Yield { let (args_start, mut opts) = { let mut opts = Opts::default(); - let args = Builtin::of(interp, cmd).args_slice(); - match parse_flags(&mut opts, args) { - Ok(Some(rest)) => { - let start = args.len() - rest.len(); - (start, opts) - } + match Builtin::parse_flags(interp, cmd, &mut opts) { + Ok(Some(start)) => (start, opts), Ok(None) => { return Self::fail_usage(interp, cmd); } @@ -87,8 +81,7 @@ impl Mkdir { } fn next(interp: &Interpreter, cmd: NodeId) -> Yield { - // NOTE: reshaped for borrowck — read scalars, drop the borrow, - // then act. + // Read scalars, drop the borrow, then act. let action = match &mut Self::state_mut(interp, cmd).state { State::Idle => panic!("Invalid state"), State::Exec(exec) => { @@ -100,37 +93,36 @@ impl Mkdir { exec.err = None; NextAction::Done(exit_code) } else { - return Yield::suspended(); + NextAction::Suspend } } else { exec.started = true; NextAction::Schedule(exec.args_start) } } - State::WaitingWriteErr => return Yield::failed(), - State::Done => return Builtin::done(interp, cmd, 0), + State::WaitingWriteErr => NextAction::Failed, + State::Done => NextAction::AlreadyDone, }; match action { + NextAction::Suspend => Yield::suspended(), + NextAction::Failed => Yield::failed(), + NextAction::AlreadyDone => Builtin::done(interp, cmd, 0), NextAction::Done(code) => { Self::state_mut(interp, cmd).state = State::Done; Builtin::done(interp, cmd, code) } NextAction::Schedule(args_start) => { - let argc = Builtin::of(interp, cmd).args_slice().len(); + let argc = Builtin::argc(interp, cmd); let task_count = argc - args_start; if let State::Exec(exec) = &mut Self::state_mut(interp, cmd).state { exec.tasks_count = task_count; } let opts = Self::state_mut(interp, cmd).opts; - let cwd = Builtin::shell(interp, cmd).cwd().to_vec(); - let evtloop = Builtin::event_loop(interp, cmd); - let interp_ptr: *mut Interpreter = interp.as_ctx_ptr(); + let cwd = Builtin::shell(interp, cmd).borrow().cwd().to_vec(); for i in args_start..argc { let path = Builtin::of(interp, cmd).arg_bytes(i).to_vec(); - let task = - ShellMkdirTask::create(cmd, opts, path, cwd.clone(), evtloop, interp_ptr); - // SAFETY: freshly heap-allocated. - unsafe { ShellTask::schedule(task) }; + let task = ShellMkdirTask::create(cmd, opts, path, cwd.clone(), interp); + ShellTask::schedule(task); } Yield::suspended() } @@ -144,14 +136,15 @@ impl Mkdir { e: Option, ) -> Yield { let pending = match &mut Self::state_mut(interp, cmd).state { - State::WaitingWriteErr => return Builtin::done(interp, cmd, 1), - State::Exec(exec) => exec.output_queue.pop_front(), + State::WaitingWriteErr => Err(()), + State::Exec(exec) => Ok(exec.output_queue.pop_front()), State::Idle | State::Done => panic!("Invalid state"), }; + let Ok(pending) = pending else { + return Builtin::done(interp, cmd, 1); + }; if let Some(task) = pending { - // SAFETY: `task` was heap-allocated in `OutputTask::new` and - // pushed by `write_err`/`write_out`; not yet freed. - return unsafe { OutputTask::::on_io_writer_chunk(task, interp, written, e) }; + return OutputTask::::on_io_writer_chunk(task, interp, written, e); } Self::next(interp, cmd) } @@ -167,7 +160,7 @@ impl Mkdir { let output_task = OutputTask::::new(cmd, OutputSrc::Arrlist(output)); let errstr: Option> = err.map(|e| { - let s = Builtin::task_error_to_string(interp, cmd, Kind::Mkdir, &e).to_vec(); + let s = Builtin::task_error_to_string(Kind::Mkdir, &e); if let State::Exec(exec) = &mut Self::state_mut(interp, cmd).state { exec.err = Some(e); } @@ -180,36 +173,43 @@ impl Mkdir { enum NextAction { Done(ExitCode), Schedule(usize), + Suspend, + Failed, + AlreadyDone, } impl OutputTaskVTable for Mkdir { fn write_err( interp: &Interpreter, cmd: NodeId, - child: *mut OutputTask, + child: Box>, errbuf: &[u8], - ) -> Option { + ) -> OutputWrite { if let State::Exec(exec) = &mut Self::state_mut(interp, cmd).state { exec.output_waiting += 1; } - if let Some(safeguard) = Builtin::of(interp, cmd).stderr.needs_io() { + let stderr_needs_io = Builtin::of(interp, cmd).stderr.needs_io(); + if let Some(safeguard) = stderr_needs_io { // OutputTask has no `WriterTag` of its own (it is not directly // dispatchable as an IOWriter child), so the enqueue is tagged - // `WriterTag::Builtin` and `child` is stashed on `output_queue`; + // `WriterTag::Builtin` and `child` is parked on `output_queue`; // `on_io_writer_chunk` pops it to route the completion back to - // the OutputTask state machine and reclaim the box. + // the OutputTask state machine. if let State::Exec(exec) = &mut Self::state_mut(interp, cmd).state { exec.output_queue.push_back(child); } let childptr = ChildPtr::new(cmd, WriterTag::Builtin); - return Some( - Builtin::of_mut(interp, cmd) - .stderr - .enqueue(childptr, errbuf, safeguard), - ); + return OutputWrite::Enqueued(Builtin::write_out( + interp, + cmd, + IoKind::Stderr, + childptr, + errbuf, + safeguard, + )); } let _ = Builtin::write_no_io(interp, cmd, IoKind::Stderr, errbuf); - None + OutputWrite::Done(child) } fn on_write_err(interp: &Interpreter, cmd: NodeId) { @@ -221,29 +221,32 @@ impl OutputTaskVTable for Mkdir { fn write_out( interp: &Interpreter, cmd: NodeId, - child: *mut OutputTask, - output: &mut OutputSrc, - ) -> Option { + child: Box>, + ) -> OutputWrite { if let State::Exec(exec) = &mut Self::state_mut(interp, cmd).state { exec.output_waiting += 1; } - if let Some(safeguard) = Builtin::of(interp, cmd).stdout.needs_io() { - // See write_err — stash `child` so the chunk callback routes to + let stdout_needs_io = Builtin::of(interp, cmd).stdout.needs_io(); + if let Some(safeguard) = stdout_needs_io { + // See write_err — park `child` so the chunk callback routes to // OutputTask::on_io_writer_chunk. - if let State::Exec(exec) = &mut Self::state_mut(interp, cmd).state { - exec.output_queue.push_back(child); - } let childptr = ChildPtr::new(cmd, WriterTag::Builtin); - let buf = output.slice().to_vec(); - return Some( - Builtin::of_mut(interp, cmd) - .stdout - .enqueue(childptr, &buf, safeguard), - ); + return OutputWrite::Enqueued(Builtin::write_out_with( + interp, + cmd, + IoKind::Stdout, + childptr, + safeguard, + |buf| { + buf.extend_from_slice(child.output.slice()); + if let State::Exec(exec) = &mut Self::state_mut(interp, cmd).state { + exec.output_queue.push_back(child); + } + }, + )); } - let buf = output.slice().to_vec(); - let _ = Builtin::write_no_io(interp, cmd, IoKind::Stdout, &buf); - None + let _ = Builtin::write_no_io(interp, cmd, IoKind::Stdout, child.output.slice()); + OutputWrite::Done(child) } fn on_write_out(interp: &Interpreter, cmd: NodeId) { @@ -272,26 +275,25 @@ pub(crate) struct ShellMkdirTask { pub task: ShellTask, } +crate::shell_task!(ShellMkdirTask); + impl ShellMkdirTask { fn create( cmd: NodeId, opts: Opts, filepath: Vec, cwd_path: Vec, - evtloop: EventLoopHandle, - interp: *mut Interpreter, - ) -> *mut ShellMkdirTask { - let mut task = Box::new(ShellMkdirTask { + interp: &Interpreter, + ) -> Box { + Box::new(ShellMkdirTask { cmd, opts, filepath, cwd_path, created_directories: Vec::new(), err: None, - task: ShellTask::new(evtloop), - }); - task.task.interp = interp; - bun_core::heap::into_raw(task) + task: ShellTask::new(interp), + }) } fn run_from_thread_pool(this: &mut ShellMkdirTask) { @@ -354,34 +356,14 @@ impl ShellMkdirTask { } } } - // Bounce-back to the main thread is posted by `shell_task_trampoline` - // via `ShellTask::on_finish::` (handles both JS and mini event - // loops). - } - - /// Reclaims ownership of the heap allocation produced by [`Self::create`] - /// and forwards it to [`Mkdir::on_shell_mkdir_task_done`]. - fn run_from_main_thread(this: NonNull, interp: &Interpreter) { - // SAFETY: `this` is a live heap allocation produced by `Self::create`; - // the dispatch contract guarantees it is not yet freed. - let mut task = unsafe { bun_core::heap::take(this.as_ptr()) }; - let cmd = task.cmd; - Mkdir::on_shell_mkdir_task_done(interp, cmd, &mut task); + // Bounce-back to the main thread is posted by `ShellTask::run_owned` + // via `ShellTask::on_finish` (handles both JS and mini event loops). } } -impl bun_event_loop::Taskable for ShellMkdirTask { - const TAG: bun_event_loop::TaskTag = bun_event_loop::task_tag::ShellMkdirTask; - /// A pool completion that will not run: drop the keep-alive and the box - /// (nothing else frees an unrun one). - unsafe fn release_unrun(this: *mut Self) { - // SAFETY: fn contract — the box the builtin scheduled. - unsafe { - (*this).task.unref_unrun(); - drop(bun_core::heap::take(this)); - } - } -} +// `runtime::dispatch::run_task`'s `task_tag::ShellMkdirTask` arm reboxes the +// pointer `ShellTask::on_finish` posted; a completion that will not run drops +// the keep-alive and the box. /// Collects each created directory into /// `created_directories` (newline-separated) when `-v` is set. Passed by value @@ -417,14 +399,19 @@ impl MkdirCtx for MkdirVerboseVTable { } impl crate::shell::interpreter::ShellTaskCtx for ShellMkdirTask { - const TASK_OFFSET: usize = core::mem::offset_of!(Self, task); - fn run_from_thread_pool(this: &mut Self) { - Self::run_from_thread_pool(this) + fn shell_task(&self) -> &ShellTask { + &self.task + } + fn shell_task_mut(&mut self) -> &mut ShellTask { + &mut self.task + } + fn run_from_thread_pool(&mut self) { + Self::run_from_thread_pool(self) } - fn run_from_main_thread(this: *mut Self, interp: &Interpreter) { - // `ShellTaskCtx` callers guarantee `this` is the live, non-null - // heap-allocated task posted via `ShellTask::schedule`. - Self::run_from_main_thread(NonNull::new(this).unwrap(), interp) + /// The task drops after `on_shell_mkdir_task_done` has taken what it needs. + fn run_from_main_thread(mut self: Box, interp: &Interpreter) { + let cmd = self.cmd; + Mkdir::on_shell_mkdir_task_done(interp, cmd, &mut self); } } @@ -465,9 +452,7 @@ impl FlagParser for Opts { self.verbose = true; None } - _ => Some(ParseFlagResult::IllegalOption( - &raw const smallflags[1 + i..], - )), + _ => Some(ParseFlagResult::IllegalOption(smallflags[1 + i..].into())), } } } diff --git a/src/runtime/shell/builtin/mv.rs b/src/runtime/shell/builtin/mv.rs index ef318c0037e0..74d2ffb8521e 100644 --- a/src/runtime/shell/builtin/mv.rs +++ b/src/runtime/shell/builtin/mv.rs @@ -29,12 +29,12 @@ pub struct MvArgs { pub enum MvState { #[default] Idle, - CheckTarget(Box), + /// `None` while the task is out on the pool; put back once it is done. + CheckTarget(Option>), Executing { task_count: usize, tasks_done: usize, error_signal: AtomicBool, - tasks: Vec, err: Option, }, Done, @@ -61,12 +61,11 @@ impl Mv { buf: &[u8], exit_code: ExitCode, ) -> Yield { - if let Some(safeguard) = Builtin::of(interp, cmd).stderr.needs_io() { + let stderr_needs_io = Builtin::of(interp, cmd).stderr.needs_io(); + if let Some(safeguard) = stderr_needs_io { Self::state_mut(interp, cmd).state = MvState::WaitingWriteErr { exit_code }; let child = ChildPtr::new(cmd, WriterTag::Builtin); - return Builtin::of_mut(interp, cmd) - .stderr - .enqueue(child, buf, safeguard); + return Builtin::write_out(interp, cmd, IoKind::Stderr, child, buf, safeguard); } let _ = Builtin::write_no_io(interp, cmd, IoKind::Stderr, buf); Builtin::done(interp, cmd, exit_code) @@ -95,12 +94,9 @@ impl Mv { if let Err(e) = Self::parse_opts(interp, cmd) { let buf: Vec = match e { MvParseError::IllegalOption(s) => Builtin::fmt_error_arena( - interp, - cmd, Some(Kind::Mv), format_args!("illegal option -- {}\n", bstr::BStr::new(s)), - ) - .to_vec(), + ), MvParseError::ShowUsage => Kind::Mv.usage_string().to_vec(), }; return Self::write_failing_error(interp, cmd, &buf, 1); @@ -108,32 +104,27 @@ impl Mv { let cwd = Builtin::cwd(interp, cmd); let target_idx = Self::state_mut(interp, cmd).args.target_idx; let target = ZBox::from_bytes(Builtin::of(interp, cmd).arg_bytes(target_idx)); - let evtloop = Builtin::event_loop(interp, cmd); - let mut task = Box::new(ShellMvCheckTargetTask { + let task = Box::new(ShellMvCheckTargetTask { cmd, cwd, target, result: None, - done: false, - task: ShellTask::new(evtloop), + task: ShellTask::new(interp), }); - task.task.interp = interp.as_ctx_ptr(); - // SAFETY: `task` is heap-allocated and outlives the worker - // call (held in `MvState::CheckTarget` below). - unsafe { ShellTask::schedule(&raw mut *task) }; - Self::state_mut(interp, cmd).state = MvState::CheckTarget(task); + Self::state_mut(interp, cmd).state = MvState::CheckTarget(None); + ShellTask::schedule(task); Yield::suspended() } Tag::CheckTarget => { - let done = match &Self::state_mut(interp, cmd).state { - MvState::CheckTarget(t) => t.done, - _ => unreachable!(), - }; + let done = matches!( + Self::state_mut(interp, cmd).state, + MvState::CheckTarget(Some(_)) + ); if !done { return Yield::suspended(); } let result = match &mut Self::state_mut(interp, cmd).state { - MvState::CheckTarget(t) => t.result.take(), + MvState::CheckTarget(Some(t)) => t.result.take(), _ => unreachable!(), }; debug_assert!(result.is_some()); @@ -145,7 +136,7 @@ impl Mv { // one source. Any other errno (EACCES, ELOOP, …) // is reported and fails regardless of source count. let target = match &Self::state_mut(interp, cmd).state { - MvState::CheckTarget(t) => t.target.as_bytes().to_vec(), + MvState::CheckTarget(Some(t)) => t.target.as_bytes().to_vec(), _ => unreachable!(), }; if e.get_errno() == bun_sys::E::ENOENT { @@ -157,113 +148,96 @@ impl Mv { None } else { let buf = Builtin::fmt_error_arena( - interp, - cmd, Some(Kind::Mv), format_args!( "{}: No such file or directory\n", bstr::BStr::new(&target) ), - ) - .to_vec(); + ); return Self::write_failing_error(interp, cmd, &buf, 1); } } else { let msg = e.msg().unwrap_or(b"unknown error"); let buf = Builtin::fmt_error_arena( - interp, - cmd, Some(Kind::Mv), format_args!( "{}: {}\n", bstr::BStr::new(&target), bstr::BStr::new(msg) ), - ) - .to_vec(); + ); return Self::write_failing_error(interp, cmd, &buf, 1); } } }; let n_sources = { - let me = Self::state_mut(interp, cmd); + let mut me = Self::state_mut(interp, cmd); me.args.target_fd = maybe_fd; me.args.target_idx - me.args.sources_start }; // Trying to move multiple files into a non-directory. if maybe_fd.is_none() && n_sources > 1 { let target = match &Self::state_mut(interp, cmd).state { - MvState::CheckTarget(t) => t.target.as_bytes().to_vec(), + MvState::CheckTarget(Some(t)) => t.target.as_bytes().to_vec(), _ => unreachable!(), }; let buf = Builtin::fmt_error_arena( - interp, - cmd, Some(Kind::Mv), format_args!("{} is not a directory\n", bstr::BStr::new(&target)), - ) - .to_vec(); + ); return Self::write_failing_error(interp, cmd, &buf, 1); } const BATCH: usize = ShellMvBatchedTask::BATCH_SIZE; let task_count = n_sources.div_ceil(BATCH); let cwd = Builtin::cwd(interp, cmd); - let evtloop = Builtin::event_loop(interp, cmd); let (sources_start, target_idx) = { let me = Self::state_mut(interp, cmd); (me.args.sources_start, me.args.target_idx) }; - let target = Builtin::of(interp, cmd).arg_bytes(target_idx); - - let mut tasks: Vec = Vec::with_capacity(task_count); - for i in 0..task_count { - let start = sources_start + i * BATCH; - let end = (start + BATCH).min(target_idx); - let mut srcs = Vec::with_capacity(end - start); - for j in start..end { - srcs.push(ZBox::from_bytes(Builtin::of(interp, cmd).arg_bytes(j))); + let mut tasks: Vec> = Vec::with_capacity(task_count); + { + let bltn = Builtin::of(interp, cmd); + let target = bltn.arg_bytes(target_idx); + for i in 0..task_count { + let start = sources_start + i * BATCH; + let end = (start + BATCH).min(target_idx); + let mut srcs = Vec::with_capacity(end - start); + for j in start..end { + srcs.push(ZBox::from_bytes(bltn.arg_bytes(j))); + } + tasks.push(Box::new(ShellMvBatchedTask { + cmd, + sources: srcs, + target: ZBox::from_bytes(target), + target_fd: maybe_fd, + cwd, + error_signal: None, + err: None, + task: ShellTask::new(interp), + })); } - tasks.push(ShellMvBatchedTask { - cmd, - idx: i, - sources: srcs, - target: ZBox::from_bytes(target), - target_fd: maybe_fd, - cwd, - error_signal: None, - err: None, - task: ShellTask::new(evtloop), - }); } Self::state_mut(interp, cmd).state = MvState::Executing { task_count, tasks_done: 0, error_signal: AtomicBool::new(false), - tasks, err: None, }; // Now that the AtomicBool has its final address, point // every task at it and schedule. - let interp_ptr: *mut Interpreter = interp.as_ctx_ptr(); - if let MvState::Executing { - error_signal, - tasks, - .. - } = &mut Self::state_mut(interp, cmd).state - { - let sig = BackRef::new(&*error_signal); - for t in tasks.iter_mut() { - t.error_signal = Some(sig); - t.task.interp = interp_ptr; - // SAFETY: `t` lives in `MvState::Executing::tasks`, - // which is fully populated before any task is scheduled - // and never grown afterward, so its address is stable - // for the worker call's lifetime. - unsafe { ShellTask::schedule(&raw mut *t) }; - } + let sig = { + let me = Self::state_mut(interp, cmd); + let MvState::Executing { error_signal, .. } = &me.state else { + unreachable!() + }; + BackRef::new(error_signal) + }; + for mut t in tasks { + t.error_signal = Some(sig); + ShellTask::schedule(t); } Yield::suspended() } @@ -283,38 +257,44 @@ impl Mv { _: usize, e: Option, ) -> Yield { - match Self::state_mut(interp, cmd).state { - MvState::WaitingWriteErr { exit_code } => { - if let Some(_err) = e { - Self::state_mut(interp, cmd).state = MvState::Err; - return Self::next(interp, cmd); - } - Builtin::done(interp, cmd, exit_code) - } + let exit_code = match Self::state_mut(interp, cmd).state { + MvState::WaitingWriteErr { exit_code } => exit_code, _ => panic!("Invalid state"), + }; + if let Some(_err) = e { + Self::state_mut(interp, cmd).state = MvState::Err; + return Self::next(interp, cmd); } + Builtin::done(interp, cmd, exit_code) } - fn check_target_task_done(interp: &Interpreter, cmd: NodeId) { - if let MvState::CheckTarget(t) = &mut Self::state_mut(interp, cmd).state { - t.done = true; + fn check_target_task_done( + interp: &Interpreter, + cmd: NodeId, + task: Box, + ) { + { + let mut me = Self::state_mut(interp, cmd); + if let MvState::CheckTarget(slot) = &mut me.state { + *slot = Some(task); + } } Self::next(interp, cmd).run(interp); } - fn batched_move_task_done(interp: &Interpreter, cmd: NodeId, task_idx: usize) { + fn batched_move_task_done(interp: &Interpreter, cmd: NodeId, mut task: ShellMvBatchedTask) { let (all_done, had_err) = { + let mut me = Self::state_mut(interp, cmd); let MvState::Executing { task_count, tasks_done, error_signal, - tasks, err, - } = &mut Self::state_mut(interp, cmd).state + } = &mut me.state else { unreachable!() }; - if let Some(e) = tasks[task_idx].err.take() { + if let Some(e) = task.err.take() { error_signal.store(true, Ordering::SeqCst); if err.is_none() { *err = Some(e); @@ -331,7 +311,7 @@ impl Mv { }; // The failing rename's errno becomes the shell exit code. let exit_code = e.errno as ExitCode; - let buf = Builtin::task_error_to_string(interp, cmd, Kind::Mv, &e).to_vec(); + let buf = Builtin::task_error_to_string(Kind::Mv, &e); Self::write_failing_error(interp, cmd, &buf, exit_code).run(interp); return; } @@ -347,14 +327,14 @@ impl Mv { } let mut idx = 0usize; while idx < argc { - let flag = Builtin::of(interp, cmd).arg_bytes(idx); - match Self::parse_flag(flag) { + let parsed = Self::parse_flag(Builtin::of(interp, cmd).arg_bytes(idx)); + match parsed { MvFlag::Done => { let filepath_args = argc - idx; if filepath_args < 2 { return Err(MvParseError::ShowUsage); } - let me = Self::state_mut(interp, cmd); + let mut me = Self::state_mut(interp, cmd); me.args.sources_start = idx; me.args.target_idx = argc - 1; return Ok(()); @@ -408,10 +388,20 @@ pub struct ShellMvCheckTargetTask { /// `Ok(Some(fd))` → directory; `Ok(None)` → not a directory; `Err(e)` → /// open error (e.g. ENOENT). pub(crate) result: Option, bun_sys::Error>>, - pub(crate) done: bool, pub task: ShellTask, } +impl Drop for ShellMvCheckTargetTask { + fn drop(&mut self) { + if let Some(Ok(Some(fd))) = self.result.take() { + closefd(fd); + } + } +} + +crate::shell_task!(ShellMvCheckTargetTask); +crate::shell_task!(ShellMvBatchedTask); + impl ShellMvCheckTargetTask { fn run_from_thread_pool(this: &mut ShellMvCheckTargetTask) { let flags = bun_sys::O::RDONLY | bun_sys::O::DIRECTORY; @@ -420,16 +410,13 @@ impl ShellMvCheckTargetTask { Err(e) if e.get_errno() == bun_sys::E::ENOTDIR => Ok(None), Err(e) => Err(e), }); - // Bounce-back is posted by `shell_task_trampoline`. + // Bounce-back is posted by `ShellTask::run_owned`. } } /// renameat() each source into the target. pub struct ShellMvBatchedTask { pub(crate) cmd: NodeId, - /// Index into `MvState::Executing::tasks` so the main-thread completion - /// can route to `Mv::batched_move_task_done`. - pub(crate) idx: usize, pub(crate) sources: Vec, pub(crate) target: ZBox, pub(crate) target_fd: Option, @@ -474,7 +461,7 @@ impl ShellMvBatchedTask { e }); } - // Bounce-back is posted by `shell_task_trampoline`. + // Bounce-back is posted by `ShellTask::run_owned`. } /// `renameat()`, falling through to [`Self::move_across_devices`] on EXDEV. @@ -679,52 +666,37 @@ impl ShellMvBatchedTask { } } -impl bun_event_loop::Taskable for ShellMvCheckTargetTask { - const TAG: bun_event_loop::TaskTag = bun_event_loop::task_tag::ShellMvCheckTargetTask; - /// Owned by the builtin's `MvState`, which frees it with the interpreter; - /// only the keep-alive is this hop's to drop. - unsafe fn release_unrun(this: *mut Self) { - // SAFETY: fn contract; the Mv state outlives the queue entry. - unsafe { (*this).task.unref_unrun() } - } -} -impl bun_event_loop::Taskable for ShellMvBatchedTask { - const TAG: bun_event_loop::TaskTag = bun_event_loop::task_tag::ShellMvBatchedTask; - /// An element of `MvState::Executing.tasks`; as `ShellMvCheckTargetTask`. - unsafe fn release_unrun(this: *mut Self) { - // SAFETY: as above. - unsafe { (*this).task.unref_unrun() } - } -} +// `runtime::dispatch::run_task`'s arms rebox the pointer `ShellTask::on_finish` +// posted; a completion that will not run drops the keep-alive and the box. -// `*mut Self` sig is forced by the `ShellTaskCtx` trait contract; the body's -// internal deref is SAFETY-commented. impl crate::shell::interpreter::ShellTaskCtx for ShellMvCheckTargetTask { - const TASK_OFFSET: usize = core::mem::offset_of!(Self, task); - fn run_from_thread_pool(this: &mut Self) { - Self::run_from_thread_pool(this) + fn shell_task(&self) -> &ShellTask { + &self.task } - // `*mut Self` sig forced by `ShellTaskCtx` trait contract; the body's internal deref is SAFETY-commented. - #[allow(clippy::not_unsafe_ptr_arg_deref)] - fn run_from_main_thread(this: *mut Self, interp: &Interpreter) { - // SAFETY: `ShellTask::run_from_main_thread` dispatch contract — `this` - // is a live `ShellMvCheckTargetTask` held in `MvState::CheckTarget`. - let this = unsafe { this.as_ref() }.unwrap(); - Mv::check_target_task_done(interp, this.cmd); + fn shell_task_mut(&mut self) -> &mut ShellTask { + &mut self.task + } + fn run_from_thread_pool(&mut self) { + Self::run_from_thread_pool(self) + } + fn run_from_main_thread(self: Box, interp: &Interpreter) { + let cmd = self.cmd; + Mv::check_target_task_done(interp, cmd, self); } } impl crate::shell::interpreter::ShellTaskCtx for ShellMvBatchedTask { - const TASK_OFFSET: usize = core::mem::offset_of!(Self, task); - fn run_from_thread_pool(this: &mut Self) { - Self::run_from_thread_pool(this) + fn shell_task(&self) -> &ShellTask { + &self.task + } + fn shell_task_mut(&mut self) -> &mut ShellTask { + &mut self.task + } + fn run_from_thread_pool(&mut self) { + Self::run_from_thread_pool(self) } - // `*mut Self` sig forced by `ShellTaskCtx` trait contract; the body's internal deref is SAFETY-commented. - #[allow(clippy::not_unsafe_ptr_arg_deref)] - fn run_from_main_thread(this: *mut Self, interp: &Interpreter) { - // SAFETY: `ShellTask::run_from_main_thread` dispatch contract — `this` - // is a live `ShellMvBatchedTask` held in `MvState::Executing::tasks`. - let this = unsafe { this.as_ref() }.unwrap(); - Mv::batched_move_task_done(interp, this.cmd, this.idx); + fn run_from_main_thread(self: Box, interp: &Interpreter) { + let cmd = self.cmd; + Mv::batched_move_task_done(interp, cmd, *self); } } diff --git a/src/runtime/shell/builtin/pwd.rs b/src/runtime/shell/builtin/pwd.rs index 354c7e9682ef..1e3e9def7add 100644 --- a/src/runtime/shell/builtin/pwd.rs +++ b/src/runtime/shell/builtin/pwd.rs @@ -29,32 +29,30 @@ impl Pwd { pub(crate) fn start(interp: &Interpreter, cmd: NodeId) -> Yield { if !Builtin::of(interp, cmd).args_slice().is_empty() { let msg: &[u8] = b"pwd: too many arguments\n"; - if let Some(safeguard) = Builtin::of(interp, cmd).stderr.needs_io() { + let stderr_needs_io = Builtin::of(interp, cmd).stderr.needs_io(); + if let Some(safeguard) = stderr_needs_io { Self::state_mut(interp, cmd).state = State::WaitingIo { kind: WaitKind::Stderr, }; let child = ChildPtr::new(cmd, WriterTag::Builtin); - return Builtin::of_mut(interp, cmd) - .stderr - .enqueue(child, msg, safeguard); + return Builtin::write_out(interp, cmd, IoKind::Stderr, child, msg, safeguard); } let _ = Builtin::write_no_io(interp, cmd, IoKind::Stderr, msg); return Builtin::done(interp, cmd, 1); } let cwd: Vec = { - let mut v = Builtin::shell(interp, cmd).cwd().to_vec(); + let mut v = Builtin::shell(interp, cmd).borrow().cwd().to_vec(); v.push(b'\n'); v }; - if let Some(safeguard) = Builtin::of(interp, cmd).stdout.needs_io() { + let stdout_needs_io = Builtin::of(interp, cmd).stdout.needs_io(); + if let Some(safeguard) = stdout_needs_io { Self::state_mut(interp, cmd).state = State::WaitingIo { kind: WaitKind::Stdout, }; let child = ChildPtr::new(cmd, WriterTag::Builtin); - return Builtin::of_mut(interp, cmd) - .stdout - .enqueue(child, &cwd, safeguard); + return Builtin::write_out(interp, cmd, IoKind::Stdout, child, &cwd, safeguard); } let _ = Builtin::write_no_io(interp, cmd, IoKind::Stdout, &cwd); Self::state_mut(interp, cmd).state = State::Done; @@ -72,8 +70,11 @@ impl Pwd { return Builtin::done(interp, cmd, 1); } let kind = match &Self::state_mut(interp, cmd).state { - State::WaitingIo { kind } => *kind, - _ => return Builtin::done(interp, cmd, 0), + State::WaitingIo { kind } => Some(*kind), + _ => None, + }; + let Some(kind) = kind else { + return Builtin::done(interp, cmd, 0); }; Self::state_mut(interp, cmd).state = State::Done; Builtin::done(interp, cmd, if kind == WaitKind::Stderr { 1 } else { 0 }) diff --git a/src/runtime/shell/builtin/rm.rs b/src/runtime/shell/builtin/rm.rs index f3ca95379533..b53b733e2f48 100644 --- a/src/runtime/shell/builtin/rm.rs +++ b/src/runtime/shell/builtin/rm.rs @@ -141,7 +141,8 @@ impl Rm { } let arg = Builtin::of(interp, cmd).arg_bytes(idx as usize).to_vec(); - match Self::parse_flag(&mut Self::state_mut(interp, cmd).opts, &arg) { + let parsed = Self::parse_flag(&mut Self::state_mut(interp, cmd).opts, &arg); + match parsed { RmParseFlag::ContinueParsing => { if let RmState::ParseOpts { idx: i, .. } = &mut Self::state_mut(interp, cmd).state @@ -170,7 +171,7 @@ impl Rm { // Check that none of the paths will delete the root. { - let cwd = Builtin::shell(interp, cmd).cwd().to_vec(); + let cwd = Builtin::shell(interp, cmd).borrow().cwd().to_vec(); // Operands are unbounded user input, so neither // step may use the fixed-size thread-local // buffers behind `join` / `normalize_string`. @@ -180,13 +181,13 @@ impl Rm { let mut normalize_buf = Vec::new(); for i in args_start..argc { - let path = Builtin::of(interp, cmd).arg_bytes(i); - let resolved: &[u8] = if Platform::AUTO.is_absolute(path) { - path + let path = Builtin::of(interp, cmd).arg_bytes(i).to_vec(); + let resolved: &[u8] = if Platform::AUTO.is_absolute(&path) { + &path } else { resolve_path::join_spill::( &mut join_spill, - &[&cwd, path], + &[&cwd, &path], ) }; if normalize_buf.len() <= resolved.len() { @@ -205,37 +206,35 @@ impl Rm { // Copy resolved before // re-borrowing `interp` mutably. let resolved_owned = resolved.to_vec(); - if let Some(safeguard) = - Builtin::of(interp, cmd).stderr.needs_io() - { + let stderr_needs_io = + Builtin::of(interp, cmd).stderr.needs_io(); + if let Some(safeguard) = stderr_needs_io { Self::state_mut(interp, cmd).state = RmState::ParseOpts { idx, wait_write_err: true, }; let child = ChildPtr::new(cmd, WriterTag::Builtin); - return Builtin::of_mut(interp, cmd) - .stderr - .enqueue_fmt( - child, - Some(Kind::Rm), - format_args!( - "\"{}\" may not be removed\n", - bstr::BStr::new(&resolved_owned) - ), - safeguard, - ); + return Builtin::write_out_fmt( + interp, + cmd, + IoKind::Stderr, + child, + Some(Kind::Rm), + format_args!( + "\"{}\" may not be removed\n", + bstr::BStr::new(&resolved_owned) + ), + safeguard, + ); } let buf = Builtin::fmt_error_arena( - interp, - cmd, Some(Kind::Rm), format_args!( "\"{}\" may not be removed\n", bstr::BStr::new(&resolved_owned) ), - ) - .to_vec(); + ); let _ = Builtin::write_no_io(interp, cmd, IoKind::Stderr, &buf); return Builtin::done(interp, cmd, 1); @@ -265,13 +264,17 @@ impl Rm { ); } RmParseFlag::IllegalOptionWithFlag => { - if let Some(safeguard) = Builtin::of(interp, cmd).stderr.needs_io() { + let stderr_needs_io = Builtin::of(interp, cmd).stderr.needs_io(); + if let Some(safeguard) = stderr_needs_io { Self::state_mut(interp, cmd).state = RmState::ParseOpts { idx, wait_write_err: true, }; let child = ChildPtr::new(cmd, WriterTag::Builtin); - return Builtin::of_mut(interp, cmd).stderr.enqueue_fmt( + return Builtin::write_out_fmt( + interp, + cmd, + IoKind::Stderr, child, Some(Kind::Rm), format_args!( @@ -282,12 +285,9 @@ impl Rm { ); } let buf = Builtin::fmt_error_arena( - interp, - cmd, Some(Kind::Rm), format_args!("illegal option -- {}\n", bstr::BStr::new(&arg[1..])), - ) - .to_vec(); + ); let _ = Builtin::write_no_io(interp, cmd, IoKind::Stderr, &buf); return Builtin::done(interp, cmd, 1); } @@ -300,29 +300,29 @@ impl Rm { }; if !started { let cwd = Builtin::cwd(interp, cmd); - let evtloop = Builtin::event_loop(interp, cmd); let opts = Self::state_mut(interp, cmd).opts; - let interp_ptr: *mut Interpreter = interp.as_ctx_ptr(); let (args_start, argc) = { - let me = Self::state_mut(interp, cmd); + let mut me = Self::state_mut(interp, cmd); let RmState::Exec(e) = &mut me.state else { unreachable!() }; e.started = true; (e.args_start, e.args_start + e.total_tasks) }; - let (sig, out_count) = match &Self::state_mut(interp, cmd).state { - RmState::Exec(e) => ( + let (sig, out_count) = { + let me = Self::state_mut(interp, cmd); + let RmState::Exec(e) = &me.state else { + unreachable!() + }; + ( bun_ptr::BackRef::new(&e.error_signal), bun_ptr::BackRef::new(&e.output_count), - ), - _ => unreachable!(), + ) }; for i in args_start..argc { - let root = Builtin::of(interp, cmd).arg_bytes(i); - let task = ShellRmTask::create( - cmd, opts, root, cwd, sig, out_count, evtloop, interp_ptr, - ); + let root = Builtin::of(interp, cmd).arg_bytes(i).to_vec(); + let task = + ShellRmTask::create(cmd, opts, &root, cwd, sig, out_count, interp); // SAFETY: freshly heap-allocated. unsafe { ShellRmTask::schedule(task) }; } @@ -336,15 +336,14 @@ impl Rm { } fn write_err_literal(interp: &Interpreter, cmd: NodeId, idx: u32, buf: &[u8]) -> Yield { - if let Some(safeguard) = Builtin::of(interp, cmd).stderr.needs_io() { + let stderr_needs_io = Builtin::of(interp, cmd).stderr.needs_io(); + if let Some(safeguard) = stderr_needs_io { Self::state_mut(interp, cmd).state = RmState::ParseOpts { idx, wait_write_err: true, }; let child = ChildPtr::new(cmd, WriterTag::Builtin); - return Builtin::of_mut(interp, cmd) - .stderr - .enqueue(child, buf, safeguard); + return Builtin::write_out(interp, cmd, IoKind::Stderr, child, buf, safeguard); } let _ = Builtin::write_no_io(interp, cmd, IoKind::Stderr, buf); Builtin::done(interp, cmd, 1) @@ -397,7 +396,7 @@ impl Rm { // stashing the error on `exec` (formatting needs &mut interp). let errstr: Option> = task_err .as_ref() - .map(|e| Builtin::task_error_to_string(interp, cmd, Kind::Rm, e).to_vec()); + .map(|e| Builtin::task_error_to_string(Kind::Rm, e)); let (tasks_done, total) = { let RmState::Exec(exec) = &mut Self::state_mut(interp, cmd).state else { panic!("Invalid state") @@ -414,15 +413,13 @@ impl Rm { }; if let Some(s) = errstr { - if let Some(safeguard) = Builtin::of(interp, cmd).stderr.needs_io() { + let stderr_needs_io = Builtin::of(interp, cmd).stderr.needs_io(); + if let Some(safeguard) = stderr_needs_io { if let RmState::Exec(exec) = &mut Self::state_mut(interp, cmd).state { exec.output_count.fetch_add(1, Ordering::SeqCst); } let child = ChildPtr::new(cmd, WriterTag::Builtin); - Builtin::of_mut(interp, cmd) - .stderr - .enqueue(child, &s, safeguard) - .run(interp); + Builtin::write_out(interp, cmd, IoKind::Stderr, child, &s, safeguard).run(interp); return; } let _ = Builtin::write_no_io(interp, cmd, IoKind::Stderr, &s); @@ -473,11 +470,11 @@ impl Rm { } }); - if let Some(safeguard) = Builtin::of(interp, cmd).stdout.needs_io() { + let stdout_needs_io = Builtin::of(interp, cmd).stdout.needs_io(); + + if let Some(safeguard) = stdout_needs_io { let child = ChildPtr::new(cmd, WriterTag::Builtin); - return Builtin::of_mut(interp, cmd) - .stdout - .enqueue(child, &buf, safeguard); + return Builtin::write_out(interp, cmd, IoKind::Stdout, child, &buf, safeguard); } let _ = Builtin::write_no_io(interp, cmd, IoKind::Stdout, &buf); let done = match &mut Self::state_mut(interp, cmd).state { @@ -555,11 +552,11 @@ impl Rm { } #[inline] - fn state_mut(interp: &Interpreter, cmd: NodeId) -> &mut Rm { - match &mut Builtin::of_mut(interp, cmd).impl_ { + fn state_mut(interp: &Interpreter, cmd: NodeId) -> core::cell::RefMut<'_, Rm> { + core::cell::RefMut::map(Builtin::of_mut(interp, cmd), |b| match &mut b.impl_ { crate::shell::builtin::Impl::Rm(r) => &mut **r, _ => unreachable!(), - } + }) } } @@ -668,9 +665,9 @@ impl ShellRmTask { cwd: bun_sys::Fd, error_signal: bun_ptr::BackRef, output_count: bun_ptr::BackRef, - evtloop: EventLoopHandle, - interp: *mut Interpreter, + interp: &Interpreter, ) -> *mut ShellRmTask { + let evtloop = interp.event_loop; let join_style = JoinStyle::from_path(root_path); // Separate allocation — see the comment on `root_task`. let root_task = bun_core::heap::into_raw(Box::new(DirTask { @@ -690,7 +687,7 @@ impl ShellRmTask { callback: DirTask::work_pool_callback, }, })); - let mut boxed = Box::new(ShellRmTask { + let boxed = Box::new(ShellRmTask { cmd, opts, cwd, @@ -701,9 +698,8 @@ impl ShellRmTask { err: bun_threading::Guarded::new(None), join_style, event_loop: evtloop, - task: ShellTask::new(evtloop), + task: ShellTask::new(interp), }); - boxed.task.interp = interp; let raw = bun_core::heap::into_raw(boxed); // SAFETY: both freshly leaked; exclusive. unsafe { (*root_task).task_manager = raw }; @@ -734,12 +730,12 @@ impl ShellRmTask { /// Recover `*ShellRmTask` from the intrusive `*WorkPoolTask` and run the /// root DirTask. unsafe fn work_pool_callback(task: *mut WorkPoolTask) { - // SAFETY: `task` is the first `#[repr(C)]` field of `ShellTask`, which - // is embedded in `ShellRmTask` at `TASK_OFFSET`. `this` is a live - // heap-allocated task; the worker thread has exclusive access to + // SAFETY: `task` is `self.task.task`, embedded in a live heap-allocated + // `ShellRmTask`; the worker thread has exclusive access to // `root_task` until it spawns subtasks. unsafe { - let this = ::from_work_task(task); + let this = + bun_ptr::container_of::(task, core::mem::offset_of!(Self, task.task)); DirTask::run_from_thread_pool_impl((*this).root_task); } } @@ -751,18 +747,29 @@ impl ShellRmTask { /// `this` is the live `heap::alloc`'d task; not touched again on this /// thread after return (unless a verbose pending-count keeps it alive). unsafe fn finish_concurrently(this: *mut ShellRmTask) { - // SAFETY: caller contract. - unsafe { ShellTask::on_finish::(this) }; - } - - /// # Safety - /// `this` must be a live `heap::alloc`'d [`ShellRmTask`] posted via - /// [`finish_concurrently`]; main thread. - fn run_from_main_thread(this: *mut ShellRmTask, interp: &Interpreter) { - // SAFETY: caller contract. + use bun_event_loop::{ArmedLoopTask, ConcurrentTask::AutoDeinit, EventLoopTask}; + use core::ptr::NonNull; + // SAFETY: caller contract. Raw place access only: queued verbose hops + // may read `*this` on the main thread concurrently, so no `Box`/`&mut` + // may cover it here; `poster` is moved out before the post, after + // which the main thread may free `*this`. unsafe { - let cmd = (*this).cmd; - Rm::on_shell_rm_task_done(interp, cmd, this); + let poster = (*this) + .task + .poster + .take() + .expect("shell task on the pool is armed"); + let armed = match &mut (*this).task.concurrent_task { + EventLoopTask::Js(ct) => { + ArmedLoopTask::Js(NonNull::from(ct.from(this, AutoDeinit::ManualDeinit))) + } + EventLoopTask::Mini(at) => ArmedLoopTask::Mini( + NonNull::new(at.from(this, shell_rm_task_run_from_main_thread_mini)) + .expect("intrusive task"), + ), + }; + poster.post(armed); + drop(poster); } } @@ -1548,8 +1555,9 @@ impl DirTask { // SAFETY: caller contract — `interp` set at create. let (interp, cmd) = unsafe { let tm = (*this).task_manager; - (&*(*tm).task.interp, (*tm).cmd) + ((*tm).task.interp, (*tm).cmd) }; + let interp = interp.get(); Rm::write_verbose(interp, cmd, this).run(interp); } @@ -1751,9 +1759,21 @@ impl bun_event_loop::Taskable for DirTask { } } +/// Mini-loop trampoline for [`ShellRmTask::finish_concurrently`]. +fn shell_rm_task_run_from_main_thread_mini(this: *mut ShellRmTask, _: *mut ()) { + // SAFETY: `this` is the live heap task `finish_concurrently` posted; the + // mini loop fires it once on the main thread. + ShellTask::run_from_main_thread::(unsafe { bun_core::heap::take(this) }); +} + impl crate::shell::interpreter::ShellTaskCtx for ShellRmTask { - const TASK_OFFSET: usize = core::mem::offset_of!(Self, task); - fn run_from_thread_pool(_this: &mut Self) { + fn shell_task(&self) -> &ShellTask { + &self.task + } + fn shell_task_mut(&mut self) -> &mut ShellTask { + &mut self.task + } + fn run_from_thread_pool(&mut self) { // Not reached: `ShellRmTask::schedule` installs `work_pool_callback` // directly (the generic trampoline auto-posts back, which would race // the recursive DirTask tree's own `finish_concurrently`). @@ -1762,8 +1782,10 @@ impl crate::shell::interpreter::ShellTaskCtx for ShellRmTask { "ShellRmTask scheduled via ShellTask::schedule; use ShellRmTask::schedule" ); } - fn run_from_main_thread(this: *mut Self, interp: &Interpreter) { - // SAFETY: `ShellTask::run_from_main_thread` dispatch contract. - Self::run_from_main_thread(this, interp) + /// Released back to the raw pending-callback protocol + /// (`decr_pending_and_maybe_deinit` frees it). + fn run_from_main_thread(self: Box, interp: &Interpreter) { + let cmd = self.cmd; + Rm::on_shell_rm_task_done(interp, cmd, bun_core::heap::into_raw(self)); } } diff --git a/src/runtime/shell/builtin/seq.rs b/src/runtime/shell/builtin/seq.rs index fbad2252f12f..cc144bb91689 100644 --- a/src/runtime/shell/builtin/seq.rs +++ b/src/runtime/shell/builtin/seq.rs @@ -18,10 +18,8 @@ pub struct Seq { start: f32, end: f32, increment: f32, - /// Borrowed from argv (NUL-terminated arena strings) or `'static` literals; - /// argv outlives the builtin — `RawSlice` invariant. - separator: bun_ptr::RawSlice, - terminator: bun_ptr::RawSlice, + separator: Vec, + terminator: Vec, } impl Default for Seq { @@ -31,8 +29,8 @@ impl Default for Seq { start: 1.0, end: 1.0, increment: 1.0, - separator: bun_ptr::RawSlice::new(b"\n"), - terminator: bun_ptr::RawSlice::EMPTY, + separator: b"\n".to_vec(), + terminator: Vec::new(), } } } @@ -44,24 +42,23 @@ impl Seq { return Self::fail(interp, cmd, Kind::Seq.usage_string()); } + let arg_at = |i: usize| -> Vec { Builtin::of(interp, cmd).arg_bytes(i).to_vec() }; let mut idx = 0usize; - // Flag parsing — operates on raw argv pointers so we can stash - // borrowed slices into separator/terminator. while idx < argc { - let arg = Builtin::of(interp, cmd).arg_bytes(idx); + let arg = arg_at(idx); if arg == b"-s" || arg == b"--separator" { idx += 1; if idx >= argc { return Self::fail(interp, cmd, b"seq: option requires an argument -- s\n"); } - let bytes = Builtin::of(interp, cmd).arg_bytes(idx); - Self::state_mut(interp, cmd).separator = bun_ptr::RawSlice::new(bytes); + let bytes = arg_at(idx); + Self::state_mut(interp, cmd).separator = bytes; idx += 1; continue; } if arg.starts_with(b"-s") && arg.len() > 2 { - Self::state_mut(interp, cmd).separator = bun_ptr::RawSlice::new(&arg[2..]); + Self::state_mut(interp, cmd).separator = arg[2..].to_vec(); idx += 1; continue; } @@ -70,13 +67,13 @@ impl Seq { if idx >= argc { return Self::fail(interp, cmd, b"seq: option requires an argument -- t\n"); } - let bytes = Builtin::of(interp, cmd).arg_bytes(idx); - Self::state_mut(interp, cmd).terminator = bun_ptr::RawSlice::new(bytes); + let bytes = arg_at(idx); + Self::state_mut(interp, cmd).terminator = bytes; idx += 1; continue; } if arg.starts_with(b"-t") && arg.len() > 2 { - Self::state_mut(interp, cmd).terminator = bun_ptr::RawSlice::new(&arg[2..]); + Self::state_mut(interp, cmd).terminator = arg[2..].to_vec(); idx += 1; continue; } @@ -90,8 +87,8 @@ impl Seq { // Positional args. macro_rules! parse_num { ($i:expr) => {{ - let s = Builtin::of(interp, cmd).arg_bytes($i); - match parse_f32(s) { + let s = arg_at($i); + match parse_f32(&s) { Some(n) if n.is_finite() => n, _ => return Self::fail(interp, cmd, b"seq: invalid argument\n"), } @@ -104,7 +101,7 @@ impl Seq { let int1 = parse_num!(idx); idx += 1; { - let me = Self::state_mut(interp, cmd); + let mut me = Self::state_mut(interp, cmd); me.end = int1; if me.start > me.end { me.increment = -1.0; @@ -115,7 +112,8 @@ impl Seq { let int2 = parse_num!(idx); idx += 1; { - let me = Self::state_mut(interp, cmd); + let mut me = Self::state_mut(interp, cmd); + let me = &mut *me; me.start = int1; me.end = int2; me.increment = if me.start < me.end { @@ -129,19 +127,22 @@ impl Seq { if idx < argc { let int3 = parse_num!(idx); { - let me = Self::state_mut(interp, cmd); + let mut me = Self::state_mut(interp, cmd); me.start = int1; me.increment = int2; me.end = int3; } - let me = Self::state_mut(interp, cmd); - if me.increment == 0.0 { + let (start, end, increment) = { + let me = Self::state_mut(interp, cmd); + (me.start, me.end, me.increment) + }; + if increment == 0.0 { return Self::fail(interp, cmd, b"seq: zero increment\n"); } - if me.start > me.end && me.increment > 0.0 { + if start > end && increment > 0.0 { return Self::fail(interp, cmd, b"seq: needs negative decrement\n"); } - if me.start < me.end && me.increment < 0.0 { + if start < end && increment < 0.0 { return Self::fail(interp, cmd, b"seq: needs positive increment\n"); } } @@ -160,8 +161,14 @@ impl Seq { // Render entirely into a local Vec, then either enqueue it or // write_no_io it; we buffer once for simplicity. let (start, end, incr, sep, term) = { - let me = Self::state_mut(interp, cmd); - (me.start, me.end, me.increment, me.separator, me.terminator) + let mut me = Self::state_mut(interp, cmd); + ( + me.start, + me.end, + me.increment, + core::mem::take(&mut me.separator), + core::mem::take(&mut me.terminator), + ) }; let mut out = Vec::new(); let mut current = start; @@ -173,7 +180,7 @@ impl Seq { // Rust `{}` for f32 prints the shortest decimal that round-trips // (no exponent, no trailing ".0"). let _ = write!(&mut out, "{}", current); - out.extend_from_slice(sep.slice()); + out.extend_from_slice(&sep); let next = current + incr; if next == current { // f32 rounding can make `current + incr` equal `current` @@ -184,15 +191,13 @@ impl Seq { } current = next; } - out.extend_from_slice(term.slice()); + out.extend_from_slice(&term); Self::state_mut(interp, cmd).state = State::Done; if needs_io { let safeguard = Builtin::of(interp, cmd).stdout.needs_io().unwrap(); let child = ChildPtr::new(cmd, WriterTag::Builtin); - return Builtin::of_mut(interp, cmd) - .stdout - .enqueue(child, &out, safeguard); + return Builtin::write_out(interp, cmd, IoKind::Stdout, child, &out, safeguard); } let _ = Builtin::write_no_io(interp, cmd, IoKind::Stdout, &out); Builtin::done(interp, cmd, 0) @@ -208,7 +213,8 @@ impl Seq { Self::state_mut(interp, cmd).state = State::Err; return Builtin::done(interp, cmd, 1); } - match Self::state_mut(interp, cmd).state { + let state = Self::state_mut(interp, cmd).state; + match state { State::Done => Builtin::done(interp, cmd, 0), State::Err => Builtin::done(interp, cmd, 1), State::Idle => { diff --git a/src/runtime/shell/builtin/touch.rs b/src/runtime/shell/builtin/touch.rs index c4ef56c0ff31..76db8eead5d9 100644 --- a/src/runtime/shell/builtin/touch.rs +++ b/src/runtime/shell/builtin/touch.rs @@ -1,8 +1,8 @@ use crate::shell::ExitCode; use crate::shell::builtin::{Builtin, BuiltinState, IoKind, Kind}; use crate::shell::interpreter::{ - EventLoopHandle, FlagParser, Interpreter, NodeId, OutputSrc, OutputTask, OutputTaskVTable, - ParseFlagResult, ShellTask, parse_flags, unsupported_flag, + FlagParser, Interpreter, NodeId, OutputSrc, OutputTask, OutputTaskVTable, OutputWrite, + ParseFlagResult, ShellTask, unsupported_flag, }; use crate::shell::io_writer::{ChildPtr, WriterTag}; use crate::shell::yield_::Yield; @@ -30,19 +30,17 @@ pub struct ExecState { /// Index into argv where filepath args start. pub(crate) args_start: usize, pub(crate) err: Option, - /// FIFO of in-flight OutputTask pointers awaiting an IOWriter chunk - /// completion. Stopgap until `WriterTag` can carry the `*mut OutputTask` - /// directly — see mkdir.rs `Exec::output_queue` for rationale. - pub(crate) output_queue: std::collections::VecDeque<*mut OutputTask>, + /// FIFO of in-flight OutputTasks awaiting an IOWriter chunk completion — + /// see mkdir.rs `Exec::output_queue` for rationale. + pub(crate) output_queue: std::collections::VecDeque>>, } impl Touch { pub(crate) fn start(interp: &Interpreter, cmd: NodeId) -> Yield { let mut opts = Opts::default(); let args_start = { - let args = Builtin::of(interp, cmd).args_slice(); - match parse_flags(&mut opts, args) { - Ok(Some(rest)) => args.len() - rest.len(), + match Builtin::parse_flags(interp, cmd, &mut opts) { + Ok(Some(start)) => start, Ok(None) => { Self::state_mut(interp, cmd).state = State::WaitingWriteErr; return Builtin::write_failing_error( @@ -76,6 +74,9 @@ impl Touch { enum Action { Done(ExitCode), Schedule(usize), + Suspend, + Failed, + AlreadyDone, } let action = match &mut Self::state_mut(interp, cmd).state { State::Idle => panic!("Invalid state"), @@ -88,34 +89,34 @@ impl Touch { exec.err = None; Action::Done(code) } else { - return Yield::suspended(); + Action::Suspend } } else { exec.started = true; Action::Schedule(exec.args_start) } } - State::WaitingWriteErr => return Yield::failed(), - State::Done => return Builtin::done(interp, cmd, 0), + State::WaitingWriteErr => Action::Failed, + State::Done => Action::AlreadyDone, }; match action { + Action::Suspend => Yield::suspended(), + Action::Failed => Yield::failed(), + Action::AlreadyDone => Builtin::done(interp, cmd, 0), Action::Done(code) => { Self::state_mut(interp, cmd).state = State::Done; Builtin::done(interp, cmd, code) } Action::Schedule(args_start) => { - let argc = Builtin::of(interp, cmd).args_slice().len(); + let argc = Builtin::argc(interp, cmd); if let State::Exec(exec) = &mut Self::state_mut(interp, cmd).state { exec.tasks_count = argc - args_start; } - let cwd = Builtin::shell(interp, cmd).cwd().to_vec(); - let evtloop = Builtin::event_loop(interp, cmd); - let interp_ptr: *mut Interpreter = interp.as_ctx_ptr(); + let cwd = Builtin::shell(interp, cmd).borrow().cwd().to_vec(); for i in args_start..argc { let path = Builtin::of(interp, cmd).arg_bytes(i).to_vec(); - let task = ShellTouchTask::create(cmd, path, cwd.clone(), evtloop, interp_ptr); - // SAFETY: freshly heap-allocated. - unsafe { ShellTask::schedule(task) }; + let task = ShellTouchTask::create(cmd, path, cwd.clone(), interp); + ShellTask::schedule(task); } Yield::suspended() } @@ -137,25 +138,18 @@ impl Touch { None }; if let Some(task) = pending { - // SAFETY: `task` was heap-allocated in `OutputTask::new` and - // pushed by `write_err`/`write_out`; not yet freed. - return unsafe { OutputTask::::on_io_writer_chunk(task, interp, written, e) }; + return OutputTask::::on_io_writer_chunk(task, interp, written, e); } Self::next(interp, cmd) } - /// # Safety - /// `task` must be a live heap allocation produced by - /// [`ShellTouchTask::create`]; ownership is reclaimed here. - fn on_shell_touch_task_done(interp: &Interpreter, cmd: NodeId, task: *mut ShellTouchTask) { - // SAFETY: task was heap-allocated in create(); reclaim. - let mut task = unsafe { bun_core::heap::take(task) }; + fn on_shell_touch_task_done(interp: &Interpreter, cmd: NodeId, mut task: ShellTouchTask) { if let State::Exec(exec) = &mut Self::state_mut(interp, cmd).state { exec.tasks_done += 1; } if let Some(e) = task.err.take() { let output_task = OutputTask::::new(cmd, OutputSrc::Arrlist(Vec::new())); - let errstr = Builtin::task_error_to_string(interp, cmd, Kind::Touch, &e).to_vec(); + let errstr = Builtin::task_error_to_string(Kind::Touch, &e); if let State::Exec(exec) = &mut Self::state_mut(interp, cmd).state { exec.err = Some(e); } @@ -170,27 +164,31 @@ impl OutputTaskVTable for Touch { fn write_err( interp: &Interpreter, cmd: NodeId, - child: *mut OutputTask, + child: Box>, errbuf: &[u8], - ) -> Option { + ) -> OutputWrite { if let State::Exec(exec) = &mut Self::state_mut(interp, cmd).state { exec.output_waiting += 1; } - if let Some(safeguard) = Builtin::of(interp, cmd).stderr.needs_io() { - // Stash so on_io_writer_chunk can route to the OutputTask state - // machine and reclaim the box (stopgap for missing WriterTag). + let stderr_needs_io = Builtin::of(interp, cmd).stderr.needs_io(); + if let Some(safeguard) = stderr_needs_io { + // Park it so on_io_writer_chunk can route the completion back to + // the OutputTask state machine (there is no WriterTag for it). if let State::Exec(exec) = &mut Self::state_mut(interp, cmd).state { exec.output_queue.push_back(child); } let childptr = ChildPtr::new(cmd, WriterTag::Builtin); - return Some( - Builtin::of_mut(interp, cmd) - .stderr - .enqueue(childptr, errbuf, safeguard), - ); + return OutputWrite::Enqueued(Builtin::write_out( + interp, + cmd, + IoKind::Stderr, + childptr, + errbuf, + safeguard, + )); } let _ = Builtin::write_no_io(interp, cmd, IoKind::Stderr, errbuf); - None + OutputWrite::Done(child) } fn on_write_err(interp: &Interpreter, cmd: NodeId) { if let State::Exec(exec) = &mut Self::state_mut(interp, cmd).state { @@ -200,27 +198,30 @@ impl OutputTaskVTable for Touch { fn write_out( interp: &Interpreter, cmd: NodeId, - child: *mut OutputTask, - output: &mut OutputSrc, - ) -> Option { + child: Box>, + ) -> OutputWrite { if let State::Exec(exec) = &mut Self::state_mut(interp, cmd).state { exec.output_waiting += 1; } - if let Some(safeguard) = Builtin::of(interp, cmd).stdout.needs_io() { - if let State::Exec(exec) = &mut Self::state_mut(interp, cmd).state { - exec.output_queue.push_back(child); - } + let stdout_needs_io = Builtin::of(interp, cmd).stdout.needs_io(); + if let Some(safeguard) = stdout_needs_io { let childptr = ChildPtr::new(cmd, WriterTag::Builtin); - let buf = output.slice().to_vec(); - return Some( - Builtin::of_mut(interp, cmd) - .stdout - .enqueue(childptr, &buf, safeguard), - ); + return OutputWrite::Enqueued(Builtin::write_out_with( + interp, + cmd, + IoKind::Stdout, + childptr, + safeguard, + |buf| { + buf.extend_from_slice(child.output.slice()); + if let State::Exec(exec) = &mut Self::state_mut(interp, cmd).state { + exec.output_queue.push_back(child); + } + }, + )); } - let buf = output.slice().to_vec(); - let _ = Builtin::write_no_io(interp, cmd, IoKind::Stdout, &buf); - None + let _ = Builtin::write_no_io(interp, cmd, IoKind::Stdout, child.output.slice()); + OutputWrite::Done(child) } fn on_write_out(interp: &Interpreter, cmd: NodeId) { if let State::Exec(exec) = &mut Self::state_mut(interp, cmd).state { @@ -241,23 +242,22 @@ pub struct ShellTouchTask { pub task: ShellTask, } +crate::shell_task!(ShellTouchTask); + impl ShellTouchTask { pub(crate) fn create( cmd: NodeId, filepath: Vec, cwd_path: Vec, - evtloop: EventLoopHandle, - interp: *mut Interpreter, - ) -> *mut ShellTouchTask { - let mut task = Box::new(ShellTouchTask { + interp: &Interpreter, + ) -> Box { + Box::new(ShellTouchTask { cmd, filepath, cwd_path, err: None, - task: ShellTask::new(evtloop), - }); - task.task.interp = interp; - bun_core::heap::into_raw(task) + task: ShellTask::new(interp), + }) } /// utimes() the path; on ENOENT @@ -306,43 +306,28 @@ impl ShellTouchTask { this.err = Some(err.with_path(filepath.as_bytes())); } } - // Worker→main bounce-back is posted by `shell_task_trampoline` after + // Worker→main bounce-back is posted by `ShellTask::run_owned` after // this returns. } - - /// # Safety - /// `this` must be a live heap allocation produced by [`Self::create`]; - /// ownership is consumed via [`Touch::on_shell_touch_task_done`]. - fn run_from_main_thread(this: *mut ShellTouchTask, interp: &Interpreter) { - // SAFETY: `this` is a live heap-allocated task. - let cmd = unsafe { (*this).cmd }; - // SAFETY: forwarded from caller's contract. - Touch::on_shell_touch_task_done(interp, cmd, this); - } } -impl bun_event_loop::Taskable for ShellTouchTask { - const TAG: bun_event_loop::TaskTag = bun_event_loop::task_tag::ShellTouchTask; - /// A pool completion that will not run: drop the keep-alive and the box - /// (nothing else frees an unrun one). - unsafe fn release_unrun(this: *mut Self) { - // SAFETY: fn contract — the box the builtin scheduled. - unsafe { - (*this).task.unref_unrun(); - drop(bun_core::heap::take(this)); - } - } -} +// `runtime::dispatch::run_task`'s `task_tag::ShellTouchTask` arm reboxes the +// pointer `ShellTask::on_finish` posted; a completion that will not run drops +// the keep-alive and the box. impl crate::shell::interpreter::ShellTaskCtx for ShellTouchTask { - const TASK_OFFSET: usize = core::mem::offset_of!(Self, task); - fn run_from_thread_pool(this: &mut Self) { - Self::run_from_thread_pool(this) + fn shell_task(&self) -> &ShellTask { + &self.task + } + fn shell_task_mut(&mut self) -> &mut ShellTask { + &mut self.task + } + fn run_from_thread_pool(&mut self) { + Self::run_from_thread_pool(self) } - fn run_from_main_thread(this: *mut Self, interp: &Interpreter) { - // SAFETY: `ShellTaskCtx` callers guarantee `this` is the live - // heap-allocated task posted via `ShellTask::schedule`. - Self::run_from_main_thread(this, interp) + fn run_from_main_thread(self: Box, interp: &Interpreter) { + let cmd = self.cmd; + Touch::on_shell_touch_task_done(interp, cmd, *self); } } @@ -375,9 +360,7 @@ impl FlagParser for Opts { b'm' => Some(ParseFlagResult::Unsupported(unsupported_flag(b"-m"))), b'r' => Some(ParseFlagResult::Unsupported(unsupported_flag(b"-r"))), b't' => Some(ParseFlagResult::Unsupported(unsupported_flag(b"-t"))), - _ => Some(ParseFlagResult::IllegalOption( - &raw const smallflags[1 + i..], - )), + _ => Some(ParseFlagResult::IllegalOption(smallflags[1 + i..].into())), } } } diff --git a/src/runtime/shell/builtin/which.rs b/src/runtime/shell/builtin/which.rs index 9d53713b2fb8..2bd050eb6b2d 100644 --- a/src/runtime/shell/builtin/which.rs +++ b/src/runtime/shell/builtin/which.rs @@ -32,12 +32,11 @@ impl Which { pub(crate) fn start(interp: &Interpreter, cmd: NodeId) -> Yield { let argc = Builtin::of(interp, cmd).args_slice().len(); if argc == 0 { - if let Some(safeguard) = Builtin::of(interp, cmd).stdout.needs_io() { + let stdout_needs_io = Builtin::of(interp, cmd).stdout.needs_io(); + if let Some(safeguard) = stdout_needs_io { Self::state_mut(interp, cmd).state = State::OneArg; let child = ChildPtr::new(cmd, WriterTag::Builtin); - return Builtin::of_mut(interp, cmd) - .stdout - .enqueue(child, b"\n", safeguard); + return Builtin::write_out(interp, cmd, IoKind::Stdout, child, b"\n", safeguard); } let _ = Builtin::write_no_io(interp, cmd, IoKind::Stdout, b"\n"); return Builtin::done(interp, cmd, 1); @@ -53,23 +52,17 @@ impl Which { match search.resolve(&arg) { Some(resolved) => { let buf = Builtin::fmt_error_arena( - interp, - cmd, None, format_args!("{}\n", bstr::BStr::new(&resolved)), - ) - .to_vec(); + ); let _ = Builtin::write_no_io(interp, cmd, IoKind::Stdout, &buf); } None => { had_not_found = true; let buf = Builtin::fmt_error_arena( - interp, - cmd, Some(Kind::Which), format_args!("{} not found\n", bstr::BStr::new(&arg)), - ) - .to_vec(); + ); let _ = Builtin::write_no_io(interp, cmd, IoKind::Stdout, &buf); } } @@ -114,8 +107,12 @@ impl Which { *had_not_found = true; *waiting_write = true; } - if let Some(safeguard) = Builtin::of(interp, cmd).stdout.needs_io() { - return Builtin::of_mut(interp, cmd).stdout.enqueue_fmt( + let stdout_needs_io = Builtin::of(interp, cmd).stdout.needs_io(); + if let Some(safeguard) = stdout_needs_io { + return Builtin::write_out_fmt( + interp, + cmd, + IoKind::Stdout, child, None, format_args!("{} not found\n", bstr::BStr::new(&arg)), @@ -123,12 +120,9 @@ impl Which { ); } let buf = Builtin::fmt_error_arena( - interp, - cmd, None, format_args!("{} not found\n", bstr::BStr::new(&arg)), - ) - .to_vec(); + ); let _ = Builtin::write_no_io(interp, cmd, IoKind::Stdout, &buf); Self::arg_complete(interp, cmd) } @@ -138,8 +132,12 @@ impl Which { { *waiting_write = true; } - if let Some(safeguard) = Builtin::of(interp, cmd).stdout.needs_io() { - return Builtin::of_mut(interp, cmd).stdout.enqueue_fmt( + let stdout_needs_io = Builtin::of(interp, cmd).stdout.needs_io(); + if let Some(safeguard) = stdout_needs_io { + return Builtin::write_out_fmt( + interp, + cmd, + IoKind::Stdout, child, None, format_args!("{}\n", bstr::BStr::new(&resolved)), @@ -147,12 +145,9 @@ impl Which { ); } let buf = Builtin::fmt_error_arena( - interp, - cmd, None, format_args!("{}\n", bstr::BStr::new(&resolved)), - ) - .to_vec(); + ); let _ = Builtin::write_no_io(interp, cmd, IoKind::Stdout, &buf); Self::arg_complete(interp, cmd) } @@ -181,10 +176,18 @@ impl Which { if let Some(err) = e { return Builtin::done(interp, cmd, err.errno as crate::shell::ExitCode); } - match Self::state_mut(interp, cmd).state { - State::OneArg => Builtin::done(interp, cmd, 1), - State::MultiArgs { .. } => Self::arg_complete(interp, cmd), - _ => Builtin::done(interp, cmd, 0), + enum Then { + Done(crate::shell::ExitCode), + ArgComplete, + } + let then = match Self::state_mut(interp, cmd).state { + State::OneArg => Then::Done(1), + State::MultiArgs { .. } => Then::ArgComplete, + _ => Then::Done(0), + }; + match then { + Then::Done(code) => Builtin::done(interp, cmd, code), + Then::ArgComplete => Self::arg_complete(interp, cmd), } } @@ -203,6 +206,7 @@ struct SearchEnv { impl SearchEnv { fn load(interp: &Interpreter, cmd: NodeId) -> Self { let shell = Builtin::shell(interp, cmd); + let shell = shell.borrow(); // `EnvMap::get` refs the returned string; balance it. let path_env = shell .export_env diff --git a/src/runtime/shell/builtin/yes.rs b/src/runtime/shell/builtin/yes.rs index b22ec9934a64..651c0d885aab 100644 --- a/src/runtime/shell/builtin/yes.rs +++ b/src/runtime/shell/builtin/yes.rs @@ -1,5 +1,5 @@ use crate::shell::ExitCode; -use crate::shell::builtin::{Builtin, BuiltinIO, BuiltinState, Impl, Kind}; +use crate::shell::builtin::{Builtin, BuiltinIO, BuiltinState, Impl, IoKind, Kind}; use crate::shell::interpreter::{EventLoopHandle, Interpreter, NodeId, OutputNeedsIOSafeGuard}; use crate::shell::io_writer::{ChildPtr, WriterTag}; use crate::shell::states::cmd::Exec; @@ -64,20 +64,21 @@ impl Yes { } let evtloop = Builtin::event_loop(interp, cmd); - let interp_ptr: *mut Interpreter = interp.as_ctx_ptr(); { - let me = Self::state_mut(interp, cmd); + let mut me = Self::state_mut(interp, cmd); me.buffer = buf; me.buffer_used = filled; me.task = Some(YesTask { - interp: interp_ptr, + interp: bun_ptr::ParentRef::new(interp), cmd, evtloop, concurrent_task: EventLoopTask::from_event_loop(evtloop), }); } - if let Some(safeguard) = Builtin::of(interp, cmd).stdout.needs_io() { + let stdout_needs_io = Builtin::of(interp, cmd).stdout.needs_io(); + + if let Some(safeguard) = stdout_needs_io { Self::state_mut(interp, cmd).state = State::WaitingIo; return Self::enqueue_chunk(interp, cmd, safeguard); } @@ -92,8 +93,9 @@ impl Yes { // are accessible simultaneously — the buffer is written zero-copy, // which matters for `yes` throughput. let err = { - let cmd_node = interp.as_cmd_mut(cmd); - let shell = cmd_node.base.shell; + let mut cmd_node = interp.as_cmd_mut(cmd); + let cmd_node = &mut *cmd_node; + let shell = cmd_node.base.shell.borrow(); let Exec::Builtin(me) = &mut cmd_node.exec else { unreachable!() }; @@ -101,8 +103,7 @@ impl Yes { let chunk = &yes.buffer[..yes.buffer_used]; let mut err = None; for _ in 0..4 { - // SAFETY: `shell` is `cmd_node.base.shell`, live for the Cmd. - if let Err(e) = unsafe { stdout.write_no_io_to(shell, chunk) } { + if let Err(e) = stdout.write_no_io_to(&shell, chunk) { err = Some(e); break; } @@ -111,12 +112,9 @@ impl Yes { }; if let Some(e) = err { let buf = Builtin::fmt_error_arena( - interp, - cmd, Some(Kind::Yes), format_args!("{}\n", bstr::BStr::new(e.name())), - ) - .to_vec(); + ); return Self::write_failing_error(interp, cmd, &buf, 1); } // Bounce back via the event loop so we don't block the main thread. @@ -139,10 +137,10 @@ impl Yes { safeguard: OutputNeedsIOSafeGuard, ) -> Yield { let child = ChildPtr::new(cmd, WriterTag::Builtin); - // `stdout` and `impl_` are disjoint fields of `Builtin` — split-borrow - // so the tiled buffer is enqueued zero-copy. - let (stdout, yes) = Self::split_stdout_state(Builtin::of_mut(interp, cmd)); - stdout.enqueue(child, &yes.buffer[..yes.buffer_used], safeguard) + Builtin::write_out_with(interp, cmd, IoKind::Stdout, child, safeguard, |buf| { + let yes = Self::state_mut(interp, cmd); + buf.extend_from_slice(&yes.buffer[..yes.buffer_used]); + }) } fn write_failing_error( @@ -191,7 +189,7 @@ impl Yes { #[repr(C)] pub struct YesTask { /// Back-ref to the owning [`Interpreter`]. - pub(crate) interp: *mut Interpreter, + pub(crate) interp: bun_ptr::ParentRef, pub(crate) cmd: NodeId, pub(crate) evtloop: EventLoopHandle, pub(crate) concurrent_task: EventLoopTask, @@ -240,8 +238,7 @@ impl YesTask { /// [`Yes::start`]. Reached only via the concurrent-task dispatch installed /// in [`enqueue`](Self::enqueue). pub(crate) fn run_from_main_thread(this: &Self) { - // SAFETY: `interp` was set in `Yes::start` and outlives the task. - let (interp, cmd) = unsafe { (&*this.interp, this.cmd) }; + let (interp, cmd) = (&*this.interp, this.cmd); Yes::write_no_io_loop(interp, cmd).run(interp); } diff --git a/src/runtime/shell/dispatch_tasks.rs b/src/runtime/shell/dispatch_tasks.rs index 7a8d1fc7b65a..a7eae6f35d96 100644 --- a/src/runtime/shell/dispatch_tasks.rs +++ b/src/runtime/shell/dispatch_tasks.rs @@ -1,32 +1,43 @@ -//! Forward-decl shell task types referenced by `runtime::dispatch::run_task`. +//! Shell task types referenced by `runtime::dispatch::run_task`. //! //! Several shell task types collapsed into the NodeId-arena state machine -//! (`interpreter.rs`); the rest are gated behind `interpreter_body_gated.rs`. -//! The high-tier dispatcher must still cast the erased `Task.ptr` to a -//! concrete type and call the per-type entry point, so the shapes are -//! declared here. Bodies that already exist elsewhere re-export through this -//! module; the rest carry the body inline (mostly `run_from_main_thread()` → -//! resume the parent state via NodeId). +//! (`interpreter.rs`). The high-tier dispatcher must still rebox the erased +//! `Task.ptr` as a concrete type and call the per-type entry point, so the +//! shapes are declared here. Bodies that already exist elsewhere re-export +//! through this module; the rest carry the body inline (mostly +//! `run_from_main_thread()` → resume the parent state via NodeId). + +use bun_ptr::ParentRef; + +use crate::shell::interpreter::{Interpreter, NodeId, ShellTask, ShellTaskCtx}; -use crate::shell::interpreter::{Interpreter, NodeId, ShellTask}; /// Task payload for [`ShellAsync`](crate::shell::states::r#async::Async)'s /// bounce back to the main thread. The state lives in `interp.nodes`, so -/// the enqueued payload is `(interp, node)`. -#[repr(C)] +/// the enqueued payload is `(interp, node)`; one is boxed per `Async` node +/// and reused for each bounce. pub(crate) struct ShellAsyncTask { - pub interp: *mut Interpreter, + pub interp: ParentRef, pub node: NodeId, + /// Intrusive node for the mini-loop post (the JS loop queues a `Task`). + pub concurrent_task: bun_event_loop::AnyTaskWithExtraContext::AnyTaskWithExtraContext, } -/// Stat task backing shell conditional expressions (`[ -f x ]` etc.). Wraps an -/// inner [`ShellTask`]. -#[repr(C)] -pub(crate) struct ShellCondExprStatTask { - pub task: CondExprStatInner, +impl ShellAsyncTask { + pub(crate) fn run(self: Box) { + crate::shell::states::r#async::Async::run_from_main_thread(self); + } } -#[repr(C)] -pub(crate) struct CondExprStatInner { +impl bun_event_loop::AnyTaskWithExtraContext::BoxedMiniTaskRunner + for ShellAsyncTask +{ + fn run_from_loop_thread(owner: Box) { + owner.run(); + } +} + +/// Stat task backing shell conditional expressions (`[ -f x ]` etc.). +pub(crate) struct ShellCondExprStatTask { pub task: ShellTask, pub cond: NodeId, pub stat: bun_sys::Result, @@ -37,21 +48,23 @@ pub(crate) struct CondExprStatInner { pub cwd_fd: bun_sys::Fd, } -impl ShellCondExprStatTask { - /// # Safety - /// `this` must be a live `heap::alloc` payload paired with the schedule - /// site. Ownership of `*this` is consumed. - // Dispatch trampoline: `this` validity is guaranteed by the `run_task` - // contract; signature is fixed by `dispatch.rs`. - #[allow(clippy::not_unsafe_ptr_arg_deref)] - pub(crate) fn run_from_main_thread(this: *mut Self, interp: &Interpreter) { - // SAFETY: live Box'd task; paired with `heap::alloc` at schedule time. - let owned = unsafe { bun_core::heap::take(this) }; +crate::shell_task!(ShellCondExprStatTask); + +impl ShellTaskCtx for ShellCondExprStatTask { + fn shell_task(&self) -> &ShellTask { + &self.task + } + fn shell_task_mut(&mut self) -> &mut ShellTask { + &mut self.task + } + fn run_from_thread_pool(&mut self) { + debug_assert!(self.path.last() == Some(&0)); + let z = bun_core::ZStr::from_buf(&self.path, self.path.len() - 1); + self.stat = crate::shell::interpreter::shell_statat(self.cwd_fd, z); + } + fn run_from_main_thread(self: Box, interp: &Interpreter) { crate::shell::states::cond_expr::CondExpr::on_stat_task_done( - interp, - owned.task.cond, - &owned.task.stat, - &owned.task.path, + interp, self.cond, &self.stat, &self.path, ); } } @@ -63,7 +76,6 @@ pub enum ShellGlobErr { } /// Glob-expansion task run off the JS thread during word expansion. -#[repr(C)] pub(crate) struct ShellGlobTask { pub task: ShellTask, pub expansion: NodeId, @@ -72,63 +84,47 @@ pub(crate) struct ShellGlobTask { pub err: Option, } -impl bun_event_loop::Taskable for ShellGlobTask { - const TAG: bun_event_loop::TaskTag = bun_event_loop::task_tag::ShellGlobTask; - /// A pool completion that will not run: drop the keep-alive and the box - /// (nothing else frees an unrun one). - unsafe fn release_unrun(this: *mut Self) { - // SAFETY: fn contract — the box the builtin scheduled. - unsafe { - (*this).task.unref_unrun(); - drop(bun_core::heap::take(this)); - } - } -} +crate::shell_task!(ShellGlobTask); -impl crate::shell::interpreter::ShellTaskCtx for ShellGlobTask { - const TASK_OFFSET: usize = core::mem::offset_of!(Self, task); - fn run_from_thread_pool(this: &mut Self) { - match Self::walk_impl(&mut this.walker, &mut this.result) { +impl ShellTaskCtx for ShellGlobTask { + fn shell_task(&self) -> &ShellTask { + &self.task + } + fn shell_task_mut(&mut self) -> &mut ShellTask { + &mut self.task + } + fn run_from_thread_pool(&mut self) { + match Self::walk_impl(&mut self.walker, &mut self.result) { Ok(Ok(())) => {} - Ok(Err(e)) => this.err = Some(ShellGlobErr::Syscall(e)), - Err(e) => this.err = Some(ShellGlobErr::Unknown(e)), + Ok(Err(e)) => self.err = Some(ShellGlobErr::Syscall(e)), + Err(e) => self.err = Some(ShellGlobErr::Unknown(e)), } } - // Dispatch trampoline: `this` validity is guaranteed by the `run_task` - // contract; signature is fixed by the `ShellTaskCtx` trait. - #[allow(clippy::not_unsafe_ptr_arg_deref)] - fn run_from_main_thread(this: *mut Self, interp: &Interpreter) { - // SAFETY: paired with `heap::alloc` in `create_and_schedule`. - let mut me = unsafe { bun_core::heap::take(this) }; + fn run_from_main_thread(mut self: Box, interp: &Interpreter) { crate::shell::states::expansion::Expansion::on_glob_walk_done( interp, - me.expansion, - core::mem::take(&mut me.result), - me.err.take(), + self.expansion, + core::mem::take(&mut self.result), + self.err.take(), ); } } impl ShellGlobTask { - /// Heap-allocate the glob task for `expansion` and schedule it on the - /// work pool; the allocation is freed in `run_from_main_thread`. + /// Box the glob task for `expansion` and schedule it on the work pool; + /// it comes back through `run_from_main_thread`. pub(crate) fn create_and_schedule( interp: &Interpreter, expansion: NodeId, walker: bun_glob::BunGlobWalkerZ, ) { - let mut task = ShellTask::new(interp.event_loop); - task.interp = interp.as_ctx_ptr(); - let this = bun_core::heap::alloc(ShellGlobTask { - task, + ShellTask::schedule(Box::new(ShellGlobTask { + task: ShellTask::new(interp), expansion, walker, result: Vec::new(), err: None, - }); - // SAFETY: `this` is a fresh heap allocation embedding `ShellTask` at - // `TASK_OFFSET`; freed in `run_from_main_thread`. - unsafe { ShellTask::schedule::(this) }; + })); } fn walk_impl( diff --git a/src/runtime/shell/interpreter.rs b/src/runtime/shell/interpreter.rs index 3cdf1eba50c1..72ad73265b53 100644 --- a/src/runtime/shell/interpreter.rs +++ b/src/runtime/shell/interpreter.rs @@ -17,18 +17,23 @@ //! //! A parent/child back-pointer tree would be borrow-checker hostile //! (overlapping `&mut` of parent and child), so all state nodes live in a -//! flat `Vec` owned by the `Interpreter`. Nodes refer to each other -//! (and to their parent) by `NodeId` — a `u32` index. Dispatch is a single -//! hoisted `match` on the parent's tag (`Interpreter::child_done`), which -//! keeps the per-tick hot path inlined (see PORTING.md §Dispatch hot-path). +//! flat arena owned by the `Interpreter`. Nodes refer to each other (and to +//! their parent) by `NodeId` — a `u32` index. Dispatch is a single hoisted +//! `match` on the parent's tag (`Interpreter::child_done`), which keeps the +//! per-tick hot path inlined (see PORTING.md §Dispatch hot-path). //! -//! State methods take `(&mut Interpreter, this: NodeId)` and look their -//! own data up via `interp.node_mut(this)` / `interp.nodes[this]`. +//! Each slot is a `RefCell` in its own heap box, so a borrowed node +//! stays put while the arena grows and overlapping borrows of one node are a +//! checked panic rather than aliasing. State methods take +//! `(&Interpreter, this: NodeId)` and look their own data up via +//! `interp.as_(this)` / `interp.as__mut(this)`; keep those borrows +//! short and never hold one across a call that re-enters the same node. use bun_collections::VecExt; use bun_jsc::JsCell; -use core::cell::Cell; +use core::cell::{Cell, Ref, RefCell, RefMut}; use core::fmt; +use std::rc::Rc; use std::sync::atomic::{AtomicBool, AtomicU32, Ordering}; use bun_sys::{self, Fd}; @@ -51,7 +56,6 @@ use crate::shell::yield_::Yield; use crate::shell::{ShellErr, ast}; bun_core::declare_scope!(SHELL, visible); -bun_core::declare_scope!(CowFd, hidden); /// `log!("...")` — scoped debug logger for the shell interpreter. Expands to /// nothing in release; the `SHELL` static is referenced by absolute path so @@ -98,9 +102,11 @@ impl fmt::Display for NodeId { } } +const NODE_CHUNK: usize = 8; +type NodeChunk = [RefCell; NODE_CHUNK]; + /// One slot in the interpreter's state arena. All state structs -/// live as enum variants in a single `Vec` so the only outstanding -/// borrow at any time is `&mut Interpreter`. +/// live as enum variants so the arena is a single homogeneous vector. pub enum Node { /// Freed slot, available for reuse by `alloc_node`. Free, @@ -135,8 +141,7 @@ impl Node { } } - /// Every state struct embeds a `Base` header at a known field; this is the - /// hoisted accessor. + /// Every state struct embeds a `Base` header; `None` for a freed slot. pub(crate) fn base(&self) -> Option<&Base> { match self { Node::Free => None, @@ -173,34 +178,33 @@ impl Node { } /// Generate `Interpreter::as_{,_mut}` typed accessors. These panic on -/// tag mismatch. +/// tag mismatch, and (being `RefCell` borrows) on an overlapping borrow of +/// the same node. macro_rules! node_accessors { ($($variant:ident => $ty:ty, $get:ident, $get_mut:ident);* $(;)?) => { impl Interpreter { $( #[inline] #[track_caller] - pub fn $get(&self, id: NodeId) -> &$ty { - match &self.nodes.get()[id.idx()] { + pub fn $get(&self, id: NodeId) -> Ref<'_, $ty> { + Ref::map(self.node(id), |n| match n { Node::$variant(v) => v, other => panic!( concat!("expected Node::", stringify!($variant), " at {}, got {:?}"), id, other.kind() ), - } + }) } #[inline] #[track_caller] - #[allow(clippy::mut_from_ref)] - pub fn $get_mut(&self, id: NodeId) -> &mut $ty { - // SAFETY: R-2 single-JS-thread invariant — see `nodes_mut`. - match unsafe { &mut self.nodes.get_mut()[id.idx()] } { + pub fn $get_mut(&self, id: NodeId) -> RefMut<'_, $ty> { + RefMut::map(self.node_mut(id), |n| match n { Node::$variant(v) => v, other => panic!( concat!("expected Node::", stringify!($variant), " at {}, got {:?}"), id, other.kind() ), - } + }) } )* } @@ -267,9 +271,14 @@ pub enum OutputNeedsIOSafeGuard { // interpreter entirely — see `DbgDepthGuard::MAX_DEPTH`). With every field // behind `UnsafeCell`, an overlapping `&Interpreter` is sound. pub struct Interpreter { - /// Flat arena of state-machine nodes. Indices are `NodeId`s; freed slots - /// are recycled via `free_list`. - pub(crate) nodes: JsCell>, + /// State-machine node arena. Indices are `NodeId`s; freed slots are + /// recycled via `free_list`. Slots live in fixed-size boxed chunks that + /// are only ever appended, so a `Ref`/`RefMut` into one stays valid while + /// the arena grows, and filling a slot goes through its own `RefCell`. + #[allow(clippy::vec_box, reason = "chunk address stability across growth")] + nodes: JsCell>>, + /// Slots handed out so far (high-water mark). + node_len: Cell, free_list: JsCell>, pub(crate) event_loop: EventLoopHandle, @@ -288,7 +297,7 @@ pub struct Interpreter { /// the mini-event-loop path always passes an empty vec. pub(crate) jsobjs: Vec, - pub(crate) root_shell: JsCell, + pub(crate) root_shell: EnvRc, pub(crate) root_io: JsCell, pub(crate) has_pending_activity: AtomicU32, @@ -308,12 +317,13 @@ pub struct Interpreter { pub(crate) estimated_size_for_gc: Cell, /// Lazily-populated UTF-8 cache for the JS-side argv (`$@`/`$N` expansion - /// when running under a Worker). See [`Interpreter::get_vm_args_utf8`]. + /// when running under a Worker). See [`Interpreter::append_var_argv`]. pub(crate) vm_args_utf8: JsCell>>, - /// `bun run` CLI context for `$N` expansion on the mini event loop. - /// Null when constructed from JS (no `ContextData` is reachable). - pub(crate) command_ctx: *mut bun_options_types::context::ContextData, + /// `bun run` CLI context for `$N` expansion on the mini event loop; the + /// caller of `init_and_run_*` owns it for longer than the interpreter + /// lives. `None` when constructed from JS. + command_ctx: Option>, } #[repr(transparent)] @@ -345,107 +355,10 @@ pub enum CleanupState { // Construction / standalone-exec entrypoints // ──────────────────────────────────────────────────────────────────────────── -impl ShellArgs { - /// Heap-allocated (returned as `Box`) because the interpreter stores - /// `Box` and state nodes hold `*const ast::*` into the arena; - /// the box must not move once `parse()` has filled `script_ast`. - pub(crate) fn init() -> Box { - Box::new(ShellArgs { - __arena: bun_alloc::Arena::new(), - // Overwritten by `parse()` before `run()`. An empty stmt list is - // a safe placeholder. - script_ast: ast::Script { stmts: &[] }, - }) - } - - #[inline] - pub(crate) fn arena(&self) -> &bun_alloc::Arena { - &self.__arena - } - - /// Store the parsed AST root alongside its owning arena. This is the single - /// self-referential lifetime-erasure point: `script` borrows `self.__arena` - /// for `'a`, but `ShellArgs` is heap-allocated and the arena is never moved - /// or dropped while the interpreter (and thus every state node holding - /// `*const ast::*`) is live. Widening `'a` → `'static` here is therefore - /// sound — every later dereference happens through raw pointers under - /// `unsafe`, which is where the real invariant is checked. - /// - /// `Interpreter::parse` returns the lifetime-tied `Script<'a>` so callers - /// that *don't* store (e.g. `TestingAPIs::shell_parse`) get the correct - /// borrow scope; only callers that move the arena into long-lived storage - /// route through this helper. - #[inline] - pub(crate) fn set_script_ast(&mut self, script: bun_shell_parser::ast::Script<'_>) { - // `ast::Script` is `bun_shell_parser::ast::Script<'static>` — identical - // type, only the lifetime parameter differs. `self.__arena` owns every - // node `script.stmts` references and is dropped only when this - // `ShellArgs` is, so the widened references remain valid for the - // interpreter's lifetime. Re-construct field-by-field via a slice - // pointer cast (`Stmt<'a>` and `Stmt<'static>` are layout-identical). - let stmts = script.stmts; - // SAFETY: lifetime-only widen; arena outlives `self` (see above). - let stmts: &'static [ast::Stmt] = - unsafe { core::slice::from_raw_parts(stmts.as_ptr().cast::(), stmts.len()) }; - self.script_ast = ast::Script { stmts }; - } - - /// Reports the arena's `allocated_bytes()` (a superset — tokens + strpool - /// + AST nodes). This is for GC `estimatedSize` reporting only, where - /// over-approximation is preferable to a tree walk on a lifetime-erased - /// AST mirror. - pub(crate) fn memory_cost(&self) -> usize { - core::mem::size_of::() + self.__arena.allocated_bytes() - } -} - /// Only used by the construction path (`Interpreter::init`). pub(crate) type ShellResult = Result; impl Interpreter { - /// Lex `src` (ASCII or Unicode), build a `Parser`, and return the root - /// `ast::Script`. Tokens and AST nodes are bump-allocated into `arena`. - /// - /// On lex error, `out_lex_err` is populated and `ParseError::Lex` returned - /// so the caller can `combineErrors()` for diagnostics; on parse error - /// `out_parse_err` is populated likewise. - pub(crate) fn parse<'a>( - arena: &'a bun_alloc::Arena, - src: &'a [u8], - jsobjs: &'a mut [crate::jsc::JSValue], - jsstrings_to_escape: &'a [bun_core::String], - out_parser: &mut Option>, - out_lex_result: &mut Option>, - ) -> crate::Result> { - use crate::shell::shell_body::{LexerAscii, LexerUnicode, ParseError, Parser}; - let jsobjs_len = jsobjs.len() as u32; - let lex_result = if bun_core::is_all_ascii(src) { - let mut lexer = LexerAscii::new(arena, src, jsstrings_to_escape, jsobjs_len); - lexer.lex().map_err(crate::Error::from)?; - lexer.get_result() - } else { - let mut lexer = LexerUnicode::new(arena, src, jsstrings_to_escape, jsobjs_len); - lexer.lex().map_err(crate::Error::from)?; - lexer.get_result() - }; - if !lex_result.errors.is_empty() { - *out_lex_result = Some(lex_result); - return Err(ParseError::Lex.into()); - } - let jsobjs_raw: &'a mut [bun_shell_parser::JSValueRaw] = { - let len = jsobjs.len(); - let ptr = jsobjs.as_mut_ptr().cast::(); - // SAFETY: `bun_jsc::JSValue` and `bun_shell_parser::JSValueRaw` are both - // `#[repr(transparent)]` over `usize` — see the `JSValueRaw` doc in - // `shell_parser/parse.rs`. Reinterpret in place via a typed pointer cast. - // Compute `len` before deriving the raw mut pointer so the shared - // reborrow inside `len()` does not stack on top of the Unique tag. - unsafe { core::slice::from_raw_parts_mut(ptr, len) } - }; - *out_parser = Some(Parser::new(arena, lex_result, jsobjs_raw)?); - Ok(out_parser.as_mut().unwrap().parse()?) - } - /// Builds the root `ShellExecEnv` (export env from the event loop's /// `DotEnv::Loader`, cwd from `getcwd()`, cwd_fd from `open(O_DIRECTORY)`), /// dups stdin into an `IOReader`, and heap-allocates the interpreter. @@ -454,17 +367,12 @@ impl Interpreter { /// /// On success the returned box owns `shargs`; on error `shargs` is /// dropped. - /// - /// Note: `allocator` parameter dropped (always global mimalloc). - /// `ctx` is stored for `bun run` argv access from builtins; held as a raw - /// pointer because the interpreter outlives any single `&mut ContextData` - /// borrow. // `ShellErr` is the shared shell-wide error type defined in `shell_body.rs`; // boxing it here would change `pub fn` signatures across every // `?`-propagating shell caller. #[allow(clippy::result_large_err)] pub(crate) fn init( - ctx: *mut bun_options_types::context::ContextData, + ctx: Option<&bun_options_types::context::ContextData>, event_loop: EventLoopHandle, shargs: Box, jsobjs: Vec, @@ -473,22 +381,19 @@ impl Interpreter { ) -> ShellResult> { // ── export_env ───────────────────────────────────────────────────── // On the `.js` event loop, take `export_env_` (or empty); on `.mini`, - // populate from `event_loop.env()` (the loop's `DotEnv::Loader`). + // populate from the loop's `DotEnv::Loader`. let export_env = if matches!(event_loop, EventLoopHandle::Js { .. }) { export_env_.unwrap_or_else(EnvMap::init) } else { - // SAFETY: `event_loop.env()` returns the `MiniEventLoop`'s - // `DotEnv::Loader`, which is set by `init_global()` and outlives - // the interpreter (thread-lifetime singleton). - let env_loader = unsafe { &mut *event_loop.env() }; - let mut export_env = EnvMap::init_with_capacity(env_loader.map.map.count()); - let mut iter = env_loader.iterator(); - while let Some(entry) = iter.next() { - let key = crate::shell::EnvStr::init_slice(&entry.key_ptr[..]); - let value = crate::shell::EnvStr::init_slice(&entry.value_ptr.value[..]); - export_env.insert(key, value); - } - export_env + event_loop.with_env(|env_loader| { + let mut export_env = EnvMap::init_with_capacity(env_loader.map.map.count()); + for (key, value) in env_loader.map.map.iter() { + let key = crate::shell::EnvStr::init_slice(&key[..]); + let value = crate::shell::EnvStr::init_slice(&value.value[..]); + export_env.insert(key, value); + } + export_env + }) }; // ── cwd / cwd_fd ─────────────────────────────────────────────────── @@ -514,6 +419,18 @@ impl Interpreter { cwd_arr.extend_from_slice(&pathbuf[..cwd_len + 1]); debug_assert_eq!(*cwd_arr.last().unwrap(), 0); + let (buffered_stdout, buffered_stderr) = CapturedBuf::new_pair(); + let root_shell = Rc::new(RefCell::new(ShellExecEnv { + buffered_stdout, + buffered_stderr, + shell_env: EnvMap::init(), + cmd_local_env: EnvMap::init(), + export_env, + __prev_cwd: cwd_arr.clone(), + __cwd: cwd_arr, + cwd_fd, + })); + // ── stdin ────────────────────────────────────────────────────────── log!("Duping stdin"); let stdin_fd_res = if bun_core::output::stdio::is_stdin_null() { @@ -532,12 +449,10 @@ impl Interpreter { } else { shell_dup(Fd::stdin()) }; + // On error `root_shell` drops here, closing `cwd_fd`. let stdin_fd = match stdin_fd_res { Ok(fd) => fd, - Err(e) => { - closefd(cwd_fd); - return Err(ShellErr::new_sys(&e)); - } + Err(e) => return Err(ShellErr::new_sys(&e)), }; let stdin_reader = IOReader::init(stdin_fd, event_loop); @@ -545,20 +460,12 @@ impl Interpreter { // ── assemble ─────────────────────────────────────────────────────── let interpreter = Box::new(Interpreter { nodes: JsCell::new(Vec::new()), + node_len: Cell::new(0), free_list: JsCell::new(Vec::new()), event_loop, args: JsCell::new(shargs), jsobjs, - root_shell: JsCell::new(ShellExecEnv { - _buffered_stdout: Bufio::default(), - _buffered_stderr: Bufio::default(), - shell_env: EnvMap::init(), - cmd_local_env: EnvMap::init(), - export_env, - __prev_cwd: cwd_arr.clone(), - __cwd: cwd_arr, - cwd_fd, - }), + root_shell, root_io: JsCell::new(IO { stdin: crate::shell::io::InKind::Fd(stdin_reader), // By default stdout/stderr should be IOWriters on dup'd @@ -581,26 +488,20 @@ impl Interpreter { cleanup_state: Cell::new(CleanupState::NeedsFullCleanup), estimated_size_for_gc: Cell::new(0), vm_args_utf8: JsCell::new(Vec::new()), - command_ctx: ctx, + command_ctx: ctx.map(bun_ptr::BackRef::new), }); // Wire the interpreter backref into root stdin so async poll // callbacks can drive `Yield::run`. - let interp_ptr: *mut Interpreter = Interpreter::as_ctx_ptr(&interpreter); if let crate::shell::io::InKind::Fd(ref r) = interpreter.root_io.get().stdin { - // SAFETY: `interp_ptr` is the live `Interpreter` just constructed. - r.set_interp(interp_ptr); + r.set_interp(&interpreter); } // ── optional cwd override ─────────────────────────────────────────── if let Some(c) = cwd_ { - // On failure, deref root_io + deinit root_shell + free. - if let Err(e) = interpreter - .root_shell - .with_mut(|rs| rs.change_cwd_impl(c, true)) - { + let res = interpreter.root_shell.borrow_mut().change_cwd_impl(c, true); + if let Err(e) = res { // `deinit_from_exec` performs the full teardown (drops - // `root_io` Arcs, frees env maps, closes `cwd_fd`, consumes - // the box). + // `root_io`, frees env maps, closes `cwd_fd`, consumes the box). interpreter.deinit_from_exec(); return Err(ShellErr::new_sys(&e)); } @@ -611,15 +512,14 @@ impl Interpreter { /// Full teardown for the standalone (`MiniEventLoop`) path. Drops root IO /// refcounts, frees the root shell env, and consumes the box. - fn deinit_from_exec(self) { + #[allow(clippy::boxed_local, reason = "consumes and frees the interpreter")] + pub(crate) fn deinit_from_exec(self: Box) { log!("deinit interpreter"); self.this_jsvalue.set(crate::jsc::JSValue::ZERO); - // `root_io` holds `Arc`/`Arc`; replacing with + // `root_io` holds `Rc`/`Rc`; replacing with // default drops the refs. self.root_io.set(IO::default()); - // Free buffered IO, env - // maps, cwd fd; do NOT free the struct itself (it's embedded). - self.root_shell.with_mut(|rs| rs.deinit_embedded(true)); + self.root_shell.borrow_mut().clear(true); } /// Standalone-shell entrypoint for `bun .sh`: parse `src` (already @@ -629,7 +529,7 @@ impl Interpreter { /// ("Failed to run " vs "Failed to run script ") and in /// not bumping `standalone_shell` analytics. pub(crate) fn init_and_run_from_file( - ctx: &mut bun_options_types::context::ContextData, + ctx: &bun_options_types::context::ContextData, mini: &'static mut bun_event_loop::MiniEventLoop::MiniEventLoop, path: &[u8], src: &[u8], @@ -646,7 +546,7 @@ impl Interpreter { /// directly; the `Result` only carries lexer/parser errors that escaped /// without a diagnostic. pub(crate) fn init_and_run_from_source( - ctx: &mut bun_options_types::context::ContextData, + ctx: &bun_options_types::context::ContextData, mini: &'static mut bun_event_loop::MiniEventLoop::MiniEventLoop, path_for_errors: &[u8], src: &[u8], @@ -660,72 +560,41 @@ impl Interpreter { /// the parse-error diagnostic — the only two behavioural deltas between /// the two entrypoints. fn init_and_run_impl( - ctx: &mut bun_options_types::context::ContextData, + ctx: &bun_options_types::context::ContextData, mini: &'static mut bun_event_loop::MiniEventLoop::MiniEventLoop, label: &[u8], src: &[u8], cwd: Option<&[u8]>, from_source: bool, ) -> crate::Result { + use bun_shell_parser::ParseFailure; if from_source { bun_analytics::features::standalone_shell.fetch_add(1, Ordering::Relaxed); } // We are the script's shell (`bun run