diff --git a/src/bundler/LinkerContext.rs b/src/bundler/LinkerContext.rs index bc47073ee17d..7eaac27860f9 100644 --- a/src/bundler/LinkerContext.rs +++ b/src/bundler/LinkerContext.rs @@ -236,24 +236,14 @@ impl<'a> LinkerContext<'a> { .get() } - /// Mutable projection of the `r#loop` BACKREF for `AnyEventLoop` dispatch - /// (`enqueue_task_concurrent*`, `tick`). Centralises the raw `NonNull` - /// deref so the three callers (`BundleV2::any_loop_mut`, `ParseTask` / - /// `ServerComponentParseTask` completion) are safe. - /// - /// `&self` receiver (not `&mut self`): the loop storage is **disjoint** - /// from `LinkerContext` (it lives in the `BundleThread` / runtime arena — - /// see [`EventLoop`]), and worker-thread completions reach this through a - /// `BackRef` (`&` only). + /// Mutable projection of the `r#loop` BACKREF, for the bundle thread's own + /// use of its loop (`BundleV2::any_loop_mut`). Other threads post to the + /// loop through `crate::post`, which reads the field raw. #[inline] - #[allow(clippy::mut_from_ref)] - pub(crate) fn any_loop_mut(&self) -> Option<&mut bun_event_loop::AnyEventLoop> { + pub(crate) fn any_loop_mut(&mut self) -> Option<&mut bun_event_loop::AnyEventLoop> { // SAFETY: BACKREF — set once in `BundleV2::init` from a loop that - // outlives the bundle pass; the pointee is disjoint from `*self`. - // Exclusivity: `Js { owner }.enqueue_task_concurrent` is `&self` - // (MPSC), and `Mini.enqueue_task_concurrent_with_extra_ctx` only - // pushes to an MPSC queue + writes the caller-owned intrusive task - // node, so concurrent worker completions do not alias loop state. + // outlives the bundle pass; the pointee is disjoint from `*self` + // (see [`EventLoop`]). self.r#loop.map(|p| unsafe { &mut *p.as_ptr() }) } diff --git a/src/bundler/ParseTask.rs b/src/bundler/ParseTask.rs index bf13ab135e1a..547aaf690df8 100644 --- a/src/bundler/ParseTask.rs +++ b/src/bundler/ParseTask.rs @@ -41,11 +41,6 @@ use bun_resolver::{self as _resolver, Resolver}; declare_scope!(ParseTask, hidden); -#[allow(non_snake_case)] -mod EventLoop { - pub(super) type Task = bun_event_loop::ConcurrentTask::ConcurrentTask; -} - // the per-file parse arena is held as `bump: &'static Bump` (the // worker arena is pinned for the entire bundle pass — see `run_with_source_code`), // so `bump.alloc_*` / `ArenaString::into_bump_str` already yield `&'static` @@ -130,7 +125,8 @@ pub enum ParseTaskStage { /// The information returned to the Bundler thread when a parse finishes. pub(crate) struct Result { - pub(crate) task: EventLoop::Task, + /// Mini-loop queue node for `crate::post` (see `post::Event::NODE`). + pub(crate) task: bun_event_loop::AnyTaskWithExtraContext::AnyTaskWithExtraContext, pub(crate) ctx: bun_ptr::ParentRef, bun_ptr::Mut>, pub(crate) value: ResultValue, pub(crate) watcher_data: WatcherData, @@ -204,7 +200,7 @@ impl ParseTask { /// (LIFETIMES.tsv) into the arena-allocated bundle, set at `init` time and /// valid until `BundleV2::deinit`. Prefer this over open-coded /// `unsafe { &*task.ctx }`; sites that mutate the bundle (e.g. - /// `on_complete`) must continue to deref the raw `ctx` field directly. + /// `ParseComplete::run`) must continue to deref the raw `ctx` field directly. /// /// # Safety /// @@ -225,7 +221,7 @@ impl ParseTask { resolve_result: &_resolver::Result, source_index: Index, // Take `*mut` so the stored BACKREF retains - // write provenance for `on_complete` (a `&BundleV2` param would shrink + // write provenance for `ParseComplete::run` (a `&BundleV2` param would shrink // provenance to read-only, making the later `&mut *ctx` UB). ctx: *mut BundleV2<'_>, ) -> ParseTask { @@ -339,7 +335,7 @@ impl Default for ParseTask { // two callbacks fire for the same `ParseTask` concurrently — the IO→worker // hand-off in `run_from_thread_pool_impl` reschedules sequentially). Writes: // `ParseTask.{stage, source_index, ...}` (own fields); result is sent via -// `ctx.loop_.enqueue_task_concurrent` (MPSC queue). Reads `ctx: &BundleV2` +// `post::` (MPSC queue). Reads `ctx: &BundleV2` // shared (`Worker::get`, `ctx.graph.pool`, `ctx.transpiler.options`). // `ParseTask` is `Send` because its non-auto-`Send` fields are bundle- // lifetime arena slices / backref pointers (`ctx`, `path`, `contents`). @@ -2809,7 +2805,7 @@ pub mod parse_worker { let result = Box::new(Result { ctx: this.ctx.expect("ParseTask.ctx unset"), - task: EventLoop::Task::default(), + task: Default::default(), value, // `ExternalFreeFunction` // doesn't derive `Copy`, so move it out (task is consumed here). @@ -2827,55 +2823,9 @@ pub mod parse_worker { // `ParseTask` is arena-owned (no Drop); `jsx` may hold owned slices from tsconfig. drop(core::mem::take(&mut this.jsx)); - // `worker.ctx` is a `BackRef` (safe `Deref`); the BACKREF deref - // of `linker.r#loop` is centralised in `LinkerContext::any_loop_mut`. - // - // The loop is effectively non-optional — `BundleV2::init` - // always sets `linker.r#loop` before scheduling any ParseTask. Running - // `on_complete` inline on the worker thread would violate - // `BundleV2::on_parse_task_complete`'s threading contract (it mutates the - // bundler graph, which is owned by the main/bundler thread). - match worker - .ctx - .linker - .any_loop_mut() - .expect("BundleV2.linker.loop must be set before scheduling ParseTask") - { - bun_event_loop::AnyEventLoop::Js { .. } => { - let ct = - bun_event_loop::ConcurrentTask::ConcurrentTask::from_callback(result, |p| { - // SAFETY: `p` is the `result` Box leaked above; ownership - // transfers to `on_complete`, which deallocates it. - unsafe { on_complete(p) }; - Ok(()) - }); - let poster = worker - .ctx - .js_poster - .as_ref() - .expect("JS-owned bundle has a poster"); - if let bun_event_loop::Posted::Refused(ct) = poster.post(ct) { - // Owning JS VM torn down mid-bundle: free the hop and the result. - // SAFETY: refused ⇒ we own the task box and the leaked result. - unsafe { - bun_event_loop::ConcurrentTask::ConcurrentTask::release_refused(ct); - drop(bun_core::heap::take(result)); - } - } - } - bun_event_loop::AnyEventLoop::Mini(mini) => { - // SAFETY: `result` is a valid heap pointer with `task` at the given offset; - // ownership transfers to the mini event loop which frees it after `on_complete_mini`. - unsafe { - mini.enqueue_task_concurrent_with_extra_ctx::>( - result, - on_complete_mini, - offset_of!(Result, task), - ); - } - } - } - // Runs at function exit, i.e. after enqueue. + // SAFETY: `result` is a fresh heap allocation that `ParseComplete::run` + // (or `refused`) frees; `result.ctx` is the bundle this task belongs to. + unsafe { crate::post::post::(result) }; worker.unget(); } @@ -2889,46 +2839,42 @@ pub mod parse_worker { } } - fn on_complete_mini(result: *mut Result, ctx: *mut BundleV2<'static>) { - // SAFETY: callback contract — `result` was heap-allocated above; `ctx` is - // the BACKREF stashed in `result.ctx`. - BundleV2::on_parse_task_complete(unsafe { &mut *result }, unsafe { &mut *ctx }); - // SAFETY: `result` is uniquely owned (callback contract). - drop_result_owned_fields(unsafe { &mut *result }); - // `drop(heap::take(result))` would run full Drop glue: - // `on_parse_task_complete` SWAPS `result.value.Success.source` with the - // graph's placeholder and moves `result.ast` out, so post-swap - // `result.value` holds the *placeholder* `Source` whose - // `contents: Cow::Borrowed` may alias plugin-/loader-provided bytes the - // graph's swapped-in Source still references (asan use-after-poison at - // process_files_to_copy:4241 in bundler_loader/_plugin tests). So: - // dealloc the box without running Drop. - // SAFETY: `result` came from `bun_core::heap::into_raw(Box)` - // above; uniquely owned. Dealloc with the same layout, no field Drop. - unsafe { std::alloc::dealloc(result.cast::(), std::alloc::Layout::new::()) }; - } + /// A worker finished parsing one file. `Result` is heap-allocated by the + /// worker and freed here. + pub(crate) struct ParseComplete; - /// # Safety - /// `result` must be a live, uniquely-owned heap allocation produced by - /// `bun_core::heap::into_raw(Box)` in `run_from_thread_pool_impl` - /// (or `ServerComponentParseTask`'s equivalent). Ownership transfers to - /// this fn, which deallocates `result` before returning. Must run on the - /// main/bundler thread (it dereferences `result.ctx` mutably). - pub(crate) unsafe fn on_complete(result: *mut Result) { - // SAFETY: result allocated via heap::alloc above; uniquely owned here. - let r = unsafe { &mut *result }; - let ctx = r.ctx; - // SAFETY: `ctx` is a ParentRef stored with write provenance - // (`from_raw_mut` in `ParseTask::init`); the BundleV2 outlives the bundle - // pass and no other `&mut BundleV2` is live on this (main) thread when the - // event-loop callback fires. `r` and `*ctx` are disjoint allocations. - BundleV2::on_parse_task_complete(r, unsafe { ctx.assume_mut() }); - drop_result_owned_fields(r); - // See `on_complete_mini` for why this is `dealloc`, not `drop(take(_))`. - // SAFETY: `result` came from `bun_core::heap::into_raw(Box)` - // above; uniquely owned. Dealloc with the same layout, no field Drop. - unsafe { std::alloc::dealloc(result.cast::(), std::alloc::Layout::new::()) }; + impl crate::post::Event for ParseComplete { + type Item = Result; + const NODE: usize = offset_of!(Result, task); + + unsafe fn bundle(result: *mut Result) -> *mut BundleV2<'static> { + // SAFETY: caller contract. + unsafe { (*result).ctx }.as_mut_ptr() + } + + unsafe fn run(result: *mut Result, bv2: &mut BundleV2<'static>) { + // SAFETY: `post`'s contract — uniquely owned heap allocation, disjoint from `*bv2`. + let r = unsafe { &mut *result }; + BundleV2::on_parse_task_complete(r, bv2); + drop_result_owned_fields(r); + // Not `drop(heap::take(result))`: `on_parse_task_complete` SWAPS + // `result.value.Success.source` with the graph's placeholder and moves + // `result.ast` out, so post-swap `result.value` holds the *placeholder* + // `Source` whose `contents: Cow::Borrowed` may alias plugin-/loader-provided + // bytes the graph's swapped-in Source still references (asan + // use-after-poison in bundler_loader/_plugin tests). Dealloc without Drop. + // SAFETY: `result` came from `heap::into_raw(Box)`; uniquely owned. + unsafe { + std::alloc::dealloc(result.cast::(), std::alloc::Layout::new::()) + }; + } + + unsafe fn refused(result: *mut Result) { + // Nothing was swapped out of it, so full Drop is right here. + // SAFETY: `result` came from `heap::into_raw(Box)`; uniquely owned. + drop(unsafe { bun_core::heap::take(result) }); + } } } // end mod parse_worker -pub(crate) use parse_worker::on_complete; +pub(crate) use parse_worker::ParseComplete; diff --git a/src/bundler/ServerComponentParseTask.rs b/src/bundler/ServerComponentParseTask.rs index b2f10b316297..b6e4abf5d350 100644 --- a/src/bundler/ServerComponentParseTask.rs +++ b/src/bundler/ServerComponentParseTask.rs @@ -2,7 +2,6 @@ //! running through the js_parser. It emits a ParseTask.Result and joins //! with the same logic that it runs though. -use core::mem::offset_of; use std::fmt::Write as _; use bun_alloc::{AllocError as OOM, Arena}; // bumpalo::Bump re-export @@ -22,12 +21,12 @@ use crate::Worker; use crate::bundle_v2::BundleV2; use crate::cache::ExternalFreeFunction; use crate::options::{Loader, Target}; -use crate::parse_task::{self, ResultValue, Success, WatcherData, on_complete}; +use crate::parse_task::{self, ParseComplete, ResultValue, Success, WatcherData}; pub(crate) struct ServerComponentParseTask { pub task: ThreadPoolTask, pub data: Data, - // BACKREF (LIFETIMES.tsv) — written through in `on_complete`. + // BACKREF (LIFETIMES.tsv) — written through in `ParseComplete::run`. // `ParentRef` (write-provenance via `NonNull::from(&mut self)` at construction) // so deref sites are safe; `None` only for the FRU `Default` placeholder. pub ctx: Option, bun_ptr::Mut>>, @@ -61,7 +60,7 @@ pub struct ClientEntryWrapper { // CONCURRENCY: thread-pool callback — runs on worker threads, one task per // `ServerComponentParseTask` (heap-allocated, scheduled exactly once). Writes: // own fields + `Log` (local) + result is posted via -// `ctx.loop_.enqueue_task_concurrent` (MPSC). Reads `ctx: &BundleV2` shared. +// `post::` (MPSC). Reads `ctx: &BundleV2` shared. // `ServerComponentParseTask` is `Send` because `ctx: *mut BundleV2` is a // backref to a `Send` type and `Source`/`Data` payloads are bundle-arena // slices. @@ -102,64 +101,12 @@ fn task_callback_wrap(thread_pool_task: *mut ThreadPoolTask) { }); let result = bun_core::heap::into_raw(result); - // `worker.ctx` is a `BackRef` (safe `Deref`); the BACKREF deref - // of `linker.r#loop` is centralised in `LinkerContext::any_loop_mut`. - // - // The loop is effectively non-optional — `BundleV2::init` - // always sets `linker.r#loop` before scheduling any ServerComponentParseTask. - // Running `on_complete` inline on the worker thread would violate - // `BundleV2::on_parse_task_complete`'s threading contract (it mutates the - // bundler graph, which is owned by the main/bundler thread). - match worker - .ctx - .linker - .any_loop_mut() - .expect("BundleV2.linker.loop must be set before scheduling ServerComponentParseTask") - { - bun_event_loop::AnyEventLoop::Js { .. } => { - let ct = bun_event_loop::ConcurrentTask::ConcurrentTask::from_callback(result, |p| { - // SAFETY: `p` is the `result` Box leaked above; ownership - // transfers to `on_complete`, which deallocates it. - unsafe { on_complete(p) }; - Ok(()) - }); - let poster = worker - .ctx - .js_poster - .as_ref() - .expect("JS-owned bundle has a poster"); - if let bun_event_loop::Posted::Refused(ct) = poster.post(ct) { - // Owning JS VM torn down mid-bundle: free the hop and the result. - // SAFETY: refused ⇒ we own the task box and the leaked result. - unsafe { - bun_event_loop::ConcurrentTask::ConcurrentTask::release_refused(ct); - drop(bun_core::heap::take(result)); - } - } - } - bun_event_loop::AnyEventLoop::Mini(mini) => { - // SAFETY: `result` is a freshly Box-leaked `parse_task::Result` (above) and - // `offset_of!(parse_task::Result, task)` is the intrusive task field within it. - unsafe { - mini.enqueue_task_concurrent_with_extra_ctx::>( - result, - on_complete_mini, - offset_of!(parse_task::Result, task), - ); - } - } - } - // Runs at function exit, i.e. after enqueue. + // SAFETY: `result` is a fresh heap allocation that `ParseComplete::run` + // (or `refused`) frees; `result.ctx` is the bundle this task belongs to. + unsafe { crate::post::post::(result) }; worker.unget(); } -fn on_complete_mini(result: *mut parse_task::Result, _ctx: *mut BundleV2<'static>) { - // `on_complete` already recovers `ctx` from `result.ctx`. - // SAFETY: callback contract — `result` is the uniquely-owned Box leaked in - // `run_from_thread_pool`; ownership transfers to `on_complete`. - unsafe { on_complete(result) }; -} - fn task_callback( task: &mut ServerComponentParseTask, log: &mut Log, diff --git a/src/bundler/bundle_v2.rs b/src/bundler/bundle_v2.rs index b117a316ffa7..92815c08e732 100644 --- a/src/bundler/bundle_v2.rs +++ b/src/bundler/bundle_v2.rs @@ -47,6 +47,7 @@ pub use api::JSBundler::Plugin as JSBundlerPlugin; /// `BundleV2.JSBundleCompletionTask` — re-exported from the canonical def below. pub use bv2_impl::JSBundleCompletionTask; +pub use bv2_impl::{PluginLoadDeferred, PluginLoadSettled, PluginResolveSettled}; /// `jsc::api::JSBundler::FileMap` — re-exported from the canonical def below. pub use api::JSBundler::FileMap; @@ -1086,7 +1087,7 @@ pub mod bv2_impl { pub bv2: *mut BundleV2<'static>, pub import_record: MiniImportRecord, pub value: ResolveValue, - /// `jsc.AnyEventLoop.Task` — intrusive node for the Mini-loop queue. + /// Mini-loop queue node for `crate::post` (see `post::Event::NODE`). pub(crate) task: bun_event_loop::AnyTaskWithExtraContext::AnyTaskWithExtraContext, /// Links in the bundle's list of requests a plugin currently /// holds (`Graph::outstanding_resolves`); bundle thread only. @@ -1204,9 +1205,7 @@ pub mod bv2_impl { /// `.defer()`ed and not yet drained: its scan-counter unit sits in /// `Graph::deferred_pending` (bundle thread only). pub(crate) deferred: bool, - /// `jsc.AnyEventLoop.Task` — intrusive node for the Mini-loop queue - /// (used by `onDefer` to notify the bundler thread when it runs - /// under a `MiniEventLoop`). + /// Mini-loop queue node for `crate::post` (see `post::Event::NODE`). pub task: bun_event_loop::AnyTaskWithExtraContext::AnyTaskWithExtraContext, /// Links in `Graph::outstanding_loads`; bundle thread only. pub(crate) outstanding: crate::Graph::OutstandingLink, @@ -1537,7 +1536,6 @@ pub mod bv2_impl { } // From bake where the loop running the bundle is also the loop running // the plugins. - // `any_loop_mut` centralises the BACKREF deref of `linker.r#loop`. let poster = self .js_poster .as_ref() @@ -3754,13 +3752,13 @@ pub mod bv2_impl { })?; let _ = self.graph.ast.append(JSAst::empty_in(self.graph.heap)); // OOM/capacity: fire-and-forget - // `bun.new(ServerComponentParseTask, …)` — heap-owned by the - // worker pool; freed via `bun.destroy` in `on_complete` after the - // result posts back to the bundle thread. + // Heap-allocated; the worker recovers it from its `task` link + // (`task_callback_wrap`) and posts a `parse_task::Result` back to + // the bundle thread. let task = bun_core::heap::into_raw(Box::new(ServerComponentParseTask { data, // SAFETY: `from_mut(self)` is the live bundle (write provenance for - // `on_complete`'s `assume_mut`) and outlives the task; `'a` erased to + // `ParseComplete::run`) and outlives the task; `'a` erased to // `'static` for the BACKREF. ctx: Some(unsafe { bun_ptr::ParentRef::from_raw_mut( @@ -4315,103 +4313,65 @@ pub mod bv2_impl { } Ok(()) } + } - pub fn on_load_async(&mut self, load: &mut jsc_api::JSBundler::Load) { - // Dispatch to the loop that *owns* `BundleV2`. - // For `Bun.build` this is a Mini loop running on the bundler thread, so - // `on_load` must land there — not on the JS plugin loop — or it will - // mutate `graph` / allocate from `graph.heap` off-thread. - match self.any_loop_mut() { - bun_event_loop::AnyEventLoop::Js { .. } => { - let ct = bun_event_loop::ConcurrentTask::ConcurrentTask::from_callback( - std::ptr::from_mut(load), - on_load_from_js_loop_raw, - ); - let poster = self - .js_poster - .as_ref() - .expect("JS-owned bundle has a poster"); - if let bun_event_loop::Posted::Refused(ct) = poster.post(ct) { - // Owning JS VM torn down mid-bundle: the hop never runs. - // SAFETY: refused ⇒ we own the task. - unsafe { - bun_event_loop::ConcurrentTask::ConcurrentTask::release_refused(ct) - }; - } - } - bun_event_loop::AnyEventLoop::Mini(mini) => { - // SAFETY: `load` is a valid &mut for the duration of the enqueue; - // the mini loop dispatches `on_load_mini` on the bundler thread. - unsafe { - mini.enqueue_task_concurrent_with_extra_ctx::>( - std::ptr::from_mut(load), - on_load_mini, - core::mem::offset_of!(jsc_api::JSBundler::Load, task), - ); - } - } - } + /// A plugin settled `load` (or an `onLoad` callback returned). + pub struct PluginLoadSettled; + + impl crate::post::Event for PluginLoadSettled { + type Item = jsc_api::JSBundler::Load; + const NODE: usize = core::mem::offset_of!(jsc_api::JSBundler::Load, task); + + unsafe fn bundle(load: *mut Self::Item) -> *mut BundleV2<'static> { + // SAFETY: caller contract; `bv2` is set in `Load::init`. + unsafe { (*load).bv2 } } - pub fn on_resolve_async(&mut self, resolve: &mut jsc_api::JSBundler::Resolve) { - // See `on_load_async` — must dispatch on the bundler's own loop. - match self.any_loop_mut() { - bun_event_loop::AnyEventLoop::Js { .. } => { - let ct = bun_event_loop::ConcurrentTask::ConcurrentTask::from_callback( - std::ptr::from_mut(resolve), - on_resolve_from_js_loop_raw, - ); - let poster = self - .js_poster - .as_ref() - .expect("JS-owned bundle has a poster"); - if let bun_event_loop::Posted::Refused(ct) = poster.post(ct) { - // Owning JS VM torn down mid-bundle: the hop never runs. - // SAFETY: refused ⇒ we own the task. - unsafe { - bun_event_loop::ConcurrentTask::ConcurrentTask::release_refused(ct) - }; - } - } - bun_event_loop::AnyEventLoop::Mini(mini) => { - // SAFETY: `resolve` is a valid &mut for the duration of the enqueue; - // the mini loop dispatches `on_resolve_mini` on the bundler thread. - unsafe { - mini.enqueue_task_concurrent_with_extra_ctx::>( - std::ptr::from_mut(resolve), - on_resolve_mini, - core::mem::offset_of!(jsc_api::JSBundler::Resolve, task), - ); - } - } - } + unsafe fn run(load: *mut Self::Item, this: &mut BundleV2<'static>) { + // SAFETY: `post`'s contract; `Load` lives in the graph arena, disjoint from `*this`. + BundleV2::on_load(unsafe { &mut *load }, this); } } - fn on_load_mini(load: *mut jsc_api::JSBundler::Load, this: *mut BundleV2<'static>) { - // SAFETY: callback contract — `load` is the ctx passed to - // `enqueue_task_concurrent_with_extra_ctx`; `this` is the BundleV2 the - // mini loop's `tick` supplies as ParentContext. - BundleV2::on_load(unsafe { &mut *load }, unsafe { &mut *this }); - } + /// A plugin settled `resolve` (or an `onResolve` callback returned). + pub struct PluginResolveSettled; - fn on_resolve_mini(resolve: *mut jsc_api::JSBundler::Resolve, this: *mut BundleV2<'static>) { - // SAFETY: see `on_load_mini`. - BundleV2::on_resolve(unsafe { &mut *resolve }, unsafe { &mut *this }); - } + impl crate::post::Event for PluginResolveSettled { + type Item = jsc_api::JSBundler::Resolve; + const NODE: usize = core::mem::offset_of!(jsc_api::JSBundler::Resolve, task); - fn on_load_from_js_loop(load: &mut jsc_api::JSBundler::Load) { - // SAFETY: `bv2` is a live backref set in `Load::init`. - let bv2 = unsafe { &mut *load.bv2 }; - BundleV2::on_load(load, bv2); + unsafe fn bundle(resolve: *mut Self::Item) -> *mut BundleV2<'static> { + // SAFETY: caller contract; `bv2` is set in `Resolve::init`. + unsafe { (*resolve).bv2 } + } + + unsafe fn run(resolve: *mut Self::Item, this: &mut BundleV2<'static>) { + // SAFETY: as for `PluginLoadSettled`. + BundleV2::on_resolve(unsafe { &mut *resolve }, this); + } } - fn on_load_from_js_loop_raw( - load: *mut jsc_api::JSBundler::Load, - ) -> bun_event_loop::JsResult<()> { - // SAFETY: `load` is a valid pointer set up by `from_callback`. - on_load_from_js_loop(unsafe { &mut *load }); - Ok(()) + /// An `onLoad` callback called `defer()`: park this load's scan-counter + /// unit in `Graph::deferred_pending` until everything else has loaded. + /// The `Load` is still outstanding, so it is only borrowed here. + pub struct PluginLoadDeferred; + + impl crate::post::Event for PluginLoadDeferred { + type Item = jsc_api::JSBundler::Load; + const NODE: usize = PluginLoadSettled::NODE; + + unsafe fn bundle(load: *mut Self::Item) -> *mut BundleV2<'static> { + // SAFETY: same contract. + unsafe { PluginLoadSettled::bundle(load) } + } + + unsafe fn run(load: *mut Self::Item, this: &mut BundleV2<'static>) { + this.thread_lock.assert_locked(); + // SAFETY: as for `PluginLoadSettled`. + unsafe { (*load).deferred = true }; + this.graph.deferred_pending += 1; + this.decrement_scan_counter(); + } } impl<'a> BundleV2<'a> { @@ -4599,20 +4559,6 @@ pub mod bv2_impl { } } - fn on_resolve_from_js_loop(resolve: &mut jsc_api::JSBundler::Resolve) { - // SAFETY: `bv2` is a live backref set in `Resolve::init`. - let bv2 = unsafe { &mut *resolve.bv2 }; - BundleV2::on_resolve(resolve, bv2); - } - - fn on_resolve_from_js_loop_raw( - resolve: *mut jsc_api::JSBundler::Resolve, - ) -> bun_event_loop::JsResult<()> { - // SAFETY: `resolve` is a valid pointer set up by `from_callback`. - on_resolve_from_js_loop(unsafe { &mut *resolve }); - Ok(()) - } - impl<'a> BundleV2<'a> { pub(crate) fn on_resolve(resolve: &mut jsc_api::JSBundler::Resolve, this: &mut BundleV2) { this.graph.outstanding_resolves.unlink(resolve); @@ -6938,17 +6884,6 @@ pub mod bv2_impl { } impl<'a> BundleV2<'a> { - pub fn on_notify_defer(&mut self) { - self.thread_lock.assert_locked(); - self.graph.deferred_pending += 1; - self.decrement_scan_counter(); - } - - pub fn on_notify_defer_mini(load: &mut jsc_api::JSBundler::Load, this: &mut BundleV2) { - load.deferred = true; - this.on_notify_defer(); - } - pub(crate) fn on_parse_task_complete( parse_result: &mut parse_task::Result, this: &mut BundleV2, @@ -7058,8 +6993,8 @@ pub mod bv2_impl { // `on_load` (copy-for-bundling path) parks plugin asset bytes // as `Cow::Owned` directly in this slot and gives the ParseTask // a borrowed alias. The full-Source swap just moved that owner - // into `result.source`; move it back so `parse_worker::on_complete`'s - // `drop(heap::take(result))` doesn't free the buffer + // into `result.source`; move it back so the graph, not the + // `Result` that `ParseComplete::run` frees, owns the buffer // `process_files_to_copy` will later `mem::take`. if matches!(result.source.contents, std::borrow::Cow::Owned(_)) { core::mem::swap( diff --git a/src/bundler/lib.rs b/src/bundler/lib.rs index 20338efed2b8..d3807f812bd4 100644 --- a/src/bundler/lib.rs +++ b/src/bundler/lib.rs @@ -110,6 +110,7 @@ pub mod linker_context_mod; pub mod options_impl; #[path = "ParseTask.rs"] pub mod parse_task; +pub mod post; pub mod transpiler; /// `linker_context/` submodule directory. Declared inline (no `mod.rs`). diff --git a/src/bundler/post.rs b/src/bundler/post.rs new file mode 100644 index 000000000000..37fe99b1cd13 --- /dev/null +++ b/src/bundler/post.rs @@ -0,0 +1,100 @@ +//! Events other threads deliver to the bundle thread: a worker finished a +//! parse, a plugin on the JS thread settled a resolve or a load or called +//! `defer()`. A bundle runs either on the JS event loop of the VM that owns it +//! (dev server) or on a mini event loop of its own (`Bun.build`, `bun build`); +//! the two loops queue work differently. [`post`] is the only place that +//! knows which loop a bundle is on, so every event has exactly one handler, +//! [`Event::run`], and cannot behave differently depending on the loop. + +use core::ptr::addr_of; + +use bun_event_loop::ConcurrentTask::ConcurrentTask; +use bun_event_loop::{AnyEventLoop, Posted}; + +use crate::bundle_v2::BundleV2; + +pub trait Event { + type Item; + + /// `offset_of!(Item, )` of the item's `AnyTaskWithExtraContext`; + /// the mini loop links the item into its queue through that field. + const NODE: usize; + + /// # Safety + /// `item` is live (it was, or is about to be, passed to [`post`]). + unsafe fn bundle(item: *mut Self::Item) -> *mut BundleV2<'static>; + + /// Runs on the bundle thread. Owns `item` to whatever extent the event + /// does: a parse result is freed here, a plugin `Load`/`Resolve` lives in + /// the bundle's arena and is only borrowed. + /// + /// # Safety + /// `item` was passed to [`post`] and has not been touched since. + unsafe fn run(item: *mut Self::Item, bv2: &mut BundleV2<'static>); + + /// The VM whose loop runs the bundle is gone, so `run` never will. + /// + /// # Safety + /// As for [`run`](Self::run). + unsafe fn refused(_item: *mut Self::Item) {} +} + +/// Queue `item` for `E::run` on the bundle thread. Callable from any thread. +/// +/// # Safety +/// `item` must stay valid until `run` or `refused` has consumed it, and +/// `E::bundle(item)` must be the live `BundleV2` this bundle pass belongs to. +pub unsafe fn post(item: *mut E::Item) { + // SAFETY: caller contract. + let bv2 = unsafe { E::bundle(item) }; + // SAFETY: `linker.loop` and `js_poster` are written once in `BundleV2::init` + // and never again, so they can be read from a worker or the JS thread while + // the bundle thread is mutating the rest of `*bv2`; only those two fields + // are touched, through raw pointers, so no reference to `*bv2` is formed. + let any_loop = unsafe { *addr_of!((*bv2).linker.r#loop) } + .expect("BundleV2.linker.loop must be set before work is posted to it"); + // SAFETY: `linker.loop` is a backref to the loop that owns this bundle pass + // and outlives it. The bundle thread may be ticking it concurrently; both + // arms below only push onto the loop's MPSC queue (the `Mini` arm also + // writes the node inside `*item`, which the caller owns) and wake it. + match unsafe { &mut *any_loop.as_ptr() } { + AnyEventLoop::Js { .. } => { + let task = ConcurrentTask::from_callback(item, run_on_js_loop::); + // SAFETY: see above. + let poster = unsafe { &*addr_of!((*bv2).js_poster) } + .as_ref() + .expect("a bundle on a JS loop has a poster"); + if let Posted::Refused(task) = poster.post(task) { + // SAFETY: `task` was created just above and never queued. + unsafe { ConcurrentTask::release_refused(task) }; + // SAFETY: caller contract; `run` will not observe `item`. + unsafe { E::refused(item) }; + } + } + AnyEventLoop::Mini(mini) => { + // SAFETY: `E::NODE` is the item's task field (trait contract) and + // the caller keeps `item` alive until it is dequeued. + unsafe { + mini.enqueue_task_concurrent_with_extra_ctx::>( + item, + run_on_mini_loop::, + E::NODE, + ); + } + } + } +} + +fn run_on_js_loop(item: *mut E::Item) -> bun_event_loop::JsResult<()> { + // SAFETY: `post`'s contract; the bundle thread holds no other `&mut BundleV2` + // while its loop is dispatching queued tasks. + unsafe { E::run(item, &mut *E::bundle(item)) }; + Ok(()) +} + +fn run_on_mini_loop(item: *mut E::Item, bv2: *mut BundleV2<'static>) { + // SAFETY: `post`'s contract. + debug_assert!(core::ptr::eq(bv2, unsafe { E::bundle(item) })); + // SAFETY: `post`'s contract; `bv2` is the bundle the mini loop is ticking for. + unsafe { E::run(item, &mut *bv2) }; +} diff --git a/src/runtime/api/JSBundler.rs b/src/runtime/api/JSBundler.rs index 53326b259f43..81ec716284af 100644 --- a/src/runtime/api/JSBundler.rs +++ b/src/runtime/api/JSBundler.rs @@ -12,7 +12,6 @@ use bun_collections::{StringMap, StringSet}; use bun_core::MutableString; use bun_core::Output; use bun_core::{String as BunString, ZigString}; -use bun_jsc::ConcurrentTask::ConcurrentTask; use bun_jsc::{self as jsc, CallFrame, JSGlobalObject, JSValue, JsError, JsResult}; use bun_options_types::compile_target::CompileTarget; use bun_options_types::schema::api; // bun.schema.api @@ -1400,33 +1399,18 @@ pub mod js_bundler { // NOTE: `Resolve`/`Load`/`MiniImportRecord`/etc. are owned by // `bun_bundler::bundle_v2::api::JSBundler` so that `BundleV2` can operate - // on them directly (`on_resolve_async`/`on_load_async`). `dispatch()` and - // `run_on_js_thread()` are also inherent methods there — they only need - // `bun_event_loop` types and the `Plugin` opaque, neither of which is a T6 - // dependency. Only the JSC-aware bits (`on_defer`, `JSBundlerPlugin__*` - // C-ABI exports) live here. + // on them directly (`on_resolve`/`on_load`, reached through `post`). + // `dispatch()` and `run_on_js_thread()` are also inherent methods there — + // they only need `bun_event_loop` types and the `Plugin` opaque, neither of + // which is a T6 dependency. Only the JSC-aware bits (`on_defer`, + // `JSBundlerPlugin__*` C-ABI exports) live here. pub use bun_bundler::bundle_v2::api::JSBundler::{ Load, LoadSuccess, LoadValue, MiniImportRecord, Resolve, ResolveSuccess, ResolveValue, }; + use bun_bundler::bundle_v2::{PluginLoadDeferred, PluginLoadSettled, PluginResolveSettled}; + use bun_bundler::post::post; - /// `&mut BundleV2` for the live backref stored on `Resolve`/`Load`. - /// - /// Centralises the `*mut BundleV2 → &mut` deref so the C++-called thunks - /// (`JSBundlerPlugin__onResolveAsync`, `on_defer`, `…__onLoadAsync`, - /// `…__addError`, `on_notify_defer_raw`) stay safe at the call site. `bv2` - /// is the back-reference set in `Resolve::init`/`Load::init`; the - /// `BundleV2` heap allocation outlives every plugin callback (owner- - /// creates-child, single-JS-thread). The `BundleV2` storage is heap- - /// disjoint from `Resolve`/`Load`, so the returned `&mut` does not alias - /// the caller's `&mut Resolve`/`&mut Load`. - #[inline] - fn bv2_mut<'a>(bv2: *mut BundleV2<'static>) -> &'a mut BundleV2<'static> { - // SAFETY: see fn doc — live backref (owner-creates-child), single - // JS-thread, disjoint heap from the `Resolve`/`Load` callers borrow. - unsafe { &mut *bv2 } - } - - /// `&mut Plugin` for the live `BundleV2` backref stored on `Resolve`/`Load`. + /// `&mut Plugin` for the `BundleV2` backref stored on `Resolve`/`Load`. /// /// Centralises the `Option → &mut T` deref so the three callers /// (`JSBundlerPlugin__onResolveAsync`, `on_defer`, @@ -1437,8 +1421,14 @@ pub mod js_bundler { /// the caller's `&mut Resolve`/`&mut Load`. #[inline] fn bv2_plugin<'a>(bv2: *mut BundleV2<'static>) -> &'a mut Plugin { + // SAFETY: `bv2` is the backref set in `Resolve::init`/`Load::init`; the + // pass outlives every plugin callback. `plugins` is written before the + // first dispatch and never again, so reading just that field through a + // raw pointer is fine while the bundle thread (for `Bun.build`, a + // different thread) holds `&mut` to the rest of `*bv2`. + let plugins = unsafe { *core::ptr::addr_of!((*bv2).plugins) }; // SAFETY: see fn doc — `plugins.is_some()`, disjoint heap. - unsafe { &mut *bv2_mut(bv2).plugins.unwrap().as_ptr() } + unsafe { &mut *plugins.expect("plugin callback without plugins").as_ptr() } } /// # Safety @@ -1479,7 +1469,8 @@ pub mod js_bundler { }); } - bv2_mut(resolve.bv2).on_resolve_async(resolve); + // SAFETY: `resolve` stays linked in the bundle until `on_resolve` consumes it. + unsafe { post::(resolve) }; } bun_output::declare_scope!(BUNDLER_DEFERRED, hidden); @@ -1507,63 +1498,15 @@ pub mod js_bundler { bstr::BStr::new(&self.path) ); - // Notify the *bundler thread* about the deferral. This will - // decrement the pending item counter and increment the deferred - // counter. Must land on `parse_task.ctx`'s `r#loop()` (the loop - // running BundleV2), which is distinct from the - // `enqueue_on_js_loop_for_plugins` target (the plugin host's JS loop) - // when `Bun.build` runs the bundler on its own Mini event loop. - // SAFETY: `parse_task.ctx` and `bv2` are valid backrefs; `r#loop()` - // points at a live `AnyEventLoop` owned by the bundle thread / - // runtime for the duration of the bundle. - unsafe { - let ctx = (*self.parse_task).ctx.expect("ParseTask.ctx unset"); - // SAFETY: write provenance from `ParseTask::init`; bundle outlives plugin. - let any_loop = ctx - .assume_mut() - .r#loop() - .expect("BundleV2.linker.loop must be set before plugins run"); - match &mut *any_loop.as_ptr() { - bun_event_loop::AnyEventLoop::Js { .. } => { - let ct = - ConcurrentTask::from_callback(ctx.as_mut_ptr(), on_notify_defer_raw); - let poster = (*ctx.as_mut_ptr()) - .js_poster - .as_ref() - .expect("JS-owned bundle has a poster"); - if let bun_event_loop::Posted::Refused(ct) = poster.post(ct) { - // Owning JS VM torn down mid-bundle: the notify never runs. - bun_event_loop::ConcurrentTask::ConcurrentTask::release_refused(ct); - } - } - bun_event_loop::AnyEventLoop::Mini(mini) => { - // `mini.enqueueTaskConcurrentWithExtraCtx( - // Load, BundleV2, this, BundleV2.onNotifyDeferMini, .task)` - mini.enqueue_task_concurrent_with_extra_ctx::>( - std::ptr::from_mut::(self), - on_notify_defer_mini_wrap, - core::mem::offset_of!(Load, task), - ); - } - } + // SAFETY: the `Load` stays outstanding (and its arena alive) until + // the bundle thread has drained the deferral and the plugin then + // settles it. + unsafe { post::(std::ptr::from_mut::(self)) }; - Ok(bv2_plugin(self.bv2).append_defer_promise()) - } + Ok(bv2_plugin(self.bv2).append_defer_promise()) } } - fn on_notify_defer_raw(ctx: *mut BundleV2<'static>) -> bun_event_loop::JsResult<()> { - bv2_mut(ctx).on_notify_defer(); - Ok(()) - } - - fn on_notify_defer_mini_wrap(load: *mut Load, ctx: *mut BundleV2<'static>) { - // SAFETY: callback contract — `load` was passed as the `Context` arg to - // `enqueue_task_concurrent_with_extra_ctx`; `ctx` is the bundle-thread - // `BundleV2` backref the mini loop's tick supplies as `extra`. - BundleV2::on_notify_defer_mini(unsafe { &mut *load }, unsafe { &mut *ctx }); - } - /// # Safety /// `load` must be the live `*mut Load` previously handed to C++ via /// `Load::dispatch`, and `global` must be the plugin's owning @@ -1625,7 +1568,8 @@ pub mod js_bundler { }); } - bv2_mut(this.bv2).on_load_async(this); + // SAFETY: `this` stays linked in the bundle until `on_load` consumes it. + unsafe { post::(this) }; } /// Opaque FFI handle for the C++ `JSBundlerPlugin`. The opaque type and @@ -1869,7 +1813,8 @@ pub mod js_bundler { let resolve = unsafe { bun_ptr::callback_ctx::(ctx) }; let msg = plugin_msg_from_js(plugin, &resolve.import_record.source_file, exception); resolve.value = ResolveValue::Err(msg); - bv2_mut(resolve.bv2).on_resolve_async(resolve); + // SAFETY: `resolve` stays linked in the bundle until `on_resolve` consumes it. + unsafe { post::(resolve) }; } 1 => { // SAFETY: C++ caller passes the live `*mut Load` it received from @@ -1877,7 +1822,8 @@ pub mod js_bundler { let load = unsafe { bun_ptr::callback_ctx::(ctx) }; let msg = plugin_msg_from_js(plugin, &load.path, exception); load.value = LoadValue::Err(msg); - bv2_mut(load.bv2).on_load_async(load); + // SAFETY: `load` stays linked in the bundle until `on_load` consumes it. + unsafe { post::(load) }; } _ => panic!("invalid error type"), } diff --git a/test/bake/dev/plugins.test.ts b/test/bake/dev/plugins.test.ts index fa1641f878af..d4de64e8dec4 100644 --- a/test/bake/dev/plugins.test.ts +++ b/test/bake/dev/plugins.test.ts @@ -66,6 +66,46 @@ devTest("onLoad", { await dev.fetch("/").equals("value: 1"); }, }); +devTest("onLoad defer() waits for the rest of the graph to load", { + framework: minimalFramework, + pluginFile: ` + let otherLoaded = false; + export default [ + { + name: 'a', + setup(build) { + build.onLoad({ filter: /\\.ts$/ }, async (args) => { + if (args.path.endsWith('other.ts')) otherLoaded = true; + if (!args.path.endsWith('trigger.ts')) return undefined; + await args.defer(); + return { contents: 'export const value = ' + otherLoaded + ';', loader: 'ts' }; + }); + }, + } + ]; + `, + files: { + "trigger.ts": ` + throw new Error('should not be loaded'); + `, + "other.ts": ` + export const other = 1; + `, + "routes/index.ts": ` + import { value } from '../trigger.ts'; + import { other } from '../other.ts'; + + export default function (req, meta) { + return new Response('other loaded before defer resolved: ' + value + ', other: ' + other); + } + `, + }, + async test(dev) { + await dev.fetch("/").equals("other loaded before defer resolved: true, other: 1"); + await dev.patch("other.ts", { find: "1", replace: "2" }); + await dev.fetch("/").equals("other loaded before defer resolved: true, other: 2"); + }, +}); devTest("onResolve + onLoad virtual file", { framework: minimalFramework, pluginFile: ` diff --git a/test/internal/source-lints/bundler-cross-thread-post.test.ts b/test/internal/source-lints/bundler-cross-thread-post.test.ts new file mode 100644 index 000000000000..873e0dad9866 --- /dev/null +++ b/test/internal/source-lints/bundler-cross-thread-post.test.ts @@ -0,0 +1,91 @@ +import { file } from "bun"; +import { expect, test } from "bun:test"; +import { realpathSync } from "fs"; +import path from "path"; +import { globAllSources } from "../../../scripts/glob-sources.ts"; + +// Everything another thread delivers to the bundle thread (a finished parse, +// a plugin's answer to an onResolve/onLoad, a `defer()` call) goes through +// `post::()` in src/bundler/post.rs. A bundle runs either on the JS loop of +// the VM that owns it (bake) or on a mini loop of its own (`Bun.build`, +// `bun build`), and the two are fed differently, so `post()` is the one place +// that matches on `AnyEventLoop::Js` / `AnyEventLoop::Mini` and the one caller +// of the mini loop's `enqueue_task_concurrent_with_extra_ctx`. Each event is +// then one `Event::run`, whichever loop it arrives on. +// +// This tree used to have that match written out at every producer (parse +// completion twice, resolve and load settlement, defer), each with its own +// pair of per-loop handlers, and the pairs drifted: `defer()` marked the +// `Load` as deferred on the mini-loop handler only. A new producer gets an +// `Event` impl and calls `post()`; it does not get its own match. + +const root = path.resolve(import.meta.dir, "..", "..", ".."); +const ALLOWED = "src/bundler/post.rs"; + +function inScope(source: string): boolean { + return ( + source.startsWith("src/bundler/") || + source.startsWith("src/bundler_jsc/") || + source === "src/runtime/api/JSBundler.rs" + ); +} + +// Only scan files tracked in HEAD (a `git stash` round-trip can leave stray +// `.rs` files in the working tree; CI runs on a clean checkout). Same guard as +// dead-code-escapes.test.ts. +const tracked: Set | null = (() => { + const r = Bun.spawnSync({ + cmd: ["git", "-C", root, "ls-tree", "-r", "--name-only", "-z", "HEAD"], + stdout: "pipe", + stderr: "ignore", + }); + if (!r.success) return null; + return new Set(r.stdout.toString().split("\0").filter(Boolean)); +})(); + +const BANNED = [/\bAnyEventLoop::Js\b/, /\benqueue_task_concurrent_with_extra_ctx\b/]; + +const offenders: string[] = []; +let scanned = 0; +let postRs: string | null = null; +for (const abs of globAllSources().rust) { + if (!abs.endsWith(".rs")) continue; + const source = path.relative(root, abs).replaceAll(path.sep, "/"); + if (!inScope(source)) continue; + if (path.relative(root, realpathSync(abs)).replaceAll(path.sep, "/") !== source) continue; + if (tracked !== null && !tracked.has(source)) continue; + scanned++; + const content = await file(abs).text(); + if (source === ALLOWED) { + postRs = content; + continue; + } + // Strip comments (whole-line, then trailing) so prose about `post()` does + // not count. None of the scanned files has `//` inside a string literal on + // a line that also names one of the banned identifiers. + const stripped = content.replace(/^[ \t]*\/\/.*$/gm, "").replace(/[ \t]\/\/.*$/gm, ""); + const lines = stripped.split("\n"); + for (let i = 0; i < lines.length; i++) { + for (const pattern of BANNED) { + if (pattern.test(lines[i])) offenders.push(`${source}:${i + 1}: ${lines[i].trim()}`); + } + } +} + +test("scans the bundler sources", () => { + // Guards against the scope/tracked/realpath filters leaving nothing to scan, + // which would make the ban below pass vacuously. + expect(scanned).toBeGreaterThan(1); +}); + +test("post() is where the bundle's loop is matched and fed", () => { + expect({ + present: postRs !== null, + matchesJsArm: postRs !== null && BANNED[0].test(postRs), + enqueuesOnMiniLoop: postRs !== null && BANNED[1].test(postRs), + }).toEqual({ present: true, matchesJsArm: true, enqueuesOnMiniLoop: true }); +}); + +test("no other bundler code dispatches on the bundle's loop or enqueues on it directly", () => { + expect(offenders).toEqual([]); +});