diff --git a/src/bundler/DeferredBatchTask.rs b/src/bundler/DeferredBatchTask.rs index e2920d2d0550..7e23907f9e34 100644 --- a/src/bundler/DeferredBatchTask.rs +++ b/src/bundler/DeferredBatchTask.rs @@ -1,84 +1,51 @@ -//! This task is run once all parse and resolve tasks have been complete -//! and we have deferred onLoad plugins that we need to resume. +//! Posted to the plugins' JS thread when the scan has nothing left to do but +//! the onLoad callbacks that called `.defer()`: the runtime's dispatch arm +//! resolves their promises (`JSBundlerPlugin__drainDeferred`) and frees this. //! -//! It enqueues a task to be run on the JS thread which resolves the promise -//! for every onLoad callback which called `.defer()`. +//! It carries only the plugin handle. A plugin that answers without awaiting +//! its `.defer()` promise lets the pass finish, and free its `BundleV2`, while +//! this task is still queued, so nothing here may point back into the pass. +//! The handle itself outlives the task: `Bun.build` destroys it from the +//! completion task, which is posted to the same queue after this, and bake's +//! plugins live as long as the dev server. + +use core::ptr::NonNull; use crate::BundleV2; -// Task is `(tag: u8, ptr: *mut ())` owned by bun_event_loop; -// runtime owns the match-loop. See PORTING.md §Dispatch. +use crate::bundle_v2::JSBundlerPlugin; use bun_event_loop::ConcurrentTask::ConcurrentTask; use bun_event_loop::{Task, task_tag}; -#[derive(Default)] pub struct DeferredBatchTask { - // Debug-only flag; zero-sized in release. - #[cfg(debug_assertions)] - running: bool, + plugins: NonNull, } impl bun_event_loop::Taskable for DeferredBatchTask { const TAG: bun_event_loop::TaskTag = task_tag::BundleV2DeferredBatchTask; - /// Embedded in its `BundleV2`, which outlives the queue entry and owns - /// everything the drain would have touched; nothing to free. - unsafe fn release_unrun(_: *mut Self) {} + unsafe fn release_unrun(this: *mut Self) { + // SAFETY: fn contract — `this` is the box `schedule` queued. + unsafe { bun_core::heap::destroy(this) }; + } } impl DeferredBatchTask { - pub(crate) fn init(&mut self) { - // Kept as `&mut self` (not `-> Self`) — this struct is embedded - // by value in BundleV2 (recovered via container_of in `get_bundle_v2`), so - // it is reset in place, never separately constructed. - #[cfg(debug_assertions)] - debug_assert!(!self.running); - // No Drop / no owned fields — pure reset. - let _ = core::mem::take(self); - } - - pub(crate) fn get_bundle_v2(&mut self) -> &mut BundleV2<'static> { - // SAFETY: `self` is always the `drain_defer_task` field of a live `BundleV2`; - // this struct is never instantiated standalone. Lifetime erased to 'static; - // callers must not outlive the owning bundle. - unsafe { - &mut *bun_core::from_field_ptr!( - BundleV2<'static>, - drain_defer_task, - std::ptr::from_mut::(self) - ) - } - } - - pub(crate) fn schedule(&mut self) { - #[cfg(debug_assertions)] - { - debug_assert!(!self.running); - self.running = false; - } - let task = ConcurrentTask::create(Task::init(std::ptr::from_mut::(self))); - - self.get_bundle_v2().enqueue_on_js_loop_for_plugins(task); - } - - pub fn run_on_js_thread(&mut self) { - // `deinit` only resets - // the debug `running` flag; nothing follows `drain_deferred`, so - // resetting the flag afterwards covers both paths. - { - let bv2 = self.get_bundle_v2(); - let rejected = bv2.completion.map(|c| c.result_is_err()).unwrap_or(false); - // The void result is discarded — see - // `Plugin::drain_deferred` for the exception-scope note. - bv2.plugins_mut().expect("plugins").drain_deferred(rejected); + /// Bundle thread (the loop that owns `bv2`). + pub(crate) fn schedule(bv2: &mut BundleV2) { + let plugins = bv2 + .plugins + .expect("a load deferred, so the pass has plugins"); + let this = bun_core::heap::into_raw(Box::new(Self { plugins })); + let task = ConcurrentTask::create(Task::init(this)); + if !bv2.enqueue_on_js_loop_for_plugins(task) { + // SAFETY: refused ⇒ the queue never took `this`; nothing else points at it. + unsafe { bun_core::heap::destroy(this) }; } - self.deinit(); } - // Not `impl Drop` — this struct is an intrusive field of `BundleV2` - // and `deinit` is a debug-flag reset, not resource teardown. - fn deinit(&mut self) { - #[cfg(debug_assertions)] - { - self.running = false; - } + /// JS thread, from the dispatch arm that owns the box. + pub fn plugins(&self) -> &JSBundlerPlugin { + // SAFETY: see the module doc — the handle is destroyed only after this + // task has run or been released. + unsafe { self.plugins.as_ref() } } } diff --git a/src/bundler/Graph.rs b/src/bundler/Graph.rs index aa13706b0826..0a2d8c04d068 100644 --- a/src/bundler/Graph.rs +++ b/src/bundler/Graph.rs @@ -5,6 +5,7 @@ use bun_ast::server_component_boundary; use bun_collections::MultiArrayList; use enum_map::EnumMap; +use crate::DeferredBatchTask::DeferredBatchTask; use crate::IndexStringMap::IndexStringMap; use crate::PathToSourceIndexMap::PathToSourceIndexMap; use crate::options; @@ -53,7 +54,8 @@ pub struct Graph<'a> { /// is "moved" into this counter (pending_items -= 1; deferred_pending += 1) /// /// When `pending_items` hits zero and there are deferred pending tasks, those - /// tasks will be run, and the count is "moved" back to `pending_items` + /// tasks will be run, and the count is "moved" back to `pending_items`; + /// a load answered before then moves its own back (`BundleV2::on_load`). pub(crate) deferred_pending: u32, /// onResolve / onLoad requests a plugin currently holds (dispatched to its @@ -243,8 +245,7 @@ impl<'a> Graph<'a> { } } - transpiler.drain_defer_task.init(); - transpiler.drain_defer_task.schedule(); + DeferredBatchTask::schedule(transpiler); return true; } @@ -274,6 +275,12 @@ impl Default for OutstandingLink { } } } +impl OutstandingLink { + /// Dispatched to the plugin and not answered yet. + pub(crate) fn is_linked(&self) -> bool { + self.linked + } +} pub trait OutstandingNode: Sized { fn link(&mut self) -> &mut OutstandingLink; } diff --git a/src/bundler/bundle_v2.rs b/src/bundler/bundle_v2.rs index 0af0e0ce2659..8fdbbac86dab 100644 --- a/src/bundler/bundle_v2.rs +++ b/src/bundler/bundle_v2.rs @@ -30,7 +30,6 @@ pub use bv2_impl::{ OnDependenciesAnalyze, singleton, }; -pub use crate::DeferredBatchTask::DeferredBatchTask; use crate::Graph::Graph; use crate::PathToSourceIndexMap::PathToSourceIndexMap; use crate::barrel_imports::RequestedExports; @@ -118,8 +117,6 @@ pub struct BundleV2<'a> { pub(crate) finalizers: Vec, - pub(crate) drain_defer_task: DeferredBatchTask, - /// Set true by DevServer. Currently every usage of the transpiler (Bun.build /// and `bun build` CLI) runs at the top of an event loop. When this is true, /// a callback is executed after all work is complete (`finishFromBakeDevServer`). @@ -206,7 +203,8 @@ impl<'a> BundleV2<'a> { } /// Mutable projection of the `plugins` backref for FFI calls that take - /// `*mut` (`drain_deferred`). The pointee is disjoint from `self` storage. + /// `*mut` (`match_on_load` / `match_on_resolve`). The pointee is disjoint + /// from `self` storage. #[inline] pub(crate) fn plugins_mut(&mut self) -> Option<&mut JSBundlerPlugin> { // SAFETY: BACKREF — see `plugins_ref`. `&mut self` ensures no other @@ -737,8 +735,6 @@ pub mod bv2_impl { context: *mut core::ffi::c_void, kind: u8, ); - #[link_name = "JSBundlerPlugin__drainDeferred"] - safe fn JSBundlerPlugin__drainDeferred(this: &mut Plugin, rejected: bool); #[link_name = "JSBundlerPlugin__hasOnBeforeParsePlugins"] safe fn JSBundlerPlugin__hasOnBeforeParsePlugins(this: &Plugin) -> i32; // `ctx`/`args`/`result` are opaque cookies the C++ side round-trips @@ -760,15 +756,6 @@ pub mod bv2_impl { ) -> i32; } impl Plugin { - /// `Plugin.drainDeferred` — resolve every onLoad - /// `.defer()` promise. The - /// only bundler caller (`DeferredBatchTask::run_on_js_thread`) - /// ignores failures, so the void FFI call is the observable - /// behaviour at this tier. - pub(crate) fn drain_deferred(&mut self, rejected: bool) { - JSBundlerPlugin__drainDeferred(self, rejected) - } - #[inline] pub(crate) fn has_on_before_parse_plugins(&self) -> bool { JSBundlerPlugin__hasOnBeforeParsePlugins(self) != 0 @@ -1199,15 +1186,17 @@ pub mod bv2_impl { pub parse_task: bun_ptr::BackRef, /// Faster path: skip the extra threadpool dispatch when the file is not found. pub was_file: bool, - /// Defer may only be called once. + /// Defer may only be called once (JS thread). pub called_defer: bool, - /// `.defer()`ed and not yet drained: its scan-counter unit sits in - /// `Graph::deferred_pending` (bundle thread only). + /// Its scan-counter unit currently 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 the onLoad answer (`on_load_async`). pub task: bun_event_loop::AnyTaskWithExtraContext::AnyTaskWithExtraContext, + /// Mini-loop queue node for the `.defer()` notification + /// (`on_defer_async`), which may still be queued when the answer is posted. + pub(crate) defer_task: + bun_event_loop::AnyTaskWithExtraContext::AnyTaskWithExtraContext, /// Links in `Graph::outstanding_loads`; bundle thread only. pub(crate) outstanding: crate::Graph::OutstandingLink, } @@ -1229,6 +1218,7 @@ pub mod bv2_impl { called_defer: false, deferred: false, task: bun_event_loop::AnyTaskWithExtraContext::AnyTaskWithExtraContext::default(), + defer_task: bun_event_loop::AnyTaskWithExtraContext::AnyTaskWithExtraContext::default(), outstanding: Default::default(), } } @@ -1321,7 +1311,6 @@ pub mod bv2_impl { use bun_sourcemap as SourceMap; use crate::AstBuilder::AstBuilder; - use crate::DeferredBatchTask::DeferredBatchTask; use crate::Graph::Graph; use crate::LinkerContext; use crate::PathToSourceIndexMap::PathToSourceIndexMap; @@ -1420,13 +1409,11 @@ pub mod bv2_impl { /// CYCLEBREAK GENUINE: `JSBundleCompletionTask` — the /// concrete struct lives in `bun_runtime` (its fields name `Config`/ - /// `Plugin`/`HTMLBundle::Route`). The bundler reads exactly two things - /// from it (whether the result is an error, and the concurrent-task + /// `Plugin`/`HTMLBundle::Route`). The bundler needs exactly two things + /// from it (whether its VM is shutting down, and the concurrent-task /// enqueue), so the high tier hands the bundler an erased owner + /// `&'static` vtable pair (same shape as [`DevServerHandle`]). pub struct CompletionDispatch { - /// Whether the completion result is an error. - pub result_is_err: unsafe fn(core::ptr::NonNull) -> bool, /// Whether the VM that owns the plugins is shutting down: stop /// waiting for their answers and fail the build (any thread). pub is_cancelled: unsafe fn(core::ptr::NonNull) -> bool, @@ -1447,18 +1434,13 @@ pub mod bv2_impl { // cross-thread call and it goes through `jsc::EventLoop`'s lock-free queue. unsafe impl Send for CompletionHandle {} // Intentionally not `Sync`: the opaque owner (`JSBundleCompletionTask`) - // is modeled as `!Sync`, and this wrapper exposes `result_is_err(&self)` + // is modeled as `!Sync`, and this wrapper exposes `is_cancelled(&self)` // in addition to the lock-free enqueue path, so blanket `&CompletionHandle` // sharing across threads is not justified. The handle only needs to *move* // to the bundle thread (`Send`), not be shared. If a cross-thread `&` ever // becomes necessary, split out an enqueue-only wrapper and make only that // type `Sync`. impl CompletionHandle { - #[inline] - pub(crate) fn result_is_err(&self) -> bool { - // SAFETY: vtable contract. - unsafe { (self.vtable.result_is_err)(self.owner) } - } #[inline] pub(crate) fn is_cancelled(&self) -> bool { // SAFETY: vtable contract. @@ -1525,15 +1507,19 @@ pub mod bv2_impl { /// Folds the JS-loop lookup + enqueue so the bundler never dereferences /// `JSBundleCompletionTask` (its layout lives in `bun_runtime`); the /// `completion` handle carries the `&'static` vtable. + /// + /// Returns `false` when the plugins' VM is already gone: the + /// `ConcurrentTask` has been freed, and whatever it pointed at is + /// still the caller's. pub(crate) fn enqueue_on_js_loop_for_plugins( &mut self, task: NonNull, - ) { + ) -> bool { debug_assert!(self.plugins.is_some()); if let Some(completion) = self.completion { // From Bun.build — the completion posts it to its VM (`loop_handle.post_task` via the vtable). completion.enqueue_task_concurrent(task); - return; + return true; } // From bake where the loop running the bundle is also the loop running // the plugins. @@ -1542,11 +1528,15 @@ pub mod bv2_impl { .js_poster .as_ref() .expect("No JavaScript event loop for transpiler plugins to run on"); - if let bun_event_loop::Posted::Refused(task) = poster.post(task) { - // The JS VM running the plugins was torn down mid-bundle; the - // plugin hop will never run. Free the task if it is heap-owned. - // SAFETY: refused ⇒ still ours. - unsafe { bun_event_loop::ConcurrentTask::ConcurrentTask::release_refused(task) }; + match poster.post(task) { + bun_event_loop::Posted::Queued => true, + bun_event_loop::Posted::Refused(task) => { + // SAFETY: refused ⇒ still ours. + unsafe { + bun_event_loop::ConcurrentTask::ConcurrentTask::release_refused(task) + }; + false + } } } @@ -2120,14 +2110,6 @@ pub mod bv2_impl { while let Some(load) = self.graph.outstanding_loads.pop() { // SAFETY: as above. let load = unsafe { &mut *load }; - if load.deferred { - // Its unit is parked in `deferred_pending`, not `pending_items`. - load.deferred = false; - self.graph.deferred_pending -= 1; - drop(core::mem::take(&mut load.path)); - drop(core::mem::take(&mut load.namespace)); - continue; - } load.value = jsc_api::JSBundler::LoadValue::Err(cancelled_msg(&load.path)); Self::on_load(load, self); } @@ -2857,7 +2839,6 @@ pub mod bv2_impl { unique_key: 0, dynamic_import_entry_points: ArrayHashMap::new(), finalizers: Vec::new(), - drain_defer_task: DeferredBatchTask::default(), asynchronous: false, has_any_top_level_await_modules: false, requested_exports: Vec::new(), @@ -4316,16 +4297,23 @@ 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. + /// Plugin host's thread: run a callback on the loop that owns this pass + /// (for `Bun.build`, the bundle thread's Mini loop; `graph` is only touched there). + /// + /// # Safety + /// `task_offset` locates an `AnyTaskWithExtraContext` field of `*request` that is + /// not queued anywhere else, and `*request` outlives the hop. + unsafe fn post_to_own_loop( + &mut self, + request: *mut C, + on_js_loop: fn(*mut C) -> bun_event_loop::JsResult<()>, + on_mini_loop: fn(*mut C, *mut BundleV2<'static>), + task_offset: usize, + ) { 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, + request, on_js_loop, ); let poster = self .js_poster @@ -4340,50 +4328,57 @@ pub mod bv2_impl { } } 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. + // SAFETY: fn contract; the tick passes this `BundleV2` as the extra ctx. 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), + mini.enqueue_task_concurrent_with_extra_ctx::>( + request, + on_mini_loop, + task_offset, ); } } } } + /// The onLoad answer is in `load.value`; run `on_load` on the owning loop. + pub fn on_load_async(&mut self, load: &mut jsc_api::JSBundler::Load) { + // SAFETY: the plugin glue answers a load once, so `task` is queued once; + // `load` is arena-owned by this pass. + unsafe { + self.post_to_own_loop( + std::ptr::from_mut(load), + on_load_from_js_loop_raw, + on_load_mini, + core::mem::offset_of!(jsc_api::JSBundler::Load, task), + ); + } + } + + /// The onLoad callback called `.defer()`; run `on_notify_defer` on the owning + /// loop. Shares the answer's queue, so the two arrive in the order they were issued. + pub fn on_defer_async(&mut self, load: &mut jsc_api::JSBundler::Load) { + // SAFETY: `called_defer` allows one `.defer()`, so `defer_task` is queued once; + // `load` is arena-owned by this pass. + unsafe { + self.post_to_own_loop( + std::ptr::from_mut(load), + on_notify_defer_from_js_loop_raw, + on_notify_defer_mini, + core::mem::offset_of!(jsc_api::JSBundler::Load, defer_task), + ); + } + } + + /// The onResolve answer is in `resolve.value`; run `on_resolve` on the owning loop. 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), - ); - } - } + // SAFETY: as `on_load_async`. + unsafe { + self.post_to_own_loop( + std::ptr::from_mut(resolve), + on_resolve_from_js_loop_raw, + on_resolve_mini, + core::mem::offset_of!(jsc_api::JSBundler::Resolve, task), + ); } } } @@ -4414,10 +4409,31 @@ pub mod bv2_impl { Ok(()) } + fn on_notify_defer_mini(load: *mut jsc_api::JSBundler::Load, this: *mut BundleV2<'static>) { + // SAFETY: see `on_load_mini`. + BundleV2::on_notify_defer(unsafe { &mut *load }, unsafe { &mut *this }); + } + + fn on_notify_defer_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`. + let load = unsafe { &mut *load }; + // SAFETY: `bv2` is a live backref set in `Load::init`. + let bv2 = unsafe { &mut *load.bv2 }; + BundleV2::on_notify_defer(load, bv2); + Ok(()) + } + impl<'a> BundleV2<'a> { pub(crate) fn on_load(load: &mut jsc_api::JSBundler::Load, this: &mut BundleV2) { this.graph.outstanding_loads.unlink(load); - load.deferred = false; + if load.deferred { + // Answered before the drain: the paths below expect the unit in `pending_items`. + load.deferred = false; + this.graph.deferred_pending -= 1; + this.increment_scan_counter(); + } // `Load` is arena-allocated (no Drop); free its owned heap fields on every exit path. struct LoadDeinitGuard(*mut jsc_api::JSBundler::Load); impl Drop for LoadDeinitGuard { @@ -6938,15 +6954,18 @@ 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) { + /// Owning loop: `load`'s onLoad callback called `.defer()`, so its scan-counter + /// unit moves to `deferred_pending` until `drain_deferred_tasks` (or `on_load`). + pub(crate) fn on_notify_defer(load: &mut jsc_api::JSBundler::Load, this: &mut BundleV2) { + this.thread_lock.assert_locked(); + // `.defer()` called after answering: `on_load` already disposed of the unit. + if !load.outstanding.is_linked() { + return; + } + debug_assert!(!load.deferred, "Load::called_defer allows one .defer()"); load.deferred = true; - this.on_notify_defer(); + this.graph.deferred_pending += 1; + this.decrement_scan_counter(); } pub(crate) fn on_parse_task_complete( diff --git a/src/jsc/bindings/JSBundlerPlugin.cpp b/src/jsc/bindings/JSBundlerPlugin.cpp index 8c87b28467d3..09fe5e234bbe 100644 --- a/src/jsc/bindings/JSBundlerPlugin.cpp +++ b/src/jsc/bindings/JSBundlerPlugin.cpp @@ -665,8 +665,11 @@ extern "C" void JSBundlerPlugin__setConfig(Bun::JSBundlerPlugin* plugin, void* c plugin->plugin.config = config; } -extern "C" void JSBundlerPlugin__drainDeferred(Bun::JSBundlerPlugin* pluginObject, bool rejected) +extern "C" void JSBundlerPlugin__drainDeferred(Bun::JSBundlerPlugin* pluginObject) { + if (pluginObject->plugin.deferredPromises.isEmpty()) + return; + auto* globalObject = pluginObject->globalObject(); MarkedArgumentBuffer arguments; pluginObject->plugin.deferredPromises.drainTo(pluginObject, arguments); @@ -676,11 +679,7 @@ extern "C" void JSBundlerPlugin__drainDeferred(Bun::JSBundlerPlugin* pluginObjec auto scope = DECLARE_THROW_SCOPE(vm); for (auto promiseValue : arguments) { JSPromise* promise = uncheckedDowncast(promiseValue); - if (rejected) { - promise->reject(vm, JSC::jsUndefined()); - } else { - promise->resolve(globalObject, vm, JSC::jsUndefined()); - } + promise->resolve(globalObject, vm, JSC::jsUndefined()); RETURN_IF_EXCEPTION(scope, ); } RETURN_IF_EXCEPTION(scope, ); diff --git a/src/runtime/api/JSBundler.rs b/src/runtime/api/JSBundler.rs index 248176152965..c0750cb535a0 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 @@ -1409,7 +1408,7 @@ pub mod js_bundler { /// /// 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` + /// `…__addError`) 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- @@ -1503,63 +1502,13 @@ 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.loop()` (the loop running - // BundleV2), which is distinct from `js_loop_for_plugins()` (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), - ); - } - } - - Ok(bv2_plugin(self.bv2).append_defer_promise()) - } + // Read before posting: the owning loop may write to the `Load` once it has it. + let bv2 = self.bv2; + bv2_mut(bv2).on_defer_async(self); + Ok(bv2_plugin(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 `ParentContext`. - 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 @@ -1652,6 +1601,7 @@ pub mod js_bundler { // sound and discharges the deref obligation at the type level. safe fn JSBundlerPlugin__globalObject(plugin: &Plugin) -> &JSGlobalObject; safe fn JSBundlerPlugin__appendDeferPromise(plugin: &mut Plugin) -> JSValue; + safe fn JSBundlerPlugin__drainDeferred(plugin: &Plugin); safe fn JSBundlerPlugin__setConfig(plugin: &mut Plugin, config: *mut c_void); safe fn JSBundlerPlugin__runSetupFunction( plugin: &Plugin, @@ -1688,6 +1638,10 @@ pub mod js_bundler { fn tombstone(&self); fn global_object(&self) -> &JSGlobalObject; fn append_defer_promise(&mut self) -> JSValue; + /// Resolve every `.defer()` promise handed out so far: the scan has + /// drained (`DeferredBatchTask`), or the build is complete and these + /// were never awaited. JS thread. + fn drain_deferred(&self); fn add_plugin( &mut self, object: JSValue, @@ -1763,6 +1717,16 @@ pub mod js_bundler { JSBundlerPlugin__appendDeferPromise(self) } + fn drain_deferred(&self) { + jsc::mark_binding(); + // The C++ side leaves a THROW_SCOPE to be checked here; resolving with + // `undefined` can only leave a termination pending, which the caller's + // next check picks up. + let _ = bun_jsc::call_check_slow(self.global_object(), || { + JSBundlerPlugin__drainDeferred(self) + }); + } + fn add_plugin( &mut self, object: JSValue, diff --git a/src/runtime/api/js_bundle_completion_task.rs b/src/runtime/api/js_bundle_completion_task.rs index 27657f0da198..cfb34c92fc85 100644 --- a/src/runtime/api/js_bundle_completion_task.rs +++ b/src/runtime/api/js_bundle_completion_task.rs @@ -634,6 +634,11 @@ impl JSBundleCompletionTask { return Ok(()); } + // `.defer()` promises whose onLoad answered without awaiting them. + if let Some(plugin) = this.plugins_mut() { + plugin.drain_deferred(); + } + if let Some(html_build_task) = this.html_build_task { this.plugins = None; // SAFETY: `html_build_task` is a backref set by `HTMLBundle::Route` which @@ -855,7 +860,6 @@ fn from_completion_handle<'a>(c: NonNull) -> &'a JSBundleCo } static COMPLETION_VTABLE: dispatch::CompletionDispatch = dispatch::CompletionDispatch { - result_is_err: |c| matches!(from_completion_handle(c).result, BundleV2Result::Err(_)), is_cancelled: |c| { from_completion_handle(c) .cancelled diff --git a/src/runtime/bake/DevServer.rs b/src/runtime/bake/DevServer.rs index c8f326837b28..b18bb499c22b 100644 --- a/src/runtime/bake/DevServer.rs +++ b/src/runtime/bake/DevServer.rs @@ -29,6 +29,7 @@ use bun_uws::{self as uws, AnyResponse, Opcode, Request, WebSocketUpgradeContext use bun_watcher::WatchItemColumns as _; use bun_wyhash::{Wyhash, hash}; +use crate::api::js_bundler::js_bundler::PluginJscExt as _; use crate::api::server::StaticRoute; use crate::api::{AnyServer, SavedRequest}; use crate::bake; @@ -3732,6 +3733,12 @@ impl<'a> HotUpdateContext<'a> { } fn finalize_bundle_cleanup(dev: &mut DevServer, bv2: &mut BundleV2, had_sent_hmr_event: bool) { + // `.defer()` promises whose onLoad answered without awaiting them. + if let Some(plugin) = dev.bundler_options.plugin { + // SAFETY: the handle this bundle ran with; the dev server owns it and + // keeps it for its own lifetime. + unsafe { plugin.as_ref() }.drain_deferred(); + } bv2.deinit_without_freeing_arena(); if let Some(cb) = &mut dev.current_bundle { cb.promise.deinit_idempotently(); diff --git a/src/runtime/dispatch.rs b/src/runtime/dispatch.rs index df0996b0438a..739a693271aa 100644 --- a/src/runtime/dispatch.rs +++ b/src/runtime/dispatch.rs @@ -478,13 +478,9 @@ pub(crate) fn run_task( ))?; } task_tag::BundleV2DeferredBatchTask => { - // `bun_bundler` is JSC-free so the exception-scope check is hoisted - // to this dispatch arm; without it, `JSBundlerPlugin__drainDeferred`'s - // THROW_SCOPE is left unchecked and trips JSC exception validation - // at the next `drainMicrotasks` scope. - let _ = bun_jsc::call_check_slow(global, || { - cast!(BundleV2DeferredBatchTask).run_on_js_thread(); - }); + // SAFETY: `DeferredBatchTask::schedule` boxed it; the arm consumes the box. + let batch = unsafe { bun_core::heap::take(cast_ptr!(BundleV2DeferredBatchTask)) }; + crate::api::JSBundler::PluginJscExt::drain_deferred(batch.plugins()); } // SAFETY: `cast_ptr!` yields the heap-allocated task; sole owner. task_tag::FlushPendingFileSinkTask => unsafe { diff --git a/test/bake/dev/plugins.test.ts b/test/bake/dev/plugins.test.ts index fa1641f878af..9fce1dc35863 100644 --- a/test/bake/dev/plugins.test.ts +++ b/test/bake/dev/plugins.test.ts @@ -110,6 +110,113 @@ devTest("onResolve + onLoad virtual file", { ]); }, }); +// The dev server runs the bundle on the JS thread itself (the other arm of the +// plugin hops exercised by test/bundler/bundler_defer.test.ts). A `defer()` issued +// after the callback already answered must not count against the scan: y's pending +// unit already belongs to the parse of its answer, and parking it again made the +// scan look finished before z (imported by that answer) was loaded, so x's +// `defer()` resolved early. Calling defer() this late is a misuse the JS side may +// reject outright (hence the try/catch); whatever still reaches the bundler must +// not count against the scan. +devTest("onLoad defer() after answering does not resolve other loads' defer() early", { + framework: minimalFramework, + pluginFile: ` + let zLoaded = false; + export default [ + { + name: 'late-defer', + setup(build) { + build.onLoad({ filter: /[\\\\/]x\\.ts$/ }, async ({ defer }) => { + await defer(); + return { contents: 'export const zLoadedBeforeXResumed = ' + zLoaded + ';', loader: 'ts' }; + }); + build.onLoad({ filter: /[\\\\/]y\\.ts$/ }, ({ defer }) => { + queueMicrotask(() => { + try { + void defer(); + } catch {} + }); + return { contents: 'import "./z.ts";', loader: 'ts' }; + }); + build.onLoad({ filter: /[\\\\/]z\\.ts$/ }, () => { + zLoaded = true; + return { contents: 'export const z = 1;', loader: 'ts' }; + }); + }, + }, + ]; + `, + files: { + "x.ts": `throw new Error('disk contents of x were bundled');`, + "y.ts": `throw new Error('disk contents of y were bundled');`, + "z.ts": `throw new Error('disk contents of z were bundled');`, + "routes/index.ts": ` + import { zLoadedBeforeXResumed } from '../x.ts'; + import '../y.ts'; + + export default function (req, meta) { + return new Response('z loaded before x resumed: ' + zLoadedBeforeXResumed); + } + `, + }, + async test(dev) { + await dev.fetch("/").equals("z loaded before x resumed: true"); + }, +}); +// x answers without awaiting its defer(): the answer is used, and the promise is +// resolved once the bundle is complete, i.e. after y (which only that answer +// imports) has been loaded. It used to resolve as soon as the scan momentarily +// looked finished. w answers only after x has called defer(), so the scan is never +// empty when that call is processed (otherwise resolving right away would be +// correct); x's answer is long so that w's parse finishes first. +devTest("onLoad defer() that is not awaited resolves when the bundle completes", { + framework: minimalFramework, + pluginFile: ` + let yLoaded = false; + globalThis.deferSettledAfterY = "pending"; + const xCalledDefer = Promise.withResolvers(); + const xContents = 'import "./y.ts";\\n' + Array.from({ length: 2000 }, (_, i) => 'export const x' + i + ' = ' + i + ';').join('\\n'); + export default [ + { + name: 'defer-without-await', + setup(build) { + build.onLoad({ filter: /[\\\\/]x\\.ts$/ }, ({ defer }) => { + defer().then(() => { globalThis.deferSettledAfterY = String(yLoaded); }); + xCalledDefer.resolve(); + return { contents: xContents, loader: 'ts' }; + }); + build.onLoad({ filter: /[\\\\/]w\\.ts$/ }, async () => { + await xCalledDefer.promise; + return { contents: 'export const w = 1;', loader: 'ts' }; + }); + build.onLoad({ filter: /[\\\\/]y\\.ts$/ }, () => { + yLoaded = true; + return { contents: 'export const y = 1;', loader: 'ts' }; + }); + }, + }, + ]; + `, + files: { + "x.ts": `throw new Error('disk contents of x were bundled');`, + "y.ts": `throw new Error('disk contents of y were bundled');`, + "w.ts": `throw new Error('disk contents of w were bundled');`, + "routes/index.ts": ` + import '../x.ts'; + import '../w.ts'; + + export default function (req, meta) { + return new Response('settled after y was loaded: ' + globalThis.deferSettledAfterY); + } + `, + }, + async test(dev) { + // The first request bundles the route; the promise's reaction runs once that + // bundle has been handed over, so read the outcome with a second request. + await dev.fetch("/"); + await dev.fetch("/").equals("settled after y was loaded: true"); + }, +}); // devTest("onLoad with watchFile", { // framework: minimalFramework, // pluginFile: ` diff --git a/test/bundler/bundler_defer.test.ts b/test/bundler/bundler_defer.test.ts index 0d00198731cf..30a05380d0e4 100644 --- a/test/bundler/bundler_defer.test.ts +++ b/test/bundler/bundler_defer.test.ts @@ -1,5 +1,5 @@ import { describe, expect, test } from "bun:test"; -import { bunEnv, bunExe, tempDir } from "harness"; +import { bunEnv, bunExe, bunRun, tempDir } from "harness"; import * as path from "node:path"; import { itBundled } from "./expectBundled"; @@ -656,3 +656,235 @@ warn: (msg: string) => console.warn(\`[WARN] \${msg}\`) expect(onFinalizeCallCount).toBe(3); }); }); + +// An onLoad callback that calls `defer()` without waiting for it. The bundler used +// to count such a load twice (once when `defer()` parked its pending unit, again +// when the parse scheduled by the answer completed) and, on the Bun.build thread, +// to queue both notifications on the same intrusive node, so a build died with +// `panic: int cast: TryFromIntError(NegOverflow)` in `BundleV2::on_parse_task_complete` +// or never finished; whether the promise ever settled depended on the same timing. +// The builds run in a subprocess because the failures take the whole process down. +describe("defer() that is not awaited", () => { + test.concurrent( + "before answering: builds with the plugin's contents and settles the promise when the build completes", + async () => { + const moduleNames = Array.from({ length: 24 }, (_, i) => `m${i}`); + using dir = tempDir( + "defer-no-await", + Object.fromEntries([ + ...moduleNames.map(name => [`${name}.ts`, `throw new Error("disk contents of ${name} were bundled");`]), + ["entry.ts", `throw new Error("disk contents of entry were bundled");`], + [ + "build.ts", + /* ts */ ` + const moduleNames = ${JSON.stringify(moduleNames)}; + const entryContents = moduleNames.map(name => 'import "./' + name + '";').join("\\n"); + for (let build = 0; build < 4; build++) { + let issued = 0; + let settled = 0; + const result = await Bun.build({ + entrypoints: [import.meta.dir + "/entry.ts"], + format: "iife", + plugins: [ + { + name: "defer-without-await", + setup(build) { + build.onLoad({ filter: /\\.ts$/ }, args => { + issued++; + args.defer().then(() => settled++); + const base = args.path.replaceAll("\\\\", "/").split("/").pop().slice(0, -".ts".length); + return { + loader: "ts", + contents: base === "entry" ? entryContents : 'globalThis.loaded.push("' + base + '");', + }; + }); + }, + }, + ], + }); + if (!result.success) { + console.log("build " + build + " failed:", result.logs.map(String)); + process.exit(1); + } + // The leftover promises are resolved before the build's own promise is. + if (issued !== moduleNames.length + 1 || settled !== issued) { + console.log("build " + build + ": " + settled + " of " + issued + " defer() promises settled"); + process.exit(1); + } + globalThis.loaded = []; + new Function(await result.outputs[0].text())(); + const loaded = globalThis.loaded; + if (loaded.length !== moduleNames.length || moduleNames.some((name, i) => loaded[i] !== name)) { + console.log("build " + build + " bundled the wrong modules:", loaded); + process.exit(1); + } + } + console.log("ok"); + `, + ], + ]), + ); + + expect(await bunRun(path.join(String(dir), "build.ts"))).toSpawn("ok"); + }, + ); + + // With a gap between defer() and the answer, the bundle thread sees the scan + // reach zero, schedules the task that resolves the promise, and then gets the + // answer and finishes the build while that task is still queued behind a busy + // JS thread. The task used to live inside the build's BundleV2 (ASAN: + // heap-use-after-free in DeferredBatchTask::run_on_js_thread); it must not + // touch the build at all. + test.concurrent("before answering: a drain scheduled before the answer outlives the build safely", async () => { + using dir = tempDir("defer-drain-outlives-build", { + "entry.ts": `throw new Error("disk contents of entry were bundled");`, + "build.ts": /* ts */ ` + // Spinning (not sleeping) keeps this thread busy; there is no event the + // bundle thread could signal, it is the bundle thread we are racing. The + // windows are generous so a loaded machine still lets it win. + const spin = (ms: number) => { + const end = performance.now() + ms; + while (performance.now() < end) {} + }; + let settled = false; + const result = await Bun.build({ + entrypoints: [import.meta.dir + "/entry.ts"], + plugins: [ + { + name: "defer-then-busy", + setup(build) { + build.onLoad({ filter: /entry\\.ts$/ }, args => { + args.defer().then(() => (settled = true)); + spin(30); // let the bundle thread take the defer() notification first + queueMicrotask(() => spin(600)); // runs right after the answer is posted + return { contents: "export const x = 1;", loader: "ts" }; + }); + }, + }, + ], + }); + if (!result.success || !settled) { + console.log("success=" + result.success + " settled=" + settled); + process.exit(1); + } + console.log("ok"); + `, + }); + + expect(await bunRun(path.join(String(dir), "build.ts"))).toSpawn("ok"); + }); + + // Here the load's pending unit already belongs to the parse its answer scheduled. + // Parking it anyway made the scan look finished early, which resolved the + // `defer()` promises of loads that were genuinely waiting before the answer's + // own imports had been loaded. Calling defer() this late is a misuse that the + // JS side may reject outright (hence the try/catch); whatever still reaches the + // bundler must not count against the scan. + test.concurrent("after answering: does not resolve other loads' defer() early", async () => { + using dir = tempDir("defer-after-answer", { + "entry.ts": `import "./x"; import "./y";`, + "x.ts": `throw new Error("disk contents of x were bundled");`, + "y.ts": `throw new Error("disk contents of y were bundled");`, + "z.ts": `throw new Error("disk contents of z were bundled");`, + "build.ts": /* ts */ ` + for (let build = 0; build < 5; build++) { + const events: string[] = []; + const result = await Bun.build({ + entrypoints: [import.meta.dir + "/entry.ts"], + plugins: [ + { + name: "late-defer", + setup(build) { + // x waits for every other module, as documented. + build.onLoad({ filter: /[\\\\/]x\\.ts$/ }, async ({ defer }) => { + events.push("x:defer"); + await defer(); + events.push("x:resumed"); + return { contents: "export const x = 1;", loader: "ts" }; + }); + // y answers synchronously (so onLoadAsync runs before the microtask), + // then calls defer() once its answer is already on its way. + build.onLoad({ filter: /[\\\\/]y\\.ts$/ }, ({ defer }) => { + queueMicrotask(() => { + try { + void defer(); + } catch {} + }); + events.push("y:load"); + return { contents: 'import "./z";', loader: "ts" }; + }); + // z only becomes known once y's answer has been parsed. + build.onLoad({ filter: /[\\\\/]z\\.ts$/ }, () => { + events.push("z:load"); + return { contents: "export const z = 1;", loader: "ts" }; + }); + }, + }, + ], + }); + if (!result.success) { + console.log("build " + build + " failed:", result.logs.map(String)); + process.exit(1); + } + const resumed = events.indexOf("x:resumed"); + const zLoaded = events.indexOf("z:load"); + if (resumed === -1 || zLoaded === -1 || zLoaded > resumed) { + console.log("build " + build + " resumed x before z was loaded:", events); + process.exit(1); + } + } + console.log("ok"); + `, + }); + + expect(await bunRun(path.join(String(dir), "build.ts"))).toSpawn("ok"); + }); + + // Terminating the worker cancels the build while one load is parked in + // `defer()` and another is still unanswered; both are failed through the + // same path, and the parked one has to give its unit back first. + test.concurrent("cancelling the build while a load is parked in defer()", async () => { + using dir = tempDir("defer-cancelled", { + "main.ts": /* ts */ ` + const worker = new Worker(new URL("./worker.ts", import.meta.url).href); + const { data } = await new Promise((resolve, reject) => { + worker.addEventListener("message", resolve, { once: true }); + worker.addEventListener("error", reject, { once: true }); + }); + if (data !== "armed") { + console.log(data); + process.exit(1); + } + await worker.terminate(); + console.log("terminated"); + `, + "worker.ts": /* ts */ ` + let entered = 0; + const armIfBothEntered = () => ++entered === 2 && postMessage("armed"); + Bun.build({ + entrypoints: ["virtual:deferring", "virtual:stuck"], + plugins: [ + { + name: "cancelled-while-deferred", + setup(build) { + build.onResolve({ filter: /^virtual:/ }, args => ({ path: args.path, namespace: "v" })); + build.onLoad({ filter: /deferring/, namespace: "v" }, async ({ defer }) => { + const everythingElse = defer(); + armIfBothEntered(); + await everythingElse; + return { contents: "export const deferring = 1;", loader: "ts" }; + }); + build.onLoad({ filter: /stuck/, namespace: "v" }, () => new Promise(armIfBothEntered)); + }, + }, + ], + }).then( + () => postMessage("unexpected: the build finished"), + error => postMessage("unexpected: the build failed before being cancelled: " + error), + ); + `, + }); + + expect(await bunRun(path.join(String(dir), "main.ts"))).toSpawn("terminated"); + }); +});