From 3ce944007c0c815dc229d8784527b1620471a0a3 Mon Sep 17 00:00:00 2001 From: Jarred Sumner Date: Tue, 25 Aug 2026 05:49:45 +0000 Subject: [PATCH 1/7] web_worker/ConsoleObject/hot_reloader/Debugger: remove unsafe WebWorker is an Arc shared by the C++ proxy (typed cpp_ref slot), its thread and the parent's child_workers; worker-thread state lives in a WorkerThread owned by thread_main and the VM in a WorkerVm handle. The console object is a Box on the VM; formatter scope state is restored by a Deref guard instead of raw-pointer guards; forEach callbacks go through typed ForEachContext/ForEachPropertyContext trampolines; C++ entry points are HOST_EXPORTs. The hot reloader is the watcher's WatcherHandler and never touches its context off-thread: reloads and directory-cache busts are posted to the owning thread as boxed tasks (BoxedTask) and coalesced there. --- src/bundler/bundle_v2.rs | 17 +- src/codegen/generate-host-exports.ts | 10 +- src/event_loop/ConcurrentTask.rs | 47 + src/event_loop/lib.rs | 2 +- src/io/lib.rs | 4 +- src/io/write.rs | 49 + src/jsc/ConsoleObject.rs | 1608 +++++++---------- src/jsc/Debugger.rs | 80 +- src/jsc/JSValue.rs | 101 +- src/jsc/VirtualMachine.rs | 149 +- .../bindings/webcore/WorkerMessagingProxy.cpp | 25 +- src/jsc/hot_reloader.rs | 813 ++++----- src/jsc/lib.rs | 8 +- src/jsc/web_worker.rs | 1161 ++++++------ src/jsc/worker_messaging_proxy.rs | 79 + src/libuv_sys/libuv.rs | 25 + src/runtime/cli/run_command.rs | 36 +- src/runtime/cli/test/ChangedFilesFilter.rs | 11 +- src/runtime/cli/test_command.rs | 18 +- src/runtime/dispatch.rs | 21 +- src/runtime/hw_exports.rs | 40 +- src/runtime/jsc_hooks.rs | 54 +- src/runtime/node/node_process.rs | 26 +- src/runtime/shell/interpreter.rs | 8 +- src/watcher/Watcher.rs | 117 +- src/watcher/lib.rs | 3 +- 26 files changed, 2154 insertions(+), 2358 deletions(-) create mode 100644 src/jsc/worker_messaging_proxy.rs diff --git a/src/bundler/bundle_v2.rs b/src/bundler/bundle_v2.rs index 00ba44ff481f..87dce7addba2 100644 --- a/src/bundler/bundle_v2.rs +++ b/src/bundler/bundle_v2.rs @@ -1458,21 +1458,22 @@ pub mod bv2_impl { /// installed after the last fallible step in `BundleV2::init`, so the /// box is never dropped while the watcher holds a pointer to it. fn __bun_jsc_enable_hot_module_reloading_for_bundler( - bv2: core::ptr::NonNull>, + bv2: &'static mut super::BundleV2<'static>, ); } /// `Watcher.enableHotModuleReloading(this, null)` for `bun build --watch`. #[inline] pub(crate) fn enable_hot_module_reloading_for_bundler(bv2: *mut super::BundleV2<'_>) { - let bv2 = core::ptr::NonNull::new(bv2.cast::>()) - .expect("BundleV2 watcher: bv2 is non-null"); // SAFETY: link-time-resolved Rust-ABI fn in `bun_jsc::hot_reloader`. - // Not `safe fn`: the callee dereferences `bv2`, so it must point to a - // live `BundleV2` whose backing allocation outlives the watcher (sole - // caller is `BundleV2::init`; the box is leaked on the success path — - // see the watch-mode caveat above). - unsafe { __bun_jsc_enable_hot_module_reloading_for_bundler(bv2) } + // `bv2` is the live boxed `BundleV2` from `BundleV2::init`, leaked on + // the success path (see the watch-mode caveat above), so widening it + // to `'static` for the process-lifetime watcher holds. + unsafe { + __bun_jsc_enable_hot_module_reloading_for_bundler( + &mut *bv2.cast::>(), + ) + } } /// Bytecode generation entry point for the linker: marks the calling diff --git a/src/codegen/generate-host-exports.ts b/src/codegen/generate-host-exports.ts index 5448dfb9372b..a70f21f10632 100644 --- a/src/codegen/generate-host-exports.ts +++ b/src/codegen/generate-host-exports.ts @@ -467,13 +467,13 @@ ${emitNoMangle(e.abi, e.symbol, "g: &JSGlobalObject", "JSValue", body)}`; case "rust": { // `extern "Rust"` link-time hook: forward the safe signature verbatim // (no pointer rewriting; `extern "Rust"` ABI == native Rust ABI). - // Reference params/returns are rejected — `extern "Rust" fn` items need - // explicit lifetimes for borrowed types and the generator does not - // synthesise them. Use raw pointers in the impl signature instead. + // Reference params need an explicit lifetime (`extern "Rust" fn` items + // cannot elide them and the generator does not synthesise one); + // reference returns are rejected. for (const p of e.params) - if (p.ty.startsWith("&")) + if (p.ty.startsWith("&") && !/^&'/.test(p.ty)) throw new Error( - `${loc}: HOST_EXPORT(${e.symbol}, rust) param \`${p.raw}\` is a reference; use a raw pointer`, + `${loc}: HOST_EXPORT(${e.symbol}, rust) param \`${p.raw}\` is a reference without an explicit lifetime`, ); if (e.ret.startsWith("&")) throw new Error( diff --git a/src/event_loop/ConcurrentTask.rs b/src/event_loop/ConcurrentTask.rs index 245c9500ea3c..fe83894d1438 100644 --- a/src/event_loop/ConcurrentTask.rs +++ b/src/event_loop/ConcurrentTask.rs @@ -164,6 +164,53 @@ pub trait Taskable { unsafe fn release_unrun(this: *mut Self); } +/// A [`Taskable`] whose queued [`Task::ptr`] is always an owned `Box` +/// ([`Task::from_boxed`] / `VmHandle::post_boxed`): the dispatch arm for +/// `TAG` reclaims the box to run it, and [`release_unrun`](Self::release_unrun) +/// receives it when it will never run. Implemented through [`boxed_task!`], +/// which is where the tag ↔ type pairing is asserted. +/// +/// # Safety +/// `TAG`'s arm in `bun_runtime::dispatch::run_task` must reclaim exactly a +/// `Box`; a mismatched tag makes the dispatcher reinterpret the box. +pub unsafe trait BoxedTask: Sized { + /// See [`Taskable::TAG`]. + const TAG: TaskTag; + /// See [`Taskable::release_unrun`]. + fn release_unrun(self: Box); +} + +impl Taskable for T { + const TAG: TaskTag = T::TAG; + unsafe fn release_unrun(this: *mut Self) { + // SAFETY: fn contract — `this` was queued under `T::TAG`, which for a + // `BoxedTask` is only ever the box `Task::from_boxed` leaked. + BoxedTask::release_unrun(unsafe { bun_core::heap::take(this) }); + } +} + +/// Pair a boxed task type with its dispatch tag: +/// +/// ```ignore +/// bun_event_loop::boxed_task! { +/// impl[const RI: bool] for MyTask => task_tag::MyTask; +/// fn release_unrun(self) { drop(self) } +/// } +/// ``` +/// +/// The tag named here and the `run_task` arm that `heap::take`s this type are +/// the two halves of one contract; keep them next to each other in review. +#[macro_export] +macro_rules! boxed_task { + (impl$([$($gen:tt)*])? for $ty:ty => $tag:expr; fn release_unrun($this:ident) $body:block) => { + // SAFETY: the invoker names the `run_task` arm that reclaims `Box<$ty>`. + unsafe impl$(<$($gen)*>)? $crate::BoxedTask for $ty { + const TAG: $crate::TaskTag = $tag; + fn release_unrun($this: ::std::boxed::Box) $body + } + }; +} + impl TaskTag { /// The tag's identifier, for diagnostics. pub fn name(self) -> &'static str { diff --git a/src/event_loop/lib.rs b/src/event_loop/lib.rs index 9a0be447363f..78a284a09bde 100644 --- a/src/event_loop/lib.rs +++ b/src/event_loop/lib.rs @@ -29,7 +29,7 @@ pub mod any_event_loop; // ─── public surface ───────────────────────────────────────────────────────── pub type JsResult = core::result::Result; -pub use ConcurrentTask::{Task, TaskTag, Taskable, task_tag}; +pub use ConcurrentTask::{BoxedTask, Task, TaskTag, Taskable, task_tag}; // snake_case alias for the file-level-struct module so higher tiers avoid // the type/module namespace collision on the PascalCase form. diff --git a/src/io/lib.rs b/src/io/lib.rs index 4df690e14bf0..ee4010cfad9a 100644 --- a/src/io/lib.rs +++ b/src/io/lib.rs @@ -478,7 +478,9 @@ pub mod write; // Byte-level `Write` trait + helpers. Downstream // crates name these as `bun_io::Write` / `bun_io::FmtAdapter` / // `bun_io::Result`. -pub use write::{AsFmt, DiscardingWriter, FixedBufferStream, FmtAdapter, IntLe, Result, Write}; +pub use write::{ + AsFmt, DiscardingWriter, FixedBufferStream, FmtAdapter, IntLe, IoWriterAdapter, Result, Write, +}; pub use max_buf as MaxBuf; pub use pipes::{Chunk, FileType, ReadState}; diff --git a/src/io/write.rs b/src/io/write.rs index 5177daab8781..ba749af28fb7 100644 --- a/src/io/write.rs +++ b/src/io/write.rs @@ -297,6 +297,55 @@ impl fmt::Write for AsFmt<'_> { } } +// ════════════════════════════════════════════════════════════════════════════ +// IoWriterAdapter — bun_io::Write → bun_core::io::Writer (vtable head) bridge +// ════════════════════════════════════════════════════════════════════════════ + +/// Wrap a `&mut dyn Write` so it can be passed where the concrete +/// [`bun_core::io::Writer`] vtable header is expected +/// (`VirtualMachine::print_stack_trace`, `print_errorlike_object`). +/// +/// `head` is the first `#[repr(C)]` field, so the `*mut io::Writer` the vtable +/// thunks receive is this struct's own address. +#[repr(C)] +pub struct IoWriterAdapter<'a> { + head: bun_core::io::Writer, + inner: &'a mut dyn Write, +} + +impl<'a> IoWriterAdapter<'a> { + pub fn new(inner: &'a mut dyn Write) -> Self { + Self { + head: bun_core::io::Writer { + write_all: Self::thunk_write_all, + flush: Self::thunk_flush, + }, + inner, + } + } + + /// The `io::Writer` view; writes and flushes go to the wrapped sink. + #[inline] + pub fn interface(&mut self) -> &mut bun_core::io::Writer { + // SAFETY: `head` is the first field of this `#[repr(C)]` struct, so the + // whole-struct pointer is a valid `*mut io::Writer` (and keeps provenance + // over `inner` for the thunks below). + unsafe { &mut *core::ptr::from_mut(self).cast::() } + } + + fn thunk_write_all(w: *mut bun_core::io::Writer, bytes: &[u8]) -> Result<()> { + // SAFETY: only installed by `new`, so `w` is the pointer `interface` produced from `&mut Self`. + let this = unsafe { &mut *w.cast::() }; + this.inner.write_all(bytes) + } + + fn thunk_flush(w: *mut bun_core::io::Writer) -> Result<()> { + // SAFETY: as in `thunk_write_all`. + let this = unsafe { &mut *w.cast::() }; + this.inner.flush() + } +} + // ════════════════════════════════════════════════════════════════════════════ #[cfg(test)] diff --git a/src/jsc/ConsoleObject.rs b/src/jsc/ConsoleObject.rs index 2dbefc463781..f673c3b55018 100644 --- a/src/jsc/ConsoleObject.rs +++ b/src/jsc/ConsoleObject.rs @@ -111,16 +111,14 @@ impl core::fmt::Display for ConsoleObject { } impl ConsoleObject { - /// `adapt_to_new_api(&mut self.stderr_buffer)` captures a raw pointer into - /// the buffer field, so the struct is self-referential once initialized: - /// the address of `*out` MUST be stable for the value's entire lifetime — - /// moving it afterwards leaves the writer adapters dangling. - pub(crate) fn init_in_place( - out: &mut core::mem::MaybeUninit, + /// Boxed because the writer adapters point into the buffer fields: the + /// value is self-referential and must stay at this address (the VM keeps + /// the box for its whole life and never moves out of it). + pub(crate) fn new( error_writer: Output::StreamType, writer: Output::StreamType, - ) -> &mut ConsoleObject { - let out = out.write(ConsoleObject { + ) -> Box { + let mut out = Box::new(ConsoleObject { stderr_buffer: [0; 4096], stdout_buffer: [0; 4096], error_writer_backing: Output::QuietWriterAdapter::uninit(), @@ -129,22 +127,23 @@ impl ConsoleObject { counts: Counter::default(), _pin: core::marker::PhantomPinned, }); - let p: *mut ConsoleObject = out; - // SAFETY: `out` is now fully initialized at its final address; the - // adapters store raw pointers into `out.stderr_buffer` / - // `out.stdout_buffer`, which remain valid for `out`'s lifetime - // *provided the caller never moves it* (see fn doc). - unsafe { - (*p).error_writer_backing = error_writer - .quiet_writer() - .adapt_to_new_api(&mut (*p).stderr_buffer); - (*p).writer_backing = writer - .quiet_writer() - .adapt_to_new_api(&mut (*p).stdout_buffer); - } + let this = &mut *out; + this.error_writer_backing = error_writer + .quiet_writer() + .adapt_to_new_api(&mut this.stderr_buffer); + this.writer_backing = writer + .quiet_writer() + .adapt_to_new_api(&mut this.stdout_buffer); out } + /// The `void*` handed to the C++ `ConsoleObject` client, which passes it + /// back (unused) as the first argument of every `Bun__ConsoleObject__*`. + #[inline] + pub(crate) fn as_cpp_ptr(&mut self) -> *mut c_void { + core::ptr::from_mut(self).cast() + } + /// Returns the buffered stderr writer interface. #[inline] pub(crate) fn error_writer(&mut self) -> &mut bun_core::io::Writer { @@ -240,34 +239,23 @@ impl MessageType { use bun_threading::Mutex; -/// `globalThis.bunVM().console` — `VirtualMachine.console` is typed -/// `*mut c_void` (erased so `virtual_machine.rs` need not name this module's -/// type). This is the single re-entry point that casts it back; every body -/// below goes through it instead of touching the field directly. +/// `globalThis.bunVM().console`, for one short read or update. Every +/// `Bun__ConsoleObject__*` entry point is a top-level JS-thread host call. #[inline] -fn vm_console(global: &JSGlobalObject) -> *mut ConsoleObject { - // SAFETY: `VirtualMachine.console` is initialized once at VM construction - // (see `init` above — written into stable boxed storage) and lives for the - // VM's lifetime; the C++ side never calls into `Bun__ConsoleObject__*` - // before that. Returned as a raw pointer (not `&'static mut`) so callers - // dereference at each use site without holding overlapping `&mut` - // references. - global.bun_vm().as_mut().console.cast::() +fn with_console(global: &JSGlobalObject, f: impl FnOnce(&mut ConsoleObject) -> R) -> R { + f(global.bun_vm().as_mut().console_mut()) } -/// `&mut ConsoleObject` accessor for the set-once `VirtualMachine.console` -/// box. Consolidates the open-coded `&mut *vm_console(global)` deref at the -/// `Bun__ConsoleObject__*` FFI entry points (each is a top-level JS-thread -/// host call, so the `&mut` is exclusive for the duration). Callers that need -/// to interleave borrows across a deferred guard (`message_with_type_and_level_`) -/// keep using the raw [`vm_console`] pointer instead. +/// The VM console's buffered stdout/stderr writer. Formatting through it runs +/// user JS, which may re-enter `console.*` and write through the same stream. #[inline] -unsafe fn vm_console_mut<'a>(global: &JSGlobalObject) -> &'a mut ConsoleObject { - // SAFETY: see [`vm_console`] — `VirtualMachine.console` is initialized once - // at VM construction to a boxed `ConsoleObject` that lives for the VM's - // lifetime; the C++ side never calls into `Bun__ConsoleObject__*` before - // that. Single-JS-thread invariant ⇒ the returned `&mut` is exclusive. - unsafe { &mut *vm_console(global) } +fn console_writer(global: &JSGlobalObject, stderr: bool) -> &mut bun_core::io::Writer { + let console = global.bun_vm().as_mut().console_mut(); + if stderr { + console.error_writer() + } else { + console.writer() + } } static STDERR_MUTEX: Mutex = Mutex::new(); @@ -349,16 +337,13 @@ impl Drop for FlushOnDrop<'_> { } /// -#[crate::host_call] -pub extern "C" fn message_with_type_and_level( - ctype: *mut ConsoleObject, +pub fn message_with_type_and_level( message_type: MessageType, level: MessageLevel, global: &JSGlobalObject, - vals: *const JSValue, - len: usize, + vals: &[JSValue], ) { - if let Err(err) = message_with_type_and_level_(ctype, message_type, level, global, vals, len) { + if let Err(err) = message_with_type_and_level_(message_type, level, global, vals) { // The exception is already set on the VM (`JsError::Thrown`); for OOM // make sure something is pending. Mirrors `host_fn::void_from_js_error`. if matches!(err, jsc::JsError::OutOfMemory) { @@ -369,26 +354,18 @@ pub extern "C" fn message_with_type_and_level( } fn message_with_type_and_level_( - _ctype: *mut ConsoleObject, message_type: MessageType, level: MessageLevel, global: &JSGlobalObject, - vals: *const JSValue, - len: usize, + vals_slice: &[JSValue], ) -> JsResult<()> { - let console: *mut ConsoleObject = vm_console(global); + let len = vals_slice.len(); // `defer console.default_indent +|= (message_type == StartGroup) as u16;` - // Capture the raw pointer (Copy) by `move` so no borrow of the local is - // held across the body; dereference only at scope-exit. let is_start_group = message_type == MessageType::StartGroup; - let _indent_guard = scopeguard::guard(console, move |console| { - // SAFETY: see `vm_console` — points at the live boxed - // `ConsoleObject` for this VM; JS-thread-only. - unsafe { - (*console).default_indent = (*console) - .default_indent - .saturating_add(is_start_group as u16); - } + let _indent_guard = scopeguard::guard(global, move |global| { + with_console(global, |console| { + console.default_indent = console.default_indent.saturating_add(is_start_group as u16) + }); }); if message_type == MessageType::StartGroup && len == 0 { @@ -397,11 +374,9 @@ fn message_with_type_and_level_( } if message_type == MessageType::EndGroup { - // SAFETY: set-once `VirtualMachine.console` box — no other - // borrow of the console is live yet; the deferred `_indent_guard` - // captured only the raw pointer. - let c = unsafe { vm_console_mut(global) }; - c.default_indent = c.default_indent.saturating_sub(1); + with_console(global, |c| { + c.default_indent = c.default_indent.saturating_sub(1) + }); return Ok(()); } @@ -422,10 +397,7 @@ fn message_with_type_and_level_( } else { "Assertion failed\n" }; - // SAFETY: no other borrow of the console is live in this - // early-return arm (the deferred `_indent_guard` only holds the raw - // pointer, not a reference). - let ew = unsafe { vm_console_mut(global) }.error_writer(); + let ew = console_writer(global, true); let _ = ew.write_all(text.as_bytes()); let _ = ew.flush(); return Ok(()); @@ -439,23 +411,13 @@ fn message_with_type_and_level_( // Snapshot before borrowing the writer; `default_indent` is not mutated // again until the deferred `_indent_guard` runs on scope exit, so the two - // later reads (FormatOptions / TablePrinter) can use this cached copy - // instead of re-dereferencing the raw `console` pointer. - // SAFETY: see [`vm_console`] — single-JS-thread; no other `&mut` is live. - let default_indent = unsafe { vm_console_mut(global) }.default_indent; - - // SAFETY: see [`vm_console`] — `console` points at the live boxed - // `ConsoleObject` for this VM; JS-thread-only. Kept as a raw deref (not - // `vm_console_mut`) so the resulting `writer` borrow does not pin a - // long-lived `&mut ConsoleObject` across the re-derive in the empty-`Log` - // arm below. - let raw_writer: &mut bun_core::io::Writer = unsafe { - if matches!(level, MessageLevel::Warning | MessageLevel::Error) { - (*console).error_writer() - } else { - (*console).writer() - } - }; + // later reads (FormatOptions / TablePrinter) use this cached copy. + let default_indent = with_console(global, |c| c.default_indent); + + let raw_writer: &mut bun_core::io::Writer = console_writer( + global, + matches!(level, MessageLevel::Warning | MessageLevel::Error), + ); // `bun_core::io::Writer: bun_io::Write` — `&mut Writer` unsize-coerces directly. let writer: &mut dyn bun_io::Write = raw_writer; @@ -487,9 +449,6 @@ fn message_with_type_and_level_( ..FormatOptions::default() }; - // SAFETY: caller (JSC C++) guarantees `vals` points to `len` JSValues. - let vals_slice = unsafe { bun_core::ffi::slice(vals, len) }; - if message_type == MessageType::Table && len >= 1 { // if value is not an object/array/iterable, don't print a table and just print it let tabular_data = vals_slice[0]; @@ -541,10 +500,9 @@ fn message_with_type_and_level_( print_options, )?; } else if message_type == MessageType::Log { - // SAFETY: see [`vm_console`]. `writer` (above) is dead in this arm — - // the only later uses are in the mutually-exclusive `Trace` block, and - // `message_type == Log` here. - let w = unsafe { (*console).writer() }; + // `writer` (above) is dead in this arm — the only later uses are in + // the mutually-exclusive `Trace` block, and `message_type == Log` here. + let w = console_writer(global, false); let _ = w.write_all(b"\n"); let _ = w.flush(); } else if message_type != MessageType::Trace { @@ -900,7 +858,7 @@ impl<'a> TablePrinter<'a> { let mut rows: Vec = Vec::new(); { if self.is_iterable { - struct Ctx<'c, 'a> { + struct Ctx<'c, 'a, const C: bool> { this: &'c mut TablePrinter<'a>, cell_text: &'c mut Vec, columns: &'c mut Vec, @@ -910,7 +868,7 @@ impl<'a> TablePrinter<'a> { } // Capture before constructing `ctx` (which mutably borrows `*self`). let tabular_data = self.tabular_data; - let mut ctx = Ctx { + let mut ctx = Ctx::<'_, '_, ENABLE_ANSI_COLORS> { this: self, cell_text: &mut cell_text, columns: &mut columns, @@ -918,36 +876,27 @@ impl<'a> TablePrinter<'a> { idx: 0, err: None, }; - extern "C" fn callback( - _: *mut jsc::VM, - _: &JSGlobalObject, - ctx: *mut c_void, - value: JSValue, - ) { - // SAFETY: ctx points to the stack `Ctx` above. - let ctx = unsafe { bun_ptr::callback_ctx::>(ctx) }; - // Once a cell failed, a JS exception may be pending (or - // the VM terminating); don't re-enter user code for the - // remaining elements. - if ctx.err.is_some() { - return; - } - match ctx.this.collect_row::( - ctx.cell_text, - ctx.columns, - RowKey::Num(ctx.idx), - value, - ) { - Ok(row) => ctx.rows.push(row), - Err(err) => ctx.err = Some(err), + impl jsc::ForEachContext for Ctx<'_, '_, C> { + fn on_value(&mut self, _: &JSGlobalObject, value: JSValue) { + // Once a cell failed, a JS exception may be pending (or + // the VM terminating); don't re-enter user code for the + // remaining elements. + if self.err.is_some() { + return; + } + match self.this.collect_row::( + self.cell_text, + self.columns, + RowKey::Num(self.idx), + value, + ) { + Ok(row) => self.rows.push(row), + Err(err) => self.err = Some(err), + } + self.idx += 1; } - ctx.idx += 1; } - tabular_data.for_each_with_context( - global_object, - (&raw mut ctx).cast::(), - callback::, - )?; + tabular_data.for_each_ctx(global_object, &mut ctx)?; if let Some(err) = ctx.err { return Err(err); } @@ -1071,59 +1020,8 @@ impl<'a> TablePrinter<'a> { // writeTrace // ─────────────────────────────────────────────────────────────────────────── -/// Adapter: erase a `&mut dyn bun_io::Write` behind the `bun_core::io::Writer` -/// vtable header so it can be handed to `VirtualMachine::print_stack_trace` / -/// `print_errorlike_object`, which take the concrete `io::Writer` type. -/// -/// `bun_core::io::Writer` is the first `#[repr(C)]` field, so the vtable thunks -/// recover `&mut Self` from the `*mut io::Writer` they receive (same pattern as -/// `Output::QuietWriterAdapter::new_interface`). -#[repr(C)] -struct DynWriteAdapter<'a> { - head: bun_core::io::Writer, - inner: &'a mut dyn bun_io::Write, -} - -impl<'a> DynWriteAdapter<'a> { - fn new(inner: &'a mut dyn bun_io::Write) -> Self { - Self { - head: bun_core::io::Writer { - write_all: Self::thunk_write_all, - flush: Self::thunk_flush, - }, - inner, - } - } - - /// Reborrow as the `io::Writer` head. - #[inline] - fn interface(&mut self) -> &mut bun_core::io::Writer { - // SAFETY: `head` is the first `#[repr(C)]` field, so `&mut self.head` - // and `&mut *self as *mut io::Writer` are the same address; the thunks - // below cast back to `*mut Self`. - &mut self.head - } - - fn thunk_write_all(w: *mut bun_core::io::Writer, bytes: &[u8]) -> Result<(), bun_core::Error> { - // SAFETY: only reachable via the `io::Writer` vtable installed in - // `Self::new`, which always passes `&mut self.head` (first repr(C) - // field) as `w`; safe-fn coerces to the unsafe-fn-ptr slot type. - let this = unsafe { &mut *w.cast::() }; - this.inner.write_all(bytes) - } - - fn thunk_flush(w: *mut bun_core::io::Writer) -> Result<(), bun_core::Error> { - // SAFETY: only reachable via the `io::Writer` vtable installed in - // `Self::new`, which always passes `&mut self.head` (first repr(C) - // field) as `w`; safe-fn coerces to the unsafe-fn-ptr slot type. - let this = unsafe { &mut *w.cast::() }; - this.inner.flush() - } -} - pub fn write_trace(writer: &mut dyn bun_io::Write, global: &JSGlobalObject) { let mut holder = crate::zig_exception::Holder::init(); - // SAFETY: per-thread VM; `console.trace()` only runs on the JS thread. let vm = VirtualMachine::get().as_mut(); let mut source_code_slice: Option = None; @@ -1144,7 +1042,7 @@ pub fn write_trace(writer: &mut dyn bun_io::Write, global: &JSGlobalObject) { ); holder.need_to_clear_parser_arena_on_deinit = need_to_clear; - let mut adapter = DynWriteAdapter::new(writer); + let mut adapter = bun_io::IoWriterAdapter::new(writer); let _ = VirtualMachine::print_stack_trace( adapter.interface(), &holder.zig_exception().stack, @@ -1479,101 +1377,32 @@ pub use formatter::{Formatter, Tag, TagOptions, TagPayload, TagResult, visited}; pub mod formatter { use super::*; - /// RAII: write `prev` back through `place` on drop. Holds a raw `*mut` so the body of the - /// scope can freely take `&mut self` without aliasing the borrow. - /// - /// NOTE(aliasing): the `addr_of_mut!(self.field)` → body uses `&mut self` - /// → guard derefs raw pointer pattern is sound under Tree Borrows but - /// pops the raw pointer's tag under Stacked Borrows. Miri runs of this - /// module require `-Zmiri-tree-borrows`. Applies to `Decrement` below and - /// the `scopeguard::defer!` raw-pointer captures throughout `print_as`. - pub(super) struct Restore { - place: *mut T, - prev: T, - } - impl Drop for Restore { - #[inline] - fn drop(&mut self) { - // SAFETY: `place` was taken via `addr_of_mut!` on a field of a - // value that outlives this guard; no other borrow is live at drop. - unsafe { *self.place = self.prev }; - } - } - - /// `saturating_sub(1)` for the integer field types `Decrement` is used on. - pub(super) trait SaturatingDec: Copy { - fn saturating_dec(self) -> Self; + /// `&mut Formatter` for a scope that changed some formatter state; `undo` + /// puts it back when the scope ends, however it ends. + pub(super) struct Scoped<'f, 'a, U: FnMut(&mut Formatter<'a>)> { + fmt: &'f mut Formatter<'a>, + undo: U, } - impl SaturatingDec for u16 { + impl<'a, U: FnMut(&mut Formatter<'a>)> core::ops::Deref for Scoped<'_, 'a, U> { + type Target = Formatter<'a>; #[inline] - fn saturating_dec(self) -> Self { - self.saturating_sub(1) + fn deref(&self) -> &Formatter<'a> { + self.fmt } } - impl SaturatingDec for u32 { + impl<'a, U: FnMut(&mut Formatter<'a>)> core::ops::DerefMut for Scoped<'_, 'a, U> { #[inline] - fn saturating_dec(self) -> Self { - self.saturating_sub(1) + fn deref_mut(&mut self) -> &mut Formatter<'a> { + self.fmt } } - - /// RAII: saturating-decrement `*place` on drop. Holds a - /// raw `*mut` so the body can freely take `&mut self`. - pub(super) struct Decrement { - place: *mut T, - } - impl Drop for Decrement { - #[inline] - fn drop(&mut self) { - // SAFETY: `place` was taken via `addr_of_mut!` on a field of a - // value that outlives this guard; no other borrow is live at drop. - unsafe { *self.place = (*self.place).saturating_dec() }; - } - } - - /// RAII: `map.remove(value)` on drop iff `*armed`. Holds - /// raw pointers so the body can freely take `&mut self`; smaller than the - /// equivalent `scopeguard::defer!` closure under ASAN stack redzones. - pub(super) struct VisitedRemove { - map: *mut visited::Map, - armed: *const bool, - value: JSValue, - } - impl Drop for VisitedRemove { + impl<'a, U: FnMut(&mut Formatter<'a>)> Drop for Scoped<'_, 'a, U> { #[inline] fn drop(&mut self) { - // SAFETY: `map`/`armed` were taken via `addr_of!` on locals that - // outlive this guard; no other borrow is live at drop. - unsafe { - if *self.armed { - let _ = (*self.map).remove(&self.value); - } - } + (self.undo)(self.fmt); } } - /// Restore a field to `prev` at scope exit without holding a live borrow - /// on `self` for the body of the scope. The guard reads at scope-exit - /// time and never aliases: it captures a raw `*mut` to the field and - /// writes through it on drop. This lets the body freely take `&mut self`. - macro_rules! defer_restore { - ($place:expr, $prev:expr) => { - Restore { - place: core::ptr::addr_of_mut!($place), - prev: $prev, - } - }; - } - - /// Saturating-decrement a field at scope exit without holding a live borrow. - macro_rules! defer_decrement { - ($place:expr) => { - Decrement { - place: core::ptr::addr_of_mut!($place), - } - }; - } - pub struct Formatter<'a> { pub global_this: &'a JSGlobalObject, @@ -1584,10 +1413,9 @@ pub mod formatter { pub(crate) remaining_values: bun_ptr::RawSlice, pub map: visited::Map, /// Pooled backing for `map`. `None` until the first cell that can have - /// circular refs is formatted; `Drop` returns it to `visited::Pool`. - /// Raw pointer (not `Box`) because `visited::Pool` owns the - /// `heap::alloc`/`from_raw` lifecycle. - pub(crate) map_node: Option>, + /// circular refs is formatted; `Drop` moves `map` back into it and the + /// guard returns it to `visited::Pool`. + pub(crate) map_node: Option, pub(crate) hide_native: bool, pub(crate) indent: u32, pub depth: u16, @@ -1692,28 +1520,53 @@ pub mod formatter { let s = self.remaining_values; self.remaining_values = bun_ptr::RawSlice::new(&s.slice()[1..]); } + + #[inline] + pub(super) fn scoped)>( + &mut self, + undo: U, + ) -> Scoped<'_, 'a, U> { + Scoped { fmt: self, undo } + } + + /// `indent`/`depth` go back down by one when the scope ends. + #[inline] + pub(super) fn indented(&mut self) -> Scoped<'_, 'a, impl FnMut(&mut Formatter<'a>)> { + self.scoped(|f| { + f.indent = f.indent.saturating_sub(1); + f.depth = f.depth.saturating_sub(1); + }) + } + + /// `quote_strings = true` until the scope ends. + #[inline] + pub(super) fn quoting_strings(&mut self) -> Scoped<'_, 'a, impl FnMut(&mut Formatter<'a>)> { + let prev = self.quote_strings; + self.quote_strings = true; + self.scoped(move |f| f.quote_strings = prev) + } + + /// `quote_keys = true` until the scope ends. + #[inline] + pub(super) fn quoting_keys(&mut self) -> Scoped<'_, 'a, impl FnMut(&mut Formatter<'a>)> { + let prev = self.quote_keys; + self.quote_keys = true; + self.scoped(move |f| f.quote_keys = prev) + } } impl Drop for Formatter<'_> { fn drop(&mut self) { if let Some(mut node) = self.map_node.take() { // Move the working map back into the pooled node, shrink if it - // ballooned, then return the node to the thread-local pool. - let map = core::mem::take(&mut self.map); - // `node_data_mut` is safe: `Map::INIT` is `Some`, so the - // pooled slot already holds an (empty, post-`take`) `Map`; - // assigning over it drops that empty value and moves `map` in. - let data = visited::node_data_mut(&mut node); - *data = map; - if data.capacity() > 512 { - data.deinit(); + // ballooned; dropping the guard returns the node to the + // thread-local pool. + *node = core::mem::take(&mut self.map); + if node.capacity() > 512 { + node.deinit(); } else { - data.clear(); + node.clear(); } - // SAFETY: `node` was obtained from `Pool::get_node()` and is - // exclusively owned by this formatter; `Map::INIT` is `Some`, - // so `data` is initialized. Ownership returns to the pool. - unsafe { visited::Pool::release(node.as_ptr()) }; } } } @@ -1809,22 +1662,8 @@ pub mod formatter { // Thread-local free list, capped at 16 nodes. bun_collections::object_pool!(pub Pool: Map, threadsafe, 16); - pub type PoolNode = bun_collections::pool::Node; - - /// Safe `&mut Map` accessor for a pooled node. `Map::INIT` is `Some`, - /// so every node returned by [`Pool::get_node`] carries an initialized - /// `data` payload, and the caller exclusively owns the node until - /// [`Pool::release`]. Centralises the `NonNull::as_mut()` + - /// `assume_init_mut()` pair so the four call sites in this file (and - /// the cause-chain guard in `VirtualMachine::print_error_instance`) - /// don't each open-code two `unsafe` operations. - #[inline] - pub(crate) fn node_data_mut(node: &mut core::ptr::NonNull) -> &mut Map { - // SAFETY: `Map::INIT` is `Some`, so `data` is initialized for - // every node from `Pool::get_node()`; the caller owns `node` - // exclusively until `Pool::release`, so forming `&mut` is sound. - unsafe { node.as_mut().data.assume_init_mut() } - } + /// A node checked out of [`Pool`]; returned to it on drop. + pub type PoolGuard = bun_collections::pool::PoolGuard<'static, Map>; } // ─────────────────────────────────────────────────────────────────────── @@ -2763,20 +2602,11 @@ pub mod formatter { pub(crate) count: usize, } - impl<'a, 'b, const C: bool, const IS_ITERATOR: bool, const SINGLE_LINE: bool> - MapIteratorCtx<'a, 'b, C, IS_ITERATOR, SINGLE_LINE> + impl jsc::ForEachContext + for MapIteratorCtx<'_, '_, C, IS_ITERATOR, SINGLE_LINE> { - pub(crate) extern "C" fn for_each( - _: *mut jsc::VM, - global_object: &JSGlobalObject, - ctx: *mut c_void, - next_value: JSValue, - ) { - // SAFETY: ctx points to the stack-allocated `Self` passed by the caller via the C forEach callback. - let Some(ctx) = (unsafe { ctx.cast::().as_mut() }) else { - return; - }; - let this = ctx; + fn on_value(&mut self, global_object: &JSGlobalObject, next_value: JSValue) { + let this = self; if this.formatter.failed { return; } @@ -2862,17 +2692,11 @@ pub mod formatter { pub(crate) is_first: bool, } - impl<'a, 'b, const C: bool, const SINGLE_LINE: bool> SetIteratorCtx<'a, 'b, C, SINGLE_LINE> { - pub(crate) extern "C" fn for_each( - _: *mut jsc::VM, - global_object: &JSGlobalObject, - ctx: *mut c_void, - next_value: JSValue, - ) { - // SAFETY: ctx points to the stack-allocated `Self` passed by the caller via the C forEach callback. - let Some(this) = (unsafe { ctx.cast::().as_mut() }) else { - return; - }; + impl jsc::ForEachContext + for SetIteratorCtx<'_, '_, C, SINGLE_LINE> + { + fn on_value(&mut self, global_object: &JSGlobalObject, next_value: JSValue) { + let this = self; if this.formatter.failed { return; } @@ -3050,13 +2874,11 @@ pub mod formatter { fn for_each_prelude( ctx: &mut Self, global_this: &JSGlobalObject, - key: *mut EncodedSlice, + key: &EncodedSlice, value: JSValue, is_symbol: bool, is_private_symbol: bool, ) -> Option { - // SAFETY: caller passes a valid `*EncodedSlice`. - let key = unsafe { &*key }; if key.eq_ascii(b"constructor") { return None; } @@ -3126,20 +2948,18 @@ pub mod formatter { } Some(tag) } + } - pub(crate) extern "C" fn for_each( + impl jsc::ForEachPropertyContext for PropertyIteratorCtx<'_, '_, C> { + fn on_property( + &mut self, global_this: &JSGlobalObject, - ctx_ptr: *mut c_void, - key: *mut EncodedSlice, + key: &EncodedSlice, value: JSValue, is_symbol: bool, is_private_symbol: bool, ) { - // SAFETY: ctx_ptr points to the stack-allocated `Self` passed by the caller via the C forEach callback. - let Some(ctx) = (unsafe { ctx_ptr.cast::().as_mut() }) else { - return; - }; - + let ctx = self; let Some(tag) = Self::for_each_prelude(ctx, global_this, key, value, is_symbol, is_private_symbol) else { @@ -3232,11 +3052,9 @@ pub mod formatter { } if self.map_node.is_none() { - let mut node = core::ptr::NonNull::new(visited::Pool::get_node()) - .expect("ObjectPool::get_node always returns a valid heap node"); - let data = visited::node_data_mut(&mut node); - data.clear(); - self.map = core::mem::take(data); + let mut node = visited::Pool::get(); + node.clear(); + self.map = core::mem::take(&mut *node); self.map_node = Some(node); } @@ -3275,19 +3093,10 @@ pub mod formatter { return Ok(()); } - // The body mutates both `self` and `remove_before_recurse`, so - // capture raw pointers and read the *current* `remove_before_recurse` - // at scope-exit time. - let _visited = VisitedRemove { - map: &raw mut self.map, - armed: &raw const remove_before_recurse, - value, - }; - // Each arm is hoisted to its own `#[inline(never)]` helper so the // `print_as` frame stays small enough to recurse 512 levels under // debug/ASAN before the stack-safety check fires. - match format { + let result = match format { Tag::StringPossiblyFormatted => { self.print_string_possibly_formatted::(writer_, value) } @@ -3347,7 +3156,12 @@ pub mod formatter { } Tag::RevokedProxy => self.print_revoked_proxy::(writer_), Tag::Proxy => self.print_proxy::(writer_, value), + }; + // The arms may flip `remove_before_recurse`; act on its final value. + if remove_before_recurse { + let _ = self.map.remove(&value); } + result } } @@ -3849,21 +3663,13 @@ pub mod formatter { } else { false }; - let map_restore_ptr: *mut visited::Map = &raw mut self.map; - scopeguard::defer! { - // SAFETY: `self.map` outlives this guard; no other borrow is - // live at the drop point. - unsafe { - if was_in_map { - let _ = (*map_restore_ptr).insert(value, ()); - } - } - } - let mut adapter = DynWriteAdapter::new(&mut *writer_); - // SAFETY: per-thread VM. + let mut adapter = bun_io::IoWriterAdapter::new(&mut *writer_); let vm = VirtualMachine::get().as_mut(); vm.print_errorlike_object(value, None, None, self, adapter.interface(), C, false); + if was_in_map { + let _ = self.map.insert(value, ()); + } Ok(()) } @@ -4151,11 +3957,10 @@ pub mod formatter { ) -> JsResult<()> { if let Some(func) = value.get(self.global_this, "toJSON")? { let result = func.call(self.global_this, value, &[])?; - let prev_quote_keys = self.quote_keys; - self.quote_keys = true; - let _r = defer_restore!(self.quote_keys, prev_quote_keys); - let tag = Tag::get(result, self.global_this)?; - return self.format::(tag, writer_, result, self.global_this); + let mut scope = self.quoting_keys(); + let this = &mut *scope; + let tag = Tag::get(result, this.global_this)?; + return this.format::(tag, writer_, result, this.global_this); } if writer_.write_all(b"{}").is_err() { @@ -4252,32 +4057,36 @@ pub mod formatter { let mut was_good_time = self.always_newline_scope || // heuristic: more than 10, probably should have a newline before it len > 10; + let writer_failed; { self.indent += 1; self.depth += 1; - let _depth = defer_decrement!(self.depth); - let _indent = defer_decrement!(self.indent); + let mut scope = self.indented(); + let mut scope = scope.quoting_strings(); + let this = &mut *scope; + let mut writer = WrappedWriter { + ctx: writer_, + failed: false, + estimated_line_length: &mut this.estimated_line_length, + }; writer.add_for_new_line(2); - let prev_quote_strings = self.quote_strings; - self.quote_strings = true; - let _qs = defer_restore!(self.quote_strings, prev_quote_strings); let mut empty_start: Option = None; 'first: { - let element = value.get_direct_index(self.global_this, 0)?; + let element = value.get_direct_index(this.global_this, 0)?; - let tag = Tag::get_advanced(element, self.global_this, tag_opts)?; + let tag = Tag::get_advanced(element, this.global_this, tag_opts)?; was_good_time = was_good_time || !tag.tag.is_primitive() - || writer.good_time_for_a_new_line(self.indent); + || writer.good_time_for_a_new_line(this.indent); - if !self.single_line && (self.ordered_properties || was_good_time) { - writer.reset_line(self.indent); + if !this.single_line && (this.ordered_properties || was_good_time) { + writer.reset_line(this.indent); writer.write_all(b"["); writer.write_all(b"\n"); - writer.write_indent(self.indent); + writer.write_indent(this.indent); writer.add_for_new_line(1); } else { writer.write_all(b"[ "); @@ -4289,11 +4098,11 @@ pub mod formatter { break 'first; } - self.format::(tag, writer_, element, self.global_this)?; + this.format::(tag, writer_, element, this.global_this)?; writer = WrappedWriter { ctx: writer_, failed: false, - estimated_line_length: &mut self.estimated_line_length, + estimated_line_length: &mut this.estimated_line_length, }; if tag.cell.is_string_like() && C { @@ -4305,7 +4114,7 @@ pub mod formatter { let mut nonempty_count: u32 = 1; while (i as u64) < len { - let element = value.get_direct_index(self.global_this, i)?; + let element = value.get_direct_index(this.global_this, i)?; if element.is_empty() { if empty_start.is_none() { empty_start = Some(i); @@ -4330,7 +4139,7 @@ pub mod formatter { writer.print_comma::(); writer.write_all(b"\n"); // we want the line break to be unconditional here *writer.estimated_line_length = 0; - writer.write_indent(self.indent); + writer.write_indent(this.indent); writer.pretty::( "... N more items".len(), format_args!( @@ -4347,13 +4156,13 @@ pub mod formatter { if let Some(empty) = empty_start { if empty > 0 { writer.print_comma::(); - if !self.single_line - && (self.ordered_properties - || writer.good_time_for_a_new_line(self.indent)) + if !this.single_line + && (this.ordered_properties + || writer.good_time_for_a_new_line(this.indent)) { was_good_time = true; writer.write_all(b"\n"); - writer.write_indent(self.indent); + writer.write_indent(this.indent); } else { writer.space(); } @@ -4380,23 +4189,23 @@ pub mod formatter { } writer.print_comma::(); - if !self.single_line - && (self.ordered_properties || writer.good_time_for_a_new_line(self.indent)) + if !this.single_line + && (this.ordered_properties || writer.good_time_for_a_new_line(this.indent)) { writer.write_all(b"\n"); was_good_time = true; - writer.write_indent(self.indent); + writer.write_indent(this.indent); } else { writer.space(); } - let tag = Tag::get_advanced(element, self.global_this, tag_opts)?; + let tag = Tag::get_advanced(element, this.global_this, tag_opts)?; - self.format::(tag, writer_, element, self.global_this)?; + this.format::(tag, writer_, element, this.global_this)?; writer = WrappedWriter { ctx: writer_, failed: false, - estimated_line_length: &mut self.estimated_line_length, + estimated_line_length: &mut this.estimated_line_length, }; if tag.cell.is_string_like() && C { @@ -4408,13 +4217,13 @@ pub mod formatter { if let Some(empty) = empty_start.take() { if empty > 0 { writer.print_comma::(); - if !self.single_line - && (self.ordered_properties - || writer.good_time_for_a_new_line(self.indent)) + if !this.single_line + && (this.ordered_properties + || writer.good_time_for_a_new_line(this.indent)) { writer.write_all(b"\n"); was_good_time = true; - writer.write_indent(self.indent); + writer.write_indent(this.indent); } else { writer.space(); } @@ -4444,33 +4253,35 @@ pub mod formatter { // Hoist field reads before `formatter: self` reborrows the // whole `*self` (struct-literal field order is not eval // order in the borrow checker's eyes once `self` is moved). - let always_newline = !self.single_line - && (self.always_newline_scope || self.good_time_for_a_new_line()); - let single_line = self.single_line; - let global_this = self.global_this; + let always_newline = !this.single_line + && (this.always_newline_scope || this.good_time_for_a_new_line()); + let single_line = this.single_line; + let global_this = this.global_this; let mut iter = PropertyIteratorCtx:: { - formatter: self, + formatter: this, writer: writer_, always_newline, single_line, parent: value, i: i as usize, }; - value.for_each_property_non_indexed( - global_this, - (&raw mut iter).cast::(), - PropertyIteratorCtx::::for_each, - )?; - if self.failed { + value.for_each_property_non_indexed_ctx(global_this, &mut iter)?; + if this.failed { return Ok(()); } writer = WrappedWriter { ctx: writer_, failed: false, - estimated_line_length: &mut self.estimated_line_length, + estimated_line_length: &mut this.estimated_line_length, }; } + writer_failed = writer.failed; } + let mut writer = WrappedWriter { + ctx: writer_, + failed: writer_failed, + estimated_line_length: &mut self.estimated_line_length, + }; if !self.single_line && (self.ordered_properties @@ -4520,12 +4331,10 @@ pub mod formatter { // so use its dedicated `from_js` FFI downcast instead of `value.as_`. if crate::DOMFormData::from_js(value).is_some() { if let Some(to_json_function) = value.get(self.global_this, "toJSON")? { - let prev_quote_keys = self.quote_keys; - self.quote_keys = true; - let _r = defer_restore!(self.quote_keys, prev_quote_keys); - - let result = to_json_function.call(self.global_this, value, &[])?; - return self.print_as::(Tag::Object, writer_, result, jsc::JSType::Object); + let mut scope = self.quoting_keys(); + let this = &mut *scope; + let result = to_json_function.call(this.global_this, value, &[])?; + return this.print_as::(Tag::Object, writer_, result, jsc::JSType::Object); } // this case should never happen @@ -4569,9 +4378,8 @@ pub mod formatter { .unwrap_or_else(|| JSValue::js_number_from_int32(0)); let length = length_value.coerce_to_i32(self.global_this)?; - let prev_quote_strings = self.quote_strings; - self.quote_strings = true; - let _qs = defer_restore!(self.quote_strings, prev_quote_strings); + let mut scope = self.quoting_strings(); + let this = &mut *scope; let map_name = if value.js_type() == jsc::JSType::WeakMap { "WeakMap" @@ -4584,28 +4392,24 @@ pub mod formatter { return Ok(()); } - if self.single_line { + if this.single_line { let _ = write!(writer_, "{map_name}({length}) {{ "); } else { let _ = writeln!(writer_, "{map_name}({length}) {{"); } { - self.indent += 1; - self.depth = self.depth.saturating_add(1); - let _i = defer_decrement!(self.indent); - let _d = defer_decrement!(self.depth); - let global_this = self.global_this; - if self.single_line { + this.indent += 1; + this.depth = this.depth.saturating_add(1); + let mut scope = this.indented(); + let this = &mut *scope; + let global_this = this.global_this; + if this.single_line { let mut iter = MapIteratorCtx:: { - formatter: self, + formatter: this, writer: writer_, count: 0, }; - value.for_each( - global_this, - (&raw mut iter).cast::(), - MapIteratorCtx::::for_each, - )?; + value.for_each_ctx(global_this, &mut iter)?; let count = iter.count; if iter.formatter.failed { return Ok(()); @@ -4615,22 +4419,18 @@ pub mod formatter { } } else { let mut iter = MapIteratorCtx:: { - formatter: self, + formatter: this, writer: writer_, count: 0, }; - value.for_each( - global_this, - (&raw mut iter).cast::(), - MapIteratorCtx::::for_each, - )?; + value.for_each_ctx(global_this, &mut iter)?; if iter.formatter.failed { return Ok(()); } } } - if !self.single_line { - let _ = self.write_indent(writer_); + if !this.single_line { + let _ = this.write_indent(writer_); } let _ = writer_.write_all(b"}"); Ok(()) @@ -4643,28 +4443,23 @@ pub mod formatter { value: JSValue, label: &'static str, ) -> JsResult<()> { - let prev_quote_strings = self.quote_strings; - self.quote_strings = true; - let _qs = defer_restore!(self.quote_strings, prev_quote_strings); + let mut scope = self.quoting_strings(); + let this = &mut *scope; let _ = write!(writer_, "{label} {{ "); { - self.indent += 1; - self.depth = self.depth.saturating_add(1); - let _i = defer_decrement!(self.indent); - let _d = defer_decrement!(self.depth); - let global_this = self.global_this; - if self.single_line { + this.indent += 1; + this.depth = this.depth.saturating_add(1); + let mut scope = this.indented(); + let this = &mut *scope; + let global_this = this.global_this; + if this.single_line { let mut iter = MapIteratorCtx:: { - formatter: self, + formatter: this, writer: writer_, count: 0, }; - value.for_each( - global_this, - (&raw mut iter).cast::(), - MapIteratorCtx::::for_each, - )?; + value.for_each_ctx(global_this, &mut iter)?; let count = iter.count; if iter.formatter.failed { return Ok(()); @@ -4675,15 +4470,11 @@ pub mod formatter { } } else { let mut iter = MapIteratorCtx:: { - formatter: self, + formatter: this, writer: writer_, count: 0, }; - value.for_each( - global_this, - (&raw mut iter).cast::(), - MapIteratorCtx::::for_each, - )?; + value.for_each_ctx(global_this, &mut iter)?; let count = iter.count; if iter.formatter.failed { return Ok(()); @@ -4693,8 +4484,8 @@ pub mod formatter { } } } - if !self.single_line { - let _ = self.write_indent(writer_); + if !this.single_line { + let _ = this.write_indent(writer_); } let _ = writer_.write_all(b"}"); Ok(()) @@ -4711,9 +4502,8 @@ pub mod formatter { .unwrap_or_else(|| JSValue::js_number_from_int32(0)); let length = length_value.coerce_to_i32(self.global_this)?; - let prev_quote_strings = self.quote_strings; - self.quote_strings = true; - let _qs = defer_restore!(self.quote_strings, prev_quote_strings); + let mut scope = self.quoting_strings(); + let this = &mut *scope; let set_name = if value.js_type() == jsc::JSType::WeakSet { "WeakSet" @@ -4726,28 +4516,24 @@ pub mod formatter { return Ok(()); } - if self.single_line { + if this.single_line { let _ = write!(writer_, "{set_name}({length}) {{ "); } else { let _ = writeln!(writer_, "{set_name}({length}) {{"); } { - self.indent += 1; - self.depth = self.depth.saturating_add(1); - let _i = defer_decrement!(self.indent); - let _d = defer_decrement!(self.depth); - let global_this = self.global_this; - if self.single_line { + this.indent += 1; + this.depth = this.depth.saturating_add(1); + let mut scope = this.indented(); + let this = &mut *scope; + let global_this = this.global_this; + if this.single_line { let mut iter = SetIteratorCtx:: { - formatter: self, + formatter: this, writer: writer_, is_first: true, }; - value.for_each( - global_this, - (&raw mut iter).cast::(), - SetIteratorCtx::::for_each, - )?; + value.for_each_ctx(global_this, &mut iter)?; let is_first = iter.is_first; if iter.formatter.failed { return Ok(()); @@ -4757,22 +4543,18 @@ pub mod formatter { } } else { let mut iter = SetIteratorCtx:: { - formatter: self, + formatter: this, writer: writer_, is_first: true, }; - value.for_each( - global_this, - (&raw mut iter).cast::(), - SetIteratorCtx::::for_each, - )?; + value.for_each_ctx(global_this, &mut iter)?; if iter.formatter.failed { return Ok(()); } } } - if !self.single_line { - let _ = self.write_indent(writer_); + if !this.single_line { + let _ = this.write_indent(writer_); } let _ = writer_.write_all(b"}"); Ok(()) @@ -4836,14 +4618,12 @@ pub mod formatter { { self.indent += 1; self.depth = self.depth.saturating_add(1); - let _i = defer_decrement!(self.indent); - let _d = defer_decrement!(self.depth); - let old_quote_strings = self.quote_strings; - self.quote_strings = true; - let _qs = defer_restore!(self.quote_strings, old_quote_strings); - self.write_indent(writer_).expect("unreachable"); + let mut scope = self.indented(); + let mut scope = scope.quoting_strings(); + let this = &mut *scope; + this.write_indent(writer_).expect("unreachable"); - if self.single_line { + if this.single_line { let _ = write!( writer_, "{}type: {}\"{}\"{}{},{} ", @@ -4868,11 +4648,11 @@ pub mod formatter { } if let Some(message_value) = - value.fast_get(self.global_this, jsc::BuiltinName::Message)? + value.fast_get(this.global_this, jsc::BuiltinName::Message)? { if message_value.is_string() { - if !self.single_line { - self.write_indent(writer_).expect("unreachable"); + if !this.single_line { + this.write_indent(writer_).expect("unreachable"); } let _ = write!( writer_, @@ -4882,13 +4662,13 @@ pub mod formatter { pf!("") ); let tag = - Tag::get_advanced(message_value, self.global_this, self.tag_opts())?; - self.format::(tag, writer_, message_value, self.global_this)?; - if self.failed { + Tag::get_advanced(message_value, this.global_this, this.tag_opts())?; + this.format::(tag, writer_, message_value, this.global_this)?; + if this.failed { return Ok(()); } - self.print_comma::(writer_).expect("unreachable"); - if !self.single_line { + this.print_comma::(writer_).expect("unreachable"); + if !this.single_line { let _ = writer_.write_all(b"\n"); } } @@ -4896,8 +4676,8 @@ pub mod formatter { match event_type { EventType::MessageEvent => { - if !self.single_line { - self.write_indent(writer_).expect("unreachable"); + if !this.single_line { + this.write_indent(writer_).expect("unreachable"); } let _ = write!( writer_, @@ -4907,24 +4687,24 @@ pub mod formatter { pf!("") ); let data: JSValue = value - .fast_get(self.global_this, jsc::BuiltinName::Data)? + .fast_get(this.global_this, jsc::BuiltinName::Data)? .unwrap_or(JSValue::UNDEFINED); - let tag = Tag::get_advanced(data, self.global_this, self.tag_opts())?; - self.format::(tag, writer_, data, self.global_this)?; - if self.failed { + let tag = Tag::get_advanced(data, this.global_this, this.tag_opts())?; + this.format::(tag, writer_, data, this.global_this)?; + if this.failed { return Ok(()); } - self.print_comma::(writer_).expect("unreachable"); - if !self.single_line { + this.print_comma::(writer_).expect("unreachable"); + if !this.single_line { let _ = writer_.write_all(b"\n"); } } EventType::ErrorEvent => { if let Some(error_value) = - value.fast_get(self.global_this, jsc::BuiltinName::Error)? + value.fast_get(this.global_this, jsc::BuiltinName::Error)? { - if !self.single_line { - self.write_indent(writer_).expect("unreachable"); + if !this.single_line { + this.write_indent(writer_).expect("unreachable"); } let _ = write!( writer_, @@ -4934,13 +4714,13 @@ pub mod formatter { pf!("") ); let tag = - Tag::get_advanced(error_value, self.global_this, self.tag_opts())?; - self.format::(tag, writer_, error_value, self.global_this)?; - if self.failed { + Tag::get_advanced(error_value, this.global_this, this.tag_opts())?; + this.format::(tag, writer_, error_value, this.global_this)?; + if this.failed { return Ok(()); } - self.print_comma::(writer_).expect("unreachable"); - if !self.single_line { + this.print_comma::(writer_).expect("unreachable"); + if !this.single_line { let _ = writer_.write_all(b"\n"); } } @@ -5033,19 +4813,19 @@ pub mod formatter { writer.write_all(b"key="); } - let old_quote_strings = self.quote_strings; - self.quote_strings = true; - let _qs = defer_restore!(self.quote_strings, old_quote_strings); - if writer.failed { self.failed = true; } - self.format::( - Tag::get_advanced(key_value, self.global_this, self.tag_opts())?, - writer_, - key_value, - self.global_this, - )?; + { + let mut scope = self.quoting_strings(); + let this = &mut *scope; + this.format::( + Tag::get_advanced(key_value, this.global_this, this.tag_opts())?, + writer_, + key_value, + this.global_this, + )?; + } writer = WrappedWriter { ctx: writer_, failed: false, @@ -5057,245 +4837,313 @@ pub mod formatter { } if let Some(props) = value.get(self.global_this, "props")? { - let prev_quote_strings = self.quote_strings; - let _qs = defer_restore!(self.quote_strings, prev_quote_strings); - self.quote_strings = true; - - let Some(props_obj) = props.get_object() else { - writer.write_all(b" />"); - if writer.failed { - self.failed = true; - } + let writer_failed = writer.failed; + let outcome = { + let mut scope = self.quoting_strings(); + let this = &mut *scope; + this.print_jsx_props::( + writer_, + props, + tag_opts, + needs_space, + is_tag_kind_primitive, + tag_name_slice.slice(), + writer_failed, + )? + }; + let Some(writer_failed) = outcome else { return Ok(()); }; - let props_iter = jsc::JSPropertyIterator::init( - self.global_this, - props_obj, - jsc::PropertyIteratorOptions { - skip_empty_name: true, - include_value: true, - }, - )?; + writer = WrappedWriter { + ctx: writer_, + failed: writer_failed, + estimated_line_length: &mut self.estimated_line_length, + }; + } - let children_prop = props.get(self.global_this, "children")?; - if props_iter.len > 0 { - { - self.indent += 1; - let _ind = defer_decrement!(self.indent); - let count_without_children = - props_iter.len - usize::from(children_prop.is_some()); + writer.write_all(b" />"); + if writer.failed { + self.failed = true; + } + Ok(()) + } - while let Some((prop, property_value)) = props_iter.next()? { - if prop.eq_ascii(b"children") { - continue; - } + /// The `props` (and children) half of [`print_jsx`](Self::print_jsx), + /// run with `quote_strings` set. `None`: the element was closed here; + /// `Some(failed)`: the caller writes the self-closing tail. + #[allow(clippy::too_many_arguments)] + fn print_jsx_props( + &mut self, + writer_: &mut dyn bun_io::Write, + props: JSValue, + tag_opts: TagOptions, + mut needs_space: bool, + is_tag_kind_primitive: bool, + tag_name_slice: &[u8], + mut writer_failed: bool, + ) -> JsResult> { + macro_rules! pf { + ($s:literal) => { + pfmt!($s, C) + }; + } + let mut writer = WrappedWriter { + ctx: writer_, + failed: writer_failed, + estimated_line_length: &mut self.estimated_line_length, + }; + let Some(props_obj) = props.get_object() else { + writer.write_all(b" />"); + if writer.failed { + self.failed = true; + } + return Ok(None); + }; + let props_iter = jsc::JSPropertyIterator::init( + self.global_this, + props_obj, + jsc::PropertyIteratorOptions { + skip_empty_name: true, + include_value: true, + }, + )?; - let tag = - Tag::get_advanced(property_value, self.global_this, tag_opts)?; + let children_prop = props.get(self.global_this, "children")?; + writer_failed = writer.failed; + if props_iter.len > 0 { + { + self.indent += 1; + let mut scope = self.scoped(|f| f.indent = f.indent.saturating_sub(1)); + let this = &mut *scope; + let mut writer = WrappedWriter { + ctx: writer_, + failed: writer_failed, + estimated_line_length: &mut this.estimated_line_length, + }; + let count_without_children = + props_iter.len - usize::from(children_prop.is_some()); - if tag.cell.is_hidden() { - continue; - } + while let Some((prop, property_value)) = props_iter.next()? { + if prop.eq_ascii(b"children") { + continue; + } - if needs_space { - writer.space(); - } - needs_space = false; + let tag = Tag::get_advanced(property_value, this.global_this, tag_opts)?; - writer.print(format_args!( - "{}{}{}={}", - pf!(""), - prop.trunc(128), - pf!(""), - pf!("") - )); - let props_i = props_iter.i.get() as usize; + if tag.cell.is_hidden() { + continue; + } - if tag.cell.is_string_like() && C { - writer.write_all(pfmt!("", true).as_bytes()); - } + if needs_space { + writer.space(); + } + needs_space = false; - if writer.failed { - self.failed = true; - } - self.format::(tag, writer_, property_value, self.global_this)?; - writer = WrappedWriter { - ctx: writer_, - failed: false, - estimated_line_length: &mut self.estimated_line_length, - }; - - if tag.cell.is_string_like() && C { - writer.write_all(pfmt!("", true).as_bytes()); - } + writer.print(format_args!( + "{}{}{}={}", + pf!(""), + prop.trunc(128), + pf!(""), + pf!("") + )); + let props_i = props_iter.i.get() as usize; - if !self.single_line - && ( - // count_without_children is necessary to prevent - // printing an extra newline if there are children - // and one prop and the child prop is the last prop - props_i + 1 < count_without_children + if tag.cell.is_string_like() && C { + writer.write_all(pfmt!("", true).as_bytes()); + } + + if writer.failed { + this.failed = true; + } + this.format::(tag, writer_, property_value, this.global_this)?; + writer = WrappedWriter { + ctx: writer_, + failed: false, + estimated_line_length: &mut this.estimated_line_length, + }; + + if tag.cell.is_string_like() && C { + writer.write_all(pfmt!("", true).as_bytes()); + } + + if !this.single_line + && ( + // count_without_children is necessary to prevent + // printing an extra newline if there are children + // and one prop and the child prop is the last prop + props_i + 1 < count_without_children // 3 is arbitrary but basically // // ^ should be one line // // ^ should be multiple lines && props_i > 3 - ) - { - writer.write_all(b"\n"); - write_indent_n(self.indent, writer.ctx).expect("unreachable"); - } else if props_i + 1 < count_without_children { - writer.space(); - } + ) + { + writer.write_all(b"\n"); + write_indent_n(this.indent, writer.ctx).expect("unreachable"); + } else if props_i + 1 < count_without_children { + writer.space(); } } + writer_failed = writer.failed; + } + let mut writer = WrappedWriter { + ctx: writer_, + failed: writer_failed, + estimated_line_length: &mut self.estimated_line_length, + }; - if let Some(children) = children_prop { - let tag = Tag::get(children, self.global_this)?; + if let Some(children) = children_prop { + let tag = Tag::get(children, self.global_this)?; - let print_children = - matches!(tag.tag.tag(), Tag::String | Tag::JSX | Tag::Array); + let print_children = + matches!(tag.tag.tag(), Tag::String | Tag::JSX | Tag::Array); - if print_children && !self.single_line { - 'print_children: { - match tag.tag.tag() { - Tag::String => { - let children_string = - children.to_js_string_view(self.global_this)?; - if children_string.is_empty() { - break 'print_children; - } - if C { - writer.write_all(pfmt!("", true).as_bytes()); - } - writer.write_all(b">"); - if children_string.length() < 128 { - writer.write_string(&children_string); - } else { - self.indent += 1; - writer.write_all(b"\n"); - write_indent_n(self.indent, writer.ctx) - .expect("unreachable"); - self.indent = self.indent.saturating_sub(1); - writer.write_string(&children_string); - writer.write_all(b"\n"); - write_indent_n(self.indent, writer.ctx) - .expect("unreachable"); + if print_children && !self.single_line { + 'print_children: { + match tag.tag.tag() { + Tag::String => { + let children_string = + children.to_js_string_view(self.global_this)?; + if children_string.is_empty() { + break 'print_children; + } + if C { + writer.write_all(pfmt!("", true).as_bytes()); + } + writer.write_all(b">"); + if children_string.length() < 128 { + writer.write_string(&children_string); + } else { + self.indent += 1; + writer.write_all(b"\n"); + write_indent_n(self.indent, writer.ctx) + .expect("unreachable"); + self.indent = self.indent.saturating_sub(1); + writer.write_string(&children_string); + writer.write_all(b"\n"); + write_indent_n(self.indent, writer.ctx) + .expect("unreachable"); + } + } + Tag::JSX => { + writer.write_all(b">\n"); + { + self.indent += 1; + write_indent_n(self.indent, writer.ctx) + .expect("unreachable"); + if writer.failed { + self.failed = true; } + let mut scope = + self.scoped(|f| f.indent = f.indent.saturating_sub(1)); + let this = &mut *scope; + this.format::( + Tag::get(children, this.global_this)?, + writer_, + children, + this.global_this, + )?; } - Tag::JSX => { - writer.write_all(b">\n"); - { - self.indent += 1; - write_indent_n(self.indent, writer.ctx) - .expect("unreachable"); - let _ind = defer_decrement!(self.indent); + writer = WrappedWriter { + ctx: writer_, + failed: false, + estimated_line_length: &mut self.estimated_line_length, + }; + writer.write_all(b"\n"); + write_indent_n(self.indent, writer.ctx).expect("unreachable"); + } + Tag::Array => { + let length = children.get_length(self.global_this)?; + if length == 0 { + break 'print_children; + } + writer.write_all(b">\n"); + { + self.indent += 1; + write_indent_n(self.indent, writer.ctx) + .expect("unreachable"); + let writer_failed = writer.failed; + let prev_quote_strings = self.quote_strings; + self.quote_strings = false; + let mut scope = self.scoped(move |f| { + f.indent = f.indent.saturating_sub(1); + f.quote_strings = prev_quote_strings; + }); + let this = &mut *scope; + let mut writer = WrappedWriter { + ctx: writer_, + failed: writer_failed, + estimated_line_length: &mut this.estimated_line_length, + }; + + let mut j: usize = 0; + while (j as u64) < length { + let child = children.get_index( + this.global_this, + u32::try_from(j).expect("int cast"), + )?; if writer.failed { - self.failed = true; + this.failed = true; } - self.format::( - Tag::get(children, self.global_this)?, + this.format::( + Tag::get_advanced( + child, + this.global_this, + this.tag_opts(), + )?, writer_, - children, - self.global_this, + child, + this.global_this, )?; writer = WrappedWriter { ctx: writer_, failed: false, - estimated_line_length: &mut self + estimated_line_length: &mut this .estimated_line_length, }; - } - writer.write_all(b"\n"); - write_indent_n(self.indent, writer.ctx) - .expect("unreachable"); - } - Tag::Array => { - let length = children.get_length(self.global_this)?; - if length == 0 { - break 'print_children; - } - writer.write_all(b">\n"); - { - self.indent += 1; - write_indent_n(self.indent, writer.ctx) - .expect("unreachable"); - let _prev_quote_strings = self.quote_strings; - self.quote_strings = false; - let _qs2 = defer_restore!( - self.quote_strings, - _prev_quote_strings - ); - let _ind = defer_decrement!(self.indent); - - let mut j: usize = 0; - while (j as u64) < length { - let child = children.get_index( - self.global_this, - u32::try_from(j).expect("int cast"), - )?; - if writer.failed { - self.failed = true; - } - self.format::( - Tag::get_advanced( - child, - self.global_this, - self.tag_opts(), - )?, - writer_, - child, - self.global_this, - )?; - writer = WrappedWriter { - ctx: writer_, - failed: false, - estimated_line_length: &mut self - .estimated_line_length, - }; - if (j as u64) + 1 < length { - writer.write_all(b"\n"); - write_indent_n(self.indent, writer.ctx) - .expect("unreachable"); - } - j += 1; + if (j as u64) + 1 < length { + writer.write_all(b"\n"); + write_indent_n(this.indent, writer.ctx) + .expect("unreachable"); } + j += 1; } - writer.write_all(b"\n"); - write_indent_n(self.indent, writer.ctx) - .expect("unreachable"); } - _ => unreachable!(), - } - - writer.write_all(b"").as_bytes()); - } else { - writer.write_all(pf!("").as_bytes()); - } - writer.write_all(tag_name_slice.slice()); - if C { - writer.write_all(pf!("").as_bytes()); + writer = WrappedWriter { + ctx: writer_, + failed: false, + estimated_line_length: &mut self.estimated_line_length, + }; + writer.write_all(b"\n"); + write_indent_n(self.indent, writer.ctx).expect("unreachable"); } - writer.write_all(b">"); + _ => unreachable!(), } - if writer.failed { - self.failed = true; + writer.write_all(b"").as_bytes()); + } else { + writer.write_all(pf!("").as_bytes()); } - return Ok(()); + writer.write_all(tag_name_slice); + if C { + writer.write_all(pf!("").as_bytes()); + } + writer.write_all(b">"); } + + if writer.failed { + self.failed = true; + } + return Ok(None); } } + writer_failed = writer.failed; } - - writer.write_all(b" />"); - if writer.failed { - self.failed = true; - } - Ok(()) + Ok(Some(writer_failed)) } #[inline(never)] @@ -5307,8 +5155,13 @@ pub mod formatter { ) -> JsResult<()> { debug_assert!(value.is_cell()); let prev_quote_strings = self.quote_strings; + let prev_always_newline_scope = self.always_newline_scope; self.quote_strings = true; - let _qs = defer_restore!(self.quote_strings, prev_quote_strings); + let mut scope = self.scoped(move |f| { + f.quote_strings = prev_quote_strings; + f.always_newline_scope = prev_always_newline_scope; + }); + let this = &mut *scope; // We want to figure out if we should print this object on one line // or multiple lines. @@ -5330,22 +5183,20 @@ pub mod formatter { // Set, or Blob // // Then, we print it each property on a new line, recursively. - let prev_always_newline_scope = self.always_newline_scope; - let _ans = defer_restore!(self.always_newline_scope, prev_always_newline_scope); - // Hoist all `self.*` reads before constructing the iterator ctx — - // `formatter: self` is a `&mut Self` reborrow, so once it's moved - // into the struct literal we can no longer touch `self` until + // Hoist all `this.*` reads before constructing the iterator ctx — + // `formatter: this` is a `&mut Self` reborrow, so once it's moved + // into the struct literal we can no longer touch `this` until // `iter` is dropped (or via `iter.formatter`). - let single_line = self.single_line; + let single_line = this.single_line; let always_newline = - !single_line && (self.always_newline_scope || self.good_time_for_a_new_line()); - if self.depth > self.max_depth { - return self.print_object_depth_exceeded::(writer_, value); + !single_line && (this.always_newline_scope || this.good_time_for_a_new_line()); + if this.depth > this.max_depth { + return this.print_object_depth_exceeded::(writer_, value); } - let ordered_properties = self.ordered_properties; - let global_this = self.global_this; + let ordered_properties = this.ordered_properties; + let global_this = this.global_this; let mut iter = PropertyIteratorCtx:: { - formatter: self, + formatter: this, writer: writer_, always_newline, single_line, @@ -5354,29 +5205,21 @@ pub mod formatter { }; if ordered_properties { - value.for_each_property_ordered( - global_this, - (&raw mut iter).cast::(), - PropertyIteratorCtx::::for_each, - )?; + value.for_each_property_ordered_ctx(global_this, &mut iter)?; } else { - value.for_each_property( - global_this, - (&raw mut iter).cast::(), - PropertyIteratorCtx::::for_each, - )?; + value.for_each_property_ctx(global_this, &mut iter)?; } - // Extract what we need from `iter` so its `&mut self` / `&mut writer_` - // reborrows end here (NLL) and the tail can use `self`/`writer_` again. + // Extract what we need from `iter` so its `&mut Formatter` / `&mut writer_` + // reborrows end here (NLL) and the tail can use `this`/`writer_` again. let iter_i = iter.i; let iter_always_newline = iter.always_newline; - if self.failed { + if this.failed { return Ok(()); } - self.print_object_tail::(writer_, value, js_type, iter_i, iter_always_newline) + this.print_object_tail::(writer_, value, js_type, iter_i, iter_always_newline) } #[inline(never)] @@ -5604,13 +5447,15 @@ pub mod formatter { global_this: &'a JSGlobalObject, ) -> JsResult<()> { let prev_global_this = self.global_this; - let _restore = defer_restore!(self.global_this, prev_global_this); self.global_this = global_this; if let TagPayload::CustomFormattedObject(obj) = result.tag { self.custom_formatted_object = obj; } - self.print_as::(result.tag.tag(), writer, value, result.cell) + let result = + self.print_as::(result.tag.tag(), writer, value, result.cell); + self.global_this = prev_global_this; + result } /// Format a single value into `writer`, propagating a JS exception @@ -5688,65 +5533,48 @@ pub mod formatter { // C-exported entry points (count / time / etc.) // ─────────────────────────────────────────────────────────────────────────── -#[unsafe(no_mangle)] -#[crate::host_call] -pub(crate) extern "C" fn Bun__ConsoleObject__count( - _console: *mut ConsoleObject, - global_this: &JSGlobalObject, - ptr: *const u8, - len: usize, -) { - // SAFETY: top-level JS-thread host call ⇒ exclusive access to the - // set-once `VirtualMachine.console` box. - let this = unsafe { vm_console_mut(global_this) }; - // SAFETY: caller passes a valid (ptr, len) pair. - let slice = unsafe { bun_core::ffi::slice(ptr, len) }; - let hash = bun_wyhash::hash(slice); +// HOST_EXPORT(Bun__ConsoleObject__count, jsc) +pub fn count(_console: *mut c_void, global_this: &JSGlobalObject, chars: &[u8]) { + let hash = bun_wyhash::hash(chars); // we don't want to store these strings, it will take too much memory - let counter = this.counts.get_or_put(hash).expect("unreachable"); - let current: u32 = if counter.found_existing { - *counter.value_ptr - } else { - 0 - } + 1; - *counter.value_ptr = current; + let current: u32 = with_console(global_this, |this| { + let counter = this.counts.get_or_put(hash).expect("unreachable"); + let current: u32 = if counter.found_existing { + *counter.value_ptr + } else { + 0 + } + 1; + *counter.value_ptr = current; + current + }); - let writer = this.writer(); + let writer = console_writer(global_this, false); if Output::enable_ansi_colors_stdout() { let _ = writeln!( writer, "{}{}{}: {}{}{}", pfmt!("", true), - bstr::BStr::new(slice), + bstr::BStr::new(chars), pfmt!("", true), pfmt!("", true), current, pfmt!("", true), ); } else { - let _ = writeln!(writer, "{}: {}", bstr::BStr::new(slice), current); + let _ = writeln!(writer, "{}: {}", bstr::BStr::new(chars), current); } let _ = writer.flush(); } -#[unsafe(no_mangle)] -#[crate::host_call] -pub(crate) extern "C" fn Bun__ConsoleObject__countReset( - _console: *mut ConsoleObject, - global_this: &JSGlobalObject, - ptr: *const u8, - len: usize, -) { - // SAFETY: top-level JS-thread host call ⇒ exclusive access to the - // set-once `VirtualMachine.console` box. - let this = unsafe { vm_console_mut(global_this) }; - // SAFETY: caller passes a valid (ptr, len) pair. - let slice = unsafe { bun_core::ffi::slice(ptr, len) }; - let hash = bun_wyhash::hash(slice); +// HOST_EXPORT(Bun__ConsoleObject__countReset, jsc) +pub fn count_reset(_console: *mut c_void, global_this: &JSGlobalObject, chars: &[u8]) { + let hash = bun_wyhash::hash(chars); // we don't delete it because deleting is implemented via tombstoning - if let Some(v) = this.counts.get_mut(&hash) { - *v = 0; - } + with_console(global_this, |this| { + if let Some(v) = this.counts.get_mut(&hash) { + *v = 0; + } + }); } type PendingTimers = bun_collections::HashMap>; @@ -5755,16 +5583,9 @@ thread_local! { static PENDING_TIME_LOGS_LOADED: Cell = const { Cell::new(false) }; } -#[unsafe(no_mangle)] -#[crate::host_call] -pub(crate) extern "C" fn Bun__ConsoleObject__time( - _console: *mut ConsoleObject, - _global: &JSGlobalObject, - chars: *const u8, - len: usize, -) { - // SAFETY: caller passes a valid (ptr, len) pair. - let id = bun_wyhash::hash(unsafe { bun_core::ffi::slice(chars, len) }); +// HOST_EXPORT(Bun__ConsoleObject__time, jsc) +pub fn time(_console: *mut c_void, _global: &JSGlobalObject, chars: &[u8]) { + let id = bun_wyhash::hash(chars); if !PENDING_TIME_LOGS_LOADED.with(|c| c.get()) { PENDING_TIME_LOGS.with_borrow_mut(|m| *m = PendingTimers::default()); PENDING_TIME_LOGS_LOADED.with(|c| c.set(true)); @@ -5778,21 +5599,13 @@ pub(crate) extern "C" fn Bun__ConsoleObject__time( }); } -#[unsafe(no_mangle)] -#[crate::host_call] -pub(crate) extern "C" fn Bun__ConsoleObject__timeEnd( - _console: *mut ConsoleObject, - _global: &JSGlobalObject, - chars: *const u8, - len: usize, -) { +// HOST_EXPORT(Bun__ConsoleObject__timeEnd, jsc) +pub fn time_end(_console: *mut c_void, _global: &JSGlobalObject, chars: &[u8]) { if !PENDING_TIME_LOGS_LOADED.with(|c| c.get()) { return; } - // SAFETY: caller passes a valid (ptr, len) pair. - let slice = unsafe { bun_core::ffi::slice(chars, len) }; - let id = bun_wyhash::hash(slice); + let id = bun_wyhash::hash(chars); // Replace the slot with `None`, returning the previous value. let Some(prev) = PENDING_TIME_LOGS.with_borrow_mut(|m| m.get_mut(&id).map(|slot| slot.take())) else { @@ -5803,31 +5616,21 @@ pub(crate) extern "C" fn Bun__ConsoleObject__timeEnd( Output::print_elapsed( (value.read() / bun_core::time::NS_PER_US) as f64 / bun_core::time::US_PER_MS as f64, ); - match len { + match chars.len() { 0 => Output::print_errorln(format_args!("")), - _ => Output::print_errorln(format_args!(" {}", bstr::BStr::new(slice))), + _ => Output::print_errorln(format_args!(" {}", bstr::BStr::new(chars))), } Output::flush(); } -#[unsafe(no_mangle)] -#[crate::host_call] -pub(crate) extern "C" fn Bun__ConsoleObject__timeLog( - _console: *mut ConsoleObject, - global: &JSGlobalObject, - chars: *const u8, - len: usize, - args: *const JSValue, - args_len: usize, -) { +// HOST_EXPORT(Bun__ConsoleObject__timeLog, jsc) +pub fn time_log(_console: *mut c_void, global: &JSGlobalObject, chars: &[u8], args: &[JSValue]) { if !PENDING_TIME_LOGS_LOADED.with(|c| c.get()) { return; } - // SAFETY: caller passes a valid (ptr, len) pair. - let slice = unsafe { bun_core::ffi::slice(chars, len) }; - let id = bun_wyhash::hash(slice); + let id = bun_wyhash::hash(chars); let Some(Some(value)) = PENDING_TIME_LOGS.with_borrow(|m| m.get(&id).copied()) else { return; }; @@ -5835,9 +5638,9 @@ pub(crate) extern "C" fn Bun__ConsoleObject__timeLog( Output::print_elapsed( (value.read() / bun_core::time::NS_PER_US) as f64 / bun_core::time::US_PER_MS as f64, ); - match len { + match chars.len() { 0 => {} - _ => Output::print_error(format_args!(" {}", bstr::BStr::new(slice))), + _ => Output::print_error(format_args!(" {}", bstr::BStr::new(chars))), } Output::flush(); @@ -5850,14 +5653,10 @@ pub(crate) extern "C" fn Bun__ConsoleObject__timeLog( .unwrap_or(DEFAULT_CONSOLE_LOG_DEPTH); fmt.stack_check = StackCheck::init(); fmt.can_throw_stack_overflow = true; - let console = vm_console(global); - // SAFETY: see [`vm_console`] — points at the live boxed `ConsoleObject` for - // this VM; JS-thread-only. Kept as a raw deref (not `vm_console_mut`) so the - // resulting `writer` borrow does not pin a long-lived `&mut ConsoleObject` - // across the `fmt.format(...)` calls below, which can re-enter JS. - let mut writer = unsafe { (*console).error_writer() }; - // SAFETY: caller passes a valid (args, args_len) pair. - for &arg in unsafe { bun_core::ffi::slice(args, args_len) } { + // The `fmt.format(...)` calls below can re-enter JS (and `console.*`); + // `writer` is the VM console's buffered stderr for the duration. + let mut writer = console_writer(global, true); + for &arg in args { let Ok(tag) = formatter::Tag::get(arg, global) else { return; }; @@ -5872,96 +5671,73 @@ pub(crate) extern "C" fn Bun__ConsoleObject__timeLog( let _ = bun_io::Write::flush(&mut writer); } -/// Stamp out the empty `Bun__ConsoleObject__*` C-ABI hooks that JSC's -/// `ConsoleClient` vtable requires but Bun leaves unimplemented. Two arms cover -/// the two trailing-arg shapes the C++ side declares in -/// `bindings/headers.h:686-694`: `(…, *const u8, usize)` for the title-string -/// hooks and `(…, *mut ScriptArguments)` for the inspector-args hooks. -/// -/// Ident concat via `${concat()}` is unstable (`macro_metavar_expr_concat`) -/// and `paste` is not a `bun_jsc` dep, so the full `Bun__ConsoleObject__` -/// export symbol is passed verbatim — same pattern as `export_callbacks!` in -/// `runtime/api/BunObject.rs`. -macro_rules! console_noop_hooks { - (str: $($name:ident),+ $(,)?) => {$( - #[unsafe(no_mangle)] - #[crate::host_call] - pub extern "C" fn $name( - _console: *mut ConsoleObject, - _global: &JSGlobalObject, - _chars: *const u8, - _len: usize, - ) { - } - )+}; - (args: $($name:ident),+ $(,)?) => {$( - #[unsafe(no_mangle)] - #[crate::host_call] - pub extern "C" fn $name( - _console: *mut ConsoleObject, - _global: &JSGlobalObject, - _args: *mut ScriptArguments, - ) { - } - )+}; -} +// JSC's `ConsoleClient` vtable requires these; Bun leaves them unimplemented. -console_noop_hooks!(str: Bun__ConsoleObject__profile, Bun__ConsoleObject__profileEnd); +// HOST_EXPORT(Bun__ConsoleObject__profile, jsc) +pub fn profile(_console: *mut c_void, _global: &JSGlobalObject, _chars: &[u8]) {} -#[unsafe(no_mangle)] -#[crate::host_call] -pub(crate) extern "C" fn Bun__ConsoleObject__takeHeapSnapshot( - _console: *mut ConsoleObject, - global_this: &JSGlobalObject, - _chars: *const u8, - _len: usize, -) { +// HOST_EXPORT(Bun__ConsoleObject__profileEnd, jsc) +pub fn profile_end(_console: *mut c_void, _global: &JSGlobalObject, _chars: &[u8]) {} + +// HOST_EXPORT(Bun__ConsoleObject__takeHeapSnapshot, jsc) +pub fn take_heap_snapshot(_console: *mut c_void, global_this: &JSGlobalObject, _chars: &[u8]) { // TODO: this does an extra JSONStringify and we don't need it to! let snapshot: [JSValue; 1] = [global_this.generate_heap_snapshot()]; - // SAFETY: re-entry into our own host shim with a stack-local args slice. - unsafe { - message_with_type_and_level( - core::ptr::null_mut(), // unused by the callee - MessageType::Log, - MessageLevel::Debug, - global_this, - snapshot.as_ptr(), - 1, - ); - } + message_with_type_and_level( + MessageType::Log, + MessageLevel::Debug, + global_this, + &snapshot, + ); } -console_noop_hooks!( - args: - Bun__ConsoleObject__timeStamp, - Bun__ConsoleObject__record, - Bun__ConsoleObject__recordEnd, - Bun__ConsoleObject__screenshot, -); - -#[unsafe(no_mangle)] -#[crate::host_call] -pub(crate) extern "C" fn Bun__ConsoleObject__messageWithTypeAndLevel( - ctype: *mut ConsoleObject, - // Taking the - // exhaustive Rust enums by value at the C ABI would be UB on an - // out-of-range discriminant, so accept the raw `u32` (matching the C++ - // header in `bindings/headers.h`) and clamp via `from_raw`. +// HOST_EXPORT(Bun__ConsoleObject__timeStamp, jsc) +pub fn time_stamp( + _console: *mut c_void, + _global: &JSGlobalObject, + _args: *mut crate::console_object::ScriptArguments, +) { +} + +// HOST_EXPORT(Bun__ConsoleObject__record, jsc) +pub fn record( + _console: *mut c_void, + _global: &JSGlobalObject, + _args: *mut crate::console_object::ScriptArguments, +) { +} + +// HOST_EXPORT(Bun__ConsoleObject__recordEnd, jsc) +pub fn record_end( + _console: *mut c_void, + _global: &JSGlobalObject, + _args: *mut crate::console_object::ScriptArguments, +) { +} + +// HOST_EXPORT(Bun__ConsoleObject__screenshot, jsc) +pub fn screenshot( + _console: *mut c_void, + _global: &JSGlobalObject, + _args: *mut crate::console_object::ScriptArguments, +) { +} + +/// Taking the exhaustive Rust enums by value at the C ABI would be UB on an +/// out-of-range discriminant, so accept the raw `u32` (matching the C++ +/// header in `bindings/headers.h`) and clamp via `from_raw`. +// HOST_EXPORT(Bun__ConsoleObject__messageWithTypeAndLevel, jsc) +pub fn message_with_type_and_level_raw( + _console: *mut c_void, message_type: u32, level: u32, global: &JSGlobalObject, - vals: *const JSValue, - len: usize, + vals: &[JSValue], ) { - // SAFETY: forwarding the same FFI args to the inner host shim. - unsafe { - message_with_type_and_level( - ctype, - MessageType::from_raw(message_type), - MessageLevel::from_raw(level), - global, - vals, - len, - ) - }; + message_with_type_and_level( + MessageType::from_raw(message_type), + MessageLevel::from_raw(level), + global, + vals, + ); } diff --git a/src/jsc/Debugger.rs b/src/jsc/Debugger.rs index 02f850138b13..3e540d07b09a 100644 --- a/src/jsc/Debugger.rs +++ b/src/jsc/Debugger.rs @@ -41,8 +41,7 @@ pub use crate::http_server_agent::HTTPServerAgent; /// The fields are private so every outside access flows through the named /// accessors below, keeping the owning module's interpretation the only one. /// -/// Both fields are `Copy`, so `Cell` gives interior mutability with zero -/// `unsafe`. +/// Both fields are `Copy`, so `Cell` is all the interior mutability needed. pub struct ErasedAgentSlot { agent: Cell<*mut c_void>, sequence: Cell, @@ -259,44 +258,18 @@ impl Debugger { // delay (Windows lacks a working `tickWithTimeout`). TODO: remove // this when tickWithTimeout actually works properly on Windows. use bun_sys::windows::libuv as uv; - use bun_sys::windows::libuv::UvHandle as _; if wait == Wait::Shortly { - let uv_loop = this.uv_loop(); - // SAFETY: `uv_loop` is a live initialized `uv_loop_t`. - unsafe { uv::uv_update_time(uv_loop) }; - let timer: *mut uv::Timer = - bun_core::heap::into_raw(Box::new(bun_core::ffi::zeroed())); - // SAFETY: `timer` freshly allocated; `uv_loop` valid. - unsafe { (*timer).init(uv_loop) }; - - extern "C" fn on_debugger_timer(handle: *mut uv::Timer) { - // SAFETY: `vm` is the per-thread singleton; called on the - // JS thread (libuv timer callback). Unwinding across - // `extern "C"` is UB so we early-return if no debugger. + fn on_debugger_timer() { + // JS thread (libuv timer callback). if let Some(d) = VirtualMachine::get().as_mut().debugger.as_deref_mut() { d.poll_ref.unref(get_vm_ctx(AllocatorType::Js)); } - // SAFETY: `handle` is a live `uv_timer_t` (`uv_handle_t` - // at offset 0); `deinit_timer` matches `uv_close_cb`. - unsafe { - uv::uv_close(handle.cast(), Some(deinit_timer)); - } - } - extern "C" fn deinit_timer(handle: *mut uv::uv_handle_t) { - // SAFETY: `handle` is the `Box` allocated above - // (cast through `uv_handle_t` at offset 0); this is the - // sole owner reclaiming it after `uv_close` completes. - drop(unsafe { bun_core::heap::take(handle.cast::()) }); - } - // SAFETY: `timer` initialized above. - unsafe { - (*timer).start( - WAIT_FOR_CONNECTION_DELAY_MS as u64, - 0, - Some(on_debugger_timer), - ); - (*timer).ref_(); } + uv::Timer::one_shot( + this.uv_loop(), + WAIT_FOR_CONNECTION_DELAY_MS as u64, + on_debugger_timer, + ); } } @@ -377,9 +350,8 @@ impl Debugger { if HAS_CREATED_DEBUGGER.swap(true, Ordering::Relaxed) { return Ok(()); } - // `#[unsafe(no_mangle)]` already prevents the linker from stripping - // the exported `Bun__*Agent*` symbols, so no explicit keep-alive - // references are needed. + // The exported (`no_mangle`) `Bun__*Agent*` symbols cannot be stripped + // by the linker, so no explicit keep-alive references are needed. // `this` is the live per-thread VM; same allocation as // `VirtualMachine::get()` — route through the safe thread-local @@ -456,17 +428,7 @@ impl Debugger { vm.is_main_thread = false; vm.event_loop_mut().ensure_waker(); - extern "C" fn start_trampoline(ctx: *mut c_void) { - // SAFETY: `ctx` is `&mut slot` below; `hold_api_lock` calls this - // synchronously on the same frame. - let init = unsafe { (*ctx.cast::>()).take() }; - Debugger::start(init.expect("init")); - } - let mut slot = Some(init); - #[allow(deprecated)] - vm.global() - .vm() - .hold_api_lock((&raw mut slot).cast(), start_trampoline); + vm.run_with_api_lock(move || Debugger::start(init)); } /// Runs inside `holdAPILock` on the debugger thread. Publishes the @@ -580,9 +542,7 @@ pub fn start_node_inspector_server(url: &mut BunString, wait_for_connection: boo opts.debugger = true; } - let global = this.global; - // SAFETY: `global` is set during `VirtualMachine::init` and outlives the VM. - if Debugger::create(VirtualMachine::get_mut_ptr(), unsafe { &*global }).is_err() { + if Debugger::create(VirtualMachine::get_mut_ptr(), this.global()).is_err() { this.as_mut().debugger = None; return false; } @@ -873,9 +833,8 @@ impl TestReporterHandle { } // HOST_EXPORT(Bun__TestReporterAgentEnable, c) -pub fn test_reporter_agent_enable(agent: *mut TestReporterHandle) { - // SAFETY: `VirtualMachine::get()` returns the per-thread singleton; called - // on the JS thread. +pub fn test_reporter_agent_enable(agent: &mut TestReporterHandle) { + // JS thread. if let Some(dbg) = VirtualMachine::get().as_mut().debugger.as_deref_mut() { bun_core::scoped_log!(TestReporterAgent, "enable"); dbg.test_reporter_agent.handle = agent; @@ -887,13 +846,10 @@ pub fn test_reporter_agent_enable(agent: *mut TestReporterHandle) { // the test runner (`bun_test.DescribeScope`), which lives in `bun_runtime::test_runner` // — a forward-dep cycle. Dispatched through [`RuntimeHooks`]. if let Some(hooks) = runtime_hooks() { - // SAFETY: `handle` is the live C++ agent just stored above. - dbg.test_reporter_agent.next_test_id = unsafe { - (hooks.retroactively_report_discovered_tests)( - dbg.test_reporter_agent.handle, - dbg.test_reporter_agent.next_test_id, - ) - }; + dbg.test_reporter_agent.next_test_id = (hooks.retroactively_report_discovered_tests)( + agent, + dbg.test_reporter_agent.next_test_id, + ); } } } diff --git a/src/jsc/JSValue.rs b/src/jsc/JSValue.rs index 09e31b639806..686460359898 100644 --- a/src/jsc/JSValue.rs +++ b/src/jsc/JSValue.rs @@ -2204,7 +2204,7 @@ struct SerializedScriptValueExternal { handle: *mut c_void, } -/// Callback signature for [`JSValue::for_each`] / [`JSValue::for_each_with_context`]. +/// Callback signature for [`JSValue::for_each`]. pub type ForEachCallback = extern "C" fn(vm: *mut crate::VM, global: &JSGlobalObject, ctx: *mut c_void, next: JSValue); @@ -2219,6 +2219,50 @@ pub(crate) type ForEachPropertyCallback = extern "C" fn( is_private_symbol: bool, ); +/// Typed receiver for [`JSValue::for_each_ctx`]: one call per iterated element. +pub trait ForEachContext { + fn on_value(&mut self, global: &JSGlobalObject, value: JSValue); +} + +/// Typed receiver for [`JSValue::for_each_property_ctx`] and its variants: one +/// call per own property. +pub trait ForEachPropertyContext { + fn on_property( + &mut self, + global: &JSGlobalObject, + key: &bun_core::EncodedSlice, + value: JSValue, + is_symbol: bool, + is_private_symbol: bool, + ); +} + +/// [`ForEachCallback`] trampoline for a `&mut C` context. +extern "C" fn for_each_context_trampoline( + _: *mut crate::VM, + global: &JSGlobalObject, + ctx: *mut c_void, + next: JSValue, +) { + // SAFETY: `ctx` is the `&mut C` `for_each_ctx` passed for this synchronous iteration. + unsafe { &mut *ctx.cast::() }.on_value(global, next); +} + +/// [`ForEachPropertyCallback`] trampoline for a `&mut C` context. +extern "C" fn for_each_property_context_trampoline( + global: &JSGlobalObject, + ctx: *mut c_void, + key: *mut bun_core::EncodedSlice, + value: JSValue, + is_symbol: bool, + is_private_symbol: bool, +) { + // SAFETY: `ctx` is the `&mut C` the `for_each_property*_ctx` caller passed for + // this synchronous iteration; `key` is the live stack `EncodedSlice` C++ passes. + let (ctx, key) = unsafe { (&mut *ctx.cast::(), &*key) }; + ctx.on_property(global, key, value, is_symbol, is_private_symbol); +} + /// `JSValue.StringFormatter` — `Display` adapter that /// coerces the value via `toBunString` at format time. pub struct StringFormatter<'a> { @@ -2437,16 +2481,59 @@ impl JSValue { JSC__JSValue__forEach(self, global, ctx, callback) }) } - /// `JSValue.forEachWithContext` — typed-ctx wrapper (callers pass - /// `*mut c_void` directly). + /// [`for_each`](Self::for_each) with a typed context. #[inline] - pub(crate) fn for_each_with_context( + pub fn for_each_ctx( self, global: &JSGlobalObject, - ctx: *mut c_void, - callback: ForEachCallback, + ctx: &mut C, + ) -> JsResult<()> { + self.for_each( + global, + core::ptr::from_mut(ctx).cast(), + for_each_context_trampoline::, + ) + } + /// [`for_each_property`](Self::for_each_property) with a typed context. + #[inline(always)] + pub fn for_each_property_ctx( + self, + global: &JSGlobalObject, + ctx: &mut C, + ) -> JsResult<()> { + self.for_each_property( + global, + core::ptr::from_mut(ctx).cast(), + for_each_property_context_trampoline::, + ) + } + /// [`for_each_property_non_indexed`](Self::for_each_property_non_indexed) + /// with a typed context. + #[inline(always)] + pub fn for_each_property_non_indexed_ctx( + self, + global: &JSGlobalObject, + ctx: &mut C, + ) -> JsResult<()> { + self.for_each_property_non_indexed( + global, + core::ptr::from_mut(ctx).cast(), + for_each_property_context_trampoline::, + ) + } + /// [`for_each_property_ordered`](Self::for_each_property_ordered) with a + /// typed context. + #[inline(always)] + pub fn for_each_property_ordered_ctx( + self, + global: &JSGlobalObject, + ctx: &mut C, ) -> JsResult<()> { - self.for_each(global, ctx, callback) + self.for_each_property_ordered( + global, + core::ptr::from_mut(ctx).cast(), + for_each_property_context_trampoline::, + ) } /// `JSValue.forEachProperty` — enumerate own props, /// invoking `callback` per (key, value, is_symbol, is_private_symbol). diff --git a/src/jsc/VirtualMachine.rs b/src/jsc/VirtualMachine.rs index 962044c4b021..6a1f044d285c 100644 --- a/src/jsc/VirtualMachine.rs +++ b/src/jsc/VirtualMachine.rs @@ -144,7 +144,7 @@ pub struct VirtualMachine { /// [`crate::hot_reloader::HotReloaderCtx::install_bun_watcher`]); null when /// hot reload is disabled. Read via [`Self::bun_watcher_ptr`]. pub bun_watcher: *mut crate::hot_reloader::ImportWatcher, - pub(crate) console: *mut crate::console_object::ConsoleObject, + pub(crate) console: Option>, // BORROW_PARAM (`&'a mut bun_ast::Log` per LIFETIMES.tsv) — raw NonNull // used because VM is self-referential and cannot carry `<'a>`. pub log: Option>, @@ -324,12 +324,11 @@ pub struct VirtualMachine { pub(crate) gc_controller: crate::GarbageCollectionController, /// The `WebWorker` whose thread this VM runs on (`None` on the main thread). - /// The thread holds a ref on it for its whole life. - pub worker: Option<*const c_void>, + pub worker: Option>, /// Workers created on this thread and not yet released by it. Parent-thread /// only: `WebWorker::create` pushes, `release_parent_poll_ref` removes, /// `join_child_workers` drains at exit. - pub child_workers: Vec<*mut crate::web_worker::WebWorker>, + pub child_workers: Vec>, /// This VM's live pool jobs (`bun_jsc::job`); JS thread only, zero-valid. pub(crate) jobs: crate::JsCell, /// The door out of this thread (`bun_jsc::vm_handle`): tickets for work @@ -1210,14 +1209,23 @@ impl VirtualMachine { self.as_mut().log.map(|mut p| unsafe { p.as_mut() }) } - /// Safe `&WebWorker` accessor for the optional owning worker. The - /// `WebWorker` is a heap allocation owned by C++ that outlives this VM. + /// The worker whose thread this VM runs on, if any. #[inline] pub fn worker_ref(&self) -> Option<&crate::web_worker::WebWorker> { - // SAFETY: `worker` is a `*const c_void` pointing at a heap `WebWorker` - // owned by C++ that outlives this VM (BACKREF — see field decl). - self.worker - .map(|w| unsafe { &*w.cast::() }) + self.worker.as_deref() + } + + /// This VM's `console` state (buffered writers, group depth, counters). + #[inline] + pub fn console_mut(&mut self) -> &mut crate::console_object::ConsoleObject { + self.console.as_deref_mut().expect("VirtualMachine.console") + } + + /// The `void*` the C++ `ConsoleObject` client round-trips back as the + /// first argument of every `Bun__ConsoleObject__*` call. + #[inline] + pub(crate) fn console_cpp_ptr(&mut self) -> *mut c_void { + self.console_mut().as_cpp_ptr() } #[inline] @@ -2305,14 +2313,14 @@ pub struct RuntimeHooks { /// worker's transpiler. `graph` is the same trait object stored in /// `vm.standalone_module_graph` (the high tier downcasts to its concrete /// `bun_standalone_graph::Graph` — the sole implementor). - pub apply_standalone_runtime_flags: unsafe fn( - transpiler: *mut Transpiler<'static>, + pub apply_standalone_runtime_flags: fn( + transpiler: &mut Transpiler<'static>, graph: &'static dyn bun_resolver::StandaloneModuleGraph, ), /// Parse a Worker's `execArgv` for the flags in [`WorkerExecArgvFlags`]. /// `None` if parsing failed. pub parse_worker_exec_argv_flags: - unsafe fn(exec_argv: &[bun_core::WTFStringImpl]) -> Option, + fn(exec_argv: &[bun_core::String]) -> Option, /// `CronJob.clearAllForVM(vm, .teardown)`. `CronJob` lives in /// `bun_runtime::api::cron`. pub stop_cron_for_vm_teardown: fn(vm: &mut VirtualMachine), @@ -2342,7 +2350,7 @@ pub struct RuntimeHooks { /// `TestReporterAgent::next_test_id` through by value; no-op returns it /// unchanged. pub retroactively_report_discovered_tests: - unsafe fn(agent: *mut crate::debugger::TestReporterHandle, next_test_id: i32) -> i32, + fn(agent: &mut crate::debugger::TestReporterHandle, next_test_id: i32) -> i32, /// Cancel every `TimeoutObject` / `ImmediateObject` still in the calling /// thread's `timer::All` heap so their JS pins and in-heap `+1` refs drop /// before the GC sweep. `timer::All` lives in `bun_runtime` (forward-dep); @@ -2597,20 +2605,13 @@ impl VirtualMachine { MAIN_THREAD_VM.store(vm, core::sync::atomic::Ordering::Release); } - // ConsoleObject is self-referential (buffers + adapters) — allocate - // stable storage and init in place. - // `console.init(Output.rawErrorWriter(), Output.rawWriter())` must - // happen BEFORE the pointer is stored/passed; the previous port left - // it as raw `MaybeUninit` (UB on first C++ read). - let mut console_box: Box> = - Box::new(core::mem::MaybeUninit::uninit()); - crate::console_object::ConsoleObject::init_in_place( - &mut console_box, + // ConsoleObject is self-referential (buffers + adapters), hence boxed; + // it must be initialized before its pointer reaches C++. + let mut console_box = crate::console_object::ConsoleObject::new( bun_core::Output::raw_error_writer(), bun_core::Output::raw_writer(), ); - let console = - bun_core::heap::into_raw(console_box).cast::(); + let console: *mut c_void = console_box.as_cpp_ptr(); let context_id = opts .context_id @@ -2624,7 +2625,7 @@ impl VirtualMachine { unsafe { use core::ptr::addr_of_mut; addr_of_mut!((*vm).global).write(core::ptr::null_mut()); - addr_of_mut!((*vm).console).write(console); + addr_of_mut!((*vm).console).write(Some(console_box)); // `log` is a fresh leaked Box; outlives the VM. addr_of_mut!((*vm).log).write(NonNull::new(log)); addr_of_mut!((*vm).main).write(bun_ptr::RawSlice::EMPTY); @@ -2732,7 +2733,7 @@ impl VirtualMachine { // the new global. `worker_ptr` is the C++ `WebCore::Worker*` (or null on // the main thread). let global = Zig__GlobalObject__create( - console.cast(), + console, context_id, opts.mini_mode, opts.eval_mode, @@ -2877,14 +2878,9 @@ impl VirtualMachine { .worker_ref() .and_then(crate::web_worker::WebWorker::exec_argv) .is_some_and(|exec_argv| { - use bun_core::WTFStringImplExt as _; - exec_argv.iter().any(|&arg| { - // SAFETY: each entry borrows the C++ `WorkerOptions` - // array, kept alive by the owning `WebCore::Worker` - // for the worker's lifetime (see `WebWorker::argv`). - !arg.is_null() - && is_bootstrap_flag(unsafe { &*arg }.to_owned_slice_z().as_bytes()) - }) + exec_argv + .iter() + .any(|arg| is_bootstrap_flag(arg.to_utf8().slice())) }); if needs_pre_execution { // The C++ side catches and reports any JS exception thrown while @@ -3302,6 +3298,59 @@ impl<'a> bun_js_printer::OnSourceMapChunk for SourceMapHandlerGetter<'a> { // impl below. // ────────────────────────────────────────────────────────────────────────── +/// The `VirtualMachine` a `WebWorker` thread built with +/// [`VirtualMachine::init_worker`] and owns: the thread's per-thread VM +/// (`VirtualMachine::get()` on that thread) until +/// [`teardown_and_free`](Self::teardown_and_free) destroys it. +#[must_use = "the VM leaks unless `teardown_and_free` consumes this"] +pub(crate) struct WorkerVm(NonNull); + +impl Drop for WorkerVm { + fn drop(&mut self) { + debug_assert!(false, "WorkerVm dropped without teardown_and_free()"); + } +} + +impl core::ops::Deref for WorkerVm { + type Target = VirtualMachine; + #[inline] + fn deref(&self) -> &VirtualMachine { + // SAFETY: the allocation `init` returned; live until `teardown_and_free` consumes `self`. + unsafe { self.0.as_ref() } + } +} + +impl WorkerVm { + /// [`VirtualMachine::teardown`] (stop → forbid script → wait → ~VM → loops + /// → destroy), then free the VM's storage and clear the thread's VM slot. + /// Worker thread; the VM's handle is unpublished and its exit handlers ran. + pub(crate) fn teardown_and_free(self) { + let vm = core::mem::ManuallyDrop::new(self).0.as_ptr(); + // SAFETY: this thread's VM, solely owned by `self` (no other thread + // holds a pointer to it — they only ever held its handle); nothing + // dereferences it after the dealloc. + unsafe { + VirtualMachine::teardown(vm, Teardown::Worker); + // `destroy()` deinits the fields; reclaim the storage `init` put on + // the global heap (`init_worker` always passes `log: None`, so the + // log box is VM-owned here). + drop((*vm).console.take()); + if let Some(log) = (*vm).log.take() { + bun_core::heap::destroy(log.as_ptr()); + } + drop((*vm).worker.take()); + VMHolder::set_vm(None); + // The VM was `alloc_zeroed(Layout::())` in `init`, + // NOT `Box::new` — dealloc the raw storage directly so field `Drop`s + // do not re-run on already-`deinit`'d state. + std::alloc::dealloc( + vm.cast::(), + core::alloc::Layout::new::(), + ); + } + } +} + /// `allocator` dropped per §Allocators (global mimalloc). #[derive(Default)] pub struct Options { @@ -4107,14 +4156,11 @@ impl VirtualMachine { Ok(vm) } - /// Note: takes `&WebWorker` (not `&mut`) — the worker thread may only - /// hold a shared reference to its `WebWorker` (the parent / main thread - /// concurrently observes it; see `web_worker.rs` worker-thread `&self` - /// note). All accesses on `worker` here are read-only. + /// Build the VM a `WebWorker` thread runs (called on that thread). pub(crate) fn init_worker( - worker: &crate::web_worker::WebWorker, + worker: &std::sync::Arc, opts: Options, - ) -> crate::CrateResult<*mut VirtualMachine> { + ) -> crate::CrateResult { let init_opts = InitOptions { transform_options: opts.args, graph: opts.graph, @@ -4126,7 +4172,7 @@ impl VirtualMachine { is_main_thread: false, // The global is created with the worker's messaging proxy, context id // and `mini` so the C++ ZigGlobalObject is born with its options wired. - worker_ptr: worker.messaging_proxy(), + worker_ptr: worker.messaging_proxy().as_mut_ptr().cast(), context_id: Some(worker.execution_context_id() as i32), mini_mode: worker.mini(), ..Default::default() @@ -4134,10 +4180,9 @@ impl VirtualMachine { // Route through // [`init`] (which already wires console / event-loop / global / jsc_vm // / RuntimeHooks) and then patch the worker-specific fields. - let vm = Self::init(init_opts)?; - // SAFETY: `vm` is the unique live VM on this thread. - let vm_ref = unsafe { &mut *vm }; - vm_ref.worker = Some(std::ptr::from_ref::(worker).cast()); + let vm = WorkerVm(NonNull::new(Self::init(init_opts)?).expect("VirtualMachine::init")); + let vm_ref = vm.as_mut(); + vm_ref.worker = Some(std::sync::Arc::clone(worker)); if worker.arm_test_gate() { vm_ref.handle.arm_test_gate(); } @@ -4180,7 +4225,7 @@ impl VirtualMachine { // SAFETY: `vm` is the unique live VM on this thread. let vm_ref = unsafe { &mut *vm }; // `console` is the opaque round-trip pointer C++ stores into the new global. - let new_global = BakeCreateProdGlobal(vm_ref.console.cast()); + let new_global = BakeCreateProdGlobal(vm_ref.console_cpp_ptr()); vm_ref.global = new_global; VMHolder::set_cached_global_object(Some(new_global)); vm_ref.regular_event_loop.global = NonNull::new(new_global); @@ -5227,7 +5272,7 @@ impl VirtualMachine { // `console` is the live per-VM ConsoleObject. let new_global: *mut JSGlobalObject = JSGlobalObject::create_for_test_isolation( JSGlobalObject::opaque_ref(old_global), - self.console.cast(), + self.console_cpp_ptr(), ); self.global = new_global; VMHolder::set_cached_global_object(Some(new_global)); @@ -6593,11 +6638,9 @@ impl VirtualMachine { for &err in &errors_to_append { // Circular-ref guard for cause chains. if formatter.map_node.is_none() { - let mut node = NonNull::new(console_object::formatter::visited::Pool::get_node()) - .expect("ObjectPool::get_node always returns a valid heap node"); - let data = console_object::formatter::visited::node_data_mut(&mut node); - data.clear(); - formatter.map = core::mem::take(data); + let mut node = console_object::formatter::visited::Pool::get(); + node.clear(); + formatter.map = core::mem::take(&mut *node); formatter.map_node = Some(node); } diff --git a/src/jsc/bindings/webcore/WorkerMessagingProxy.cpp b/src/jsc/bindings/webcore/WorkerMessagingProxy.cpp index cda4ce937db5..3fc1688ac9a4 100644 --- a/src/jsc/bindings/webcore/WorkerMessagingProxy.cpp +++ b/src/jsc/bindings/webcore/WorkerMessagingProxy.cpp @@ -49,7 +49,8 @@ WTF_MAKE_TZONE_ALLOCATED_IMPL(WorkerMessagingProxy); extern "C" { // Allocates the thread object holding one ref for the caller, takes the keep-alive on the parent -// event loop, and spawns the thread. Null (with errorMessage set) if nothing was started. +// event loop, and spawns the thread. Null (with errorMessage set) if nothing was started. The +// string arrays are copied before it returns. void* WebWorker__create( WorkerMessagingProxy*, void* parentVM, @@ -62,12 +63,12 @@ void* WebWorker__create( bool unrefByDefault, bool evalMode, bool isNodeWorker, - StringImpl** argvPtr, + const BunString* argvPtr, size_t argvLen, bool defaultExecArgv, - StringImpl** execArgvPtr, + const BunString* execArgvPtr, size_t execArgvLen, - BunString* preloadModulesPtr, + const BunString* preloadModulesPtr, size_t preloadModulesLen); // Raise a TerminationException in the worker VM at its next safepoint and wake its loop. Any thread. void WebWorker__requestTermination(void*); @@ -133,12 +134,10 @@ ExceptionOr WorkerMessagingProxy::startWorkerGlobalScope(const String& scr preloadModules.append(Bun::toString(str)); } - static_assert(sizeof(WTF::String) == sizeof(WTF::StringImpl*)); - std::span execArgv = m_options.execArgv - .transform([](Vector& vec) -> std::span { - return { reinterpret_cast(vec.begin()), vec.size() }; - }) - .value_or(std::span {}); + Vector argv = m_options.argv.map([](const String& str) { return Bun::toString(str); }); + Vector execArgv; + if (m_options.execArgv) + execArgv = m_options.execArgv->map([](const String& str) { return Bun::toString(str); }); // The thread holds a ref on the proxy until releaseWorkerThread(). ref(); @@ -157,10 +156,10 @@ ExceptionOr WorkerMessagingProxy::startWorkerGlobalScope(const String& scr m_options.unref, m_options.evalMode, m_options.kind == WorkerOptions::Kind::Node, - reinterpret_cast(m_options.argv.begin()), - m_options.argv.size(), + argv.begin(), + argv.size(), !m_options.execArgv.has_value(), - execArgv.data(), + execArgv.begin(), execArgv.size(), preloadModules.begin(), preloadModules.size()); diff --git a/src/jsc/hot_reloader.rs b/src/jsc/hot_reloader.rs index 3660c0526a98..f43baef5cdc3 100644 --- a/src/jsc/hot_reloader.rs +++ b/src/jsc/hot_reloader.rs @@ -1,5 +1,6 @@ use core::marker::PhantomData; use core::sync::atomic::{AtomicU32, Ordering}; +use std::sync::Arc; #[cfg(not(windows))] use bun_collections::StringHashMap; @@ -9,15 +10,17 @@ use bun_core::ZStr; #[cfg(not(windows))] use bun_paths::SEP; use bun_paths::strings; +use bun_ptr::ThreadBound; #[cfg(not(windows))] use bun_resolver::fs::PathName; use bun_resolver::fs::{self as Fs, FileSystem}; use bun_sys::{self, Fd}; +#[cfg(not(windows))] +use bun_watcher::ChangedFilePath; use bun_watcher::WatchItemColumns as _; -use bun_watcher::{ChangedFilePath, Op as WatchOp, Watcher}; +use bun_watcher::{Op as WatchOp, Watcher}; -use crate::Task as JscTask; -use crate::event_loop::{ConcurrentTaskItem as ConcurrentTask, EventLoop}; +use crate::event_loop::EventLoop; use crate::virtual_machine::VirtualMachine; use bun_event_loop::task_tag; @@ -92,32 +95,24 @@ impl HotReloaderCtx for VirtualMachine { Some(self.handle()) } - fn bun_watcher_mut(&mut self) -> &mut Watcher { - // `VirtualMachine.bun_watcher` is the - // `*mut ImportWatcher` (see the field comment in - // VirtualMachine.rs), and `get_context` only runs after - // `enable_hot_module_reloading` has populated it, so the `.None` arm - // is unreachable. - // SAFETY: `bun_watcher` is the `*mut ImportWatcher` set by - // `enable_hot_module_reloading`; non-null whenever the reloader is - // running. The cast recovers the concrete type the field was erased to. - match unsafe { &mut *self.bun_watcher.cast::() } { - ImportWatcher::Hot(w) | ImportWatcher::Watch(w) => &mut **w, - ImportWatcher::None => unreachable!("bun_watcher_mut on un-enabled reloader"), - } + fn post_reload_task( + handle: &crate::VmHandle, + task: Box>, + ) -> Result<(), Box>> { + handle.post_boxed(crate::LoopKind::Regular, task) } - fn reload(&mut self, _task: &mut dyn HotReloadTaskView) { + fn reload(&self, _task: &dyn HotReloadTaskView) { // The inherent `reload` ignores its task argument, so pass `None` // rather than threading the dyn view through. - VirtualMachine::reload(self, None); + VirtualMachine::reload(self.as_mut(), None); } - fn bust_dir_cache(&mut self, path: &[u8]) -> bool { - VirtualMachine::bust_dir_cache(self, path) + fn bust_dir_cache(&self, path: &[u8]) -> bool { + VirtualMachine::bust_dir_cache(self.as_mut(), path) } - fn get_loaders(&self) -> &bun_ast::LoaderHashTable { + fn loaders(&self) -> &bun_ast::LoaderHashTable { &self.transpiler.options.loaders } @@ -130,17 +125,9 @@ impl HotReloaderCtx for VirtualMachine { } fn is_watcher_enabled(&self) -> bool { - // The field is a typed `*mut ImportWatcher`; a null pointer means no - // watcher is installed. - if self.bun_watcher.is_null() { - return false; - } - // SAFETY: `bun_watcher` is the `*mut ImportWatcher` set by - // `install_bun_watcher`; the cast recovers the concrete type. - !matches!( - unsafe { &*self.bun_watcher.cast::() }, - ImportWatcher::None - ) + // `install_bun_watcher` only ever installs `Hot`/`Watch`, so a + // non-null pointer is an enabled watcher. + !self.bun_watcher_ptr().is_null() } fn watcher_top_level_dir(&self) -> &'static [u8] { @@ -151,27 +138,22 @@ impl HotReloaderCtx for VirtualMachine { &mut self, watcher: Box, reload_immediately: bool, - ) -> *mut Watcher { - let mut iw = Box::new(if reload_immediately { - ImportWatcher::Watch(watcher) - } else { - ImportWatcher::Hot(watcher) - }); - let watcher_ptr: *mut Watcher = match &mut *iw { - ImportWatcher::Hot(w) | ImportWatcher::Watch(w) => &raw mut **w, + ) -> &mut Watcher { + // Leaked: the watcher (and this enum around it) live for the process. + let import_watcher: &'static mut ImportWatcher = + Box::leak(Box::new(if reload_immediately { + ImportWatcher::Watch(watcher) + } else { + ImportWatcher::Hot(watcher) + })); + self.bun_watcher = core::ptr::from_mut(import_watcher); + let watcher = match import_watcher { + ImportWatcher::Hot(w) | ImportWatcher::Watch(w) => &mut **w, ImportWatcher::None => unreachable!(), }; - self.bun_watcher = bun_core::heap::into_raw(iw); - // Wire the resolver's directory-watch callback at the same time. - // `Watcher::get_resolve_watcher` erases the `*mut Watcher` into the - // resolver's `AnyResolveWatcher` vtable (re-exported from - // `bun_watcher`, so it's the same type). - // SAFETY: `watcher_ptr` was just installed into `self.bun_watcher` - // via `heap::alloc` and is live for the VM's lifetime. - self.transpiler.resolver.watcher = Some(unsafe { (*watcher_ptr).get_resolve_watcher() }); - - watcher_ptr + self.transpiler.resolver.watcher = Some(watcher.get_resolve_watcher()); + watcher } fn compute_clear_screen(&self) -> bool { @@ -190,8 +172,13 @@ pub type HotReloadTask = Task; pub type WatchReloadTask = Task; /// Trait bound on `Ctx` exposing the operations the reloader needs. -/// Implemented by `VirtualMachine` and `bun.bake.DevServer`. -pub trait HotReloaderCtx { +/// Implemented by `VirtualMachine` and `bun_bundler::BundleV2`. +/// +/// The watcher thread never touches the `Ctx`: everything it needs is copied +/// in [`NewHotReloader::enable_hot_module_reloading`] (on the owning thread), +/// and `reload` / `bust_dir_cache` run on the owning thread from the posted +/// [`Task`]. +pub trait HotReloaderCtx: Sized + 'static { type EventLoop; /// The handle the watcher thread posts reload tasks through (captured @@ -199,19 +186,26 @@ pub trait HotReloaderCtx { /// reload the whole process before ever enqueueing (`bun build --watch`). fn reload_handle(&self) -> Option; - /// Implementor returns the live `Watcher` regardless of how it's stored. - fn bun_watcher_mut(&mut self) -> &mut Watcher; + /// Watcher thread: queue `task` on the owning thread through `handle` + /// (from [`reload_handle`](Self::reload_handle)); it comes back if the + /// owner is already closed. Only contexts with a handle are ever asked. + fn post_reload_task( + handle: &crate::VmHandle, + task: Box>, + ) -> Result<(), Box>>; - /// Called from `Task::run` to perform the actual reload. The const-generic - /// task is erased via the `HotReloadTaskView` so this trait isn't - /// recursively generic. - fn reload(&mut self, task: &mut dyn HotReloadTaskView); + /// Called from `Task::run` (owning thread) to perform the actual reload. + /// The const-generic task is erased via the `HotReloadTaskView` so this + /// trait isn't recursively generic. + fn reload(&self, task: &dyn HotReloadTaskView); - /// Returns whether anything was busted. - fn bust_dir_cache(&mut self, path: &[u8]) -> bool; + /// Called from `Task::run` (owning thread) for each directory the watcher + /// saw change. Returns whether anything was busted. + fn bust_dir_cache(&self, path: &[u8]) -> bool; - /// `&transpiler.options.loaders`. - fn get_loaders(&self) -> &bun_ast::LoaderHashTable; + /// `&transpiler.options.loaders`; snapshotted at init for the watcher + /// thread's extension → loader lookups. + fn loaders(&self) -> &bun_ast::LoaderHashTable; fn log_level_at_least_info(&self) -> bool { false @@ -226,12 +220,12 @@ pub trait HotReloaderCtx { fn watcher_top_level_dir(&self) -> &'static [u8]; /// Installs the watcher and wires the resolver's watch callback. - /// Returns the now-installed `*mut Watcher` so the caller can `start()` it. + /// Returns the now-installed watcher so the caller can `start()` it. fn install_bun_watcher( &mut self, watcher: Box, reload_immediately: bool, - ) -> *mut Watcher; + ) -> &mut Watcher; fn compute_clear_screen(&self) -> bool; } @@ -241,10 +235,9 @@ pub trait HotReloaderCtx { pub trait HotReloadTaskView { fn count(&self) -> u8; fn hashes(&self) -> &[u32]; - fn paths(&self) -> &[&'static [u8]]; } -impl HotReloadTaskView +impl HotReloadTaskView for Task { fn count(&self) -> u8 { @@ -253,12 +246,9 @@ impl HotReloadTaskView fn hashes(&self) -> &[u32] { &self.hashes[..self.count as usize] } - fn paths(&self) -> &[&'static [u8]] { - &self.paths[..self.count as usize] - } } -/// When non-null, `on_file_update` records the absolute path of every file +/// When set, `on_file_update` records the absolute path of every file /// it sees change before triggering a reload. Used by `bun test --changed /// --watch` so the restarted process can narrow its changed-file set to /// what the watcher actually observed (instead of re-querying git, which @@ -266,48 +256,21 @@ impl HotReloadTaskView /// the one that was just edited). /// /// Set by `ChangedFilesFilter` on the main thread before the watcher thread -/// starts; after that point only the watcher thread touches it. Its -/// contents are written to `watch_changed_trigger_file` immediately -/// before `reload_process`; the new process reads and deletes that file. -// Written once on main thread before watcher thread starts, then -// watcher-thread-only. `OnceLock` carries the publish. +/// starts; after that point only the watcher thread uses it (the set itself +/// is behind a mutex). Its contents are written to +/// `WATCH_CHANGED_TRIGGER_FILE` immediately before `reload_process`; the new +/// process reads and deletes that file. pub static WATCH_CHANGED_PATHS: std::sync::OnceLock = std::sync::OnceLock::new(); -/// `Send + Sync` newtype around the arena-allocated `StringSet` pointer so it -/// can sit inside a `OnceLock`. The set is written once on the main thread -/// before the watcher thread starts, then mutated only from the watcher -/// thread — never concurrently — so cross-thread publication of the raw -/// pointer is sound. -pub struct WatchChangedPaths(core::ptr::NonNull); +/// The set behind [`WATCH_CHANGED_PATHS`]. Written by the watcher thread, +/// read back on the same thread right before the process is replaced. +pub struct WatchChangedPaths(bun_threading::Guarded); impl WatchChangedPaths { #[inline] - pub fn new(set: &'static mut StringSet) -> Self { - Self(core::ptr::NonNull::from(set)) - } - - /// Reborrow the wrapped `StringSet`. Single audited `unsafe` for the - /// set-once `NonNull` deref so the two callers below - /// (`record_changed_path`, `flush_changed_paths_for_reload`) are safe. - /// - /// Soundness: published exactly once via `OnceLock` before the watcher - /// thread starts; thereafter only the watcher thread reaches the callers, - /// so the `&mut` is exclusive. Lives in the process-lifetime CLI arena. - #[inline] - #[allow(clippy::mut_from_ref)] - fn get_mut(&self) -> &mut StringSet { - // SAFETY: see doc comment — single-writer (watcher thread) after - // init-once publish; allocation outlives the process. - unsafe { &mut *self.0.as_ptr() } + pub fn new(set: StringSet) -> Self { + Self(bun_threading::Guarded::new(set)) } } -// SAFETY: published exactly once before the watcher thread starts; thereafter -// only the watcher thread dereferences it (see module docs above). The -// allocation lives in the process-lifetime CLI arena. -unsafe impl Send for WatchChangedPaths {} -// SAFETY: `&WatchChangedPaths` is shared via `OnceLock`, but the only mutating -// access (`get_mut`) is confined to the watcher thread after the init-once -// publish, so no two threads ever hold `&mut StringSet` concurrently. -unsafe impl Sync for WatchChangedPaths {} /// Absolute path of the temp file `flush_changed_paths_for_reload` writes /// the changed-path list into. The same path is exported via the @@ -328,7 +291,7 @@ fn record_changed_path(path: &[u8]) { if path.is_empty() { return; } - bun_core::handle_oom(set.get_mut().insert(path)); + bun_core::handle_oom(set.0.lock().insert(path)); } /// Write the recorded changed paths to the trigger file so the next @@ -350,7 +313,7 @@ fn flush_changed_paths_for_reload() { let Some(&dest) = WATCH_CHANGED_TRIGGER_FILE.get() else { return; }; - let set = set.get_mut(); + let set = set.0.lock(); if set.count() == 0 { return; } @@ -377,23 +340,35 @@ unsafe extern "C" { // before the watcher thread starts. static CLEAR_SCREEN: core::sync::atomic::AtomicBool = core::sync::atomic::AtomicBool::new(false); -pub struct NewHotReloader { - /// BACKREF to the owning context (Bundler / VM transpiler store) that - /// created this reloader. Set once at init and never reassigned; the - /// context outlives the reloader (and every `Task` it spawns). - pub ctx: bun_ptr::BackRef, +/// The watcher thread's half of hot reloading: owned by the [`Watcher`] as its +/// [`bun_watcher::WatcherHandler`], it turns file-change batches into +/// [`Task`]s posted to the owning thread. +pub struct NewHotReloader { + /// The owning context (VM / bundler); carried into each [`Task`] and only + /// followed there, on the owning thread. The context outlives the + /// reloader (both live for the process). + ctx: ThreadBound, pub(crate) verbose: bool, - pub(crate) pending_count: AtomicU32, + /// Reloads and directory busts posted and not yet run; shared with every + /// [`Task`]. + pending: Arc, pub(crate) main: MainFile, + /// `Ctx::loaders()` as of init. #[cfg(not(windows))] - pub(crate) tombstones: StringHashMap<*mut Fs::EntriesOption>, + loaders: bun_ast::LoaderHashTable, + + /// Directory-entry slots seen for a directory, kept so later kqueue events + /// for it can still map changed names after the resolver evicted the slot + /// from its index. The slots are process-lifetime BSS storage. + #[cfg(not(windows))] + pub(crate) tombstones: StringHashMap>, /// See [`HotReloaderCtx::reload_handle`]. pub(crate) reload_handle: Option, - _event_loop: PhantomData<*mut EventLoopType>, + _event_loop: PhantomData EventLoopType>, } pub struct MainFile { @@ -456,16 +431,38 @@ impl MainFile { } } -pub struct Task { +/// What the watcher thread has handed the owning thread and it has not yet +/// consumed. Whichever [`Task`] runs first takes all of it, so a later task +/// finding nothing left is a no-op (reloads and busts coalesce alike). +struct Pending { + /// Reloads posted and not yet run. + count: AtomicU32, + /// Directories whose resolver caches to bust before the next reload. + dirs: bun_threading::Guarded>>, +} + +/// One batch of changes for the owning thread: the hashes of changed watched +/// files (a reload if any). Built on the watcher thread, posted boxed, run and +/// dropped on the owning thread, where it first drains [`Pending::dirs`]. +pub struct Task { pub(crate) count: u8, pub(crate) hashes: [u32; 8], - // Only meaningful for the DevServer Ctx, but Rust can't branch a field - // type on a generic parameter without specialization, so it is stored - // unconditionally (8 fat pointers of overhead for non-DevServer Ctx). - pub(crate) paths: [&'static [u8]; 8], - /// Left `None` until [`Self::enqueue`] populates it on the heap copy. - pub(crate) concurrent_task: Option, - pub(crate) reloader: *mut NewHotReloader, + /// Directories that changed in this batch; moved to [`Pending::dirs`] on enqueue. + dirs: Vec>, + ctx: ThreadBound, + pending: Arc, + _event_loop: PhantomData EventLoopType>, +} + +// Only the VM's tasks are ever queued (`bun build --watch` has no loop); the +// `run_task` arms for these two tags `heap::take` exactly these types. +bun_event_loop::boxed_task! { + impl[const RELOAD_IMMEDIATELY: bool] for Task => + if RELOAD_IMMEDIATELY { task_tag::WatchReloadTask } else { task_tag::HotReloadTask }; + fn release_unrun(self) { + // A file change the watcher thread posted that will not reload anything. + drop(self) + } } impl @@ -473,99 +470,29 @@ impl where Ctx: HotReloaderCtx, { - pub(crate) fn init_empty( - reloader: *mut NewHotReloader, - ) -> Self { + fn new(reloader: &NewHotReloader) -> Self { Self { - reloader, - hashes: [0u32; 8], - // See the `paths` field comment for why this is unconditional. - paths: [b"".as_slice(); 8], count: 0, - concurrent_task: None, + hashes: [0u32; 8], + dirs: Vec::new(), + ctx: reloader.ctx.clone(), + pending: Arc::clone(&reloader.pending), + _event_loop: PhantomData, } } - /// Per-field raw read of the reloader's `pending_count`. - /// - /// `reloader` is a BACKREF: the `NewHotReloader` heap-allocates every - /// `Task` (via [`Self::enqueue`]) and is itself leaked for the process - /// lifetime in `enable_hot_module_reloading`, so it strictly outlives - /// every `Task` it spawns. The pointer is never null (set in - /// [`Self::init_empty`] / copied in [`Self::enqueue`]). - /// - /// We deliberately do **not** expose a whole-struct `&NewHotReloader` - /// accessor: [`Self::run`] executes on the JS event-loop thread while the - /// watcher thread may be inside `on_file_update(&mut self)` writing - /// non-`UnsafeCell` fields (`main.is_waiting_for_dir_change`, `tombstones`). - /// Materializing `&NewHotReloader` on the JS thread would assert those - /// bytes are frozen, which is a data race / Stacked-Borrows violation even - /// though this side never reads them. Instead, project to the single - /// `AtomicU32` field via `addr_of!` so no `&NewHotReloader` is formed. - #[inline] - fn pending_count(&self) -> &AtomicU32 { - // SAFETY: BACKREF — see doc comment above. `addr_of!` forms a place - // projection without an intermediate `&NewHotReloader`; `pending_count` - // is `AtomicU32` (interior-mutable) so a cross-thread `&` to it is sound. - unsafe { &*core::ptr::addr_of!((*self.reloader).pending_count) } - } - - /// Per-field raw read of the reloader's `ctx` pointer. See - /// [`Self::pending_count`] for why no whole-struct `&NewHotReloader` - /// accessor is exposed. - #[inline] - fn ctx_ptr(&self) -> *mut Ctx { - // SAFETY: BACKREF — reloader outlives every Task; `addr_of!` avoids - // forming `&NewHotReloader`. `ctx` is set once at init and never - // mutated, so a racy raw read of the pointer value is fine. - unsafe { (*core::ptr::addr_of!((*self.reloader).ctx)).as_ptr() } - } - - pub(crate) fn append(&mut self, id: u32) { - if self.count == 8 { - self.enqueue(); - self.count = 0; + /// Owning thread. Returns whether a reload ran (the dispatcher then leaves + /// the tick early without draining microtasks). + #[allow(clippy::boxed_local, reason = "reclaim point for the boxed task")] + pub fn run(self: Box) -> bool { + let ctx = self.ctx.get(); + let dirs = core::mem::take(&mut *self.pending.dirs.lock()); + for dir in &dirs { + let _ = ctx.bust_dir_cache(dir); + } + if self.count == 0 { + return false; } - - self.hashes[self.count as usize] = id; - self.count += 1; - } - - /// The dispatched task was heap-allocated in [`Self::enqueue`] via - /// `heap::alloc`; the event loop calls this after `run()` to free it. - /// - /// # Safety - /// `this` must have been created via `heap::alloc` in [`Self::enqueue`] - /// and must not be used after this call. - pub unsafe fn deinit(this: *mut Self) { - // SAFETY: precondition — `this` came from heap::alloc in `enqueue`. - drop(unsafe { bun_core::heap::take(this) }); - } -} - -impl bun_event_loop::Taskable - for Task -where - Ctx: HotReloaderCtx, -{ - const TAG: bun_event_loop::TaskTag = if RELOAD_IMMEDIATELY { - task_tag::WatchReloadTask - } else { - task_tag::HotReloadTask - }; - /// A file change the watcher thread posted that will not reload anything. - unsafe fn release_unrun(this: *mut Self) { - // SAFETY: fn contract — the box `enqueue` posted. - unsafe { Self::deinit(this) } - } -} - -impl - Task -where - Ctx: HotReloaderCtx, -{ - pub fn run(&mut self) { // Since we rely on the event loop for hot reloads, there can be // a delay before the next reload begins. In the time between the // last reload and the next one, we shouldn't schedule any more @@ -575,71 +502,83 @@ where // Note that we set the count _before_ we reload, so that if we // get another hot reload request while we're reloading, we'll // still enqueue it. - while self.pending_count().swap(0, Ordering::Relaxed) > 0 { - let ctx = self.ctx_ptr(); - // SAFETY: ctx outlives reloader (BACKREF). - unsafe { (*ctx).reload(self) }; + while self.pending.count.swap(0, Ordering::Relaxed) > 0 { + ctx.reload(&*self); } + true + } + + pub(crate) fn append( + &mut self, + reloader: &NewHotReloader, + id: u32, + ) { + if self.count == 8 { + self.enqueue(reloader); + self.count = 0; + } + + self.hashes[self.count as usize] = id; + self.count += 1; + } + + fn bust_dir(&mut self, path: &[u8]) { + self.dirs.push(path.into()); } - pub(crate) fn enqueue(&mut self) { + /// Watcher thread: post what this batch collected (if anything) to the + /// owning thread, leaving `self` empty. + pub(crate) fn enqueue( + &mut self, + reloader: &NewHotReloader, + ) { crate::mark_binding!(); - if self.count == 0 { + if self.count == 0 && self.dirs.is_empty() { return; } - // With --watch-kill-signal listeners registered, reload via the event - // loop so the JS thread emits them before execve (node runs the child's - // handlers on kill); otherwise execve immediately (node's default kill). - if RELOAD_IMMEDIATELY && !crate::posix_signal_handle::watch_kill_signal_has_listeners() { - crate::node_compile_cache::persist_now(); - Output::flush(); - flush_changed_paths_for_reload(); - bun_core::reload_process( - CLEAR_SCREEN.load(core::sync::atomic::Ordering::Relaxed), - false, - ); - unreachable!(); - } + if self.count > 0 { + // With --watch-kill-signal listeners registered, reload via the event + // loop so the JS thread emits them before execve (node runs the child's + // handlers on kill); otherwise execve immediately (node's default kill). + if RELOAD_IMMEDIATELY && !crate::posix_signal_handle::watch_kill_signal_has_listeners() + { + crate::node_compile_cache::persist_now(); + Output::flush(); + flush_changed_paths_for_reload(); + bun_core::reload_process( + CLEAR_SCREEN.load(core::sync::atomic::Ordering::Relaxed), + false, + ); + unreachable!(); + } - self.pending_count().fetch_add(1, Ordering::Relaxed); - - BunDebugger__willHotReload(); - let that = bun_core::heap::into_raw(Box::new(Self { - reloader: self.reloader, - count: self.count, - paths: self.paths, - hashes: self.hashes, - concurrent_task: None, - })); - // SAFETY: `that` was just allocated above and is exclusively owned here. - unsafe { - let concurrent = (*that).concurrent_task.insert(ConcurrentTask { - task: JscTask::init(that), - ..Default::default() + self.pending.count.fetch_add(1, Ordering::Relaxed); + + BunDebugger__willHotReload(); + } + let reloads = self.count > 0; + // `bun build --watch` has no loop to post to; it only ever reaches + // here for directory busts, which its idle process has no use for. + if let Some(handle) = &reloader.reload_handle { + self.pending.dirs.lock().append(&mut self.dirs); + let task = Box::new(Self { + count: self.count, + hashes: self.hashes, + dirs: Vec::new(), + ctx: self.ctx.clone(), + pending: Arc::clone(&self.pending), + _event_loop: PhantomData, }); - // `&that.concurrent_task` is an interior pointer into the - // Box-allocated Task. `RELOAD_IMMEDIATELY` already diverged above, so - // a handle is always present here. - // Field-only access to avoid forming a whole-struct `&NewHotReloader` - // (see `Self::pending_count` doc). - let handle = (*core::ptr::addr_of!((*self.reloader).reload_handle)) - .as_ref() - .expect("reload_handle set for a reloader that enqueues"); - if let crate::vm_handle::Posted::Refused(_) = handle.post( - crate::LoopKind::Regular, - core::ptr::NonNull::from(concurrent), - ) { - // VM torn down while a change was pending: drop the reload task. - Self::deinit(that); - } + // VM torn down while a change was pending: the box comes back and drops. + let _ = Ctx::post_reload_task(handle, task); } - self.count = 0; + self.dirs.clear(); // The JS thread emits kill-signal listeners then execve; if it's stuck in sync code it // never drains the posted task. Arm a one-shot timer that forces the reload after a // bounded window (node's watcher SIGKILLs an unresponsive child after its grace period). - if RELOAD_IMMEDIATELY { + if RELOAD_IMMEDIATELY && reloads { arm_watch_reload_grace_timer(); } } @@ -708,7 +647,7 @@ fn arm_watch_reload_grace_timer() { } } -impl +impl NewHotReloader where Ctx: HotReloaderCtx, @@ -717,33 +656,30 @@ where bun_core::pretty_errorln!("watcher: {}", args); } - /// # Safety - /// `this` must point to a live `Ctx` (VirtualMachine / DevServer / BundleV2) - /// that outlives the leaked `NewHotReloader` allocated here — i.e. for the - /// process lifetime. - pub unsafe fn enable_hot_module_reloading(this: *mut Ctx, entry_path: Option<&'static [u8]>) { - // SAFETY: precondition — `this` is the live owning `Ctx`; it outlives - // the reloader allocated below. Borrows are scoped per access. - if unsafe { (*this).is_watcher_enabled() } { + /// Owning thread. `this` (VirtualMachine / BundleV2) lives for the + /// process, as do the watcher and reloader started here. + pub fn enable_hot_module_reloading(this: &mut Ctx, entry_path: Option<&'static [u8]>) { + if this.is_watcher_enabled() { return; } - let reloader = bun_core::heap::into_raw(Box::new(Self { - // SAFETY: `this` is the live owning context; it outlives the reloader. - ctx: unsafe { bun_ptr::BackRef::from_raw_mut(this) }, - // SAFETY: see above. - verbose: unsafe { (*this).log_level_at_least_info() }, - pending_count: AtomicU32::new(0), + let reloader = Box::new(Self { + ctx: ThreadBound::new(this), + verbose: this.log_level_at_least_info(), + pending: Arc::new(Pending { + count: AtomicU32::new(0), + dirs: bun_threading::Guarded::new(Vec::new()), + }), main: MainFile::init(entry_path.unwrap_or(b"")), #[cfg(not(windows))] + loaders: bun_core::handle_oom(this.loaders().clone()), + #[cfg(not(windows))] tombstones: StringHashMap::default(), - // SAFETY: see above. - reload_handle: unsafe { (*this).reload_handle() }, + reload_handle: this.reload_handle(), _event_loop: PhantomData, - })); + }); - // SAFETY: see above; `watcher_top_level_dir` returns `&'static [u8]`. - let watcher = match Watcher::init(reloader, unsafe { (*this).watcher_top_level_dir() }) { + let watcher = match Watcher::init_with_handler(reloader, this.watcher_top_level_dir()) { Ok(w) => w, Err(err) => { bun_core::handle_error_return_trace(&err); @@ -754,18 +690,13 @@ where } }; - // SAFETY: see above. - let watcher_ptr = unsafe { (*this).install_bun_watcher(watcher, RELOAD_IMMEDIATELY) }; + let clear_screen = this.compute_clear_screen(); + let watcher = this.install_bun_watcher(watcher, RELOAD_IMMEDIATELY); - // SAFETY: single-threaded init; watcher thread not yet started. - CLEAR_SCREEN.store( - // SAFETY: see above. - unsafe { (*this).compute_clear_screen() }, - core::sync::atomic::Ordering::Relaxed, - ); + // Single-threaded init; watcher thread not yet started. + CLEAR_SCREEN.store(clear_screen, core::sync::atomic::Ordering::Relaxed); - // SAFETY: `watcher_ptr` was just installed into the ctx and is live. - if let Err(err) = unsafe { (*watcher_ptr).start() } { + if let Err(err) = watcher.start() { bun_core::handle_error_return_trace(&err); bun_core::pretty_errorln!( "error: Failed to start File Watcher: {}", @@ -777,12 +708,12 @@ where } #[cfg(not(windows))] - fn put_tombstone(&mut self, key: &[u8], value: *mut Fs::EntriesOption) { + fn put_tombstone(&mut self, key: &[u8], value: bun_ptr::BackRef) { self.tombstones.put(key, value).expect("unreachable"); } #[cfg(not(windows))] - fn get_tombstone(&mut self, key: &[u8]) -> Option<*mut Fs::EntriesOption> { + fn get_tombstone(&mut self, key: &[u8]) -> Option> { self.tombstones.get(key).copied() } @@ -794,85 +725,35 @@ where } } - /// Single audited `&mut Ctx` reborrow through the [`BackRef`]. The owning - /// context outlives this reloader (set once at init, never reassigned — - /// see field doc), and `&mut self` ensures no other reborrow through this - /// reloader is live. Centralizes the per-call-site `BackRef::get_mut` - /// deref previously open-coded at each `bust_dir_cache` site. - #[inline] - fn ctx_mut(&mut self) -> &mut Ctx { - // SAFETY: BACKREF invariant — ctx outlives the reloader; `&mut self` - // gives exclusivity for the returned borrow's duration. - unsafe { self.ctx.get_mut() } - } - - pub(crate) fn get_context(&mut self) -> &mut Watcher { - self.ctx_mut().bun_watcher_mut() - } - #[inline(never)] - pub(crate) fn on_file_update( - &mut self, - events: &mut [bun_watcher::WatchEvent], - changed_files: &[ChangedFilePath], - watchlist: &bun_watcher::WatchList, - ) { - let slice = watchlist.slice(); + pub(crate) fn on_file_update(&mut self, batch: &mut bun_watcher::FileUpdateBatch<'_>) { + // `Slice` is a by-value set of column pointers into the watchlist; + // evictions below are only buffered (`remove_at_index::`) and + // applied by `flush_evictions` after the loop, so the columns stay put. + let slice = batch.watcher().watchlist.slice(); + let mut counts = slice; + let counts = counts.items_mut::<"count", u32>(); let file_paths = slice.items_file_path(); - // Note: `WatchItemColumns` doesn't expose a `count` accessor; reach - // through the generic SoA column directly. The loop below mutates the - // column in place — build the &mut from the raw column pointer rather - // than ref-casting `&[u32]` (which is UB). - // SAFETY: column `Count` is `u32`; `items_raw` yields a pointer valid - // for `slice.len()` elements; the watcher thread is the sole writer of - // this column for the loop's duration and no other `&` to it is live. - let counts: &mut [u32] = - unsafe { bun_core::ffi::slice_mut(slice.items_raw::<"count", u32>(), slice.len()) }; let kinds = slice.items_kind(); let hashes = slice.items_hash(); let parents = slice.items_parent_hash(); let file_descriptors = slice.items_fd(); - // Note: reshaped for borrowck — `ctx` is held as a raw pointer so - // `self` can be reborrowed inside the loop body for tombstone access, - // and so the deferred `flush_evictions` doesn't hold `&mut Watcher` - // across the loop. - let ctx: *mut Watcher = std::ptr::from_mut(self.get_context()); - // Wrap the Task itself in a guard so any exit path (including future - // early-returns) flushes the buffered hashes via `enqueue()`. - // Dereferenced as `&mut *current_task` for the loop body below. - // - // Note: declared *before* `_flush` so `flush_evictions()` runs - // **before** `enqueue()` on drop. The reverse order opens a window - // where the JS thread can pick up the concurrent task, look the file up - // in the watchlist, and read the cached fd while this thread is still - // about to `close()` + `swap_remove()` it in `flush_evictions` — - // surfacing as `EBADF`/`EISDIR reading ""` in hot.test.ts under - // load. Evicting first is side-effect-free: `enqueue` carries hashes - // (not indices) and never reads the watchlist. - let mut current_task = scopeguard::guard( - Task::::init_empty(self), - |mut t| t.enqueue(), - ); - // See the note above for why this drops *before* `current_task`. - let _flush = scopeguard::guard(ctx, |ctx| { - Output::flush(); - // SAFETY: the Watcher outlives this call (it owns the Reloader that calls us). - unsafe { (*ctx).flush_evictions() }; - }); + let event_count = batch.events().len(); + let mut current_task = Task::::new(self); let fs: &mut FileSystem = FileSystem::instance(); let rfs: &mut Fs::file_system::RealFS = &mut fs.fs; #[cfg(windows)] - let _ = (changed_files, parents, file_descriptors, rfs); + let _ = (parents, file_descriptors, rfs); let mut _on_file_update_path_buf = bun_paths::path_buffer_pool::get(); - for event in events.iter() { + for event_i in 0..event_count { + let event = batch.events()[event_i]; // Stale udata: kevent.udata can outlive a swapRemove in flushEvictions. if event.index as usize >= file_paths.len() { continue; } let file_path: &[u8] = &file_paths[event.index as usize]; - let update_count = counts[event.index as usize] + 1; - counts[event.index as usize] = update_count; + counts[event.index as usize] += 1; let kind = kinds[event.index as usize]; // so it's consistent with the rest @@ -885,19 +766,12 @@ where if event.op.contains(WatchOp::DELETE) || (event.op.contains(WatchOp::RENAME) && IS_KQUEUE) { - // SAFETY: the Watcher outlives this call (it owns the - // Reloader that calls us); `remove_at_index::` - // only buffers the eviction, and the borrow is scoped - // to this call so it never overlaps the watchlist - // column slices above. - unsafe { - (*ctx).remove_at_index::( - bun_watcher::Kind::File, - event.index, - 0, - &[], - ) - }; + batch.watcher_mut().remove_at_index::( + bun_watcher::Kind::File, + event.index, + 0, + &[], + ); } if self.verbose { @@ -937,7 +811,7 @@ where } } - current_task.append(current_hash); + current_task.append(self, current_hash); } } bun_watcher::Kind::Directory => { @@ -946,15 +820,19 @@ where // on windows we receive file events for all items affected by a directory change // so we only need to clear the directory cache. all other effects will be handled // by the file events - let _ = self.ctx_mut().bust_dir_cache( - strings::paths::without_trailing_slash_windows_path(file_path), - ); + current_task.bust_dir(strings::paths::without_trailing_slash_windows_path( + file_path, + )); continue; } #[cfg(not(windows))] { let mut affected_buf: [&[u8]; 128] = [b"".as_slice(); 128]; - let mut entries_option: Option<*mut Fs::EntriesOption> = None; + // inotify names live in the watcher's event buffer; copied + // out so the watcher can be borrowed for evictions below. + let mut names_buf: [ChangedFilePath; bun_watcher::MAX_COUNT] = + [None; bun_watcher::MAX_COUNT]; + let mut entries_option: Option> = None; // Note: the affected-name element type differs by // platform (kqueue vs inotify). Split into two locals; @@ -965,9 +843,8 @@ where let affected_len: usize = 'brk: { if IS_KQUEUE { - // SAFETY: hot-reload runs single-threaded on the JS thread; - // no other live `&mut EntriesOption` for this key here. if let Some(existing) = rfs.entries.get(file_path) { + let existing = bun_ptr::BackRef::new(existing); self.put_tombstone(file_path, existing); entries_option = Some(existing); } else if let Some(existing) = self.get_tombstone(file_path) { @@ -992,7 +869,6 @@ where let exists = if basename.len() < name_buf.len() { name_buf[..basename.len()].copy_from_slice(basename); name_buf[basename.len()] = 0; - // SAFETY: name_buf[..=basename.len()] is NUL-terminated. let z = ZStr::from_buf(&name_buf[..], basename.len()); bun_sys::faccessat( file_descriptors[event.index as usize], @@ -1005,7 +881,7 @@ where if exists { self.main.is_waiting_for_dir_change = false; record_changed_path(self.main.file); - current_task.append(self.main.hash); + current_task.append(self, self.main.hash); } } } @@ -1027,7 +903,6 @@ where zbuf[..affected_path.len()] .copy_from_slice(affected_path); zbuf[affected_path.len()] = 0; - // SAFETY: zbuf is NUL-terminated at len. let z = ZStr::from_buf( &zbuf[..], affected_path.len(), @@ -1053,12 +928,15 @@ where break 'brk affected_i; } - affected_inotify = event.names(changed_files); + let names = event.names(batch.changed_files()); + names_buf[..names.len()].copy_from_slice(names); + affected_inotify = &names_buf[..names.len()]; break 'brk affected_inotify.len(); }; if affected_len > 0 && !IS_KQUEUE { if let Some(existing) = rfs.entries.get(file_path) { + let existing = bun_ptr::BackRef::new(existing); self.put_tombstone(file_path, existing); entries_option = Some(existing); } else if let Some(existing) = self.get_tombstone(file_path) { @@ -1066,9 +944,9 @@ where } } - let _ = self.ctx_mut().bust_dir_cache( - strings::paths::without_trailing_slash_windows_path(file_path), - ); + current_task.bust_dir(strings::paths::without_trailing_slash_windows_path( + file_path, + )); // The watched entrypoint has a per-file inotify watch on its inode. // An atomic rename (`rename(tmp, entrypoint)`) or a rm+recreate over @@ -1105,25 +983,23 @@ where zbuf[..self.main.file.len()] .copy_from_slice(self.main.file); zbuf[self.main.file.len()] = 0; - // SAFETY: zbuf is NUL-terminated at len. let z = ZStr::from_buf(&zbuf[..], self.main.file.len()); bun_sys::access(z, libc::F_OK).is_ok() } }; if main_exists { record_changed_path(self.main.file); - current_task.append(self.main.hash); + current_task.append(self, self.main.hash); } break; } } if let Some(dir_ent) = entries_option { - // SAFETY: dir_ent points into rfs.entries (or a tombstoned copy); - // both outlive this loop iteration. Shared access only — - // `entries()` takes `&self` and per-entry mutation below goes - // through the entry's own mutex + cells. - let dir_ent = unsafe { &*dir_ent }; + // Shared access only — `entries()` takes `&self` and + // per-entry mutation below goes through the entry's + // own mutex + cells. + let dir_ent: &Fs::EntriesOption = dir_ent.get(); let mut last_file_hash: bun_watcher::HashType = bun_watcher::HashType::MAX; @@ -1140,10 +1016,8 @@ where continue; } - // `ctx` is a BACKREF that outlives the reloader. let loader = self - .ctx - .get_loaders() + .loaders .get(PathName::find_extname(changed_name)) .copied() .unwrap_or(bun_ast::Loader::File); @@ -1180,40 +1054,34 @@ where } path_string = ent.abs_path; file_hash = Watcher::get_hash(path_string.as_bytes()); - for (entry_id, hash) in hashes.iter().enumerate() { - if *hash == file_hash { - if file_descriptors[entry_id].is_valid() { - if prev_entry_id != entry_id { - record_changed_path( - path_string.as_bytes(), - ); - current_task.append(hashes[entry_id]); - if self.verbose { - Self::debug(format_args!( - "Removing file: {}", - bstr::BStr::new( - path_string.as_bytes() - ) - )); - } - // SAFETY: see the - // File-arm call - // above. - unsafe { - (*ctx).remove_at_index::( - bun_watcher::Kind::File, - entry_id as u16, - 0, - &[], + if let Some(entry_id) = + hashes.iter().position(|hash| *hash == file_hash) + { + if file_descriptors[entry_id].is_valid() { + if prev_entry_id != entry_id { + record_changed_path(path_string.as_bytes()); + current_task.append(self, hashes[entry_id]); + if self.verbose { + Self::debug(format_args!( + "Removing file: {}", + bstr::BStr::new( + path_string.as_bytes() ) - }; + )); } + batch + .watcher_mut() + .remove_at_index::( + bun_watcher::Kind::File, + entry_id as u16, + 0, + &[], + ); } - - prev_entry_id = entry_id; - _ = prev_entry_id; - break; } + + prev_entry_id = entry_id; + _ = prev_entry_id; } break 'brk path_string.as_bytes(); @@ -1280,26 +1148,29 @@ where } } - // Drop order (LIFO): `_flush` guard → Output::flush() + - // ctx.flush_evictions(), then `current_task` guard → enqueue(). See - // the note on `current_task` above for why this order matters. + // Evict *before* enqueueing. The reverse order opens a window where the + // JS thread can pick up the task, look the file up in the watchlist, + // and read the cached fd while this thread is still about to `close()` + // + `swap_remove()` it in `flush_evictions` — surfacing as + // `EBADF`/`EISDIR reading ""` in hot.test.ts under load. Evicting + // first is side-effect-free: `enqueue` carries hashes (not indices) and + // never reads the watchlist. + Output::flush(); + batch.watcher_mut().flush_evictions(); + current_task.enqueue(self); } } -/// `Watcher::init` stores the `NewHotReloader` as its opaque context and -/// dispatches file-change/error callbacks through this trait. -impl bun_watcher::WatcherContext +/// `Watcher::init_with_handler` owns the `NewHotReloader` and dispatches +/// file-change/error callbacks (watcher thread) through this trait. +impl bun_watcher::WatcherHandler for NewHotReloader where Ctx: HotReloaderCtx, + Self: Send, { - fn on_file_update( - &mut self, - events: &mut [bun_watcher::WatchEvent], - changed_files: &[bun_watcher::ChangedFilePath], - watchlist: &bun_watcher::WatchList, - ) { - Self::on_file_update(self, events, changed_files, watchlist); + fn on_file_update(&mut self, batch: &mut bun_watcher::FileUpdateBatch<'_>) { + Self::on_file_update(self, batch); } fn on_error(&mut self, err: bun_sys::Error) { @@ -1309,11 +1180,12 @@ where // ── `bun build --watch` (Ctx = BundleV2) ───────────────────────────────── // `RELOAD_IMMEDIATELY = true` means the watcher thread `execve()`s on the -// first change (Task::enqueue diverges), so `event_loop()` / `reload()` are -// never reached. The bundler crate (T5) can't name this generic, so it calls -// in via the `#[no_mangle]` hook below. +// first file change (Task::enqueue diverges) and there is no loop to post +// directory busts to, so `reload()` / `bust_dir_cache()` are never reached. +// The bundler crate (T5) can't name this generic, so it calls in via the +// `extern "Rust"` hook below. -impl<'a> HotReloaderCtx for bun_bundler::BundleV2<'a> { +impl HotReloaderCtx for bun_bundler::BundleV2<'static> { type EventLoop = bun_event_loop::AnyEventLoop; fn reload_handle(&self) -> Option { @@ -1322,27 +1194,25 @@ impl<'a> HotReloaderCtx for bun_bundler::BundleV2<'a> { None } - fn bun_watcher_mut(&mut self) -> &mut Watcher { - let handle = self - .bun_watcher - .expect("bun_watcher_mut on un-enabled BundleV2 reloader"); - // SAFETY: `Box` leaked via `into_raw` in `install_bun_watcher`; - // live for the process (BundleV2 is leaked under --watch — see - // `generate_from_cli`). - unsafe { &mut *handle.as_ptr() } + fn post_reload_task( + _handle: &crate::VmHandle, + task: Box>, + ) -> Result<(), Box>> { + // No handle (see `reload_handle`), so never asked. + Err(task) } - fn reload(&mut self, _task: &mut dyn HotReloadTaskView) { - // RELOAD_IMMEDIATELY=true never enqueues `Task::run` for BundleV2 - // (diverges or kill-signal branch; no listeners registered there). + fn reload(&self, _task: &dyn HotReloadTaskView) { + // No handle, so no task ever runs for BundleV2. unreachable!() } - fn bust_dir_cache(&mut self, path: &[u8]) -> bool { - bun_bundler::BundleV2::bust_dir_cache(self, path) + fn bust_dir_cache(&self, _path: &[u8]) -> bool { + // No handle, so no task ever runs for BundleV2. + unreachable!() } - fn get_loaders(&self) -> &bun_ast::LoaderHashTable { + fn loaders(&self) -> &bun_ast::LoaderHashTable { &self.transpiler.options.loaders } @@ -1363,15 +1233,13 @@ impl<'a> HotReloaderCtx for bun_bundler::BundleV2<'a> { &mut self, watcher: Box, _reload_immediately: bool, - ) -> *mut Watcher { - // `watcher_nn` is a fresh non-null heap allocation; live for the - // process (BundleV2 is leaked under --watch — see `generate_from_cli`). - let watcher_nn = bun_core::heap::into_raw_nn(watcher); - let watcher_ptr: *mut Watcher = watcher_nn.as_ptr(); - self.bun_watcher = Some(watcher_nn); - // SAFETY: `watcher_ptr` was just installed; live for the process. - self.transpiler.resolver.watcher = Some(unsafe { (*watcher_ptr).get_resolve_watcher() }); - watcher_ptr + ) -> &mut Watcher { + // Leaked: live for the process (BundleV2 is leaked under --watch — see + // `generate_from_cli`). + let watcher: &'static mut Watcher = Box::leak(watcher); + self.bun_watcher = Some(core::ptr::NonNull::from(&mut *watcher)); + self.transpiler.resolver.watcher = Some(watcher.get_resolve_watcher()); + watcher } fn compute_clear_screen(&self) -> bool { @@ -1387,15 +1255,10 @@ impl<'a> HotReloaderCtx for bun_bundler::BundleV2<'a> { type BundlerWatcher = NewHotReloader, bun_event_loop::AnyEventLoop, true>; -/// CYCLEBREAK extern hook: called from `BundleV2::init` (T5) when -/// `cli_watch_flag` is set. Defined here (not in -/// `bun_bundler`) because the bundler crate can't name `NewHotReloader`. -#[unsafe(no_mangle)] -fn __bun_jsc_enable_hot_module_reloading_for_bundler( - bv2: core::ptr::NonNull>, -) { - // SAFETY: `bv2` is the `&mut *Box>` formed in - // `BundleV2::init`; the lifetime is `'static` for the only caller (build - // command leaks the CLI arena), and the box is leaked under --watch. - unsafe { BundlerWatcher::enable_hot_module_reloading(bv2.as_ptr(), None) }; +/// CYCLEBREAK hook: called from `BundleV2::init` (T5) when `cli_watch_flag` +/// is set. Defined here (not in `bun_bundler`) because the bundler crate +/// can't name `NewHotReloader`. +// HOST_EXPORT(__bun_jsc_enable_hot_module_reloading_for_bundler, rust) +pub fn enable_hot_module_reloading_for_bundler(bv2: &'static mut bun_bundler::BundleV2<'static>) { + BundlerWatcher::enable_hot_module_reloading(bv2, None); } diff --git a/src/jsc/lib.rs b/src/jsc/lib.rs index 03faa8dec0e7..6f1563627a89 100644 --- a/src/jsc/lib.rs +++ b/src/jsc/lib.rs @@ -175,9 +175,9 @@ pub mod uuid; pub mod weak; pub use self::js_value::{ - CoerceTo, ComparisonResult, ForEachCallback, FromAny, FromJsEnum, JSValue, - Protected as ProtectedJSValue, ProxyField, SerializedFlags, SerializedScriptValue, - TemporalType, + CoerceTo, ComparisonResult, ForEachCallback, ForEachContext, ForEachPropertyContext, FromAny, + FromJsEnum, JSValue, Protected as ProtectedJSValue, ProxyField, SerializedFlags, + SerializedScriptValue, TemporalType, }; // LAYERING (PORTING.md §Dispatch): the task dispatch covers every concrete @@ -1218,7 +1218,9 @@ pub use self::runtime_transpiler_store::RuntimeTranspilerStore; #[path = "web_worker.rs"] pub mod web_worker; +pub mod worker_messaging_proxy; pub use self::web_worker::WebWorker; +pub use self::worker_messaging_proxy::WorkerMessagingProxy; // LAYERING: `Jest`/`TestScope`/`Expect`/`Snapshot` live in // `bun_runtime::test_runner` — a forward-dep on `bun_runtime`, which itself diff --git a/src/jsc/web_worker.rs b/src/jsc/web_worker.rs index 95d4acfeb292..f5c96e7aae1f 100644 --- a/src/jsc/web_worker.rs +++ b/src/jsc/web_worker.rs @@ -1,12 +1,13 @@ //! The thread that runs a Worker's global scope. //! -//! One `WebWorker` per worker thread. It is atomically refcounted and does not -//! belong to either side alone: +//! One `WebWorker` per worker thread, shared (`Arc`) between: //! //! - the C++ `WorkerMessagingProxy` (the parent<->worker relationship object, -//! see WorkerMessagingProxy.h) holds one ref from `create()` until it has -//! joined the thread (`releaseWorkerThread()`), and -//! - the running thread holds one for the whole of `thread_main`. +//! see WorkerMessagingProxy.h), which is handed a pointer by `create()` and +//! keeps it until it has joined the thread (`releaseWorkerThread()` → +//! `WebWorker__deref`); the ref backing that pointer is [`WebWorker::cpp_ref`], +//! - the running thread, which owns one for the whole of `thread_main`, and +//! - the parent VM's `child_workers` list while the worker is registered. //! //! `proxy` points back at the messaging proxy, which the thread also holds a //! ref on, so it is valid for the thread's whole life. Everything the thread @@ -20,7 +21,7 @@ //! `beforeExit` on a natural drain. //! 3. `shutdown()` — 'exit' handlers, stop phase, join own children, JSC VM //! teardown, free per-thread state, `workerGlobalScopeDestroyed`. -//! Then the thread drops its self-ref and returns; the parent joins it. +//! Then the thread drops its ref and returns; the parent joins it. //! //! Children: every worker created on a thread is registered on that thread's //! `VirtualMachine.child_workers` (parent thread only). When a thread exits — @@ -30,38 +31,39 @@ //! is Node's `stop_sub_worker_contexts()`; there is no process-global list. //! //! Threads: everything the worker thread needs from its parent VM (transform -//! options, an env snapshot, the standalone graph) is copied on the parent -//! thread in `create()`, and the thread holds a `Ticket` on the parent for its -//! whole life, so the parent cannot be destroyed under it. The parent (or an -//! exiting ancestor) reaches the worker's VM only through `vm_handle` — the -//! worker VM's uncounted handle, published once the VM exists — never through -//! a pointer to it. - -use crate::JsCell; -use core::cell::Cell; -use core::ffi::c_void; +//! options, an env snapshot, the standalone graph, argv) is copied on the +//! parent thread in `create()`, and the thread holds a `Ticket` on the parent +//! for its whole life, so the parent cannot be destroyed under it. The parent +//! (or an exiting ancestor) reaches the worker's VM only through `vm_handle` — +//! the worker VM's uncounted handle, published once the VM exists — never +//! through a pointer to it. + use core::ptr::NonNull; use core::sync::atomic::{AtomicBool, Ordering}; +use std::sync::Arc; use std::thread::JoinHandle; -use bun_core::{EncodedSlice, String as BunString, WTFStringImpl}; +use bun_core::{EncodedSlice, String as BunString, Utf8Bytes, strings}; use bun_io::KeepAlive; +use bun_ptr::{BackRef, ThreadBound}; +use bun_threading::Guarded; -use crate::virtual_machine::{self, VirtualMachine, runtime_hooks}; -use crate::{self as jsc, EncodedSliceJsc as _, JSGlobalObject, JSValue, JsError, LogJsc}; +use crate::virtual_machine::{self, VirtualMachine, WorkerVm, runtime_hooks}; +use crate::worker_messaging_proxy::WorkerMessagingProxy; +use crate::{self as jsc, EncodedSliceJsc as _, JSGlobalObject, JSPromise, JSValue, JsError, LogJsc}; bun_core::define_scoped_log!(log, Worker, hidden); -#[derive(bun_ptr::ThreadSafeRefCounted)] pub struct WebWorker { // ---- Immutable after `create()` (any thread) ---------------------------- - /// The C++ `WorkerMessagingProxy`; the thread holds a ref on it, so it is - /// valid for as long as this thread runs. Opaque here. - messaging_proxy: *mut c_void, - /// The `VirtualMachine` of the thread that created this worker. - /// **Parent thread only** (`child_workers`, `parent_poll_ref`); the worker - /// thread never dereferences it — what it needs was copied below. - parent: *mut VirtualMachine, + /// The C++ `WorkerMessagingProxy`; the thread holds a ref on it, so it + /// outlives this object (the proxy drops the pointer to us before it drops + /// that ref, see `releaseWorkerThread()`). + proxy: BackRef, + /// The `VirtualMachine` of the thread that created this worker; only that + /// thread follows it (`child_workers`, `parent_poll_ref`). What the worker + /// thread needs was copied below. + parent: ThreadBound, /// The parent's `--hot` / `--watch` mode, inherited by the worker VM. hot_reload: crate::virtual_machine::HotReload, /// Whether the worker VM arms `bun_jsc::vm_handle`'s test gate (debug @@ -76,18 +78,14 @@ pub struct WebWorker { /// constructor): loads `node:worker_threads` before preloads and the entry point. is_node_worker: bool, store_fd: bool, - /// Borrowed from the proxy's `WorkerOptions` (alive as long as the proxy). - argv_ptr: *const WTFStringImpl, - argv_len: usize, - exec_argv_ptr: *const WTFStringImpl, - exec_argv_len: usize, - inherit_exec_argv: bool, + argv: Box<[WorkerString]>, + /// `None`: the worker inherits the parent's `execArgv`. + exec_argv: Option>, unresolved_specifier: Box<[u8]>, preloads: Vec>, name: bun_core::ZBox, // ---- Cross-thread ---------------------------------------------------------- - ref_count: bun_ptr::ThreadSafeRefCount, /// Set by the parent (`requestTermination`), by an exiting ancestor, or by /// the worker itself (`process.exit()`); polled by the worker loop between /// ticks and turned into a JSC TerminationException for running script. @@ -95,33 +93,66 @@ pub struct WebWorker { /// The worker VM's uncounted handle: how the parent (or an exiting /// ancestor) asks it to terminate. `None` before `start_vm()` publishes it /// and after `shutdown()` unpublishes it. - vm_handle: bun_threading::Guarded>, - - // ---- Parent-thread only --------------------------------------------------- - /// Keep-alive on the parent's event loop: taken in `create()`, toggled by - /// `.ref()`/`.unref()`, released when the parent releases the thread. - parent_poll_ref: JsCell, - /// Taken by the parent to join the OS thread. - join_handle: JsCell>>, - - // ---- Worker-thread only ----------------------------------------------------- - // Mutated only on the worker thread, but through `&self` because other - // threads hold `&WebWorker` concurrently; hence the cells. - /// The worker's `VirtualMachine`; null before `start_vm()` and after - /// `shutdown()`. - vm: Cell<*mut VirtualMachine>, - status: Cell, - // The VM's allocator IS this arena. - arena: JsCell>, - /// Cloned env for the worker VM; boxed on the global heap because the arena - /// does not run `Drop`. Reclaimed in `shutdown()`. - worker_env_loader: Cell<*mut bun_dotenv::Loader>, + vm_handle: Guarded>, /// `process.exit(code)` ran; later error paths must not overwrite its code. exit_called: AtomicBool, /// The parent asked this thread to stop (`worker.terminate()` or an exiting /// parent) while its VM was live — as opposed to the thread stopping itself, /// or being stopped before it started. Written under the `vm_handle` lock. terminated_by_parent: AtomicBool, + /// The ref behind the pointer `create()` returned to the C++ proxy + /// (`m_workerThread`); released by `WebWorker__deref`. + cpp_ref: Guarded>>, + + // ---- Parent-thread only --------------------------------------------------- + /// Keep-alive on the parent's event loop: taken in `create()`, toggled by + /// `.ref()`/`.unref()`, released when the parent releases the thread. + parent_poll_ref: Guarded, + /// Taken by the parent to join the OS thread. + join_handle: Guarded>>, +} + +/// An `argv` / `execArgv` entry copied from the `WorkerOptions` in `create()` +/// in its original width, so the worker builds the same JS string from it. +pub enum WorkerString { + Latin1(Box<[u8]>), + Utf16(Box<[u16]>), + Utf8(Box<[u8]>), +} + +impl WorkerString { + fn new(s: &BunString) -> WorkerString { + if s.is_utf16() { + WorkerString::Utf16(s.utf16().into()) + } else if s.is_utf8() { + WorkerString::Utf8(s.byte_slice().into()) + } else { + WorkerString::Latin1(s.latin1().into()) + } + } + + /// A fresh JS-heap-independent string for the calling thread. + pub fn to_bun_string(&self) -> BunString { + match self { + WorkerString::Latin1(b) if b.is_empty() => BunString::EMPTY, + WorkerString::Utf16(w) if w.is_empty() => BunString::EMPTY, + WorkerString::Utf8(b) if b.is_empty() => BunString::EMPTY, + WorkerString::Latin1(b) => BunString::clone_latin1(b), + WorkerString::Utf16(w) => BunString::clone_utf16(w), + WorkerString::Utf8(b) => BunString::clone_utf8(b), + } + } + + pub fn to_utf8(&self) -> Utf8Bytes<'static> { + match self { + WorkerString::Latin1(b) => match strings::allocate_latin1_into_utf8(b) { + Ok(v) => Utf8Bytes::Owned(v), + Err(_) => bun_core::out_of_memory(), + }, + WorkerString::Utf16(w) => Utf8Bytes::Owned(strings::to_utf8_alloc(w)), + WorkerString::Utf8(b) => Utf8Bytes::Owned(b.to_vec()), + } + } } /// Copied from the parent VM on its thread at `new Worker()`; consumed by @@ -132,6 +163,19 @@ struct WorkerVmInit { proxy_env_slots: jsc::rare_data::ProxyEnvSlots, } +/// What only the worker thread touches, owned by `thread_main`'s frame. +struct WorkerThread { + /// The worker's `VirtualMachine`; `None` before `start_vm()` and after + /// `shutdown()`. + vm: Option, + status: Status, + /// The VM's allocator IS this arena (`VirtualMachine.arena` points at it). + arena: Option>, + /// Cloned env for the worker VM (`VirtualMachine.transpiler.env` points at + /// it). Dropped in `shutdown()` after the VM. + env_loader: Option>, +} + enum EntryOutcome { Continue, /// The entry module rejected and no handler took it: the worker exits. @@ -151,28 +195,13 @@ pub enum Status { Terminated, } -// `JSGlobalObject` is an opaque FFI handle (ZST); it crosses FFI as `&`/`*const` -// even when C++ mutates through it. `proxy` is the opaque C++ `WorkerMessagingProxy*` -// round-tripped from `create()`; it is only ever handed back to C++. +// `JSGlobalObject` is an opaque FFI handle (ZST); it crosses FFI as `&` even +// when C++ mutates through it. unsafe extern "C" { - safe fn WebWorker__workerThreadStarted(proxy: *mut c_void); - safe fn WebWorker__workerGlobalScopeStarted(proxy: *mut c_void, global: &JSGlobalObject); - safe fn WebWorker__workerGlobalScopeDestroyed( - proxy: *mut c_void, - exit_code: i32, - stopped_by_parent: bool, - ); - safe fn WebWorker__parentContextWillDestroy(proxy: *mut c_void); safe fn WebWorker__entrySettled(global: &JSGlobalObject); /// Loads `node:worker_threads` in this VM (it rebinds process stdio and /// registers parentPort). May leave an exception pending. safe fn Bun__Worker__loadNodeWorkerThreadsModule(global: &JSGlobalObject); - safe fn WebWorker__dispatchError( - global: &JSGlobalObject, - proxy: *mut c_void, - message: BunString, - err: JSValue, - ); safe fn Bun__freeSharedHeaderBufferForThreadExit(); // Raw FFI (no RAII guard) so `thread_main` can take the API lock and abandon // it with the VM — see the note there. @@ -190,141 +219,62 @@ pub fn join_child_workers(parent: &mut VirtualMachine) { // pushes, `release_parent_poll_ref()` removes, and this takes the rest. let children = core::mem::take(&mut parent.child_workers); for child in children { - // SAFETY: registered children are live until the parent releases them - // (the proxy's ref); this is that release. - let messaging_proxy = unsafe { (*child).messaging_proxy }; - WebWorker__parentContextWillDestroy(messaging_proxy); + child.parent_context_will_destroy(); } } /// The messaging proxy of the worker running on `vm`'s thread, or null on the /// main thread. Used by the worker-side script bindings (parentPort.postMessage, /// workerData, ...). -#[unsafe(no_mangle)] -extern "C" fn WebWorker__getMessagingProxy(vm: &VirtualMachine) -> *mut c_void { +// HOST_EXPORT(WebWorker__getMessagingProxy, c) +pub fn get_messaging_proxy( + vm: &VirtualMachine, +) -> *mut crate::worker_messaging_proxy::WorkerMessagingProxy { vm.worker_ref() - .map(|w| w.messaging_proxy) + .map(|w| w.messaging_proxy().as_mut_ptr()) .unwrap_or(core::ptr::null_mut()) } -impl Drop for WebWorker { - fn drop(&mut self) { - log!("[{}] destroy", self.execution_context_id); - debug_assert!( - self.join_handle.with_mut(|h| h.is_none()), - "worker thread was never joined" - ); - } -} - -impl WebWorker { - pub(crate) fn has_requested_terminate(&self) -> bool { - self.requested_terminate.load(Ordering::Acquire) - } - - /// Worker thread only. - #[inline] - fn vm_ptr(&self) -> *mut VirtualMachine { - self.vm.get() - } - - /// Closure-scoped `&mut KeepAlive` accessor for `parent_poll_ref`. The cell - /// is touched only on the parent thread (`set_ref`, - /// `release_parent_poll_ref`, `create`) so no lock is required; `JsCell` - /// provides the interior mutability because `WebWorker` is shared `&self` - /// across threads. - #[inline] - fn with_parent_poll_ref(&self, f: impl FnOnce(&mut KeepAlive) -> R) -> R { - self.parent_poll_ref.with_mut(f) - } - - /// Whether this worker was started in eval mode (entry source is a - /// string, not a file). - #[inline] - pub fn eval_mode(&self) -> bool { - self.eval_mode - } - - /// Borrowed from the C++ `WorkerOptions` (kept alive by the owning - /// `WebCore::Worker`). - #[inline] - pub fn argv(&self) -> &[WTFStringImpl] { - // SAFETY: `argv_ptr[..argv_len]` is borrowed from C++ WorkerOptions - // (BACKREF — kept alive by the owning Worker for `self`'s lifetime). - // `(null, 0)` is tolerated by `ffi::slice`. - unsafe { bun_core::ffi::slice(self.argv_ptr, self.argv_len) } - } - - /// `None` when - /// `inherit_exec_argv` (the worker inherits the parent's execArgv), - /// otherwise `Some(slice)` (possibly empty) borrowed from C++ WorkerOptions. - #[inline] - pub fn exec_argv(&self) -> Option<&[WTFStringImpl]> { - if self.inherit_exec_argv { - return None; - } - // SAFETY: see `argv()`. - Some(unsafe { bun_core::ffi::slice(self.exec_argv_ptr, self.exec_argv_len) }) - } - - fn set_requested_terminate(&self) -> bool { - self.requested_terminate.swap(true, Ordering::Release) - } - - // ========================================================================= - // Construction (parent thread) - // ========================================================================= +// ========================================================================= +// Construction (parent thread) +// ========================================================================= + +/// Allocate the thread object (returning the pointer whose ref the calling +/// proxy owns), take a keep-alive on the parent event loop, register as a +/// child of the parent VM, and spawn the thread. On any failure returns +/// null with `error_message` set and nothing to clean up. +// HOST_EXPORT(WebWorker__create, c) +pub fn create( + proxy: &crate::worker_messaging_proxy::WorkerMessagingProxy, + parent: &mut VirtualMachine, + name_str: &BunString, + specifier_str: &BunString, + error_message: &mut BunString, + _parent_context_id: u32, + this_context_id: u32, + mini: bool, + default_unref: bool, + eval_mode: bool, + is_node_worker: bool, + argv: &[BunString], + inherit_exec_argv: bool, + exec_argv: &[BunString], + preload_modules: &[BunString], +) -> *const crate::web_worker::WebWorker { + jsc::mark_binding(); + log!("[{}] create", this_context_id); - /// Allocate the thread object (one ref, owned by the calling proxy), take a - /// keep-alive on the parent event loop, register as a child of the parent VM, - /// and spawn the thread. On any failure returns null with `error_message` - /// set and nothing to clean up. - #[unsafe(export_name = "WebWorker__create")] - pub(crate) unsafe extern "C" fn create( - proxy: *mut c_void, - parent: *mut VirtualMachine, - name_str: &BunString, - specifier_str: &BunString, - error_message: &mut BunString, - _parent_context_id: u32, - this_context_id: u32, - mini: bool, - default_unref: bool, - eval_mode: bool, - is_node_worker: bool, - argv_ptr: *const WTFStringImpl, - argv_len: usize, - inherit_exec_argv: bool, - exec_argv_ptr: *const WTFStringImpl, - exec_argv_len: usize, - preload_modules_ptr: *const BunString, - preload_modules_len: usize, - ) -> *mut WebWorker { - jsc::mark_binding(); - log!("[{}] create", this_context_id); + let spec_slice = specifier_str.to_utf8(); - let spec_slice = specifier_str.to_utf8(); + let mut preloads: Vec> = Vec::with_capacity(preload_modules.len()); + { let mut temp_log = bun_ast::Log::default(); - // SAFETY: `parent` is the calling thread's live VM (BACKREF); borrows - // are scoped to each statement. - let prev_log = unsafe { - let prev = (*parent).transpiler.log; - (*parent).transpiler.set_log(&raw mut temp_log); - prev - }; - // RAII: log pointer restored and temp log dropped on every return path. - let mut restore = scopeguard::guard(temp_log, move |log| { - // SAFETY: `parent` outlives the guard (this call's frame). - unsafe { (*parent).transpiler.set_log(prev_log) }; - drop(log); + let prev_log = parent.transpiler.log; + parent.transpiler.set_log(&raw mut temp_log); + // RAII: log pointer restored on every return path. + let mut parent = scopeguard::guard(&mut *parent, move |parent| { + parent.transpiler.set_log(prev_log); }); - let temp_log = &mut *restore; - - // SAFETY: caller passed valid (ptr,len) (or `(null,0)`); slice borrowed from C++. - let preload_modules: &[BunString] = - unsafe { bun_core::ffi::slice(preload_modules_ptr, preload_modules_len) }; - - let mut preloads: Vec> = Vec::with_capacity(preload_modules_len); for module in preload_modules { let utf8_slice = module.to_utf8(); // node: builtin specifiers skip the file resolver — the worker-side @@ -333,246 +283,259 @@ impl WebWorker { preloads.push(utf8_slice.slice().to_vec().into_boxed_slice()); continue; } - // SAFETY: `parent` is the live VM on the calling (parent) thread; - // `resolve_entry_point_specifier` takes the raw pointer. - if let Some(preload) = unsafe { - resolve_entry_point_specifier(parent, utf8_slice.slice(), error_message, temp_log) - } { + if let Some(preload) = resolve_entry_point_specifier( + &mut parent, + utf8_slice.slice(), + error_message, + &mut temp_log, + ) { preloads.push(preload.to_vec().into_boxed_slice()); } if !error_message.is_empty() { - // preloads dropped by RAII. - return core::ptr::null_mut(); + return core::ptr::null(); } } + } - // Everything the worker thread needs from this VM is copied here, on - // its own thread; the worker never dereferences `parent`. - // SAFETY: `parent` is the calling thread's live VM. - let parent_ref = unsafe { &*parent }; - let store_fd = parent_ref.transpiler.resolver.store_fd; - let mut transform_options = (*parent_ref.transpiler.options.transform_options).clone(); - if !inherit_exec_argv { - let hooks = runtime_hooks().expect("RuntimeHooks not installed"); - // SAFETY: caller passed valid (ptr,len) borrowed from C++ WorkerOptions; - // the hook only reads the slice. - let parsed = unsafe { - (hooks.parse_worker_exec_argv_flags)(bun_core::ffi::slice( - exec_argv_ptr, - exec_argv_len, - )) - }; - if let Some(flags) = parsed { - let parent_allows_addons = transform_options.allow_addons.unwrap_or(true); - transform_options.allow_addons = Some(parent_allows_addons && flags.allow_addons); - let parent_allows_ffi_cc = transform_options.allow_ffi_cc.unwrap_or(true); - transform_options.allow_ffi_cc = Some(parent_allows_ffi_cc && flags.allow_ffi_cc); - } + // Everything the worker thread needs from this VM is copied here, on + // its own thread; the worker never dereferences `parent`. + let store_fd = parent.transpiler.resolver.store_fd; + let mut transform_options = (*parent.transpiler.options.transform_options).clone(); + if !inherit_exec_argv { + let hooks = runtime_hooks().expect("RuntimeHooks not installed"); + // `None` on parse failure keeps the parent's settings. + if let Some(flags) = (hooks.parse_worker_exec_argv_flags)(exec_argv) { + let parent_allows_addons = transform_options.allow_addons.unwrap_or(true); + transform_options.allow_addons = Some(parent_allows_addons && flags.allow_addons); + let parent_allows_ffi_cc = transform_options.allow_ffi_cc.unwrap_or(true); + transform_options.allow_ffi_cc = Some(parent_allows_ffi_cc && flags.allow_ffi_cc); } - // The worker's `process.env` starts as a copy of the parent's now (as in - // Node). Proxy-env values may be RefCountedEnvValue bytes owned by the - // parent's proxy_env_storage: snapshot slots + map under its lock so - // every slice copied is backed by a ref the snapshot holds. - let mut proxy_env_slots = jsc::rare_data::ProxyEnvSlots::default(); - let mut env_loader = { - let parent_slots = parent_ref.proxy_env_storage.lock(); - proxy_env_slots.clone_from(&parent_slots); - match parent_ref.env_loader().clone_for_worker() { - Ok(loader) => loader, - Err(_) => { - *error_message = BunString::static_("Out of memory"); - return core::ptr::null_mut(); - } + } + // The worker's `process.env` starts as a copy of the parent's now (as in + // Node). Proxy-env values may be RefCountedEnvValue bytes owned by the + // parent's proxy_env_storage: snapshot slots + map under its lock so + // every slice copied is backed by a ref the snapshot holds. + let mut proxy_env_slots = jsc::rare_data::ProxyEnvSlots::default(); + let mut env_loader = { + let parent_slots = parent.proxy_env_storage.lock(); + proxy_env_slots.clone_from(&parent_slots); + match parent.env_loader().clone_for_worker() { + Ok(loader) => loader, + Err(_) => { + *error_message = BunString::static_(b"Out of memory"); + return core::ptr::null(); } - }; - proxy_env_slots.sync_into(&mut env_loader.map); - let init = WorkerVmInit { - transform_options, - env_loader, - proxy_env_slots, - }; - - // The construction ref: handed to C++ on success, dropped on failure. - let worker = bun_ptr::RefPtr::new(WebWorker { - messaging_proxy: proxy, - parent, - hot_reload: parent_ref.hot_reload, - arm_test_gate: cfg!(debug_assertions) - && parent_ref.is_main_thread() - && bun_core::env_var::feature_flag::BUN_DEBUG_TEST_WORKER_TEARDOWN_GATE::get() - .unwrap_or(false), - execution_context_id: this_context_id, - mini, - eval_mode, - is_node_worker, - store_fd, - argv_ptr, - argv_len, - exec_argv_ptr, - exec_argv_len, - inherit_exec_argv, - unresolved_specifier: spec_slice.slice().to_vec().into_boxed_slice(), - preloads, - name: if name_str.is_empty() { - bun_core::ZBox::default() - } else { - name_str.to_owned_slice_z() - }, - ref_count: bun_ptr::ThreadSafeRefCount::init(), - requested_terminate: AtomicBool::new(false), - vm_handle: bun_threading::Guarded::new(None), - vm: Cell::new(core::ptr::null_mut()), - parent_poll_ref: JsCell::new(KeepAlive::init()), - join_handle: JsCell::new(None), - status: Cell::new(Status::Start), - arena: JsCell::new(None), - worker_env_loader: Cell::new(core::ptr::null_mut()), - exit_called: AtomicBool::new(false), - terminated_by_parent: AtomicBool::new(false), - }); - let worker_ref = bun_ptr::ParentRef::from(worker.as_non_null()); - - // Keep the parent's event loop alive until the parent releases this - // thread, unless the user opted out with `{ ref: false }`. - if !default_unref { - // `bun_io::js_vm_ctx()` is this (the parent) thread's loop. - worker_ref.with_parent_poll_ref(|p| p.ref_(bun_io::js_vm_ctx())); } + }; + proxy_env_slots.sync_into(&mut env_loader.map); + let init = WorkerVmInit { + transform_options, + env_loader, + proxy_env_slots, + }; - // The thread's own ref, taken before it exists so it can never observe zero. - let thread_ref = worker.clone(); - // The thread is something of this VM's on another thread for as long as - // it runs: the parent joins it before its own teardown's wait, which - // this ticket would otherwise hold. - let parent_ticket = parent_ref.ticket(); - /// What the worker thread is handed: its refcounted `WebWorker` (the ref - /// taken above is the thread's), the parent's snapshot, and a ticket on - /// the parent VM. - struct ThreadStart { - worker: bun_ptr::RefPtr, - init: WorkerVmInit, - _parent_ticket: crate::Ticket, + let worker = Arc::new(WebWorker { + // The proxy holds a ref on itself on this worker's behalf until it has + // joined the thread and dropped its pointer to us (`releaseWorkerThread`). + proxy: BackRef::new(proxy), + // The parent VM joins every child (`join_child_workers`) before it is + // destroyed; followed only on this (the parent's) thread. + parent: ThreadBound::new(parent), + hot_reload: parent.hot_reload, + arm_test_gate: cfg!(debug_assertions) + && parent.is_main_thread() + && bun_core::env_var::feature_flag::BUN_DEBUG_TEST_WORKER_TEARDOWN_GATE::get() + .unwrap_or(false), + execution_context_id: this_context_id, + mini, + eval_mode, + is_node_worker, + store_fd, + argv: argv.iter().map(WorkerString::new).collect(), + exec_argv: (!inherit_exec_argv).then(|| exec_argv.iter().map(WorkerString::new).collect()), + unresolved_specifier: spec_slice.slice().to_vec().into_boxed_slice(), + preloads, + name: if name_str.is_empty() { + bun_core::ZBox::default() + } else { + name_str.to_owned_slice_z() + }, + requested_terminate: AtomicBool::new(false), + vm_handle: Guarded::new(None), + exit_called: AtomicBool::new(false), + terminated_by_parent: AtomicBool::new(false), + cpp_ref: Guarded::new(None), + parent_poll_ref: Guarded::new(KeepAlive::init()), + join_handle: Guarded::new(None), + }); + + // Keep the parent's event loop alive until the parent releases this + // thread, unless the user opted out with `{ ref: false }`. + if !default_unref { + // `bun_io::js_vm_ctx()` is this (the parent) thread's loop. + worker.parent_poll_ref.lock().ref_(bun_io::js_vm_ctx()); + } + + // The thread is something of this VM's on another thread for as long as + // it runs: the parent joins it before its own teardown's wait, which + // this ticket would otherwise hold. + let parent_ticket = parent.ticket(); + let thread_worker = Arc::clone(&worker); + let spawn = std::thread::Builder::new() + .stack_size(bun_threading::thread_pool::DEFAULT_THREAD_STACK_SIZE as usize) + .spawn(move || { + let _parent_ticket = parent_ticket; + thread_worker.thread_main(init); + }); + match spawn { + Ok(handle) => { + *worker.join_handle.lock() = Some(handle); + parent.child_workers.push(Arc::clone(&worker)); + let ptr = Arc::as_ptr(&worker); + let cpp_ref = Arc::clone(&worker); + *worker.cpp_ref.lock() = Some(cpp_ref); + ptr } - // SAFETY: `WebWorker` is shared across threads by design (atomics, - // `Guarded`, thread-confined cells — see the struct doc) and holds no - // parent-VM state; `init` is an owned copy — byte buffers, scalars and - // `Arc`s, no JSC or atom strings; the parent VM - // itself is kept by `_parent_ticket`. - unsafe impl Send for ThreadStart {} - let start = ThreadStart { - worker: thread_ref, - init, - _parent_ticket: parent_ticket, - }; - let spawn = std::thread::Builder::new() - .stack_size(bun_threading::thread_pool::DEFAULT_THREAD_STACK_SIZE as usize) - .spawn(move || { - let start = start; - start.worker.thread_main(start.init); - // The thread's ref (and the parent ticket) drop here. - }); - match spawn { - Ok(handle) => { - worker_ref.join_handle.set(Some(handle)); - let worker = worker.into_raw(); - // SAFETY: `parent` is the calling thread's VM; parent-thread-only list. - unsafe { (*parent).child_workers.push(worker) }; - worker - } - Err(_) => { - // The thread's ref went down with the closure; ours drops on return. - worker_ref.with_parent_poll_ref(|p| p.unref(bun_io::js_vm_ctx())); - *error_message = BunString::static_("Failed to spawn worker thread"); - core::ptr::null_mut() - } + Err(_) => { + worker.parent_poll_ref.lock().unref(bun_io::js_vm_ctx()); + *error_message = BunString::static_(b"Failed to spawn worker thread"); + core::ptr::null() } } +} - /// Drop one ref; the last one frees the allocation (`Drop` below). Any thread. - /// - /// # Safety - /// `this` came from `create()` and the caller owns one ref on it. - #[unsafe(export_name = "WebWorker__deref")] - pub(crate) unsafe extern "C" fn deref(this: *mut WebWorker) { - // SAFETY: fn contract. - unsafe { bun_ptr::ThreadSafeRefCount::::deref(this) }; +/// Drop the ref behind the pointer `create()` returned: the proxy's last use +/// of it, on the thread that owns the parent context. +// HOST_EXPORT(WebWorker__deref, c) +pub fn release_cpp_ref(this: bun_ptr::ThisPtr) { + let cpp_ref = this.cpp_ref.lock().take(); + drop(cpp_ref); +} + +/// Block until the OS thread has returned. Parent thread; the worker has +/// either reported `workerGlobalScopeDestroyed` or been asked to terminate +/// by an exiting parent. Termination interrupts script, not a native call +/// the worker is blocked in, so this waits as long as that call does (as +/// Node's JoinThread does). +// HOST_EXPORT(WebWorker__join, c) +pub fn join(this: &crate::web_worker::WebWorker) { + let handle = this.join_handle.lock().take(); + if let Some(handle) = handle { + log!("[{}] join", this.execution_context_id); + // A panic on the worker thread has already been reported by the panic + // hook; the join result carries nothing further. + let _ = handle.join(); } +} - /// Block until the OS thread has returned. Parent thread; the worker has - /// either reported `workerGlobalScopeDestroyed` or been asked to terminate - /// by an exiting parent. Termination interrupts script, not a native call - /// the worker is blocked in, so this waits as long as that call does (as - /// Node's JoinThread does). - #[unsafe(export_name = "WebWorker__join")] - pub(crate) extern "C" fn join(this: *mut WebWorker) { - let this = bun_ptr::ParentRef::from(NonNull::new(this).expect("WebWorker FFI ptr")); - if let Some(handle) = this.join_handle.with_mut(Option::take) { - log!("[{}] join", this.execution_context_id); - // A panic on the worker thread has already been reported by the panic - // hook; the join result carries nothing further. - let _ = handle.join(); - } +// ========================================================================= +// Parent-thread API (called from C++ via JS) +// ========================================================================= + +/// worker.ref()/.unref(). Parent thread; the proxy gates out calls once +/// the keep-alive has been released. +// HOST_EXPORT(WebWorker__setRef, c) +pub fn set_ref(this: &crate::web_worker::WebWorker, value: bool) { + let mut poll = this.parent_poll_ref.lock(); + if value { + poll.ref_(bun_io::js_vm_ctx()); + } else { + poll.unref(bun_io::js_vm_ctx()); } +} - // ========================================================================= - // Parent-thread API (called from C++ via JS) - // ========================================================================= +/// Ask the thread to stop: set `requested_terminate`, raise a +/// TerminationException in its VM at the next safepoint, wake its loop. +/// Any thread the proxy is used from may call this. +// HOST_EXPORT(WebWorker__requestTermination, c) +pub fn request_termination(this: &crate::web_worker::WebWorker) { + // The handle's lock is taken *before* the flag is published: a worker + // that breaks out of its loop because it saw the flag then blocks in + // shutdown() (unpublish) until `terminated_by_parent` and the stop are + // set here, instead of racing past with neither. + let handle = this.vm_handle.lock(); + if this.set_requested_terminate() { + return; + } + log!("[{}] requestTermination", this.execution_context_id); + if let Some(handle) = &*handle { + // Node: being stopped only counts (exit code 1) once the environment + // exists and before the thread starts tearing it down on its own. + this.terminated_by_parent.store(true, Ordering::Relaxed); + // From now on the worker's native code enters no script and settles + // no promises (Node's `ExitEnv` → `is_stopping`), even before its + // thread notices; a TerminationException is raised at its next + // safepoint and its loop woken. + handle.request_termination(); + } +} - /// worker.ref()/.unref(). Parent thread; the proxy holds a ref on `this` - /// and gates out calls once the keep-alive has been released. - #[unsafe(export_name = "WebWorker__setRef")] - pub(crate) extern "C" fn set_ref(this: *mut WebWorker, value: bool) { - let this = bun_ptr::ParentRef::from(NonNull::new(this).expect("WebWorker FFI ptr")); - this.with_parent_poll_ref(|poll| { - if value { - poll.ref_(bun_io::js_vm_ctx()); - } else { - poll.unref(bun_io::js_vm_ctx()); - } - }); +/// The parent is releasing this thread: drop the keep-alive on the parent's +/// loop and forget it as a child. Parent thread. +// HOST_EXPORT(WebWorker__releaseParentPollRef, c) +pub fn release_parent_poll_ref(this: &crate::web_worker::WebWorker) { + this.parent_poll_ref.lock().unref(bun_io::js_vm_ctx()); + let children = &mut this.parent.get().as_mut().child_workers; + if let Some(i) = children + .iter() + .position(|c| core::ptr::eq(Arc::as_ptr(c), this)) + { + drop(children.swap_remove(i)); } +} - /// Ask the thread to stop: set `requested_terminate`, raise a - /// TerminationException in its VM at the next safepoint, wake its loop. - /// Any thread that holds a ref (the proxy) may call this. - #[unsafe(export_name = "WebWorker__requestTermination")] - pub(crate) extern "C" fn request_termination(this: *mut WebWorker) { - let this = bun_ptr::ParentRef::from(NonNull::new(this).expect("WebWorker FFI ptr")); - // The handle's lock is taken *before* the flag is published: a worker - // that breaks out of its loop because it saw the flag then blocks in - // shutdown() (unpublish) until `terminated_by_parent` and the stop are - // set here, instead of racing past with neither. - let handle = this.vm_handle.lock(); - if this.set_requested_terminate() { - return; - } - log!("[{}] requestTermination", this.execution_context_id); - if let Some(handle) = &*handle { - // Node: being stopped only counts (exit code 1) once the environment - // exists and before the thread starts tearing it down on its own. - this.terminated_by_parent.store(true, Ordering::Relaxed); - // From now on the worker's native code enters no script and settles - // no promises (Node's `ExitEnv` → `is_stopping`), even before its - // thread notices; a TerminationException is raised at its next - // safepoint and its loop woken. - handle.request_termination(); - } +impl Drop for WebWorker { + fn drop(&mut self) { + log!("[{}] destroy", self.execution_context_id); + debug_assert!( + self.join_handle.lock().is_none(), + "worker thread was never joined" + ); } +} - /// The parent is releasing this thread: drop the keep-alive on the parent's - /// loop and forget it as a child. Parent thread. - #[unsafe(export_name = "WebWorker__releaseParentPollRef")] - pub(crate) extern "C" fn release_parent_poll_ref(this: *mut WebWorker) { - let this_ref = bun_ptr::ParentRef::from(NonNull::new(this).expect("WebWorker FFI ptr")); - this_ref.with_parent_poll_ref(|p| p.unref(bun_io::js_vm_ctx())); - // SAFETY: parent thread; `parent` outlives its children (it joins them). - let children = unsafe { &mut (*this_ref.parent).child_workers }; - if let Some(i) = children.iter().position(|&c| core::ptr::eq(c, this)) { - children.swap_remove(i); - } +impl WebWorker { + pub(crate) fn has_requested_terminate(&self) -> bool { + self.requested_terminate.load(Ordering::Acquire) } + /// The parent context is exiting: terminate, join and release this worker + /// (`WorkerMessagingProxy::parentContextWillDestroy`). Parent thread only — + /// `parent.get()` refuses any other. + fn parent_context_will_destroy(self: Arc) { + let _: &VirtualMachine = self.parent.get(); + let proxy = self.proxy; + // Ours is not the ref that keeps this alive through its release (the + // proxy's is); drop it first so the release frees it where it always has. + drop(self); + proxy.parent_context_will_destroy(); + } + + /// Whether this worker was started in eval mode (entry source is a + /// string, not a file). + #[inline] + pub fn eval_mode(&self) -> bool { + self.eval_mode + } + + #[inline] + pub fn argv(&self) -> &[WorkerString] { + &self.argv + } + + /// `None` when the worker inherits the parent's execArgv, otherwise the + /// (possibly empty) list it was created with. + #[inline] + pub fn exec_argv(&self) -> Option<&[WorkerString]> { + self.exec_argv.as_deref() + } + + fn set_requested_terminate(&self) -> bool { + self.requested_terminate.swap(true, Ordering::Release) + } + + #[inline] pub(crate) fn hot_reload(&self) -> crate::virtual_machine::HotReload { self.hot_reload @@ -591,8 +554,8 @@ impl WebWorker { /// The C++ `WorkerMessagingProxy`, handed to `Zig__GlobalObject__create` so /// the worker's global is born knowing its options (env, argv, workerData). #[inline] - pub(crate) fn messaging_proxy(&self) -> *mut c_void { - self.messaging_proxy + pub(crate) fn messaging_proxy(&self) -> &WorkerMessagingProxy { + self.proxy.get() } #[inline] @@ -604,12 +567,7 @@ impl WebWorker { // Worker thread // ========================================================================= - // Worker-thread call chain takes `&self` (NOT `&mut self`): the parent / - // main thread may concurrently hold `&WebWorker` (`request_termination`, - // an exiting ancestor), so materialising `&mut WebWorker` here would - // be aliased-&mut UB. Worker-thread-only mutable fields are wrapped in - // `Cell` / `UnsafeCell` instead. - fn thread_main(&self, init: WorkerVmInit) { + fn thread_main(self: Arc, init: WorkerVmInit) { bun_analytics::features::workers_spawned.fetch_add(1, Ordering::Relaxed); if !self.name.is_empty() { @@ -620,47 +578,50 @@ impl WebWorker { )); } + let mut thread = WorkerThread { + vm: None, + status: Status::Start, + arena: None, + env_loader: None, + }; + // Terminated before we even started — straight to shutdown so the // parent still gets its close event. if self.has_requested_terminate() { - self.shutdown(); + self.shutdown(&mut thread); return; } - let vm_ptr = match self.start_vm(init) { - Ok(vm) => vm, + match self.start_vm(&mut thread, init) { + Ok(true) => {} + // `start_vm()` observed `requested_terminate` and already ran `shutdown()`. + Ok(false) => return, Err(err) => { bun_core::output::panic(format_args!( "An unhandled error occurred while starting a worker: {}\n", err.name() )); } - }; - - // `start_vm()` observed `requested_terminate` and already ran `shutdown()`. - if vm_ptr.is_null() { - return; } - // `start_vm()` installed `vm_ptr` as this thread's per-thread VM - // (`VirtualMachine::init` → `VMHolder`), so the safe thread-local - // accessor returns the same allocation. - debug_assert!(core::ptr::eq(vm_ptr, VirtualMachine::get_mut_ptr())); let global = VirtualMachine::get().global(); // Take the API lock for the thread's whole life and abandon it with the // VM (`shutdown()` destroys the `JSC::VM`; there is nothing to unlock). // Raw FFI rather than the RAII guard, whose `&VM` would dangle. JSC__VM__getAPILock(global.vm()); - self.spin(); + self.spin(&mut thread); } /// Phase 1: build the worker's arena + VirtualMachine and publish `vm`. /// - /// Returns the published VM pointer; `Ok(null)` means the early-terminate - /// checkpoint already ran `shutdown()`. - fn start_vm(&self, init: WorkerVmInit) -> Result<*mut VirtualMachine, crate::CrateError> { - debug_assert!(self.status.get() == Status::Start); - debug_assert!(self.vm_ptr().is_null()); + /// `Ok(false)` means the early-terminate checkpoint already ran `shutdown()`. + fn start_vm( + self: &Arc, + thread: &mut WorkerThread, + init: WorkerVmInit, + ) -> Result { + debug_assert!(thread.status == Status::Start); + debug_assert!(thread.vm.is_none()); let hooks = runtime_hooks().expect("RuntimeHooks not installed"); let WorkerVmInit { @@ -669,71 +630,57 @@ impl WebWorker { proxy_env_slots, } = init; - // worker-thread only field; no other thread reads `arena`. - self.arena.set(Some(bun_alloc::Arena::new())); + let arena = thread.arena.insert(Box::new(bun_alloc::Arena::new())); + let arena = NonNull::from(&mut **arena); - // `heap::alloc`'d and stashed on `self` so `shutdown()` step 5 reclaims - // it on every path — including the early-terminate checkpoint below, - // which calls `shutdown()` before the VM exists. - let loader_ptr: *mut bun_dotenv::Loader = bun_core::heap::into_raw(Box::new(env_loader)); - self.worker_env_loader.set(loader_ptr); + // Stashed on `thread` so `shutdown()` reclaims it on every path — + // including the early-terminate checkpoint below, which calls + // `shutdown()` before the VM exists. + let env_loader = thread.env_loader.insert(Box::new(env_loader)); + let env_loader = NonNull::from(&mut **env_loader); // Checkpoint before the expensive part: initWorker builds a full JSC // VM. If a parent's request_termination() fired while we were cloning the env // above, bail now rather than spending ~50–100ms (release) creating a // VM that will immediately tear down. if self.has_requested_terminate() { - self.shutdown(); - return Ok(core::ptr::null_mut()); + self.shutdown(thread); + return Ok(false); } - let vm = VirtualMachine::init_worker( + let vm = thread.vm.insert(VirtualMachine::init_worker( self, virtual_machine::Options { args: transform_options, - env_loader: NonNull::new(loader_ptr), + env_loader: Some(env_loader), store_fd: self.store_fd, graph: crate::virtual_machine::standalone_module_graph(), ..Default::default() }, - )?; - // Scoped `&mut VirtualMachine` for the worker-specific fields; ends - // before anything else on this thread re-derives access to the VM. + )?); { - // SAFETY: init_worker returns a valid heap-allocated VM ptr; - // not yet published, so this `&mut` is exclusive. - let vm_ref = unsafe { &mut *vm }; - // arena initialised above; worker-thread only field. `with_mut` - // scopes a `&mut Option` to the closure; we extract the raw - // address (escaping as `*mut`, no borrow) for the VM backref. - vm_ref.arena = self - .arena - .with_mut(|a| NonNull::new(std::ptr::from_mut(a.as_mut().unwrap()))); - - *vm_ref.proxy_env_storage.lock() = proxy_env_slots; - - vm_ref.is_main_thread = false; + let vm = vm.as_mut(); + vm.arena = Some(arena); + *vm.proxy_env_storage.lock() = proxy_env_slots; + vm.is_main_thread = false; VirtualMachine::set_is_main_thread_vm(false); - vm_ref.on_unhandled_rejection = on_unhandled_rejection; + vm.on_unhandled_rejection = on_unhandled_rejection; } // Publish now (rather than at the end of startVM) so that: // - a concurrent request_termination() (parent, or an exiting ancestor) can // wake us once JS starts running, and - // - early returns below reach spin()/shutdown() with this.vm set, + // - early returns below reach spin()/shutdown() with the VM set, // so teardownJSCVM/vm.deinit() run and the just-built JSC::VM // heap is not leaked. // We do NOT call shutdown() directly from here: shutdown() with a - // non-null vm runs vm.onExit() (JS), which requires holdAPILock. + // VM runs vm.onExit() (JS), which requires holdAPILock. // Instead we return; threadMain enters holdAPILock(spin) and spin()'s // first check observes requested_terminate. - self.vm.set(vm); - // SAFETY: `vm` is the live VM just built on this thread. - *self.vm_handle.lock() = Some(unsafe { (*vm).handle() }); + *self.vm_handle.lock() = Some(vm.handle()); - // SAFETY: `vm` is a valid heap-allocated VM ptr (checked above). - unsafe { - let b = &mut (*vm).transpiler; + { + let b = &mut vm.as_mut().transpiler; b.resolver.env_loader = NonNull::new(b.env); b.options.env.behavior = bun_options_types::schema::api::DotEnvBehavior::LoadAllWithoutInlining; @@ -748,75 +695,66 @@ impl WebWorker { // walks the resolver's global dir_cache) and entry-point loading. // spin() will observe the flag and shutdown() under the API lock. if self.has_requested_terminate() { - return Ok(vm); + return Ok(true); } - // SAFETY: this thread's live VM; per-expression derefs, no long-lived `&mut`. - unsafe { - if (*vm).transpiler.configure_defines().is_err() { - // Fall through to spin() → shutdown() for full teardown under - // the API lock (flushLogs runs JS). Set terminate so spin() - // bails immediately; vm.log carries the error for flushLogs. - (*vm).exit_handler.exit_code = 1; - let _ = self.set_requested_terminate(); - return Ok(vm); - } - - (*vm).load_extra_env_and_source_code_printer(); + if vm.as_mut().transpiler.configure_defines().is_err() { + // Fall through to spin() → shutdown() for full teardown under + // the API lock (flushLogs runs JS). Set terminate so spin() + // bails immediately; vm.log carries the error for flushLogs. + vm.as_mut().exit_handler.exit_code = 1; + let _ = self.set_requested_terminate(); + return Ok(true); } - Ok(vm) + + vm.as_mut().load_extra_env_and_source_code_printer(); + Ok(true) } /// Phase 2: post 'online', load the entry point, run the event loop. /// Runs inside `holdAPILock`. Always ends by calling `shutdown()`. - /// - /// Returns `()` so the thread can unwind-free fall out of the - /// `extern "C"` trampoline — see `shutdown`. - fn spin(&self) { + fn spin(&self, thread: &mut WorkerThread) { log!("[{}] spin start", self.execution_context_id); - // vm set in start_vm; non-null past this point. Mutation goes through - // `vm.as_mut()` which forms a fresh short-lived `&mut` per call (the - // `JsCell` escape hatch — provenance from the thread-local `*mut`). - let vm_ptr: *mut VirtualMachine = self.vm_ptr(); - // This IS the worker thread's per-thread VM (set by + // The VM set in start_vm IS the worker thread's per-thread VM (set by // `VirtualMachine::init` → `VMHolder`), so the safe thread-local - // accessor returns the same allocation. - debug_assert!(core::ptr::eq(vm_ptr, VirtualMachine::get_mut_ptr())); + // accessor returns the same allocation. Mutation goes through + // `vm.as_mut()`, which forms a fresh short-lived `&mut` per call. + debug_assert!( + thread + .vm + .as_deref() + .is_some_and(|vm| core::ptr::eq(vm, VirtualMachine::get())) + ); let vm: &VirtualMachine = VirtualMachine::get(); - debug_assert!(self.status.get() == Status::Start); - self.set_status(Status::Starting); + debug_assert!(thread.status == Status::Start); + self.set_status(thread, Status::Starting); // Terminated during startVM() (or startVM() short-circuited here on // configureDefines failure) — shut down under the API lock so the // JSC::VM built by initWorker is torn down rather than leaked. if self.has_requested_terminate() { self.flush_logs(vm); - return self.shutdown(); + return self.shutdown(thread); } // 'online' before the entry runs, as in node: it precedes anything the entry posts. - WebWorker__workerThreadStarted(self.messaging_proxy); + self.proxy.worker_thread_started(); - // `preloads` is owned by `self` (heap `WebWorker` outlives the VM). // `preload: Vec>` — clone the boxes (cheap, ≤handful). vm.as_mut().preload.clone_from(&self.preloads); // Resolve the entry point on the worker thread (the parent only stored - // the raw specifier). The returned slice is BORROWED — every exit from - // spin() goes through shutdown() which is noreturn, so a `defer free` - // here would never run anyway. + // the raw specifier). The returned slice is borrowed; every exit from + // spin() goes through shutdown(). let mut resolve_error = BunString::EMPTY; let vm_log = vm.log_mut().unwrap(); - // SAFETY: `vm_ptr` is the live worker-thread VM. - let path = match unsafe { - resolve_entry_point_specifier( - vm_ptr, - &self.unresolved_specifier, - &mut resolve_error, - vm_log, - ) - } { + let path = match resolve_entry_point_specifier( + vm.as_mut(), + &self.unresolved_specifier, + &mut resolve_error, + vm_log, + ) { Some(p) => p, None => { vm.as_mut().exit_handler.exit_code = 1; @@ -828,14 +766,14 @@ impl WebWorker { vm_log.add_error(None, bun_ast::Loc::EMPTY, err.slice().to_vec()); } self.flush_logs(vm); - return self.shutdown(); + return self.shutdown(thread); } }; // Terminated while resolving — exit code 0, no error. if self.has_requested_terminate() { self.flush_logs(vm); - return self.shutdown(); + return self.shutdown(thread); } // Node runs its worker bootstrap (parentPort, stdio, process overrides) @@ -852,7 +790,7 @@ impl WebWorker { } self.flush_logs(vm); WebWorker__entrySettled(global); - return self.shutdown(); + return self.shutdown(thread); } } @@ -861,7 +799,7 @@ impl WebWorker { // which outlive the worker VM. `vm.main` stores it as a raw BACKREF // (see `VirtualMachine::set_main`); no lifetime extension needed. let promise = match vm.as_mut().load_entry_point_for_web_worker(path) { - Ok(p) => p, + Ok(p) => JSPromise::opaque_mut(p), Err(_) => { // process.exit() may have run during load; don't clobber its code. if !self.exit_called.load(Ordering::Relaxed) { @@ -869,7 +807,7 @@ impl WebWorker { } self.flush_logs(vm); WebWorker__entrySettled(vm.global()); - return self.shutdown(); + return self.shutdown(thread); } }; @@ -883,50 +821,45 @@ impl WebWorker { // later) is the entry's uncaught error at that moment — the worker stops // unless a handler took it — and is reported exactly once. The loader // marks this promise handled, so nothing else would report it. + // `promise` is rooted (`entry_promise`) for the loop's duration. + let promise_value = promise.to_js(); let mut entry_rejection_seen = false; let mut observe_entry = |vm: &VirtualMachine| -> EntryOutcome { - // SAFETY: `promise` is a live JSC heap cell, rooted below for the loop's duration. - unsafe { - if entry_rejection_seen || (*promise).status() != jsc::js_promise::Status::Rejected - { - return EntryOutcome::Continue; - } - entry_rejection_seen = true; - // Same rule as the main thread (run_command): a CJS worker - // entry's top-level throw is an uncaughtException; only an - // ESM entry rejection reports origin "unhandledRejection". - let is_rejection = !vm.as_mut().entry_point_result.evaluated_as_cjs; - let handled = vm.as_mut().uncaught_exception( - vm.global(), - (*promise).result(vm.jsc_vm()), - is_rejection, - ); - if handled { - EntryOutcome::Continue - } else { - EntryOutcome::Stop - } + if entry_rejection_seen || promise.status() != jsc::js_promise::Status::Rejected { + return EntryOutcome::Continue; + } + entry_rejection_seen = true; + // Same rule as the main thread (run_command): a CJS worker + // entry's top-level throw is an uncaughtException; only an + // ESM entry rejection reports origin "unhandledRejection". + let is_rejection = !vm.as_mut().entry_point_result.evaluated_as_cjs; + let handled = vm.as_mut().uncaught_exception( + vm.global(), + promise.result(vm.jsc_vm()), + is_rejection, + ); + if handled { + EntryOutcome::Continue + } else { + EntryOutcome::Stop } }; if let EntryOutcome::Stop = observe_entry(vm) { // exit_code is already 1 from uncaught_exception; re-setting it here // would clobber a process.on('exit') change to process.exitCode. - return self.shutdown(); + return self.shutdown(thread); } // A still-pending entry promise is an unsettled top-level await: as in // Node the worker counts as started once its module graph is executing, // and the await continues in the normal event loop below — messages, // timers and I/O keep flowing meanwhile. Rooted for the loop's duration. - let entry_promise = crate::Strong::create( - JSValue::from_cell(promise.cast::()), - vm.global(), - ); + let entry_promise = crate::Strong::create(promise_value, vm.global()); self.flush_logs(vm); log!("[{}] event loop start", self.execution_context_id); // Pending -> Running: messages and tasks queued while the entry loaded are delivered. - WebWorker__workerGlobalScopeStarted(self.messaging_proxy, vm.global()); - self.set_status(Status::Running); + self.proxy.worker_global_scope_started(vm.global()); + self.set_status(thread, Status::Running); // don't run the GC if we don't actually need to if vm.standalone_module_graph.is_none() @@ -979,8 +912,7 @@ impl WebWorker { vm.as_mut().on_before_exit(); // Drained with the entry still pending: an unsettled top-level await, // Node's exit 13 (unless the user chose a nonzero exit code). - // SAFETY: rooted by `entry_promise`. - if unsafe { (*promise).status() } == jsc::js_promise::Status::Pending + if promise.status() == jsc::js_promise::Status::Pending && vm.exit_handler.exit_code == 0 { vm.as_mut().exit_handler.exit_code = 13; @@ -989,7 +921,7 @@ impl WebWorker { drop(entry_promise); self.flush_logs(vm); - self.shutdown(); + self.shutdown(thread); } /// Phase 3: unpublish the VM's handle (a racing `requestTermination` now @@ -998,70 +930,42 @@ impl WebWorker { /// destroy), free the thread's remaining state, and last of all report /// `workerGlobalScopeDestroyed` — the parent joins this thread from that /// task, so nothing after it may touch the parent. - fn shutdown(&self) { + fn shutdown(&self, thread: &mut WorkerThread) { jsc::mark_binding(); - self.set_status(Status::Terminated); + self.set_status(thread, Status::Terminated); bun_analytics::features::workers_terminated.fetch_add(1, Ordering::Relaxed); log!("[{}] shutdown", self.execution_context_id); - // worker-thread only field; no other thread reads `arena`. - let mut arena = self.arena.replace(None); - let env_loader = self.worker_env_loader.replace(core::ptr::null_mut()); + let mut arena = thread.arena.take(); + let env_loader = thread.env_loader.take(); // ---- 1. Unpublish vm ------------------------------------------------ drop(self.vm_handle.lock().take()); - let vm_ptr = self.vm.replace(core::ptr::null_mut()); + let vm = thread.vm.take(); // ---- 2. User exit handlers ----------------------------------------- let mut exit_code: i32 = 0; - if !vm_ptr.is_null() { - // SAFETY: vm_ptr valid; no other thread holds a pointer to it (they - // only ever held its handle) — `&mut` is exclusive. - let vm = unsafe { &mut *vm_ptr }; - vm.is_shutting_down = true; - vm.on_exit(); - exit_code = i32::from(vm.exit_handler.exit_code); + if let Some(vm) = vm { + { + let vm = vm.as_mut(); + vm.is_shutting_down = true; + vm.on_exit(); + exit_code = i32::from(vm.exit_handler.exit_code); + } log!( "[{}] shutdown: exit handlers done", self.execution_context_id ); // ---- 3–5. Stop, forbid script, wait, ~VM, loops, destroy ---------- - // SAFETY: this thread's VM; sole owner. - unsafe { VirtualMachine::teardown(vm_ptr, crate::virtual_machine::Teardown::Worker) }; - - // `destroy()` deinits the fields; reclaim the storage `init` put on - // the global heap (worker `init_worker` always passes `log: None`, - // so the log box is VM-owned here). - // SAFETY: sole owner; nothing past this point dereferences the VM. - unsafe { - let console = core::mem::replace(&mut (*vm_ptr).console, core::ptr::null_mut()); - if !console.is_null() { - bun_core::heap::destroy(console); - } - if let Some(log) = (*vm_ptr).log.take() { - bun_core::heap::destroy(log.as_ptr()); - } - virtual_machine::VMHolder::set_vm(None); - // The VM was `alloc_zeroed(Layout::())` in - // `init`, NOT `Box::new` — dealloc the raw storage directly so - // field `Drop`s do not re-run on already-`deinit`'d state. - std::alloc::dealloc( - vm_ptr.cast::(), - core::alloc::Layout::new::(), - ); - } + vm.teardown_and_free(); } log!( "[{}] shutdown: VirtualMachine destroyed", self.execution_context_id ); - // Reclaim the cloned env (`heap::alloc`'d in `start_vm()`; see field doc). - if !env_loader.is_null() { - // SAFETY: `heap::alloc`'d in `start_vm`; sole owner; the VM is - // gone so its raw `transpiler.env` borrow is dead. - drop(unsafe { bun_core::heap::take(env_loader) }); - } + // The VM is gone, so its `transpiler.env` borrow of this is dead. + drop(env_loader); // This thread's C++ thread_local destructors are not guaranteed to run // before the process exits, so free the HPACK scratch buffer that any // http2 session on this thread allocated. @@ -1078,11 +982,8 @@ impl WebWorker { // its forced unwind would cross `extern "C"` frames and abort). // A worker stopped by its parent that never called process.exit() did // not choose `exit_code`; the proxy decides what that reads as per kind. - WebWorker__workerGlobalScopeDestroyed( - self.messaging_proxy, - exit_code, - self.stopped_by_parent(), - ); + self.proxy + .worker_global_scope_destroyed(exit_code, self.stopped_by_parent()); } /// worker.terminate() from the parent, and the worker did not also exit on @@ -1093,22 +994,16 @@ impl WebWorker { && !self.exit_called.load(Ordering::Relaxed) } - /// process.exit() inside the worker. Worker-thread only. - /// - /// Takes `&self` (not `&mut self`) because `request_termination` / - /// other threads may concurrently hold `&WebWorker` on another - /// thread; producing `&mut` here would be aliased-&mut UB. + /// process.exit() inside the worker. Worker thread. pub fn exit(&self) { self.exit_called.store(true, Ordering::Relaxed); let _ = self.set_requested_terminate(); - // Stop subsequent JS at the next safepoint. `this.vm` is null during - // `vm.onExit()` (shutdown nulls it first), so a re-entrant + // Stop subsequent JS at the next safepoint. The handle is unpublished + // before `vm.onExit()` (shutdown step 1), so a re-entrant // process.exit() from an exit handler does not re-arm the trap. - let vm_ptr = self.vm_ptr(); - if !vm_ptr.is_null() { - // From an immediate this runs before the turn's poll; the wake is what ends it. - // SAFETY: this thread's live VM. - unsafe { (*vm_ptr).handle_ref().request_termination() }; + // From an immediate this runs before the turn's poll; the wake is what ends it. + if let Some(handle) = &*self.vm_handle.lock() { + handle.request_termination(); } } @@ -1116,13 +1011,13 @@ impl WebWorker { // Helpers (worker thread) // ========================================================================= - fn set_status(&self, status: Status) { + fn set_status(&self, thread: &mut WorkerThread, status: Status) { log!( "[{}] status: {}", self.execution_context_id, <&'static str>::from(status) ); - self.status.set(status); + thread.status = status; } /// Report the VM log (entry resolution / load errors) to the parent as the @@ -1152,7 +1047,7 @@ impl WebWorker { } }; let dispatch = jsc::host_fn::from_js_host_call_generic(global, || { - WebWorker__dispatchError(global, self.messaging_proxy, str, err) + self.proxy.dispatch_error(global, str, err) }); if let Err(e) = dispatch { let _ = crate::task::report_error_or_terminate(global, e); @@ -1183,19 +1078,14 @@ fn on_unhandled_rejection( // A parse failure rejects with a BuildMessage, which doesn't survive structured // clone. Node reports a SyntaxError; build a real one from the formatted parse // error so the subtype reaches the parent intact. - if let Some(bm) = error_instance.as_::() { - // SAFETY: as_ returned a live BuildMessage cell, read-only on the - // worker (JS) thread that owns it. - let text: &[u8] = unsafe { &(*bm).msg.data.text }; - error_instance = EncodedSlice::utf8(text).to_syntax_error_instance(global_object); + if let Some(bm) = error_instance.as_class_ref::() { + error_instance = EncodedSlice::utf8(&bm.msg.data.text).to_syntax_error_instance(global_object); } let mut array: Vec = Vec::new(); - // `worker_ref()` is the safe BACKREF accessor — `vm.worker` points at the - // heap `WebWorker` owned by C++ that outlives `vm`. `&WebWorker` (not - // `&mut`) — see worker-thread `&self` note. let worker = vm.worker_ref().expect("Assertion failure: no worker"); + let proxy = worker.proxy; let format_result = jsc::console_object::format2( jsc::console_object::MessageLevel::Debug, @@ -1230,12 +1120,7 @@ fn on_unhandled_rejection( // last-resort error handler and about to arm termination. let error_message = BunString::clone_utf8(&array); if jsc::host_fn::from_js_host_call_generic(global_object, || { - WebWorker__dispatchError( - global_object, - worker.messaging_proxy, - error_message, - error_instance, - ); + proxy.dispatch_error(global_object, error_message, error_instance); }) .is_err() { @@ -1246,6 +1131,7 @@ fn on_unhandled_rejection( // termination exception makes dispatchExitInternal skip 'exit' (as terminate() should), // and its processIsExiting guard stops shutdown() from running them twice. virtual_machine::ExitHandler::dispatch_on_exit(vm); + let worker = vm.worker_ref().expect("Assertion failure: no worker"); let _ = worker.set_requested_terminate(); // Do NOT call `worker.shutdown()` here — // `shutdown()` RETURNS, so calling it here would destroy @@ -1265,15 +1151,9 @@ fn on_unhandled_rejection( /// Resolve a worker entry-point specifier to a path the module loader can /// consume. The returned slice is BORROWED — it aliases `str`, the /// standalone module graph, or the resolver's arena; the caller must NOT -/// free it. -/// -/// # Safety -/// `parent` must point at this thread's live `VirtualMachine`. Passed as a raw -/// pointer (not `&mut`) because callers hold other borrows into the VM (its -/// log) across the call; per-use `(*parent)` derefs keep any autoref scoped to -/// the single expression. -unsafe fn resolve_entry_point_specifier<'s>( - parent: *mut VirtualMachine, +/// free it. `parent` is the calling thread's VM. +fn resolve_entry_point_specifier<'s>( + parent: &mut VirtualMachine, str: &'s [u8], error_message: &mut BunString, log: &mut bun_ast::Log, @@ -1281,8 +1161,7 @@ unsafe fn resolve_entry_point_specifier<'s>( // In a `bun build --compile` executable, a relative specifier names an embedded entry point (relative to the // embedded root) before it names a file on disk, and an absolute one may be an embedded path in either syntax // (`new URL("./w.ts", import.meta.url)`). - // SAFETY: per fn contract; `standalone_module_graph` is a read-only field. - if let Some(graph) = unsafe { (*parent).standalone_module_graph } + if let Some(graph) = parent.standalone_module_graph && let Some(name) = graph.resolve(graph.base_public_path_with_default_suffix(), str) { return Some(name); @@ -1310,18 +1189,10 @@ unsafe fn resolve_entry_point_specifier<'s>( } } - // SAFETY: per fn contract; `global` is a read-only field, and the resolver - // (`transpiler`) is mutated only on `parent`'s owning thread — both call - // sites (`create()` on the parent thread, `spin()` on the worker thread) - // satisfy that. - let global = unsafe { (*parent).global }; - // SAFETY: same as above — `parent`'s `transpiler` is mutated only on its - // owning thread (the caller's thread per fn contract). - let resolved_entry_point = match unsafe { (*parent).transpiler.resolve_entry_point(str) } { + let resolved_entry_point = match parent.transpiler.resolve_entry_point(str) { Ok(r) => r, Err(_) => { - // `global` valid for VM lifetime; safe ZST-handle deref (panics on null). - let global = JSGlobalObject::opaque_ref(global); + let global = parent.global(); let out: jsc::JsResult = (|| { let out = log.to_js(global, format_args!("Error resolving Worker entry point"))?; out.to_bun_string(global) diff --git a/src/jsc/worker_messaging_proxy.rs b/src/jsc/worker_messaging_proxy.rs new file mode 100644 index 000000000000..1e26dd3a6443 --- /dev/null +++ b/src/jsc/worker_messaging_proxy.rs @@ -0,0 +1,79 @@ +//! FFI surface of `WebCore::WorkerMessagingProxy` (WorkerMessagingProxy.h), +//! the parent<->worker relationship object a [`WebWorker`](crate::WebWorker) +//! thread reports to. + +use bun_core::String as BunString; + +use crate::{JSGlobalObject, JSValue}; + +bun_opaque::opaque_ffi! { + /// `WebCore::WorkerMessagingProxy`. `ThreadSafeRefCounted` on the C++ side; + /// every entry point below is documented for the thread it is called from. + pub struct WorkerMessagingProxy; +} + +// SAFETY: the C++ object is `ThreadSafeRefCounted` and built to be reached from +// both the parent and the worker thread; `&WorkerMessagingProxy` covers no +// Rust-visible bytes (opaque ZST), so sharing it exposes no Rust state. +unsafe impl Send for WorkerMessagingProxy {} +// SAFETY: as above. +unsafe impl Sync for WorkerMessagingProxy {} + +unsafe extern "C" { + /// Worker thread: post 'online' to the parent, before the entry point runs. + safe fn WebWorker__workerThreadStarted(proxy: &WorkerMessagingProxy); + /// Worker thread: post `workerGlobalScopeStarted` to the parent and start + /// delivering queued messages. + safe fn WebWorker__workerGlobalScopeStarted( + proxy: &WorkerMessagingProxy, + global: &JSGlobalObject, + ); + /// Worker thread, last call: post `workerGlobalScopeDestroyed` to the + /// parent, which joins the thread from that task. + safe fn WebWorker__workerGlobalScopeDestroyed( + proxy: &WorkerMessagingProxy, + exit_code: i32, + stopped_by_parent: bool, + ); + /// Parent thread, while it is exiting: terminate, join and release the + /// worker thread (`WorkerMessagingProxy::parentContextWillDestroy`). + safe fn WebWorker__parentContextWillDestroy(proxy: &WorkerMessagingProxy); + /// Worker thread: dispatch 'error' on the worker's global scope, then + /// report it to the parent's `Worker` object. May leave an exception pending. + safe fn WebWorker__dispatchError( + global: &JSGlobalObject, + proxy: &WorkerMessagingProxy, + message: BunString, + err: JSValue, + ); +} + +impl WorkerMessagingProxy { + #[inline] + pub fn worker_thread_started(&self) { + WebWorker__workerThreadStarted(self) + } + + #[inline] + pub fn worker_global_scope_started(&self, global: &JSGlobalObject) { + WebWorker__workerGlobalScopeStarted(self, global) + } + + #[inline] + pub fn worker_global_scope_destroyed(&self, exit_code: i32, stopped_by_parent: bool) { + WebWorker__workerGlobalScopeDestroyed(self, exit_code, stopped_by_parent) + } + + /// Only from the thread that owns the worker's parent context; callers go + /// through `WebWorker::parent_context_will_destroy`, which checks that. + #[inline] + pub(crate) fn parent_context_will_destroy(&self) { + WebWorker__parentContextWillDestroy(self) + } + + /// May leave an exception pending on `global`. + #[inline] + pub fn dispatch_error(&self, global: &JSGlobalObject, message: BunString, err: JSValue) { + WebWorker__dispatchError(global, self, message, err) + } +} diff --git a/src/libuv_sys/libuv.rs b/src/libuv_sys/libuv.rs index 9175e67e2677..2088c9248a52 100644 --- a/src/libuv_sys/libuv.rs +++ b/src/libuv_sys/libuv.rs @@ -1506,6 +1506,31 @@ impl Timer { panic!("internal error: uv_timer_stop failed"); } } + + /// Run `callback` once on `loop_`'s thread `timeout_ms` from now (the + /// loop's cached time is refreshed first), from a ref'd timer that then + /// closes and frees itself. `loop_` is the caller's live loop, as for + /// [`init`](Self::init). + pub fn one_shot(loop_: *mut Loop, timeout_ms: u64, callback: fn()) { + unsafe extern "C" fn fire(handle: *mut Timer) { + // SAFETY: `handle` is the box `one_shot` leaked; `data` holds the `fn()`. + let callback: fn() = unsafe { mem::transmute::<*mut c_void, fn()>((*handle).data) }; + callback(); + // SAFETY: as above; libuv is done with the handle once `free` runs. + unsafe { (*handle).close(free) }; + } + unsafe extern "C" fn free(handle: *mut Timer) { + // SAFETY: the box `one_shot` leaked; closed, so libuv holds no pointer to it. + drop(unsafe { Box::from_raw(handle) }); + } + // SAFETY: `loop_` is a live initialised loop (fn contract, as `uv_timer_init` below). + unsafe { uv_update_time(loop_) }; + let timer: &mut Timer = Box::leak(Box::new(bun_core::ffi::zeroed::())); + timer.init(loop_); + timer.data = callback as *mut c_void; + timer.start(timeout_ms, 0, Some(fire)); + timer.ref_(); + } } // ────────────────────────────────────────────────────────────────────────── diff --git a/src/runtime/cli/run_command.rs b/src/runtime/cli/run_command.rs index ffa4802bb9d0..a33b47846699 100644 --- a/src/runtime/cli/run_command.rs +++ b/src/runtime/cli/run_command.rs @@ -1387,24 +1387,10 @@ impl Run<'_> { // ── hot-reloader enable ───────────────────────────────────────────── match ctx.debug.hot_reload { cli::command::HotReload::Hot => { - // SAFETY: `vm` is the boxed-and-leaked main-thread VM - // (process-lifetime); it outlives the leaked reloader. - unsafe { - bun_jsc::hot_reloader::HotReloader::enable_hot_module_reloading( - std::ptr::from_mut::(vm), - Some(entry), - ) - } + bun_jsc::hot_reloader::HotReloader::enable_hot_module_reloading(vm, Some(entry)) } cli::command::HotReload::Watch => { - // SAFETY: `vm` is the boxed-and-leaked main-thread VM - // (process-lifetime); it outlives the leaked reloader. - unsafe { - bun_jsc::hot_reloader::WatchReloader::enable_hot_module_reloading( - std::ptr::from_mut::(vm), - Some(entry), - ) - } + bun_jsc::hot_reloader::WatchReloader::enable_hot_module_reloading(vm, Some(entry)); bun_jsc::posix_signal_handle::enable_watch_mode_signals( ctx.debug.watch_kill_signal, ); @@ -1539,18 +1525,12 @@ impl Run<'_> { } result }; - // SAFETY: `vals[..1]` is the single stack `to_print`; null - // `ctype` routes to the VM's stdout/stderr default. - unsafe { - bun_jsc::ConsoleObject::message_with_type_and_level( - ::core::ptr::null_mut(), - bun_jsc::ConsoleObject::MessageType::Log, - bun_jsc::ConsoleObject::MessageLevel::Log, - vm.global(), - &raw const to_print, - 1, - ); - } + bun_jsc::ConsoleObject::message_with_type_and_level( + bun_jsc::ConsoleObject::MessageType::Log, + bun_jsc::ConsoleObject::MessageLevel::Log, + vm.global(), + &[to_print], + ); } vm.on_before_exit(); diff --git a/src/runtime/cli/test/ChangedFilesFilter.rs b/src/runtime/cli/test/ChangedFilesFilter.rs index 85e4d83e0bf7..68239b2e1bf7 100644 --- a/src/runtime/cli/test/ChangedFilesFilter.rs +++ b/src/runtime/cli/test/ChangedFilesFilter.rs @@ -367,16 +367,13 @@ pub(crate) fn init_watch_trigger() { fresh }; - // Process-lifetime singletons — store in the CLI arena so the borrows - // are legitimately `&'static`. + // Process-lifetime: the path lives in the CLI arena so the borrow is + // legitimately `&'static`. Both are set once here, before the watcher + // thread starts; see doc on `hot_reloader::WATCH_CHANGED_PATHS`. let arena = crate::cli::cli_arena(); - let set: &'static mut StringSet = arena.alloc(StringSet::new()); - // Written once on the main thread before the watcher thread starts; - // after that only the watcher thread touches these. See doc on - // `hot_reloader::WATCH_CHANGED_PATHS`. let _ = jsc::hot_reloader::WATCH_CHANGED_TRIGGER_FILE.set(arena.alloc(path).as_zstr()); let _ = jsc::hot_reloader::WATCH_CHANGED_PATHS - .set(jsc::hot_reloader::WatchChangedPaths::new(set)); + .set(jsc::hot_reloader::WatchChangedPaths::new(StringSet::new())); } } diff --git a/src/runtime/cli/test_command.rs b/src/runtime/cli/test_command.rs index cd5264d8837b..9f7aa32b482d 100644 --- a/src/runtime/cli/test_command.rs +++ b/src/runtime/cli/test_command.rs @@ -2295,24 +2295,10 @@ impl TestCommand { match vm.hot_reload { jsc::virtual_machine::HotReload::Hot => { - // SAFETY: `vm` is the process-lifetime main-thread VM; it - // outlives the leaked reloader. - unsafe { - jsc::hot_reloader::HotReloader::enable_hot_module_reloading( - std::ptr::from_mut::(vm), - None, - ); - } + jsc::hot_reloader::HotReloader::enable_hot_module_reloading(vm, None); } jsc::virtual_machine::HotReload::Watch => { - // SAFETY: `vm` is the process-lifetime main-thread VM; it - // outlives the leaked reloader. - unsafe { - jsc::hot_reloader::WatchReloader::enable_hot_module_reloading( - std::ptr::from_mut::(vm), - None, - ); - } + jsc::hot_reloader::WatchReloader::enable_hot_module_reloading(vm, None); } _ => {} } diff --git a/src/runtime/dispatch.rs b/src/runtime/dispatch.rs index ddb7a77ee97f..0a05d233c8ad 100644 --- a/src/runtime/dispatch.rs +++ b/src/runtime/dispatch.rs @@ -378,21 +378,16 @@ pub(crate) fn run_task( // ── hot-reload (early-returns from the drain loop) ─────────────── task_tag::HotReloadTask => { - let t = cast_ptr!(hot_reloader::HotReloadTask); - // The task was heap-allocated in `Task::enqueue`; `deinit` frees it. - // SAFETY: tag identifies pointee; live Box'd HotReloadTask. - unsafe { (*t).run() }; - // SAFETY: paired with heap::alloc in `Task::enqueue`. - unsafe { hot_reloader::HotReloadTask::deinit(t) }; - return Ok(RunTaskResult::EarlyReturn); + // SAFETY: boxed by `hot_reloader::Task::enqueue`; the arm consumes it. + if unsafe { bun_core::heap::take(cast_ptr!(hot_reloader::HotReloadTask)) }.run() { + return Ok(RunTaskResult::EarlyReturn); + } } task_tag::WatchReloadTask => { - let t = cast_ptr!(hot_reloader::WatchReloadTask); - // SAFETY: tag identifies pointee; live Box'd WatchReloadTask. - unsafe { (*t).run() }; - // SAFETY: paired with heap::alloc in `Task::enqueue`. - unsafe { hot_reloader::WatchReloadTask::deinit(t) }; - return Ok(RunTaskResult::EarlyReturn); + // SAFETY: boxed by `hot_reloader::Task::enqueue`; the arm consumes it. + if unsafe { bun_core::heap::take(cast_ptr!(hot_reloader::WatchReloadTask)) }.run() { + return Ok(RunTaskResult::EarlyReturn); + } } // ── bake dev-server (cold — hoisted to `run_task_cold`) ────────── task_tag::BakeHotReloadEvent => run_task_cold(task), diff --git a/src/runtime/hw_exports.rs b/src/runtime/hw_exports.rs index 8773e10d9d11..00d6ebd95d87 100644 --- a/src/runtime/hw_exports.rs +++ b/src/runtime/hw_exports.rs @@ -306,20 +306,12 @@ pub fn on_resolve_entry_point_result( callframe: &CallFrame, ) -> bun_jsc::JsResult { let result = callframe.argument(0); - // SAFETY: `vals[..len]` is the single stack `result`; `ctype` is ignored by - // `message_with_type_and_level` (it always resolves the per-VM console via - // `vm_console(global)`), so null is fine. - unsafe { - bun_jsc::ConsoleObject::message_with_type_and_level( - core::ptr::null_mut(), - bun_jsc::ConsoleObject::MessageType::Log, - bun_jsc::ConsoleObject::MessageLevel::Log, - global, - &raw const result, - 1, - ); - } - // SAFETY: bun_vm() never null for a Bun-owned global. + bun_jsc::ConsoleObject::message_with_type_and_level( + bun_jsc::ConsoleObject::MessageType::Log, + bun_jsc::ConsoleObject::MessageLevel::Log, + global, + &[result], + ); bun_core::Global::exit(u32::from(global.bun_vm().as_mut().exit_handler.exit_code)); } @@ -329,20 +321,12 @@ pub fn on_reject_entry_point_result( callframe: &CallFrame, ) -> bun_jsc::JsResult { let result = callframe.argument(0); - // SAFETY: `vals[..len]` is the single stack `result`; `ctype` is ignored by - // `message_with_type_and_level` (it always resolves the per-VM console via - // `vm_console(global)`), so null is fine. - unsafe { - bun_jsc::ConsoleObject::message_with_type_and_level( - core::ptr::null_mut(), - bun_jsc::ConsoleObject::MessageType::Log, - bun_jsc::ConsoleObject::MessageLevel::Log, - global, - &raw const result, - 1, - ); - } - // SAFETY: bun_vm() never null for a Bun-owned global. + bun_jsc::ConsoleObject::message_with_type_and_level( + bun_jsc::ConsoleObject::MessageType::Log, + bun_jsc::ConsoleObject::MessageLevel::Log, + global, + &[result], + ); bun_core::Global::exit(u32::from(global.bun_vm().as_mut().exit_handler.exit_code)); } diff --git a/src/runtime/jsc_hooks.rs b/src/runtime/jsc_hooks.rs index c1025dde549f..ada84a821604 100644 --- a/src/runtime/jsc_hooks.rs +++ b/src/runtime/jsc_hooks.rs @@ -16,7 +16,6 @@ //! 4. `__bun_get_vm_ctx` / `__bun_stdio_blob_store_new` / //! `__bun_http_sync_download_*` — low-tier extern impls. -use bun_core::WTFStringImplExt as _; use bun_options_types::LoaderExt as _; use core::cell::Cell; use core::ffi::c_void; @@ -1566,15 +1565,12 @@ static __BUN_RUNTIME_HOOKS: RuntimeHooks = RuntimeHooks { // WebWorker / Debugger runtime hooks // ════════════════════════════════════════════════════════════════════════════ -/// Apply the standalone-executable graph's runtime flags to the transpiler. -/// -/// # Safety -/// `transpiler` is the worker VM's live `&mut Transpiler` (not yet visible to -/// any other thread); `graph` is the process-lifetime trait object whose data -/// pointer is a `bun_standalone_graph::Graph` (the only implementor — set in +/// Apply the standalone-executable graph's runtime flags to the (worker VM's) +/// transpiler. `graph` is the process-lifetime trait object whose data pointer +/// is a `bun_standalone_graph::Graph` (the only implementor — set in /// `init_with_module_graph` / inherited from the parent VM). -unsafe fn apply_standalone_runtime_flags( - transpiler: *mut bun_bundler::Transpiler<'static>, +fn apply_standalone_runtime_flags( + transpiler: &mut bun_bundler::Transpiler<'static>, graph: &'static dyn bun_resolver::StandaloneModuleGraph, ) { // SAFETY: per fn contract — sole implementor; trait-object data pointer IS @@ -1587,30 +1583,20 @@ unsafe fn apply_standalone_runtime_flags( .cast::() .cast::() }; - // SAFETY: per fn contract. - crate::run_main::apply_standalone_runtime_flags(unsafe { &mut *transpiler }, graph); + crate::run_main::apply_standalone_runtime_flags(transpiler, graph); } /// Scan a Worker's `execArgv` for `--no-addons` and `--no-ffi-cc`. Like the /// CLI parser, the scan stops at the first positional. -/// -/// # Safety -/// Each `WTFStringImpl` in `exec_argv` is a live WTF string (the C++ -/// `Worker::create` array, kept alive for the worker's lifetime). -unsafe fn parse_worker_exec_argv_flags( - exec_argv: &[bun_core::WTFStringImpl], -) -> Option { +fn parse_worker_exec_argv_flags(exec_argv: &[bun_core::String]) -> Option { let mut flags = WorkerExecArgvFlags { allow_addons: true, allow_ffi_cc: true, }; - for &arg in exec_argv { - if arg.is_null() { - continue; - } - // SAFETY: per fn contract — `arg` is a live `WTFStringImpl*`. - let owned = unsafe { &*arg }.to_owned_slice_z(); - let bytes = owned.as_bytes(); + for arg in exec_argv { + let owned = arg.to_utf8(); + let bytes = owned.slice(); + // `stop_after_positional_at = 1` — first non-flag token ends parsing. if bytes.first() != Some(&b'-') { break; } @@ -1861,12 +1847,10 @@ fn stop_active_handles(vm: &mut VirtualMachine, reason: StopReason) -> SweepResu /// Returns `next_test_id` advanced past every ID assigned (the caller writes /// it back into `TestReporterAgent::next_test_id`). /// -/// # Safety -/// `agent` is a live C++ `Inspector::TestReporterAgent::Handle*` (just stored -/// into `debugger.test_reporter_agent.handle` by the caller). Called on the JS -/// thread. -unsafe fn retroactively_report_discovered_tests( - agent: *mut bun_jsc::debugger::TestReporterHandle, +/// `agent` is the live C++ `Inspector::TestReporterAgent::Handle` (just stored +/// into `debugger.test_reporter_agent.handle` by the caller). JS thread. +fn retroactively_report_discovered_tests( + agent: &mut bun_jsc::debugger::TestReporterHandle, next_test_id: i32, ) -> i32 { use crate::test_runner::bun_test::{DescribeScope, Phase, TestScheduleEntry}; @@ -1910,7 +1894,7 @@ unsafe fn retroactively_report_discovered_tests( return max_id; fn retroactively_report_scope( - agent: *mut TestReporterHandle, + agent: &mut TestReporterHandle, scope: &mut DescribeScope, parent_id: i32, max_id: &mut i32, @@ -1928,8 +1912,7 @@ unsafe fn retroactively_report_discovered_tests( let name = bun_core::String::from_bytes( describe.base.name.as_deref().unwrap_or(b"(unnamed)"), ); - // SAFETY: `agent` is a live C++ handle (fn contract). - unsafe { &mut *agent }.report_test_found_with_location( + agent.report_test_found_with_location( test_id, &name, TestType::Describe, @@ -1955,8 +1938,7 @@ unsafe fn retroactively_report_discovered_tests( let name = bun_core::String::from_bytes( test_entry.base.name.as_deref().unwrap_or(b"(unnamed)"), ); - // SAFETY: `agent` is a live C++ handle (fn contract). - unsafe { &mut *agent }.report_test_found_with_location( + agent.report_test_found_with_location( test_id, &name, TestType::Test, diff --git a/src/runtime/node/node_process.rs b/src/runtime/node/node_process.rs index 619f925b98d3..960b3d99bc39 100644 --- a/src/runtime/node/node_process.rs +++ b/src/runtime/node/node_process.rs @@ -37,21 +37,6 @@ extern "C" fn get_exec_path(global_object: &JSGlobalObject) -> JSValue { EncodedSlice::from_bytes(out.as_bytes()).to_js(global_object) } -/// A worker's `argv`/`execArgv` strings live in its parent-thread -/// `WorkerOptions`; the worker thread gets its own copy (the parent may -/// atomize its impls), and an empty one is spelled as `BunString::EMPTY`. -pub(crate) fn worker_option_string(wtf: bun_core::WTFStringImpl) -> bun_core::String { - // SAFETY: non-null impl borrowed from the live `WorkerOptions`. - let imp = unsafe { &*wtf }; - if imp.length() == 0 { - bun_core::String::EMPTY - } else if imp.is_8bit() { - bun_core::String::clone_latin1(imp.latin1_slice()) - } else { - bun_core::String::clone_utf16(imp.utf16_slice()) - } -} - // ───────────────────────────── argv (C++ accessor wrappers) ───────────────── pub(crate) fn get_argv(global: &JSGlobalObject) -> JsResult { @@ -228,8 +213,8 @@ mod _impl { if let Some(worker) = vm.worker_ref() { // was explicitly overridden for the worker? if let Some(exec_argv) = worker.exec_argv() { - return JSValue::create_array_from_iter(global_object, exec_argv.iter(), |&wtf| { - super::worker_option_string(wtf).into_js(global_object) + return JSValue::create_array_from_iter(global_object, exec_argv.iter(), |arg| { + arg.to_bun_string().into_js(global_object) }); } } @@ -390,12 +375,7 @@ mod _impl { } if let Some(worker) = worker { - args_list.extend( - worker - .argv() - .iter() - .map(|&arg| super::worker_option_string(arg)), - ); + args_list.extend(worker.argv().iter().map(|arg| arg.to_bun_string())); } else { for arg in &vm.argv { let str_ = BunString::borrow_utf8(arg); diff --git a/src/runtime/shell/interpreter.rs b/src/runtime/shell/interpreter.rs index 3cdf1eba50c1..faec7557f9f6 100644 --- a/src/runtime/shell/interpreter.rs +++ b/src/runtime/shell/interpreter.rs @@ -1612,10 +1612,7 @@ impl Interpreter { int -= 1; } - if let Some(worker_ptr) = vm.worker { - // SAFETY: `vm.worker` is set in `VirtualMachine::init_worker` - // to a live `*WebWorker` for the worker's lifetime. - let worker = unsafe { &*worker_ptr.cast::() }; + if let Some(worker) = vm.worker_ref() { let argv = worker.argv(); if int as usize >= argv.len() { return; @@ -1623,8 +1620,7 @@ impl Interpreter { if vm_args_utf8.len() != argv.len() { vm_args_utf8.reserve(argv.len()); for arg in argv { - vm_args_utf8 - .push(crate::node::process::worker_option_string(*arg).into_utf8()); + vm_args_utf8.push(arg.to_utf8()); } } out.extend_from_slice(vm_args_utf8[int as usize].slice()); diff --git a/src/watcher/Watcher.rs b/src/watcher/Watcher.rs index 177487c6fe4c..827e1194c078 100644 --- a/src/watcher/Watcher.rs +++ b/src/watcher/Watcher.rs @@ -132,10 +132,51 @@ pub struct Watcher { pub(crate) ctx: *mut (), pub(crate) on_file_update: fn(*mut (), &mut [WatchEvent], &[ChangedFilePath], &WatchList), pub(crate) on_error: fn(*mut (), sys::Error), + /// Owned callback receiver (see [`Watcher::init_with_handler`]); when set, + /// `ctx`/`on_file_update`/`on_error` are unused. + pub(crate) handler: Option>, pub thread_lock: ThreadLock, } +/// A batch of file-change events being delivered to a [`WatcherHandler`] on +/// the watcher thread while `Watcher.mutex` is held. The handler may evict +/// entries through [`watcher_mut`](Self::watcher_mut) +/// (`remove_at_index::` + `flush_evictions`). +pub struct FileUpdateBatch<'a> { + watcher: &'a mut Watcher, + event_count: usize, + changed_count: usize, +} + +impl FileUpdateBatch<'_> { + #[inline] + pub fn events(&self) -> &[WatchEvent] { + &self.watcher.watch_events[..self.event_count] + } + /// Platform buffer of changed names inside watched directories; index it + /// with [`WatchEvent::names`]. + #[inline] + pub fn changed_files(&self) -> &[ChangedFilePath] { + &self.watcher.changed_filepaths[..self.changed_count] + } + #[inline] + pub fn watcher(&self) -> &Watcher { + self.watcher + } + #[inline] + pub fn watcher_mut(&mut self) -> &mut Watcher { + self.watcher + } +} + +/// Callback receiver owned by a [`Watcher`] (see [`Watcher::init_with_handler`]). +/// Both methods run on the watcher thread. +pub trait WatcherHandler: Send + 'static { + fn on_file_update(&mut self, batch: &mut FileUpdateBatch<'_>); + fn on_error(&mut self, err: sys::Error); +} + /// Context types passed to `Watcher::init` implement this trait. /// The default `on_watch_error` forwards to `on_error`. pub trait WatcherContext { @@ -187,13 +228,47 @@ impl Watcher { unsafe { (*ctx_opaque.cast::()).on_watch_error(err) } } + Self::init_raw( + ctx.cast::<()>(), + on_file_update_wrapped::, + on_error_wrapped::, + None, + top_level_dir, + ) + } + + /// [`init`](Self::init) with an owned callback receiver instead of a raw + /// context pointer. + pub fn init_with_handler( + handler: Box, + top_level_dir: &'static [u8], + ) -> Result, crate::Error> { + fn unused_update(_: *mut (), _: &mut [WatchEvent], _: &[ChangedFilePath], _: &WatchList) {} + fn unused_error(_: *mut (), _: sys::Error) {} + Self::init_raw( + core::ptr::null_mut(), + unused_update, + unused_error, + Some(handler), + top_level_dir, + ) + } + + fn init_raw( + ctx: *mut (), + on_file_update: fn(*mut (), &mut [WatchEvent], &[ChangedFilePath], &WatchList), + on_error: fn(*mut (), sys::Error), + handler: Option>, + top_level_dir: &'static [u8], + ) -> Result, crate::Error> { let this = Box::new(Watcher { watchlist: WatchList::default(), mutex: Mutex::default(), cwd: top_level_dir, - ctx: ctx.cast::<()>(), - on_file_update: on_file_update_wrapped::, - on_error: on_error_wrapped::, + ctx, + on_file_update, + on_error, + handler, platform: Platform::new(top_level_dir)?, watch_events: vec![WatchEvent::default(); MAX_COUNT].into_boxed_slice(), changed_filepaths: [const { None }; MAX_COUNT], @@ -221,14 +296,24 @@ impl Watcher { /// and `self.changed_filepaths[..changed_count]` and calls this instead of /// open-coding the lock/trace/callback sequence. pub(crate) fn dispatch_file_updates(&mut self, event_count: usize, changed_count: usize) { - let _guard = self.mutex.lock_guard(); - if !self.running.load() { - return; + self.mutex.lock(); + if self.running.load() { + let events = &mut self.watch_events[..event_count]; + let changed = &self.changed_filepaths[..changed_count]; + WatcherTrace::write_events(&self.watchlist, events, changed); + match self.handler.take() { + Some(mut handler) => { + handler.on_file_update(&mut FileUpdateBatch { + watcher: self, + event_count, + changed_count, + }); + self.handler = Some(handler); + } + None => (self.on_file_update)(self.ctx, events, changed, &self.watchlist), + } } - let events = &mut self.watch_events[..event_count]; - let changed = &self.changed_filepaths[..changed_count]; - WatcherTrace::write_events(&self.watchlist, events, changed); - (self.on_file_update)(self.ctx, events, changed, &self.watchlist); + self.mutex.unlock(); } pub fn start(&mut self) -> Result<(), crate::Error> { @@ -313,6 +398,13 @@ impl Watcher { } } + /// [`shutdown`](Self::shutdown) for the owning `Box` returned by + /// [`init`](Self::init) / [`init_with_handler`](Self::init_with_handler). + pub fn shutdown_boxed(self: Box, close_descriptors: bool) { + // SAFETY: `self` is the unique owning box, released here. + unsafe { Self::shutdown(Box::into_raw(self), close_descriptors) } + } + pub fn get_hash(filepath: &[u8]) -> HashType { bun_wyhash::hash(filepath) as HashType } @@ -359,7 +451,10 @@ impl Watcher { self.platform.stop(); let running = self.running.load(); if running { - (self.on_error)(self.ctx, err); + match self.handler.as_mut() { + Some(handler) => handler.on_error(err), + None => (self.on_error)(self.ctx, err), + } } running } diff --git a/src/watcher/lib.rs b/src/watcher/lib.rs index 76619cf6836e..71dccce7226a 100644 --- a/src/watcher/lib.rs +++ b/src/watcher/lib.rs @@ -35,7 +35,8 @@ pub use error::{Error, Result}; pub use WatchItemKind as Kind; pub use watcher_impl::{ - AnyResolveWatcher, ChangedFilePath, Event, FdOwnership, HashType, MAX_COUNT, + AnyResolveWatcher, ChangedFilePath, Event, FdOwnership, FileUpdateBatch, HashType, MAX_COUNT, MAX_EVICTION_COUNT, Op, PackageJSON, REQUIRES_FILE_DESCRIPTORS, WATCH_OPEN_FLAGS, WatchEvent, WatchItem, WatchItemColumns, WatchItemIndex, WatchItemKind, WatchList, Watcher, WatcherContext, + WatcherHandler, }; From ea638b8a06715d31266ae0caf9490e579404386d Mon Sep 17 00:00:00 2001 From: Jarred Sumner Date: Tue, 25 Aug 2026 06:54:05 +0000 Subject: [PATCH 2/7] BundleV2: drop the bust_dir_cache forwarder the hot reloader no longer calls --- src/bundler/bundle_v2.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/bundler/bundle_v2.rs b/src/bundler/bundle_v2.rs index 87dce7addba2..e4e3a971b530 100644 --- a/src/bundler/bundle_v2.rs +++ b/src/bundler/bundle_v2.rs @@ -7634,10 +7634,6 @@ pub mod bv2_impl { } } - /// To satisfy the interface from NewHotReloader() - pub fn bust_dir_cache(&mut self, path: &[u8]) -> bool { - self.transpiler.resolver.bust_dir_cache(path) - } } pub use bun_core::cheap_prefix_normalizer; From 6c0148e41ceb0d0dbebf67f8f52f28be097ea038 Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Tue, 25 Aug 2026 06:56:48 +0000 Subject: [PATCH 3/7] [autofix.ci] apply automated fixes --- src/bundler/bundle_v2.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/bundler/bundle_v2.rs b/src/bundler/bundle_v2.rs index e4e3a971b530..2bbda7739a9c 100644 --- a/src/bundler/bundle_v2.rs +++ b/src/bundler/bundle_v2.rs @@ -7633,7 +7633,6 @@ pub mod bv2_impl { this.on_after_decrement_scan_counter(); } } - } pub use bun_core::cheap_prefix_normalizer; From c843e056af29aebea3fa46dae27c0d883a276ef6 Mon Sep 17 00:00:00 2001 From: Jarred Sumner Date: Thu, 27 Aug 2026 11:04:00 +0000 Subject: [PATCH 4/7] Rebase fixups: BoxedTask/TaskHop take the napi-lifecycle shape (with refused + ConcurrentTask::create_boxed), FSWatchTask and the reload tasks go through it; EncodedSlice/Utf8Bytes spellings --- src/event_loop/ConcurrentTask.rs | 278 ++++++++++++++++++++++------ src/event_loop/lib.rs | 2 +- src/jsc/VmHandle.rs | 122 +++++++----- src/jsc/hot_reloader.rs | 48 ++--- src/jsc/web_worker.rs | 8 +- src/runtime/bake/dev_server/mod.rs | 8 +- src/runtime/dispatch.rs | 55 +++--- src/runtime/node/node_fs_watcher.rs | 25 ++- 8 files changed, 386 insertions(+), 160 deletions(-) diff --git a/src/event_loop/ConcurrentTask.rs b/src/event_loop/ConcurrentTask.rs index fe83894d1438..7d38b9fcfcd2 100644 --- a/src/event_loop/ConcurrentTask.rs +++ b/src/event_loop/ConcurrentTask.rs @@ -5,8 +5,8 @@ //! //! The task is run on a thread pool and then the result is returned to the main JavaScript thread. //! -//! If `auto_delete` is true, the task is automatically deallocated when it's finished. -//! Otherwise, it's expected that the containing struct will deallocate the task. +//! A heap carrier (`create*`) is deallocated by the event loop once its task is +//! dispatched; an intrusive one is part of the struct that owns it. use crate::ManagedTask; use bun_threading::UnboundedQueue; @@ -164,49 +164,111 @@ pub trait Taskable { unsafe fn release_unrun(this: *mut Self); } -/// A [`Taskable`] whose queued [`Task::ptr`] is always an owned `Box` -/// ([`Task::from_boxed`] / `VmHandle::post_boxed`): the dispatch arm for -/// `TAG` reclaims the box to run it, and [`release_unrun`](Self::release_unrun) -/// receives it when it will never run. Implemented through [`boxed_task!`], -/// which is where the tag ↔ type pairing is asserted. +/// A task whose queued pointer is a [`ThisPtr`](bun_ptr::ThisPtr) to +/// `Target`, which keeps itself alive for the task; the dispatcher runs it or +/// releases it through here. A zero-sized hop type per tag, declared with +/// [`task_hop!`] next to `Target` so the ref protocol lives there. /// /// # Safety -/// `TAG`'s arm in `bun_runtime::dispatch::run_task` must reclaim exactly a -/// `Box`; a mismatched tag makes the dispatcher reinterpret the box. +/// `TAG` is dispatched (in `bun_runtime::dispatch`) to this impl and no other +/// task type uses it; and `Target`'s protocol keeps the pointee of every +/// [`task`](Self::task) it queues alive until `run` / `release_unrun` (a +/// `RefPtr` slot held for the queued task, or an owner that outlives the queue). +pub unsafe trait TaskHop { + type Target; + /// The tag constant from [`task_tag`]; the `bun_runtime::dispatch` match + /// arms MUST agree. + const TAG: TaskTag; + fn run(this: bun_ptr::ThisPtr) -> crate::JsResult<()>; + /// As [`Taskable::release_unrun`]. + fn release_unrun(this: bun_ptr::ThisPtr); + + #[inline] + fn task(this: bun_ptr::ThisPtr) -> Task { + Task::new(Self::TAG, this.as_ptr().cast::<()>()) + } +} + +/// Declares `$hop`, the [`TaskHop`] that `task_tag::$tag` dispatches to for +/// `$target`, forwarding to `$run` / `$release`. The doc comment on the +/// invocation is where `$target`'s liveness protocol for the queued task is +/// stated. +#[macro_export] +macro_rules! task_hop { + ($(#[$m:meta])* $v:vis $hop:ident for $target:ty => $tag:ident; run = $run:expr; release_unrun = $release:expr $(;)?) => { + $(#[$m])* + $v struct $hop; + // SAFETY: see macro doc — one hop per tag; the invoker documents the liveness protocol. + unsafe impl $crate::TaskHop for $hop { + type Target = $target; + const TAG: $crate::TaskTag = $crate::task_tag::$tag; + #[inline] + fn run(this: ::bun_ptr::ThisPtr<$target>) -> $crate::JsResult<()> { + ($run)(this) + } + #[inline] + fn release_unrun(this: ::bun_ptr::ThisPtr<$target>) { + ($release)(this) + } + } + }; +} + +/// A task that owns its payload: queued as the `Box` leaked into +/// [`Task::ptr`], handed back as that box when the dispatcher runs or +/// releases it. Implement via [`boxed_task!`]. +/// +/// # Safety +/// `TAG` is dispatched (in `bun_runtime::dispatch`) to this impl and no other +/// task type uses it, so the dispatcher's `Box::::from_raw` gets back +/// what [`into_task`](Self::into_task) leaked. pub unsafe trait BoxedTask: Sized { - /// See [`Taskable::TAG`]. + /// The tag constant from [`task_tag`]; the `bun_runtime::dispatch` match + /// arms MUST agree. const TAG: TaskTag; - /// See [`Taskable::release_unrun`]. + fn run(self: Box) -> crate::JsResult<()>; + /// As [`Taskable::release_unrun`]. fn release_unrun(self: Box); -} + /// A weak post was refused: the VM this was for has closed (any thread; + /// its heap may be gone). Free the task without touching that VM. + fn refused(self: Box); -impl Taskable for T { - const TAG: TaskTag = T::TAG; - unsafe fn release_unrun(this: *mut Self) { - // SAFETY: fn contract — `this` was queued under `T::TAG`, which for a - // `BoxedTask` is only ever the box `Task::from_boxed` leaked. - BoxedTask::release_unrun(unsafe { bun_core::heap::take(this) }); + /// The JS thread's own enqueue (`EventLoop::enqueue_task`), which cannot + /// be refused; off-thread posts go through `ConcurrentTask::create_boxed`. + #[inline] + fn into_task(self: Box) -> Task { + Task::new(Self::TAG, Box::into_raw(self).cast::<()>()) } } -/// Pair a boxed task type with its dispatch tag: -/// -/// ```ignore -/// bun_event_loop::boxed_task! { -/// impl[const RI: bool] for MyTask => task_tag::MyTask; -/// fn release_unrun(self) { drop(self) } -/// } -/// ``` -/// -/// The tag named here and the `run_task` arm that `heap::take`s this type are -/// the two halves of one contract; keep them next to each other in review. +/// Implements [`BoxedTask`] for `$ty` as the task `task_tag::$tag` dispatches +/// to, forwarding to `$run` / `$release` / `$refused` (each `fn(Box<$ty>)`). +/// The `impl[generics] $ty => $tag_expr` form is for a generic `$ty` whose +/// tag depends on its parameters. #[macro_export] macro_rules! boxed_task { - (impl$([$($gen:tt)*])? for $ty:ty => $tag:expr; fn release_unrun($this:ident) $body:block) => { - // SAFETY: the invoker names the `run_task` arm that reclaims `Box<$ty>`. - unsafe impl$(<$($gen)*>)? $crate::BoxedTask for $ty { + ($ty:ty => $tag:ident; run = $run:expr; release_unrun = $release:expr; refused = $refused:expr $(;)?) => { + $crate::boxed_task! { + impl[] $ty => $crate::task_tag::$tag; + run = $run; release_unrun = $release; refused = $refused; + } + }; + (impl[$($gen:tt)*] $ty:ty => $tag:expr; run = $run:expr; release_unrun = $release:expr; refused = $refused:expr $(;)?) => { + // SAFETY: see macro doc — one boxed task type per tag. + unsafe impl<$($gen)*> $crate::BoxedTask for $ty { const TAG: $crate::TaskTag = $tag; - fn release_unrun($this: ::std::boxed::Box) $body + #[inline] + fn run(self: ::std::boxed::Box) -> $crate::JsResult<()> { + ($run)(self) + } + #[inline] + fn release_unrun(self: ::std::boxed::Box) { + ($release)(self) + } + #[inline] + fn refused(self: ::std::boxed::Box) { + ($refused)(self) + } } }; } @@ -252,6 +314,10 @@ impl Taskable for crate::ManagedTask::ManagedTask { } // ──────────────────────────────────────────────────────────────────────────── +/// How a heap carrier (`ConcurrentTask::create*`) frees itself, and the +/// payload if the carrier owns it, when a weak post is refused. +type ReleaseRefused = unsafe fn(core::ptr::NonNull); + #[repr(C)] pub struct ConcurrentTask { pub task: Task, @@ -259,10 +325,13 @@ pub struct ConcurrentTask { /// path (`atomic_store_next`, called once per completed work-pool task via /// `enqueue_task_concurrent`) is a single release-store — no read-modify-write. pub next: Link, - /// If `true`, the task is heap-owned and freed by the event loop after - /// dispatch. Immutable after construction; read only on the consumer thread, - /// so it does not need to share a word with the contended `next` link. - pub auto_delete: bool, + /// `Some` on a heap carrier (`create*`): the event loop frees the carrier + /// after dispatching its task, and a refused weak post frees carrier and + /// owned payload through it ([`ConcurrentTask::release_refused`]). `None` + /// on an intrusive carrier, which belongs to its container. Immutable after + /// construction; read only on the consumer thread, so it does not need to + /// share a word with the contended `next` link. + release_refused: Option, } impl Default for ConcurrentTask { @@ -272,12 +341,12 @@ impl Default for ConcurrentTask { // byte + raw pointer); caller must set a real task before use. task: unsafe { bun_core::ffi::zeroed_unchecked() }, next: Link::new(), - auto_delete: false, + release_refused: None, } } } -// `auto_delete` is deliberately its own field rather than packed into bit 0 +// `release_refused` is deliberately its own field rather than packed into bit 0 // of `next`: `Task` is already two words here (tag is not packed into the // pointer), so the struct was never 16B, and profiling (build/create-next // benches) showed @@ -288,7 +357,7 @@ impl Default for ConcurrentTask { const _: () = assert!( core::mem::size_of::() == core::mem::size_of::() + 2 * core::mem::size_of::(), - "ConcurrentTask = Task + next ptr + auto_delete (padded)" + "ConcurrentTask = Task + next ptr + release_refused" ); // SAFETY: `link()` always projects to the same embedded `next: Link` @@ -319,11 +388,55 @@ impl ConcurrentTask { bun_core::heap::into_raw(Box::new(init)) } + /// A heap carrier for `task`, whose payload the carrier does not own + /// (intrusive, or freed by whoever posts it) — except a `ManagedTask`, + /// which it does. pub fn create(task: Task) -> core::ptr::NonNull { + /// # Safety + /// `this` is a refused heap carrier. + unsafe fn carrier_only(this: core::ptr::NonNull) { + // SAFETY: fn contract; `create*` boxed it. + drop(unsafe { bun_core::heap::take(this.as_ptr()) }); + } + /// # Safety + /// `this` is a refused heap carrier whose payload is a `ManagedTask`. + unsafe fn managed(this: core::ptr::NonNull) { + // SAFETY: fn contract; refused ⇒ both boxes are ours. + unsafe { + let task = bun_core::heap::take(this.as_ptr()).task; + crate::ManagedTask::ManagedTask::release(task.ptr.cast()); + } + } + let release: ReleaseRefused = if task.tag == task_tag::ManagedTask { + managed + } else { + carrier_only + }; + Self::create_with(task, release) + } + + /// A heap carrier owning the boxed `task`: a refused weak post hands it to + /// [`BoxedTask::refused`]. + pub fn create_boxed(task: Box) -> core::ptr::NonNull { + /// # Safety + /// `this` is a refused heap carrier built by `create_boxed::`. + unsafe fn boxed(this: core::ptr::NonNull) { + // SAFETY: fn contract; refused ⇒ both boxes are ours, and the + // payload is the `Box` `into_task` leaked. + let task = unsafe { + let task = bun_core::heap::take(this.as_ptr()).task; + Box::from_raw(task.ptr.cast::()) + }; + T::refused(task); + } + Self::create_with(task.into_task(), boxed::) + } + + fn create_with(task: Task, release: ReleaseRefused) -> core::ptr::NonNull { let raw = ConcurrentTask::new(ConcurrentTask { task, next: Link::new(), - auto_delete: true, + release_refused: Some(release), }); // SAFETY: `new` heap-allocates via `heap::into_raw` — never null. unsafe { core::ptr::NonNull::new_unchecked(raw) } @@ -349,12 +462,23 @@ impl ConcurrentTask { of: *mut T, auto_deinit: AutoDeinit, ) -> &mut ConcurrentTask { - bun_core::mark_binding!(); - *self = ConcurrentTask { - task: Task::init(of), + debug_assert!(auto_deinit == AutoDeinit::ManualDeinit); + self.from_task(Task::init(of)) + } + + /// An intrusive carrier loaded with `task`, ready to post. + pub const fn intrusive(task: Task) -> ConcurrentTask { + ConcurrentTask { + task, next: Link::new(), - auto_delete: auto_deinit == AutoDeinit::AutoDeinit, - }; + release_refused: None, + } + } + + /// Load this intrusive carrier with `task`, ready to post. + pub fn from_task(&mut self, task: Task) -> &mut ConcurrentTask { + bun_core::mark_binding!(); + *self = Self::intrusive(task); self } @@ -375,25 +499,65 @@ impl ConcurrentTask { } /// A weak poster got `task` back because the target VM has closed: free - /// it if it is a heap task (`create*`); an intrusive one belongs to its - /// container. + /// it if it is a heap carrier (`create*`), with the payload if the carrier + /// owns it; an intrusive one belongs to its container. /// /// # Safety /// `task` was just refused and is not queued anywhere. pub unsafe fn release_refused(task: core::ptr::NonNull) { - // SAFETY: fn contract. - let inner = unsafe { Self::into_task(task) }; - // A callback task (`from_callback`, `ManagedTask::new*`) owns a heap - // `ManagedTask` behind `task.ptr` as well. - if inner.tag == crate::task_tag::ManagedTask { - // SAFETY: as above; refused ⇒ ours. - unsafe { crate::ManagedTask::ManagedTask::release(inner.ptr.cast()) }; + // SAFETY: fn contract; `release_refused` was installed by the + // constructor that knows the carrier's and payload's ownership. + unsafe { + if let Some(release) = task.as_ref().release_refused { + release(task); + } } } - /// Returns whether this task should be automatically deallocated after execution. + /// Whether this is a heap carrier (`create*`) the event loop frees after + /// dispatching its task. #[inline] pub fn auto_delete(&self) -> bool { - self.auto_delete + self.release_refused.is_some() + } +} + +#[cfg(test)] +mod tests { + use super::*; + use core::sync::atomic::{AtomicUsize, Ordering}; + + static REFUSED: AtomicUsize = AtomicUsize::new(0); + static DROPPED: AtomicUsize = AtomicUsize::new(0); + + struct Payload(#[allow(dead_code)] Box<[u8; 64]>); + impl Drop for Payload { + fn drop(&mut self) { + DROPPED.fetch_add(1, Ordering::SeqCst); + } + } + crate::boxed_task! { + Payload => NapiFinalizerTask; + run = |_task: Box| Ok(()); + release_unrun = |_task: Box| {}; + refused = |_task: Box| { REFUSED.fetch_add(1, Ordering::SeqCst); }; + } + + #[test] + fn refused_boxed_carrier_releases_its_payload() { + let carrier = ConcurrentTask::create_boxed(Box::new(Payload(Box::new([7; 64])))); + // SAFETY: never queued; ours. + unsafe { ConcurrentTask::release_refused(carrier) }; + assert_eq!(REFUSED.load(Ordering::SeqCst), 1); + assert_eq!(DROPPED.load(Ordering::SeqCst), 1); + + let mut intrusive = + ConcurrentTask::intrusive(Box::new(Payload(Box::new([7; 64]))).into_task()); + // SAFETY: never queued; an intrusive carrier is left alone. + unsafe { ConcurrentTask::release_refused(core::ptr::NonNull::from(&mut intrusive)) }; + assert_eq!(DROPPED.load(Ordering::SeqCst), 1); + // SAFETY: the payload leaked into the intrusive carrier above. + drop(unsafe { Box::from_raw(intrusive.task.ptr.cast::()) }); + assert_eq!(DROPPED.load(Ordering::SeqCst), 2); } } diff --git a/src/event_loop/lib.rs b/src/event_loop/lib.rs index 78a284a09bde..0bcc34362dc6 100644 --- a/src/event_loop/lib.rs +++ b/src/event_loop/lib.rs @@ -29,7 +29,7 @@ pub mod any_event_loop; // ─── public surface ───────────────────────────────────────────────────────── pub type JsResult = core::result::Result; -pub use ConcurrentTask::{BoxedTask, Task, TaskTag, Taskable, task_tag}; +pub use ConcurrentTask::{BoxedTask, Task, TaskHop, TaskTag, Taskable, task_tag}; // snake_case alias for the file-level-struct module so higher tiers avoid // the type/module namespace collision on the PascalCase form. diff --git a/src/jsc/VmHandle.rs b/src/jsc/VmHandle.rs index 30496aa5303e..e8a88581610c 100644 --- a/src/jsc/VmHandle.rs +++ b/src/jsc/VmHandle.rs @@ -335,35 +335,66 @@ impl VmHandle { /// the VM is closed. For posters that hold no ticket (their payload is /// their own to free on refusal). pub fn post(&self, kind: LoopKind, task: NonNull) -> Posted { - test_gate::weak_post(&self.0, task, |task| { - let Some(_a) = self.enter() else { - return Posted::Refused(task); - }; - self.0.deliver(kind, task); + // SAFETY: handed over by the caller and not yet queued anywhere. + let tag = unsafe { task.as_ref() }.task.tag; + if self.post_with(|| tag, |s| s.deliver(kind, task)) { Posted::Queued - }) + } else { + Posted::Refused(task) + } } - /// Queue an owned `T` on the VM's `kind` loop (dispatched under `T::TAG`, - /// whose arm reclaims the box), or hand it back if the VM is closed. - pub fn post_boxed( + /// The weak-post gate: `deliver` runs iff the VM is not closed (and then + /// inside the gate, so the close waits for it). Whether it ran. `tag` is + /// only for the debug build's late-post report. + fn post_with( &self, - kind: LoopKind, - task: Box, - ) -> Result<(), Box> { - let raw: *mut T = bun_core::heap::into_raw(task); - let ct = ConcurrentTaskItem::create(bun_event_loop::Task::init(raw)); - match self.post(kind, ct) { - Posted::Queued => Ok(()), - Posted::Refused(ct) => { - // SAFETY: refused ⇒ nothing else holds the carrier `create` boxed - // or the payload we boxed into it two lines up. - unsafe { - drop(bun_core::heap::take(ct.as_ptr())); - Err(bun_core::heap::take(raw)) - } + tag: impl FnOnce() -> bun_event_loop::TaskTag, + deliver: impl FnOnce(&Shared), + ) -> bool { + test_gate::before_weak_post(&self.0); + let queued = match self.enter() { + Some(_a) => { + deliver(&self.0); + true } + None => false, + }; + test_gate::after_weak_post(&self.0, tag, queued); + queued + } + + /// Queue `H`'s hop for `this` on the VM's `kind` loop and wake it. `false` + /// if the VM is closed: nothing was queued, and whatever the hop would have + /// carried is still the caller's. + pub fn post_hop( + &self, + kind: LoopKind, + this: bun_ptr::ThisPtr, + ) -> bool { + self.post_with( + || H::TAG, + |s| s.deliver(kind, ConcurrentTaskItem::create(H::task(this))), + ) + } + + /// Queue a [`BoxedTask`](bun_event_loop::BoxedTask) on the VM's `kind` + /// loop and wake it, or hand it to + /// [`BoxedTask::refused`](bun_event_loop::BoxedTask::refused) if the VM is + /// closed. Whether it was queued. + pub fn post_boxed(&self, kind: LoopKind, task: Box) -> bool { + let mut task = Some(task); + let queued = self.post_with( + || T::TAG, + |s| { + let task = task.take().expect("posted once"); + s.deliver(kind, ConcurrentTaskItem::create_boxed(task)); + }, + ); + if let Some(task) = task { + T::refused(task); } + queued } /// Queue a C++ `EventLoopTask` on the VM's `kind` loop from another @@ -565,8 +596,7 @@ pub extern "C" fn Bun__VM__currentLoopKind(vm: &VirtualMachine) -> LoopKind { // under the gate, not in production. #[cfg(debug_assertions)] mod test_gate { - use super::{Ordering, Posted, Shared, State, Ticket, VmHandle}; - type Task = core::ptr::NonNull; + use super::{Ordering, Shared, State, Ticket, VmHandle}; impl VmHandle { pub(crate) fn arm_test_gate(&self) { @@ -614,20 +644,24 @@ mod test_gate { )); } } - pub(super) fn weak_post(s: &Shared, task: Task, post: impl FnOnce(Task) -> Posted) -> Posted { - if !armed(s) { - return post(task); + pub(super) fn before_weak_post(s: &Shared) { + if armed(s) { + park_until_draining(s); + } + } + pub(super) fn after_weak_post( + s: &Shared, + tag: impl FnOnce() -> bun_event_loop::TaskTag, + queued: bool, + ) { + if armed(s) { + let outcome = if queued { + "released by the wait" + } else { + "refused" + }; + say(format_args!("late post: {} ({outcome})", tag().name())); } - park_until_draining(s); - // SAFETY: handed over by the caller and not yet queued anywhere. - let tag = unsafe { task.as_ref() }.task.tag; - let r = post(task); - let outcome = match r { - Posted::Queued => "released by the wait", - Posted::Refused(_) => "refused", - }; - say(format_args!("late post: {} ({outcome})", tag.name())); - r } /// The wait began: parked posts go now. pub(super) fn draining(h: &VmHandle) { @@ -643,8 +677,7 @@ mod test_gate { } #[cfg(not(debug_assertions))] mod test_gate { - use super::{Posted, Shared, Ticket, VmHandle}; - type Task = core::ptr::NonNull; + use super::{Shared, Ticket, VmHandle}; impl VmHandle { #[inline(always)] pub(crate) fn arm_test_gate(&self) {} @@ -652,8 +685,13 @@ mod test_gate { #[inline(always)] pub(super) fn before_ticket_post(_: &Ticket) {} #[inline(always)] - pub(super) fn weak_post(_: &Shared, task: Task, post: impl FnOnce(Task) -> Posted) -> Posted { - post(task) + pub(super) fn before_weak_post(_: &Shared) {} + #[inline(always)] + pub(super) fn after_weak_post( + _: &Shared, + _: impl FnOnce() -> bun_event_loop::TaskTag, + _: bool, + ) { } #[inline(always)] pub(super) fn draining(_: &VmHandle) {} diff --git a/src/jsc/hot_reloader.rs b/src/jsc/hot_reloader.rs index f43baef5cdc3..d86bb5dbdbec 100644 --- a/src/jsc/hot_reloader.rs +++ b/src/jsc/hot_reloader.rs @@ -98,8 +98,8 @@ impl HotReloaderCtx for VirtualMachine { fn post_reload_task( handle: &crate::VmHandle, task: Box>, - ) -> Result<(), Box>> { - handle.post_boxed(crate::LoopKind::Regular, task) + ) { + handle.post_boxed(crate::LoopKind::Regular, task); } fn reload(&self, _task: &dyn HotReloadTaskView) { @@ -187,12 +187,12 @@ pub trait HotReloaderCtx: Sized + 'static { fn reload_handle(&self) -> Option; /// Watcher thread: queue `task` on the owning thread through `handle` - /// (from [`reload_handle`](Self::reload_handle)); it comes back if the - /// owner is already closed. Only contexts with a handle are ever asked. + /// (from [`reload_handle`](Self::reload_handle)); it is dropped unrun if + /// the owner is already closed. Only contexts with a handle are ever asked. fn post_reload_task( handle: &crate::VmHandle, task: Box>, - ) -> Result<(), Box>>; + ); /// Called from `Task::run` (owning thread) to perform the actual reload. /// The const-generic task is erased via the `HotReloadTaskView` so this @@ -454,15 +454,15 @@ pub struct Task EventLoopType>, } -// Only the VM's tasks are ever queued (`bun build --watch` has no loop); the -// `run_task` arms for these two tags `heap::take` exactly these types. +// Only the VM's tasks are ever queued (`bun build --watch` has no loop). A +// task released unrun or refused is a file change that will not reload +// anything: dropping it is all there is to do. bun_event_loop::boxed_task! { - impl[const RELOAD_IMMEDIATELY: bool] for Task => + impl[const RELOAD_IMMEDIATELY: bool] Task => if RELOAD_IMMEDIATELY { task_tag::WatchReloadTask } else { task_tag::HotReloadTask }; - fn release_unrun(self) { - // A file change the watcher thread posted that will not reload anything. - drop(self) - } + run = |task: Box| { task.run(); Ok(()) }; + release_unrun = drop; + refused = drop; } impl @@ -481,17 +481,23 @@ where } } - /// Owning thread. Returns whether a reload ran (the dispatcher then leaves - /// the tick early without draining microtasks). + /// Whether [`run`](Self::run) reloads (the dispatcher then leaves the + /// tick early without draining microtasks) or only busts directory caches. + pub fn reloads(&self) -> bool { + self.count > 0 + } + + /// Owning thread: bust the pending directory caches, then reload if this + /// batch carries changed files. #[allow(clippy::boxed_local, reason = "reclaim point for the boxed task")] - pub fn run(self: Box) -> bool { + pub fn run(self: Box) { let ctx = self.ctx.get(); let dirs = core::mem::take(&mut *self.pending.dirs.lock()); for dir in &dirs { let _ = ctx.bust_dir_cache(dir); } if self.count == 0 { - return false; + return; } // Since we rely on the event loop for hot reloads, there can be // a delay before the next reload begins. In the time between the @@ -505,7 +511,6 @@ where while self.pending.count.swap(0, Ordering::Relaxed) > 0 { ctx.reload(&*self); } - true } pub(crate) fn append( @@ -570,8 +575,7 @@ where pending: Arc::clone(&self.pending), _event_loop: PhantomData, }); - // VM torn down while a change was pending: the box comes back and drops. - let _ = Ctx::post_reload_task(handle, task); + Ctx::post_reload_task(handle, task); } self.dirs.clear(); @@ -1196,10 +1200,10 @@ impl HotReloaderCtx for bun_bundler::BundleV2<'static> { fn post_reload_task( _handle: &crate::VmHandle, - task: Box>, - ) -> Result<(), Box>> { + _task: Box>, + ) { // No handle (see `reload_handle`), so never asked. - Err(task) + unreachable!() } fn reload(&self, _task: &dyn HotReloadTaskView) { diff --git a/src/jsc/web_worker.rs b/src/jsc/web_worker.rs index f5c96e7aae1f..3f2b34ec41b9 100644 --- a/src/jsc/web_worker.rs +++ b/src/jsc/web_worker.rs @@ -50,7 +50,9 @@ use bun_threading::Guarded; use crate::virtual_machine::{self, VirtualMachine, WorkerVm, runtime_hooks}; use crate::worker_messaging_proxy::WorkerMessagingProxy; -use crate::{self as jsc, EncodedSliceJsc as _, JSGlobalObject, JSPromise, JSValue, JsError, LogJsc}; +use crate::{ + self as jsc, EncodedSliceJsc as _, JSGlobalObject, JSPromise, JSValue, JsError, LogJsc, +}; bun_core::define_scoped_log!(log, Worker, hidden); @@ -535,7 +537,6 @@ impl WebWorker { self.requested_terminate.swap(true, Ordering::Release) } - #[inline] pub(crate) fn hot_reload(&self) -> crate::virtual_machine::HotReload { self.hot_reload @@ -1079,7 +1080,8 @@ fn on_unhandled_rejection( // clone. Node reports a SyntaxError; build a real one from the formatted parse // error so the subtype reaches the parent intact. if let Some(bm) = error_instance.as_class_ref::() { - error_instance = EncodedSlice::utf8(&bm.msg.data.text).to_syntax_error_instance(global_object); + error_instance = + EncodedSlice::utf8(&bm.msg.data.text).to_syntax_error_instance(global_object); } let mut array: Vec = Vec::new(); diff --git a/src/runtime/bake/dev_server/mod.rs b/src/runtime/bake/dev_server/mod.rs index e8b18a04c1e0..a2fd9014146e 100644 --- a/src/runtime/bake/dev_server/mod.rs +++ b/src/runtime/bake/dev_server/mod.rs @@ -939,10 +939,10 @@ impl WatcherAtomics { // Not atomic because the dev server is not running events right now. (*this).dbg_server_event = Some(ev); } - (*ev).concurrent_task = bun_event_loop::ConcurrentTask::ConcurrentTask { - task: bun_event_loop::Task::init(ev), - ..Default::default() - }; + (*ev).concurrent_task = + bun_event_loop::ConcurrentTask::ConcurrentTask::intrusive( + bun_event_loop::Task::init(ev), + ); // The queued node pointer is derived from `ev` (allocation-root // provenance) so it stays valid across `Drop for DevServer`'s // writes. Refused ⇒ the VM is torn down; the event is one of diff --git a/src/runtime/dispatch.rs b/src/runtime/dispatch.rs index 0a05d233c8ad..8b5211975171 100644 --- a/src/runtime/dispatch.rs +++ b/src/runtime/dispatch.rs @@ -189,6 +189,15 @@ pub(crate) fn run_task( task.ptr.cast::<$ty>() }; } + /// `<$ty as BoxedTask>::run` on the queued box, SAFETY spelled once. + macro_rules! boxed { + ($ty:ty) => {{ + // SAFETY: §Dispatch — `task.tag` is `<$ty as BoxedTask>::TAG`, set by + // `BoxedTask::into_task` together with the `Box<$ty>` it leaked into + // `task.ptr`; reclaimed exactly once, here. + <$ty as bun_event_loop::BoxedTask>::run(unsafe { Box::from_raw(cast_ptr!($ty)) }) + }}; + } /// `CompressionStream::::run_from_js_thread` takes `*mut T` (full /// allocation provenance — R-2) so its trailing `T::deref()` may free the box. macro_rules! compression_arm { @@ -378,24 +387,26 @@ pub(crate) fn run_task( // ── hot-reload (early-returns from the drain loop) ─────────────── task_tag::HotReloadTask => { - // SAFETY: boxed by `hot_reloader::Task::enqueue`; the arm consumes it. - if unsafe { bun_core::heap::take(cast_ptr!(hot_reloader::HotReloadTask)) }.run() { + // SAFETY: as `boxed!`. + let task = unsafe { Box::from_raw(cast_ptr!(hot_reloader::HotReloadTask)) }; + let reloads = task.reloads(); + bun_event_loop::BoxedTask::run(task)?; + if reloads { return Ok(RunTaskResult::EarlyReturn); } } task_tag::WatchReloadTask => { - // SAFETY: boxed by `hot_reloader::Task::enqueue`; the arm consumes it. - if unsafe { bun_core::heap::take(cast_ptr!(hot_reloader::WatchReloadTask)) }.run() { + // SAFETY: as `boxed!`. + let task = unsafe { Box::from_raw(cast_ptr!(hot_reloader::WatchReloadTask)) }; + let reloads = task.reloads(); + bun_event_loop::BoxedTask::run(task)?; + if reloads { return Ok(RunTaskResult::EarlyReturn); } } // ── bake dev-server (cold — hoisted to `run_task_cold`) ────────── task_tag::BakeHotReloadEvent => run_task_cold(task), - task_tag::FSWatchTask => { - // SAFETY: boxed by the watcher's `EventSink` / `enqueue_one`; the - // arm consumes it. - unsafe { bun_core::heap::take(cast_ptr!(FSWatchTask)) }.run()?; - } + task_tag::FSWatchTask => boxed!(FSWatchTask)?, // ── node:fs libuv-request ops (Windows) ────────────────────────── #[cfg(windows)] @@ -580,18 +591,10 @@ const _: () = assert!( "dispatch::run_task / release_task_unrun arm count out of sync with bun_event_loop::task_tag", ); -// ── tag ↔ payload for the boxed fs watcher tasks ──────────────────────────── +// ── tag ↔ payload for the boxed fs.watchFile tasks ────────────────────────── // The payload types are plain owned boxes with safe `run`/`release_unrun`; the // raw-pointer reclaim lives here next to the arms that do the same. -impl bun_event_loop::Taskable for FSWatchTask { - const TAG: bun_event_loop::TaskTag = task_tag::FSWatchTask; - unsafe fn release_unrun(this: *mut Self) { - // SAFETY: fn contract — the box queued under this tag. - FSWatchTask::release_unrun(unsafe { bun_core::heap::take(this) }); - } -} - impl bun_event_loop::Taskable for crate::node::node_fs_stat_watcher::StatWatcherHop { const TAG: bun_event_loop::TaskTag = task_tag::StatWatcherHop; unsafe fn release_unrun(this: *mut Self) { @@ -1237,6 +1240,16 @@ fn __bun_release_task_unrun(task: bun_event_loop::Task) { unsafe { <$ty as Taskable>::release_unrun(task.ptr.cast::<$ty>()) } }}; } + /// `<$ty as BoxedTask>::release_unrun` on the queued box, SAFETY spelled once. + macro_rules! release_boxed { + ($ty:ty) => {{ + // SAFETY: as `release!`; the tag is `<$ty as BoxedTask>::TAG` and + // `task.ptr` the `Box<$ty>` `into_task` leaked. + <$ty as bun_event_loop::BoxedTask>::release_unrun(unsafe { + Box::from_raw(task.ptr.cast::<$ty>()) + }) + }}; + } match task.tag { task_tag::AnyTaskJob => { // The one erased tag: every payload is a `Job` reached through its header. @@ -1258,9 +1271,9 @@ fn __bun_release_task_unrun(task: bun_event_loop::Task) { task_tag::FetchTaskletPromiseSettle => { release!(crate::webcore::fetch::fetch_tasklet::FetchTaskletPromiseSettle) } - task_tag::FSWatchTask => release!(FSWatchTask), - task_tag::HotReloadTask => release!(hot_reloader::HotReloadTask), - task_tag::WatchReloadTask => release!(hot_reloader::WatchReloadTask), + task_tag::FSWatchTask => release_boxed!(FSWatchTask), + task_tag::HotReloadTask => release_boxed!(hot_reloader::HotReloadTask), + task_tag::WatchReloadTask => release_boxed!(hot_reloader::WatchReloadTask), task_tag::JSBundleCompletionTask => { release!(crate::api::js_bundle_completion_task::JSBundleCompletionTask) } diff --git a/src/runtime/node/node_fs_watcher.rs b/src/runtime/node/node_fs_watcher.rs index 8c384b47941c..77c94177f773 100644 --- a/src/runtime/node/node_fs_watcher.rs +++ b/src/runtime/node/node_fs_watcher.rs @@ -177,14 +177,22 @@ impl FSWatchTask { result } - /// The VM is tearing down and nobody will emit these events: drop them and - /// the activity unit the batch took. + /// The VM is tearing down (or was already closed when the watcher thread + /// posted) and nobody will emit these events: drop them and the activity + /// unit the batch took. #[allow(clippy::boxed_local, reason = "reclaim point for the boxed task")] pub(crate) fn release_unrun(self: Box) { self.activity.unref_task(); } } +bun_event_loop::boxed_task! { + FSWatchTask => FSWatchTask; + run = FSWatchTask::run; + release_unrun = FSWatchTask::release_unrun; + refused = FSWatchTask::release_unrun; +} + /// One `FSWatcher`'s end of a shared POSIX path watch: what the watcher /// thread holds (under the manager lock) to batch events and hand them to the /// owner's JS thread. @@ -269,10 +277,8 @@ impl EventSink { Arc::clone(&self.activity), core::mem::take(&mut self.batch), ); - if let Err(task) = self.vm.post_boxed(self.loop_kind, task) { - // VM torn down: nobody will emit these events. - task.release_unrun(); - } + // Refused (VM torn down): nobody will emit these events. + self.vm.post_boxed(self.loop_kind, task); return; } // closed or detached so just drop the events @@ -375,11 +381,10 @@ impl FSWatcher { let task = FSWatchTask::new(ThreadBound::new(self), Arc::clone(&self.activity), entries); let vm = self.global_this.bun_vm(); #[cfg(not(windows))] - if let Err(task) = vm.handle().post_boxed(vm.current_loop_kind(), task) { - task.release_unrun(); - } + vm.handle().post_boxed(vm.current_loop_kind(), task); #[cfg(windows)] - vm.event_loop_mut().enqueue_task(Task::from_boxed(task)); + vm.event_loop_mut() + .enqueue_task(bun_event_loop::BoxedTask::into_task(task)); } /// libuv delivers fs events on the JS thread; each becomes its own task. From bac6162fc440b396b807e150382743601a259d25 Mon Sep 17 00:00:00 2001 From: Jarred Sumner Date: Thu, 27 Aug 2026 11:34:41 +0000 Subject: [PATCH 5/7] source-lints: vm-thread-door inventory for the worker/hot-reloader changes; no dead_code escape in the ConcurrentTask test --- src/event_loop/ConcurrentTask.rs | 4 ++-- .../vm-thread-door.inventory.json | 21 +++++++++---------- 2 files changed, 12 insertions(+), 13 deletions(-) diff --git a/src/event_loop/ConcurrentTask.rs b/src/event_loop/ConcurrentTask.rs index 7d38b9fcfcd2..093e5c16cd7d 100644 --- a/src/event_loop/ConcurrentTask.rs +++ b/src/event_loop/ConcurrentTask.rs @@ -530,10 +530,10 @@ mod tests { static REFUSED: AtomicUsize = AtomicUsize::new(0); static DROPPED: AtomicUsize = AtomicUsize::new(0); - struct Payload(#[allow(dead_code)] Box<[u8; 64]>); + struct Payload(Box<[u8; 64]>); impl Drop for Payload { fn drop(&mut self) { - DROPPED.fetch_add(1, Ordering::SeqCst); + DROPPED.fetch_add(usize::from(self.0[0] == 7), Ordering::SeqCst); } } crate::boxed_task! { diff --git a/test/internal/source-lints/vm-thread-door.inventory.json b/test/internal/source-lints/vm-thread-door.inventory.json index b798132c18af..864e6cb6ca1d 100644 --- a/test/internal/source-lints/vm-thread-door.inventory.json +++ b/test/internal/source-lints/vm-thread-door.inventory.json @@ -43,19 +43,10 @@ ] }, "src/jsc/hot_reloader.rs": { - "thread spawn": 1, - "unsafe impl Send": [ - "WatchChangedPaths" - ], - "unsafe impl Sync": [ - "WatchChangedPaths" - ] + "thread spawn": 1 }, "src/jsc/web_worker.rs": { - "thread spawn": 1, - "unsafe impl Send": [ - "ThreadStart" - ] + "thread spawn": 1 }, "src/jsc/webcore_types.rs": { "unsafe impl Send": [ @@ -69,6 +60,14 @@ "Store" ] }, + "src/jsc/worker_messaging_proxy.rs": { + "unsafe impl Send": [ + "WorkerMessagingProxy" + ], + "unsafe impl Sync": [ + "WorkerMessagingProxy" + ] + }, "src/runtime/api/JSTranspiler.rs": { "unsafe impl Send": [ "TransformTask" From d5082bf7c688e92f65d0afe6e24d762bab438a5d Mon Sep 17 00:00:00 2001 From: Jarred Sumner Date: Thu, 27 Aug 2026 11:44:46 +0000 Subject: [PATCH 6/7] node_fs_watcher: drop the now-unused Windows Task import --- src/runtime/node/node_fs_watcher.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/runtime/node/node_fs_watcher.rs b/src/runtime/node/node_fs_watcher.rs index 77c94177f773..ea76700ec40b 100644 --- a/src/runtime/node/node_fs_watcher.rs +++ b/src/runtime/node/node_fs_watcher.rs @@ -5,8 +5,6 @@ use std::sync::Arc; use bun_collections::smallvec::SmallVec; use bun_core::Output; use bun_core::strings; -#[cfg(windows)] -use bun_event_loop::Task; use bun_io::KeepAlive; use bun_jsc::abort_signal::{AbortSubscription, OnAbort}; use bun_jsc::bun_string_jsc; From 010bb5b5e06c68b54b2fae934a8f115c2a17a294 Mon Sep 17 00:00:00 2001 From: Jarred Sumner Date: Sat, 29 Aug 2026 09:56:58 +0000 Subject: [PATCH 7/7] ConcurrentTask: BoxedTask/TaskHop implementors write their own unsafe impl (napi-lifecycle shape); FSWatchTask and the reload task converted --- src/event_loop/ConcurrentTask.rs | 96 ++++++++--------------------- src/jsc/hot_reloader.rs | 28 ++++++--- src/runtime/node/node_fs_watcher.rs | 18 ++++-- 3 files changed, 58 insertions(+), 84 deletions(-) diff --git a/src/event_loop/ConcurrentTask.rs b/src/event_loop/ConcurrentTask.rs index 093e5c16cd7d..ae1ca2ba8650 100644 --- a/src/event_loop/ConcurrentTask.rs +++ b/src/event_loop/ConcurrentTask.rs @@ -166,8 +166,8 @@ pub trait Taskable { /// A task whose queued pointer is a [`ThisPtr`](bun_ptr::ThisPtr) to /// `Target`, which keeps itself alive for the task; the dispatcher runs it or -/// releases it through here. A zero-sized hop type per tag, declared with -/// [`task_hop!`] next to `Target` so the ref protocol lives there. +/// releases it through here. A zero-sized hop type per tag, declared next to +/// `Target` with its own `unsafe impl` stating the ref protocol. /// /// # Safety /// `TAG` is dispatched (in `bun_runtime::dispatch`) to this impl and no other @@ -189,34 +189,9 @@ pub unsafe trait TaskHop { } } -/// Declares `$hop`, the [`TaskHop`] that `task_tag::$tag` dispatches to for -/// `$target`, forwarding to `$run` / `$release`. The doc comment on the -/// invocation is where `$target`'s liveness protocol for the queued task is -/// stated. -#[macro_export] -macro_rules! task_hop { - ($(#[$m:meta])* $v:vis $hop:ident for $target:ty => $tag:ident; run = $run:expr; release_unrun = $release:expr $(;)?) => { - $(#[$m])* - $v struct $hop; - // SAFETY: see macro doc — one hop per tag; the invoker documents the liveness protocol. - unsafe impl $crate::TaskHop for $hop { - type Target = $target; - const TAG: $crate::TaskTag = $crate::task_tag::$tag; - #[inline] - fn run(this: ::bun_ptr::ThisPtr<$target>) -> $crate::JsResult<()> { - ($run)(this) - } - #[inline] - fn release_unrun(this: ::bun_ptr::ThisPtr<$target>) { - ($release)(this) - } - } - }; -} - /// A task that owns its payload: queued as the `Box` leaked into /// [`Task::ptr`], handed back as that box when the dispatcher runs or -/// releases it. Implement via [`boxed_task!`]. +/// releases it. /// /// # Safety /// `TAG` is dispatched (in `bun_runtime::dispatch`) to this impl and no other @@ -241,38 +216,6 @@ pub unsafe trait BoxedTask: Sized { } } -/// Implements [`BoxedTask`] for `$ty` as the task `task_tag::$tag` dispatches -/// to, forwarding to `$run` / `$release` / `$refused` (each `fn(Box<$ty>)`). -/// The `impl[generics] $ty => $tag_expr` form is for a generic `$ty` whose -/// tag depends on its parameters. -#[macro_export] -macro_rules! boxed_task { - ($ty:ty => $tag:ident; run = $run:expr; release_unrun = $release:expr; refused = $refused:expr $(;)?) => { - $crate::boxed_task! { - impl[] $ty => $crate::task_tag::$tag; - run = $run; release_unrun = $release; refused = $refused; - } - }; - (impl[$($gen:tt)*] $ty:ty => $tag:expr; run = $run:expr; release_unrun = $release:expr; refused = $refused:expr $(;)?) => { - // SAFETY: see macro doc — one boxed task type per tag. - unsafe impl<$($gen)*> $crate::BoxedTask for $ty { - const TAG: $crate::TaskTag = $tag; - #[inline] - fn run(self: ::std::boxed::Box) -> $crate::JsResult<()> { - ($run)(self) - } - #[inline] - fn release_unrun(self: ::std::boxed::Box) { - ($release)(self) - } - #[inline] - fn refused(self: ::std::boxed::Box) { - ($refused)(self) - } - } - }; -} - impl TaskTag { /// The tag's identifier, for diagnostics. pub fn name(self) -> &'static str { @@ -530,29 +473,42 @@ mod tests { static REFUSED: AtomicUsize = AtomicUsize::new(0); static DROPPED: AtomicUsize = AtomicUsize::new(0); - struct Payload(Box<[u8; 64]>); + struct Payload { + _buf: Box<[u8; 64]>, + } impl Drop for Payload { fn drop(&mut self) { - DROPPED.fetch_add(usize::from(self.0[0] == 7), Ordering::SeqCst); + DROPPED.fetch_add(1, Ordering::SeqCst); } } - crate::boxed_task! { - Payload => NapiFinalizerTask; - run = |_task: Box| Ok(()); - release_unrun = |_task: Box| {}; - refused = |_task: Box| { REFUSED.fetch_add(1, Ordering::SeqCst); }; + // SAFETY: test-only; never dispatched, so the borrowed tag cannot collide. + unsafe impl BoxedTask for Payload { + const TAG: TaskTag = task_tag::NapiFinalizerTask; + fn run(self: Box) -> crate::JsResult<()> { + Ok(()) + } + fn release_unrun(self: Box) {} + fn refused(self: Box) { + REFUSED.fetch_add(1, Ordering::SeqCst); + } } #[test] fn refused_boxed_carrier_releases_its_payload() { - let carrier = ConcurrentTask::create_boxed(Box::new(Payload(Box::new([7; 64])))); + let carrier = ConcurrentTask::create_boxed(Box::new(Payload { + _buf: Box::new([7; 64]), + })); // SAFETY: never queued; ours. unsafe { ConcurrentTask::release_refused(carrier) }; assert_eq!(REFUSED.load(Ordering::SeqCst), 1); assert_eq!(DROPPED.load(Ordering::SeqCst), 1); - let mut intrusive = - ConcurrentTask::intrusive(Box::new(Payload(Box::new([7; 64]))).into_task()); + let mut intrusive = ConcurrentTask::intrusive( + Box::new(Payload { + _buf: Box::new([7; 64]), + }) + .into_task(), + ); // SAFETY: never queued; an intrusive carrier is left alone. unsafe { ConcurrentTask::release_refused(core::ptr::NonNull::from(&mut intrusive)) }; assert_eq!(DROPPED.load(Ordering::SeqCst), 1); diff --git a/src/jsc/hot_reloader.rs b/src/jsc/hot_reloader.rs index d86bb5dbdbec..4c819d226281 100644 --- a/src/jsc/hot_reloader.rs +++ b/src/jsc/hot_reloader.rs @@ -454,15 +454,25 @@ pub struct Task EventLoopType>, } -// Only the VM's tasks are ever queued (`bun build --watch` has no loop). A -// task released unrun or refused is a file change that will not reload -// anything: dropping it is all there is to do. -bun_event_loop::boxed_task! { - impl[const RELOAD_IMMEDIATELY: bool] Task => - if RELOAD_IMMEDIATELY { task_tag::WatchReloadTask } else { task_tag::HotReloadTask }; - run = |task: Box| { task.run(); Ok(()) }; - release_unrun = drop; - refused = drop; +// SAFETY: the only task type for `task_tag::WatchReloadTask` / +// `task_tag::HotReloadTask` (one per `RELOAD_IMMEDIATELY`); only the VM's +// tasks are ever queued (`bun build --watch` has no loop) and the carrier owns +// the box (`VmHandle::post_boxed`). A task released unrun or refused is a file +// change that will not reload anything: dropping it is all there is to do. +unsafe impl bun_event_loop::BoxedTask + for Task +{ + const TAG: bun_event_loop::TaskTag = if RELOAD_IMMEDIATELY { + task_tag::WatchReloadTask + } else { + task_tag::HotReloadTask + }; + fn run(self: Box) -> bun_event_loop::JsResult<()> { + Task::run(self); + Ok(()) + } + fn release_unrun(self: Box) {} + fn refused(self: Box) {} } impl diff --git a/src/runtime/node/node_fs_watcher.rs b/src/runtime/node/node_fs_watcher.rs index ea76700ec40b..f38a75142eb1 100644 --- a/src/runtime/node/node_fs_watcher.rs +++ b/src/runtime/node/node_fs_watcher.rs @@ -184,11 +184,19 @@ impl FSWatchTask { } } -bun_event_loop::boxed_task! { - FSWatchTask => FSWatchTask; - run = FSWatchTask::run; - release_unrun = FSWatchTask::release_unrun; - refused = FSWatchTask::release_unrun; +// SAFETY: the only task type for `task_tag::FSWatchTask`; the queued +// carrier owns the box (`ConcurrentTask::create_boxed`). +unsafe impl bun_event_loop::BoxedTask for FSWatchTask { + const TAG: bun_event_loop::TaskTag = bun_event_loop::task_tag::FSWatchTask; + fn run(self: Box) -> bun_event_loop::JsResult<()> { + FSWatchTask::run(self) + } + fn release_unrun(self: Box) { + FSWatchTask::release_unrun(self) + } + fn refused(self: Box) { + FSWatchTask::release_unrun(self) + } } /// One `FSWatcher`'s end of a shared POSIX path watch: what the watcher