diff --git a/src/bundler/ParseTask.rs b/src/bundler/ParseTask.rs index 67402efd94a..e58ba5156a9 100644 --- a/src/bundler/ParseTask.rs +++ b/src/bundler/ParseTask.rs @@ -2852,7 +2852,7 @@ pub mod parse_worker { .any_loop_mut() .expect("BundleV2.linker.loop must be set before scheduling ParseTask") { - bun_event_loop::AnyEventLoop::Js { owner } => { + bun_event_loop::AnyEventLoop::Js { owner, generation } => { owner.enqueue_task_concurrent( bun_event_loop::ConcurrentTask::ConcurrentTask::from_callback(result, |p| { // SAFETY: `p` is the `result` Box leaked above; ownership @@ -2860,6 +2860,7 @@ pub mod parse_worker { unsafe { on_complete(p) }; Ok(()) }), + *generation, ); } bun_event_loop::AnyEventLoop::Mini(mini) => { diff --git a/src/bundler/ServerComponentParseTask.rs b/src/bundler/ServerComponentParseTask.rs index e8a51b602cd..cde720b35cd 100644 --- a/src/bundler/ServerComponentParseTask.rs +++ b/src/bundler/ServerComponentParseTask.rs @@ -121,7 +121,7 @@ fn task_callback_wrap(thread_pool_task: *mut ThreadPoolTask) { .any_loop_mut() .expect("BundleV2.linker.loop must be set before scheduling ServerComponentParseTask") { - bun_event_loop::AnyEventLoop::Js { owner } => { + bun_event_loop::AnyEventLoop::Js { owner, generation } => { owner.enqueue_task_concurrent( bun_event_loop::ConcurrentTask::ConcurrentTask::from_callback(result, |p| { // SAFETY: `p` is the `result` Box leaked above; ownership @@ -129,6 +129,7 @@ fn task_callback_wrap(thread_pool_task: *mut ThreadPoolTask) { unsafe { on_complete(p) }; Ok(()) }), + *generation, ); } bun_event_loop::AnyEventLoop::Mini(mini) => { diff --git a/src/bundler/bundle_v2.rs b/src/bundler/bundle_v2.rs index a622920a50b..b56abdff2d2 100644 --- a/src/bundler/bundle_v2.rs +++ b/src/bundler/bundle_v2.rs @@ -1565,8 +1565,8 @@ pub mod bv2_impl { // the plugins. // `any_loop_mut` centralises the BACKREF deref of `linker.r#loop`. match &*self.any_loop_mut() { - bun_event_loop::AnyEventLoop::Js { owner } => { - owner.enqueue_task_concurrent(task); + bun_event_loop::AnyEventLoop::Js { owner, generation } => { + owner.enqueue_task_concurrent(task, *generation); } bun_event_loop::AnyEventLoop::Mini(_) => { panic!("No JavaScript event loop for transpiler plugins to run on"); @@ -4194,12 +4194,13 @@ pub mod bv2_impl { // `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 { owner } => { + bun_event_loop::AnyEventLoop::Js { owner, generation } => { owner.enqueue_task_concurrent( bun_event_loop::ConcurrentTask::ConcurrentTask::from_callback( std::ptr::from_mut(load), on_load_from_js_loop_raw, ), + *generation, ); } bun_event_loop::AnyEventLoop::Mini(mini) => { @@ -4219,12 +4220,13 @@ pub mod bv2_impl { 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 { owner } => { + bun_event_loop::AnyEventLoop::Js { owner, generation } => { owner.enqueue_task_concurrent( bun_event_loop::ConcurrentTask::ConcurrentTask::from_callback( std::ptr::from_mut(resolve), on_resolve_from_js_loop_raw, ), + *generation, ); } bun_event_loop::AnyEventLoop::Mini(mini) => { diff --git a/src/event_loop/AnyEventLoop.rs b/src/event_loop/AnyEventLoop.rs index 98b75524e59..1c7df0ad7ac 100644 --- a/src/event_loop/AnyEventLoop.rs +++ b/src/event_loop/AnyEventLoop.rs @@ -45,6 +45,22 @@ fn jsc_event_loop_handle(js_event_loop: *mut ()) -> JsEventLoop { unsafe { JsEventLoop::new(JsEventLoopKind::Jsc, js_event_loop) } } +/// Registration generation of a live JS event loop, captured at handle +/// construction and carried next to the erased pointer so the cross-thread +/// `enqueue_task_concurrent` dispatch can be validated against the live-VM +/// registry even after the loop (a terminated worker's) is freed. 0 for the +/// null "never dispatched" placeholder handle. +#[inline] +fn js_loop_generation(owner: JsEventLoop) -> u64 { + if owner.owner.is_null() { + 0 + } else { + // The dispatch dereferences the loop — sound here because every + // handle constructor requires the loop to be live at construction. + owner.live_generation() + } +} + /// Useful for code that may need an event loop and could be used from either JavaScript or directly without JavaScript. /// Unlike jsc.EventLoopHandle, this owns the event loop when it's not a JavaScript event loop. // Variant order/discriminant must match `crate::EventLoopKind`. @@ -54,6 +70,12 @@ pub enum AnyEventLoop<'a> { /// `link_interface!` invariant ("owner is live for every dispatch") is /// established once at construction; dispatch is safe. owner: JsEventLoop, + /// Registration generation captured with `owner` — see + /// [`js_loop_generation`]. Carried by the cross-thread + /// `enqueue_task_concurrent` dispatch, which is the one method that + /// must tolerate `owner` pointing at a freed (terminated worker) + /// loop. + generation: u64, }, Mini(Box>), } @@ -73,7 +95,7 @@ impl<'a> Default for AnyEventLoop<'a> { impl<'a> AnyEventLoop<'a> { pub fn iteration_number(&self) -> u64 { match self { - AnyEventLoop::Js { owner } => owner.iteration_number(), + AnyEventLoop::Js { owner, .. } => owner.iteration_number(), // SAFETY: see `MiniEventLoop::loop_ptr()` invariant. AnyEventLoop::Mini(mini) => unsafe { (*mini.loop_ptr()).iteration_number() }, } @@ -97,11 +119,15 @@ impl<'a> AnyEventLoop<'a> { /// `js_event_loop` is a live erased `*mut jsc::EventLoop` /// that outlives every dispatch through the returned /// `AnyEventLoop`. The pointer is not dereferenced here — it's stored - /// opaquely in [`JsEventLoop`] and only dereferenced at dispatch sites. + /// opaquely in [`JsEventLoop`] and only dereferenced at dispatch sites + /// (the generation capture reads it once, while it is live per the + /// constructor contract). #[inline] pub fn js(js_event_loop: *mut ()) -> AnyEventLoop<'static> { + let owner = jsc_event_loop_handle(js_event_loop); AnyEventLoop::Js { - owner: jsc_event_loop_handle(js_event_loop), + owner, + generation: js_loop_generation(owner), } } @@ -109,8 +135,10 @@ impl<'a> AnyEventLoop<'a> { /// Replaces `jsc::VirtualMachine::get().event_loop()` for tier-≤4 callers /// (e.g. `bun_install::PackageManager`). pub fn js_current() -> AnyEventLoop<'static> { + let owner = JsEventLoop::current(); AnyEventLoop::Js { - owner: JsEventLoop::current(), + owner, + generation: js_loop_generation(owner), } } @@ -122,7 +150,7 @@ impl<'a> AnyEventLoop<'a> { is_done: fn(*mut core::ffi::c_void) -> bool, ) { match self { - AnyEventLoop::Js { owner } => { + AnyEventLoop::Js { owner, .. } => { while !is_done(context) { owner.tick(); owner.auto_tick(); @@ -158,7 +186,7 @@ impl<'a> AnyEventLoop<'a> { // returns; the borrow ends at the bottom of this loop body before // the next `is_done` call. match unsafe { &mut *this } { - AnyEventLoop::Js { owner } => { + AnyEventLoop::Js { owner, .. } => { owner.tick(); owner.auto_tick(); } @@ -176,7 +204,7 @@ impl<'a> AnyEventLoop<'a> { pub fn tick_once(&mut self, context: *mut core::ffi::c_void) { match self { - AnyEventLoop::Js { owner } => { + AnyEventLoop::Js { owner, .. } => { let _ = context; owner.tick(); owner.auto_tick_active(); @@ -284,6 +312,9 @@ pub enum EventLoopHandle { /// [`AnyEventLoop::Js`]. `JsEventLoop` is `Copy`, so the handle stays /// `Copy`. owner: JsEventLoop, + /// Registration generation captured with `owner` — see + /// [`js_loop_generation`] and [`AnyEventLoop::Js`]. + generation: u64, }, // `BackRef` (not `&mut`) because the handle is `Copy` and // stored in `uws::InternalLoopData` as a non-owning backref. @@ -369,8 +400,10 @@ impl EventLoopHandle { /// that are overwritten before use). #[inline] pub fn init(js_event_loop: *mut ()) -> EventLoopHandle { + let owner = jsc_event_loop_handle(js_event_loop); EventLoopHandle::Js { - owner: jsc_event_loop_handle(js_event_loop), + owner, + generation: js_loop_generation(owner), } } @@ -395,7 +428,7 @@ impl EventLoopHandle { // which is what the `EventLoopCtxKind::Js` `link_impl_EventLoopCtx!` // (in `bun_jsc`) is written for. Both are per-thread singletons // that outlive the ctx. - EventLoopHandle::Js { owner } => unsafe { + EventLoopHandle::Js { owner, .. } => unsafe { bun_io::EventLoopCtx::new(bun_io::EventLoopCtxKind::Js, owner.bun_vm()) }, // `mini` is a `BackRef` to the live per-thread singleton (see @@ -435,12 +468,18 @@ impl EventLoopHandle { ptr: *mut core::ffi::c_void, ) -> EventLoopHandle { match tag { - 1 => EventLoopHandle::Js { + 1 => { // SAFETY: `(tag, ptr)` was produced by `into_tag_ptr` on a // still-live event loop, so `ptr` is a live erased - // `*mut jsc::EventLoop`. Same boundary as `EventLoopHandle::init`. - owner: unsafe { JsEventLoop::new(JsEventLoopKind::Jsc, ptr.cast::<()>()) }, - }, + // `*mut jsc::EventLoop`. Same boundary as `EventLoopHandle::init` + // (which also licenses the generation read in + // `js_loop_generation`). + let owner = unsafe { JsEventLoop::new(JsEventLoopKind::Jsc, ptr.cast::<()>()) }; + EventLoopHandle::Js { + owner, + generation: js_loop_generation(owner), + } + } // `(tag, ptr)` came from `into_tag_ptr` on a live loop, so `ptr` // is non-null. `BackRef: From>`. 2 => EventLoopHandle::Mini(NonNull::new(ptr.cast()).expect("non-null mini ptr").into()), @@ -471,7 +510,10 @@ impl EventLoopHandle { pub fn from_any(any: &mut AnyEventLoop<'static>) -> EventLoopHandle { match any { - AnyEventLoop::Js { owner } => EventLoopHandle::Js { owner: *owner }, + AnyEventLoop::Js { owner, generation } => EventLoopHandle::Js { + owner: *owner, + generation: *generation, + }, AnyEventLoop::Mini(mini) => EventLoopHandle::Mini(BackRef::new_mut(&mut **mini)), } } @@ -479,15 +521,17 @@ impl EventLoopHandle { /// `EventLoopHandle` for the current thread's JS event loop. Replaces /// `jsc::EventLoopHandle.init(jsc::VirtualMachine.get())` for tier-≤4 callers. pub fn js_current() -> EventLoopHandle { + let owner = JsEventLoop::current(); EventLoopHandle::Js { - owner: JsEventLoop::current(), + owner, + generation: js_loop_generation(owner), } } /// Erased `*mut jsc::JSGlobalObject` or null (Mini has no JS global). pub fn global_object(self) -> *mut () { match self { - EventLoopHandle::Js { owner } => owner.global_object(), + EventLoopHandle::Js { owner, .. } => owner.global_object(), EventLoopHandle::Mini(_) => core::ptr::null_mut(), } } @@ -495,7 +539,7 @@ impl EventLoopHandle { /// Erased `*mut jsc::VirtualMachine` or null. pub fn bun_vm(self) -> *mut () { match self { - EventLoopHandle::Js { owner } => owner.bun_vm(), + EventLoopHandle::Js { owner, .. } => owner.bun_vm(), EventLoopHandle::Mini(_) => core::ptr::null_mut(), } } @@ -503,7 +547,7 @@ impl EventLoopHandle { /// Erased `*mut webcore::blob::Store`. pub fn stdout(self) -> *mut () { match self { - EventLoopHandle::Js { owner } => owner.stdout(), + EventLoopHandle::Js { owner, .. } => owner.stdout(), EventLoopHandle::Mini(mut mini) => mini_mut(&mut mini).stdout(), } } @@ -511,19 +555,19 @@ impl EventLoopHandle { /// Erased `*mut webcore::blob::Store`. pub fn stderr(self) -> *mut () { match self { - EventLoopHandle::Js { owner } => owner.stderr(), + EventLoopHandle::Js { owner, .. } => owner.stderr(), EventLoopHandle::Mini(mut mini) => mini_mut(&mut mini).stderr(), } } pub fn enter(self) { - if let EventLoopHandle::Js { owner } = self { + if let EventLoopHandle::Js { owner, .. } = self { owner.enter(); } } pub fn exit(self) { - if let EventLoopHandle::Js { owner } = self { + if let EventLoopHandle::Js { owner, .. } = self { owner.exit(); } } @@ -542,7 +586,7 @@ impl EventLoopHandle { /// for the brief region they need `&mut`. pub fn file_polls(self) -> *mut bun_io::file_poll::Store { match self { - EventLoopHandle::Js { owner } => owner.file_polls(), + EventLoopHandle::Js { owner, .. } => owner.file_polls(), EventLoopHandle::Mini(mut mini) => std::ptr::from_mut(mini_mut(&mut mini).file_polls()), } } @@ -557,7 +601,7 @@ impl EventLoopHandle { match self { // `JsEventLoop::put_file_poll` takes a raw `*mut FilePoll`; pass // the decayed `poll_ptr` straight through. - EventLoopHandle::Js { owner } => { + EventLoopHandle::Js { owner, .. } => { owner.put_file_poll(poll_ptr.as_ptr(), was_ever_registered) } // ctx only touches `after_event_loop_callback{,_ctx}`, field-disjoint @@ -573,11 +617,14 @@ impl EventLoopHandle { pub fn enqueue_task_concurrent(self, task: EventLoopTaskPtr) { match self { - EventLoopHandle::Js { owner } => { + EventLoopHandle::Js { owner, generation } => { // SAFETY: caller guarantees `task.js` is the active union member // when `self` is `Js`, and points at a live `ConcurrentTask` - // (non-null). - owner.enqueue_task_concurrent(unsafe { NonNull::new_unchecked(task.js) }) + // (non-null). The dispatch validates `(owner, generation)` against + // the live-VM registry, so a freed (terminated worker) loop + // drops the task instead of being dereferenced. + owner + .enqueue_task_concurrent(unsafe { NonNull::new_unchecked(task.js) }, generation) } EventLoopHandle::Mini(mut mini) => { // SAFETY: caller guarantees `task.mini` is the active union @@ -591,7 +638,7 @@ impl EventLoopHandle { pub fn r#loop(self) -> *mut UwsLoop { match self { - EventLoopHandle::Js { owner } => owner.uws_loop(), + EventLoopHandle::Js { owner, .. } => owner.uws_loop(), // `loop_ptr` takes `&self`; safe via `BackRef: Deref`. EventLoopHandle::Mini(mini) => mini.loop_ptr(), } @@ -630,7 +677,7 @@ impl EventLoopHandle { /// Same `Copy`-handle aliasing concern as [`file_polls`]. pub fn pipe_read_buffer(self) -> *mut [u8] { match self { - EventLoopHandle::Js { owner } => owner.pipe_read_buffer(), + EventLoopHandle::Js { owner, .. } => owner.pipe_read_buffer(), EventLoopHandle::Mini(mut mini) => { std::ptr::from_mut::<[u8]>(mini_mut(&mut mini).pipe_read_buffer()) } @@ -649,7 +696,7 @@ impl EventLoopHandle { pub fn env(self) -> *mut DotEnvLoader<'static> { match self { - EventLoopHandle::Js { owner } => owner.env(), + EventLoopHandle::Js { owner, .. } => owner.env(), // `env` must be set — caller invariant. `env_ptr()` takes // `&self` and returns `Option>` (mutable // provenance). Safe via `BackRef: Deref`. @@ -664,7 +711,7 @@ impl EventLoopHandle { pub fn top_level_dir(self) -> &'static [u8] { match self { // SAFETY: slice borrowed for VM lifetime. - EventLoopHandle::Js { owner } => unsafe { &*owner.top_level_dir() }, + EventLoopHandle::Js { owner, .. } => unsafe { &*owner.top_level_dir() }, // SAFETY: `BackRef::get()` ties the borrow to the local `mini`, but // the pointee is the per-thread singleton (process-lifetime); widen // to `'static` so the return type matches the Js arm. @@ -676,7 +723,7 @@ impl EventLoopHandle { self, ) -> Result { match self { - EventLoopHandle::Js { owner } => owner.create_null_delimited_env_map(), + EventLoopHandle::Js { owner, .. } => owner.create_null_delimited_env_map(), EventLoopHandle::Mini(mini) => { // `env_ptr()` takes `&self` — safe via `BackRef: Deref`. // `env` must be set (caller invariant). diff --git a/src/event_loop/lib.rs b/src/event_loop/lib.rs index 8a9a5dab864..1c5e4517b96 100644 --- a/src/event_loop/lib.rs +++ b/src/event_loop/lib.rs @@ -64,7 +64,8 @@ bun_dispatch::link_interface! { fn enter(); fn exit(); fn enqueue_task(task: Task); - fn enqueue_task_concurrent(task: core::ptr::NonNull); + fn enqueue_task_concurrent(task: core::ptr::NonNull, generation: u64); + fn live_generation() -> u64; fn env() -> *mut bun_dotenv::Loader<'static>; fn top_level_dir() -> *const [u8]; fn create_null_delimited_env_map() -> Result; diff --git a/src/install/PackageManager.rs b/src/install/PackageManager.rs index 2159986104a..60d326c8ae4 100644 --- a/src/install/PackageManager.rs +++ b/src/install/PackageManager.rs @@ -982,7 +982,7 @@ impl PackageManager { // `WakeHandler.handler`'s second arg is the erased // `*mut PackageManager` (`bun_install_types` cannot name this // type); cast back to `*mut c_void` here. - (on_wake.get_handler())(ctx.as_ptr(), this.cast::()); + (on_wake.get_handler())(ctx.as_ptr(), this.cast::(), on_wake.generation); } (*core::ptr::addr_of_mut!((*this).event_loop)).wakeup(); } diff --git a/src/install_types/resolver_hooks.rs b/src/install_types/resolver_hooks.rs index b1aea8524e7..7ec97e0ef4a 100644 --- a/src/install_types/resolver_hooks.rs +++ b/src/install_types/resolver_hooks.rs @@ -1370,14 +1370,20 @@ pub struct TaskCallbackContext { #[derive(Default, Copy, Clone)] pub struct WakeHandler { pub context: Option>, - pub handler: Option, + /// Live-VM-registry generation of the VM that owns `context`, captured at + /// registration (0 when no handler is installed). Passed back to + /// `handler` so the cross-thread wake can be validated against the + /// registry after the owning (worker) VM may have been freed; opaque to + /// this crate. + pub generation: u64, + pub handler: Option, pub on_dependency_error: Option, } impl WakeHandler { #[inline] - pub fn get_handler(&self) -> fn(*mut c_void, *mut c_void) { + pub fn get_handler(&self) -> fn(*mut c_void, *mut c_void, u64) { // `handler` is Some whenever `context` is Some: the sole installer // (runtime::jsc_hooks) sets `context`, `handler`, and // `on_dependency_error` together in one struct literal, and callers diff --git a/src/jsc/AsyncModule.rs b/src/jsc/AsyncModule.rs index 3782d9fb717..a09f51859a5 100644 --- a/src/jsc/AsyncModule.rs +++ b/src/jsc/AsyncModule.rs @@ -360,7 +360,7 @@ impl Queue { }); } - pub fn on_wake_handler(ctx: *mut c_void, _: *mut c_void) { + pub fn on_wake_handler(ctx: *mut c_void, _: *mut c_void, generation: u64) { bun_core::scoped_log!(AsyncModule, "onWake"); let queue = ctx.cast::(); let task = ConcurrentTaskItem::create_from(queue); @@ -376,7 +376,12 @@ impl Queue { unsafe { bun_core::from_field_ptr!(VirtualMachine, modules, queue) }; // Checked: the wake can fire after a worker VM that owned `queue` was // freed by terminate(); the pointer arithmetic above performs no read. - let _ = VirtualMachine::try_enqueue_task_concurrent(vm, task); + // `generation` was registered next to `ctx` (jsc_hooks), so the + // reassembled handle rejects a new VM reusing the freed address. + let _ = VirtualMachine::try_enqueue_task_concurrent( + crate::virtual_machine::VmHandle::from_raw_parts(vm as usize, generation), + task, + ); } pub fn on_poll(&mut self) { diff --git a/src/jsc/ConcurrentPromiseTask.rs b/src/jsc/ConcurrentPromiseTask.rs index ec0b9a03f8f..b70e9e5a9c6 100644 --- a/src/jsc/ConcurrentPromiseTask.rs +++ b/src/jsc/ConcurrentPromiseTask.rs @@ -2,11 +2,10 @@ use bun_event_loop::ConcurrentTask::{AutoDeinit, ConcurrentTask, TaskTag, Taskab use bun_io::{self as Async, KeepAlive}; use bun_threading::{IntrusiveWorkTask as _, WorkPoolTask, work_pool::WorkPool}; -use crate::event_loop::EventLoop; +use crate::event_loop::{EventLoop, LoopHandle}; use crate::js_promise::{JSPromise, Strong as JSPromiseStrong}; use crate::virtual_machine::VirtualMachine; use crate::{JSGlobalObject, JsTerminated}; -use bun_ptr::BackRef; /// The `Context` type parameter for [`ConcurrentPromiseTask`] must implement this trait: /// - `run(&mut self)` — performs the work on the thread pool @@ -31,9 +30,11 @@ pub struct ConcurrentPromiseTask<'a, Context: ConcurrentPromiseTaskContext> { // Owned here so dropping the task frees the context. pub ctx: Box, pub task: WorkPoolTask, - /// BACKREF — captured from the JS-thread VM at create time; the VM (and its - /// `EventLoop`) outlives every task scheduled on it. - pub event_loop: BackRef, + /// Captured from the JS-thread VM at create time. A handle (not a + /// reference): the owning worker VM can be freed by terminate() while + /// this task sits in the pool, so `on_finish` must go through the + /// registry-checked enqueue. + pub event_loop: LoopHandle, pub promise: JSPromiseStrong, pub global_this: &'a JSGlobalObject, pub concurrent_task: ConcurrentTask, @@ -57,9 +58,10 @@ impl Taskable for ConcurrentPromiseTask<' impl<'a, Context: ConcurrentPromiseTaskContext> ConcurrentPromiseTask<'a, Context> { pub fn create_on_js_thread(global_this: &'a JSGlobalObject, value: Box) -> Box { - // `VirtualMachine::get()` returns the JS-thread singleton; the VM and - // its `EventLoop` outlive every task scheduled on it. - let event_loop = BackRef::new(VirtualMachine::get().as_mut().event_loop_shared()); + // `VirtualMachine::get()` returns the JS-thread singleton. + let event_loop = VirtualMachine::get() + .event_loop_shared() + .concurrent_handle(); let mut this = Box::new(Self { event_loop, ctx: value, @@ -116,9 +118,9 @@ impl<'a, Context: ConcurrentPromiseTaskContext> ConcurrentPromiseTask<'a, Contex ); // `task` is the live `concurrent_task` field of the heap-allocated // job; the queue takes ownership of its intrusive `next` link. - // `event_loop` may point into a worker VM freed by terminate() while - // the pool task ran — checked enqueue only. - let _ = EventLoop::try_enqueue_task_concurrent(event_loop.as_ptr(), task); + // `event_loop` may denote a worker VM's loop freed by terminate() + // while the pool task ran — checked enqueue only. + let _ = EventLoop::try_enqueue_task_concurrent(event_loop, task); } /// Frees the heap allocation backing this task. diff --git a/src/jsc/CppTask.rs b/src/jsc/CppTask.rs index 3b67d2b7e25..d6d4bdd5cd2 100644 --- a/src/jsc/CppTask.rs +++ b/src/jsc/CppTask.rs @@ -80,8 +80,10 @@ impl ConcurrentCppTask { unsafe { EventLoopTaskNoContext::run(cpp_task) }; if let Some(vm) = maybe_vm { // Checked: runs on the work-pool thread; the creating VM may be a - // worker freed by terminate() while this task ran. - VirtualMachine::try_unref_concurrently(vm.as_ptr()); + // worker freed by terminate() while this task ran. Address-only: + // the pointer was captured by C++ (`EventLoopTaskNoContext`) and + // carries no generation. + VirtualMachine::try_unref_concurrently_addr_only(vm.as_ptr()); } } } @@ -93,7 +95,7 @@ pub(crate) extern "C" fn ConcurrentCppTask__createAndRun(cpp_task: *mut EventLoo // the centralised non-null deref proof. C++ just handed it over. if let Some(vm) = EventLoopTaskNoContext::opaque_ref(cpp_task).get_vm() { // Checked for symmetry with the pool-thread unref in `run_owned`. - VirtualMachine::try_ref_concurrently(vm.as_ptr()); + VirtualMachine::try_ref_concurrently_addr_only(vm.as_ptr()); } WorkPool::schedule_new(ConcurrentCppTask { cpp_task, diff --git a/src/jsc/JSCScheduler.rs b/src/jsc/JSCScheduler.rs index be51d283d04..1926a97c399 100644 --- a/src/jsc/JSCScheduler.rs +++ b/src/jsc/JSCScheduler.rs @@ -44,10 +44,12 @@ pub(crate) extern "C" fn Bun__eventLoop__incrementRefConcurrently( crate::mark_binding!(); // Checked: called from JSC helper threads, which can outlive a // terminated worker's VM (the counter of a freed loop needs no balancing). + // Address-only: the pointer was captured by C++ (`JSVMClientData::bunVM`) + // and carries no generation. if delta > 0 { - VirtualMachine::try_ref_concurrently(jsc_vm); + VirtualMachine::try_ref_concurrently_addr_only(jsc_vm); } else { - VirtualMachine::try_unref_concurrently(jsc_vm); + VirtualMachine::try_unref_concurrently_addr_only(jsc_vm); } } @@ -60,7 +62,12 @@ pub(crate) extern "C" fn Bun__queueJSCDeferredWorkTaskConcurrently( // Checked: called from JSC concurrent threads, which can outlive a // terminated worker's VM. `create_from` heap-allocates with the // auto-delete bit set (freed by the checked enqueue when the VM is gone). - let _ = VirtualMachine::try_enqueue_task_concurrent(jsc_vm, ConcurrentTask::create_from(task)); + // Address-only: the pointer was captured by C++ (`JSVMClientData::bunVM`) + // and carries no generation. + let _ = VirtualMachine::try_enqueue_task_concurrent_addr_only( + jsc_vm, + ConcurrentTask::create_from(task), + ); } /// # Safety diff --git a/src/jsc/RuntimeTranspilerStore.rs b/src/jsc/RuntimeTranspilerStore.rs index 3c4d674963c..d6370caf5f9 100644 --- a/src/jsc/RuntimeTranspilerStore.rs +++ b/src/jsc/RuntimeTranspilerStore.rs @@ -350,6 +350,8 @@ impl RuntimeTranspilerStore { global_this: BackRef::new(global_object), non_threadsafe_referrer: OwnedString::new(referrer), vm, + // JS thread; `global_object`'s VM is this thread's live VM. + vm_handle: global_object.bun_vm().concurrent_handle(), log: bun_ast::Log::init(), loader, promise: StrongOptional::create(JSValue::from_cell(promise), global_object), @@ -406,6 +408,9 @@ pub struct TranspilerJob { // raw pointers/BackRefs are used (BACKREF — VM owns the // store and outlives every job). pub vm: *mut VirtualMachine, + /// Schedule-time handle for `vm`, for the one access that must tolerate + /// the VM being gone: the pool-thread `dispatch_to_main_thread`. + pub vm_handle: crate::virtual_machine::VmHandle, pub global_this: BackRef, pub fetcher: Fetcher, pub poll_ref: KeepAlive, @@ -514,7 +519,7 @@ impl TranspilerJob { } pub(crate) fn dispatch_to_main_thread(&mut self) { - let vm = self.vm; + let vm = self.vm_handle; let job = NonNull::from(&mut *self); // Both the store's queue and the event loop live inside the VM // allocation, which may be a worker VM freed by terminate() while diff --git a/src/jsc/VirtualMachine.rs b/src/jsc/VirtualMachine.rs index c498b9a0f36..47583a8473e 100644 --- a/src/jsc/VirtualMachine.rs +++ b/src/jsc/VirtualMachine.rs @@ -286,6 +286,11 @@ pub struct VirtualMachine { /// cross-thread enqueue helpers ([`Self::with_live_vm`] closures) read it /// from producer threads while a swap may be in progress. pub event_loop: core::sync::atomic::AtomicPtr, + /// Registration generation from [`live_vm_registry`], stamped once in + /// `register_vm` (0 = not yet registered). Atomic only because the + /// lock-free main-VM fast paths read it from producer threads; a stale + /// read there falls through to the locked registry check. + pub(crate) live_generation: core::sync::atomic::AtomicU64, pub ref_strings: crate::ref_string::Map, pub ref_strings_mutex: bun_threading::Mutex, @@ -499,15 +504,19 @@ impl VMHolder { /// static-rooted and never freed), which enables the lock-free fast path in /// the checked entry points below. /// -/// Known residual (pre-existing, strictly narrower than the bug this fixes): -/// liveness is keyed by address only, so if a new VM is allocated at a dead -/// VM's address, a stale producer that captured the old pointer passes the -/// check and its task is delivered to the new VM instead of being dropped. -/// That mis-delivery was already possible before this registry existed (the -/// stale push landed at the same reused address, plus every freed-memory -/// interleaving that is now closed). Eliminating it requires producers to -/// carry a schedule-time generation token alongside the pointer — a -/// follow-up that touches every producer struct. +/// Each registration is stamped with a process-unique generation +/// ([`mint_generation`]) that producers capture at schedule time inside a +/// [`VmHandle`](super::VmHandle) / [`LoopHandle`](crate::event_loop::LoopHandle) +/// and bring back to the liveness check. The generation is what makes the +/// check immune to address reuse: if a new VM is allocated at a dead VM's +/// address, a stale producer's handle still carries the dead registration's +/// generation, fails the `(addr, generation)` match, and the task is dropped instead +/// of being delivered to the wrong VM. +/// +/// Residual: entry points whose pointer is captured by C++ and cannot carry a +/// generation yet (`JSVMClientData::bunVM` via JSCScheduler, +/// `EventLoopTaskNoContext` via CppTask) still check by address alone — see +/// the `*_addr_only` variants below. /// /// Lock ordering: this lock is a leaf. The critical sections only touch the /// target's MPSC queue (wait-free push) and `wakeup()` (a syscall); they take @@ -516,22 +525,43 @@ pub(crate) mod live_vm_registry { use super::VirtualMachine; use crate::event_loop::EventLoop; use bun_threading::Guarded; + use core::sync::atomic::{AtomicU64, Ordering}; #[derive(Copy, Clone, Eq, PartialEq)] pub(crate) struct Entry { pub(crate) vm: usize, pub(crate) loop_: usize, + /// Generation stamped at registration; never reused process-wide. + pub(crate) generation: u64, } pub(crate) static REGISTRY: Guarded> = Guarded::new(Vec::new()); - /// Register `vm` and both of its embedded event loops. Called once as - /// the final step of `VirtualMachine::init()`, after every fallible + /// Process-unique generation source. Starts at 1 so generation 0 can + /// serve as the "never matches anything" value of a zeroed/placeholder + /// handle (zero-initialised VM allocations carry it until `register_vm`). + static NEXT_GENERATION: AtomicU64 = AtomicU64::new(1); + + fn mint_generation() -> u64 { + // Relaxed: uniqueness is all that's needed; the value is published to + // producers via the same synchronisation that hands them the handle. + NEXT_GENERATION.fetch_add(1, Ordering::Relaxed) + } + + /// Register `vm` and both of its embedded event loops under one freshly + /// minted generation, and stamp that generation into + /// `vm.live_generation` / both loops' `live_generation` so + /// `concurrent_handle()` can build handles without the lock. Called once + /// as the final step of `VirtualMachine::init()`, after every fallible /// init step has succeeded. pub(crate) fn register_vm(vm: *mut VirtualMachine) { - // SAFETY: `vm` is the freshly initialised allocation; `addr_of!` only - // projects field addresses, no reads. + let generation = mint_generation(); + // SAFETY: `vm` is the freshly initialised allocation with no other + // live borrows; `init()` has exclusive access at this point. let (regular, macro_) = unsafe { + (*vm).live_generation.store(generation, Ordering::Relaxed); + (*vm).regular_event_loop.live_generation = generation; + (*vm).macro_event_loop.live_generation = generation; ( core::ptr::addr_of!((*vm).regular_event_loop), core::ptr::addr_of!((*vm).macro_event_loop), @@ -541,10 +571,12 @@ pub(crate) mod live_vm_registry { reg.push(Entry { vm: vm as usize, loop_: regular as usize, + generation, }); reg.push(Entry { vm: vm as usize, loop_: macro_ as usize, + generation, }); } @@ -557,12 +589,20 @@ pub(crate) mod live_vm_registry { } /// Register a loop that lives outside the VM allocation (the boxed - /// spawnSync event loop). Removed with `unregister_loop` when the box is - /// freed. + /// spawnSync event loop) under its own generation, stamped into + /// `loop_.live_generation`. Removed with `unregister_loop` when the box + /// is freed. pub(crate) fn register_extra_loop(vm: *mut VirtualMachine, loop_: *mut EventLoop) { + let generation = mint_generation(); + // SAFETY: `loop_` is the freshly boxed allocation; the caller has + // exclusive access until it hands the pointer out. + unsafe { + (*loop_).live_generation = generation; + } REGISTRY.lock().push(Entry { vm: vm as usize, loop_: loop_ as usize, + generation, }); } @@ -570,22 +610,91 @@ pub(crate) mod live_vm_registry { REGISTRY.lock().retain(|e| e.loop_ != loop_ as usize); } - /// `true` iff `loop_` is one of the immortal main-thread VM's embedded - /// loops. Address arithmetic only; the main VM allocation is never freed. - pub(crate) fn is_main_vm_loop(loop_: *mut EventLoop) -> bool { + /// `true` iff `handle` denotes one of the immortal main-thread VM's + /// embedded loops, in its current (only) generation. Lock-free: the main + /// VM allocation is never freed, so the address + generation comparison + /// proves liveness without the registry. A stale (pre-`register_vm`) + /// generation read only causes a spurious `false`, and the caller then + /// gets the precise answer from the locked registry. + pub(crate) fn is_main_vm_loop(handle: crate::event_loop::LoopHandle) -> bool { let main = super::MAIN_THREAD_VM.load(core::sync::atomic::Ordering::Acquire); if main.is_null() { return false; } // SAFETY: `main` is the live, never-freed main-thread VM; `addr_of!` - // only projects field addresses, no reads. - let (regular, macro_) = unsafe { + // only projects field addresses, and `live_generation` is atomic. + let (regular, macro_, generation) = unsafe { ( - core::ptr::addr_of!((*main).regular_event_loop), - core::ptr::addr_of!((*main).macro_event_loop), + core::ptr::addr_of!((*main).regular_event_loop) as usize, + core::ptr::addr_of!((*main).macro_event_loop) as usize, + (*main).live_generation.load(Ordering::Relaxed), ) }; - core::ptr::eq(loop_, regular.cast_mut()) || core::ptr::eq(loop_, macro_.cast_mut()) + handle.generation() == generation && (handle.addr() == regular || handle.addr() == macro_) + } +} + +/// Schedule-time identity of a [`VirtualMachine`] for cross-thread producers. +/// +/// Producers that capture a VM on the JS thread and deliver a completion from +/// another thread (HTTP client thread, work pool, watcher threads, napi addon +/// threads) store this instead of `*mut VirtualMachine` / `&VirtualMachine`. +/// The address alone cannot distinguish a live VM from a new VM reusing a +/// dead worker's allocation; the generation (minted per registration in +/// [`live_vm_registry`], never reused) can. The checked entry points +/// ([`VirtualMachine::try_enqueue_task_concurrent`] and friends) verify +/// `(addr, generation)` against the registry under its lock before touching the VM. +/// +/// Plain data (`Copy`, `Send`, `Sync`): holding one neither keeps the VM +/// alive nor permits dereferencing it. The loop-pointer analogue is +/// [`crate::event_loop::LoopHandle`]. +#[derive(Copy, Clone, Eq, PartialEq, Debug)] +pub struct VmHandle { + addr: usize, + generation: u64, +} + +impl VmHandle { + /// Handle that matches no VM, for struct fields initialised before the + /// owning VM is known. Checked entry points treat it as "VM gone". + pub const fn dangling() -> Self { + VmHandle { + addr: 0, + generation: 0, + } + } + + /// Reassemble a handle whose parts were carried separately (e.g. the + /// package-manager `WakeHandler`, whose context pointer and generation + /// travel as distinct fields because the low-tier crate cannot name this + /// type). No liveness is implied. + pub(crate) fn from_raw_parts(addr: usize, generation: u64) -> Self { + VmHandle { addr, generation } + } + + /// The registration generation, for producers that must carry the + /// handle's parts through an API that cannot name this type (reassembled + /// with `from_raw_parts`). + pub fn generation(self) -> u64 { + self.generation + } + + /// Borrow the VM on its own thread, where liveness is structural (the + /// thread exists only while its VM does). Panics if called anywhere else, + /// including on a thread whose current VM reuses a dead VM's address — + /// the generation comparison catches that. + #[track_caller] + pub fn vm_on_owning_thread(self) -> &'static VirtualMachine { + let vm = VirtualMachine::get(); + assert!( + core::ptr::from_ref(vm) as usize == self.addr + && vm + .live_generation + .load(core::sync::atomic::Ordering::Relaxed) + == self.generation, + "VmHandle used on a thread that does not own its VirtualMachine" + ); + vm } } @@ -1273,6 +1382,12 @@ impl VirtualMachine { self.macro_event_loop.virtual_machine = NonNull::new(std::ptr::from_mut(self)); self.macro_event_loop.global = NonNull::new(self.global); self.macro_event_loop.concurrent_tasks = Default::default(); + // `EventLoop::default()` zeroed the registration generation that + // `register_vm` stamped; restore it (same address, same + // registration — the registry entry is untouched). + self.macro_event_loop.live_generation = self + .live_generation + .load(core::sync::atomic::Ordering::Relaxed); } // Idempotent; outside the `has_enabled_macro_mode` guard because // `__bun_macro_context_deinit` runs per-job (RuntimeTranspilerStore @@ -3727,9 +3842,65 @@ impl VirtualMachine { self.event_loop_mut().enqueue_task_concurrent(task); } - /// Run `f` against `vm` only if it is still alive, tolerating `vm` having - /// been freed (terminated worker). Returns `None` without touching `*vm` - /// when it is gone. + /// Schedule-time [`VmHandle`] for this VM, for producers that deliver a + /// completion from another thread through the checked entry points below. + /// Call on the JS thread (or anywhere `self` is provably live). + #[inline] + pub fn concurrent_handle(&self) -> VmHandle { + VmHandle { + addr: core::ptr::from_ref(self) as usize, + generation: self + .live_generation + .load(core::sync::atomic::Ordering::Relaxed), + } + } + + /// Shared body of [`Self::with_live_vm`] / [`Self::with_live_vm_addr_only`]: + /// run `f` against the VM at `addr` only while the registry proves it + /// live. `generation` is `None` for the address-only residual (C++-captured + /// pointers that cannot carry a generation yet). + fn with_live_vm_impl( + addr: usize, + generation: Option, + f: impl FnOnce(&VirtualMachine) -> R, + ) -> Option { + if addr == 0 { + return None; + } + // Fast path: the main-thread VM is allocated once and never freed, so + // an address (+ generation) match proves liveness without the lock. A + // stale (pre-`register_vm`) generation read only causes a spurious + // miss into the locked path below. + let main = MAIN_THREAD_VM.load(core::sync::atomic::Ordering::Acquire); + if main as usize == addr { + // SAFETY: main-thread VM, never freed; `live_generation` is atomic. + let main = unsafe { &*main }; + if generation.is_none_or(|g| { + g == main + .live_generation + .load(core::sync::atomic::Ordering::Relaxed) + }) { + return Some(f(main)); + } + } + let reg = live_vm_registry::REGISTRY.lock(); + if !reg + .iter() + .any(|e| e.vm == addr && generation.is_none_or(|g| e.generation == g)) + { + return None; + } + // SAFETY: the VM at `addr` is registered-live, and `unregister_vm` + // (which happens-before any free of the VM) takes the same lock we + // hold, so the VM cannot be freed while `f` runs. + Some(f(unsafe { &*(addr as *const VirtualMachine) })) + } + + /// Run `f` against the VM identified by `handle` only if that exact + /// registration is still alive, tolerating the VM having been freed + /// (terminated worker) — and, unlike an address check, tolerating a new + /// VM reusing the dead VM's allocation. Returns `None` without touching + /// the VM when it is gone. /// /// For the immortal main-thread VM this is lock-free; for every other VM, /// `f` runs under the [`live_vm_registry`] lock, which `unregister_vm` @@ -3739,41 +3910,35 @@ impl VirtualMachine { /// `f` may be on a non-JS thread — `f` must restrict itself to the /// documented thread-safe subset (the same contract as the `Sync` impl), /// which is why this helper is crate-private rather than `pub`. + pub(crate) fn with_live_vm( + handle: VmHandle, + f: impl FnOnce(&VirtualMachine) -> R, + ) -> Option { + Self::with_live_vm_impl(handle.addr, Some(handle.generation), f) + } + + /// Address-only variant of [`Self::with_live_vm`] for producers whose VM + /// pointer was captured by C++ and cannot carry a generation yet + /// (`JSVMClientData::bunVM`, `EventLoopTaskNoContext`). Residual: a new + /// VM allocated at a dead VM's address passes this check — see + /// [`live_vm_registry`]. // Deliberately takes `*mut` and is NOT `unsafe`: accepting a possibly // dangling pointer is the function's contract, and no deref happens until // the registry proves the pointee live (and holds off its free). #[allow(clippy::not_unsafe_ptr_arg_deref)] - pub(crate) fn with_live_vm( + pub(crate) fn with_live_vm_addr_only( vm: *mut VirtualMachine, f: impl FnOnce(&VirtualMachine) -> R, ) -> Option { - if vm.is_null() { - return None; - } - // Fast path: the main-thread VM is allocated once and never freed, so - // an address match proves liveness without the lock. - if core::ptr::eq( - vm, - MAIN_THREAD_VM.load(core::sync::atomic::Ordering::Acquire), - ) { - // SAFETY: main-thread VM, never freed. - return Some(f(unsafe { &*vm })); - } - let reg = live_vm_registry::REGISTRY.lock(); - if !reg.iter().any(|e| e.vm == vm as usize) { - return None; - } - // SAFETY: `vm` is registered-live, and `unregister_vm` (which - // happens-before any free of the VM) takes the same lock we hold, so - // the VM cannot be freed while `f` runs. - Some(f(unsafe { &*vm })) + Self::with_live_vm_impl(vm as usize, None, f) } - /// Cross-thread enqueue that tolerates `vm` having been freed (terminated - /// worker). Producers that captured `vm` at schedule time and deliver a - /// completion from another thread (HTTP client thread, work pool, watcher - /// threads, napi addon threads) must use this instead of dereferencing - /// `vm` directly — see [`live_vm_registry`]. + /// Cross-thread enqueue that tolerates the handle's VM having been freed + /// (terminated worker). Producers that captured the handle at schedule + /// time ([`Self::concurrent_handle`]) and deliver a completion from + /// another thread (HTTP client thread, work pool, watcher threads, napi + /// addon threads) must use this instead of dereferencing a stored VM + /// pointer — see [`live_vm_registry`]. /// /// Returns `false` when the VM is gone: the task was not queued, and /// `task`'s node was freed if it was `auto_delete` (the payload is @@ -3781,10 +3946,27 @@ impl VirtualMachine { /// terminated worker's queue, which is the pre-existing behavior for /// tasks that lost this race by a few milliseconds). pub fn try_enqueue_task_concurrent( + handle: VmHandle, + task: core::ptr::NonNull, + ) -> bool { + match Self::with_live_vm(handle, |vm| { + vm.event_loop_shared().enqueue_task_concurrent(task); + }) { + Some(()) => true, + None => { + crate::event_loop::discard_unqueued_concurrent_task(task); + false + } + } + } + + /// Address-only variant of [`Self::try_enqueue_task_concurrent`] — same + /// residual as [`Self::with_live_vm_addr_only`]. + pub fn try_enqueue_task_concurrent_addr_only( vm: *mut VirtualMachine, task: core::ptr::NonNull, ) -> bool { - match Self::with_live_vm(vm, |vm| { + match Self::with_live_vm_addr_only(vm, |vm| { vm.event_loop_shared().enqueue_task_concurrent(task); }) { Some(()) => true, @@ -3795,34 +3977,37 @@ impl VirtualMachine { } } - /// Like [`VirtualMachine::is_shutting_down`], but callable with a pointer - /// that may already be freed (terminated worker): a freed VM reports - /// `true`. For HTTP-thread / work-pool completion paths that branch on - /// shutdown before touching VM-owned state. - pub fn is_shutting_down_or_freed(vm: *mut VirtualMachine) -> bool { - Self::live_shutting_down_state(vm).unwrap_or(true) + /// Like [`VirtualMachine::is_shutting_down`], but keyed by a schedule-time + /// handle whose VM may already be freed (terminated worker): a freed VM — + /// or a new VM reusing its address — reports `true`. For HTTP-thread / + /// work-pool completion paths that branch on shutdown before touching + /// VM-owned state. + pub fn is_shutting_down_or_freed(handle: VmHandle) -> bool { + Self::live_shutting_down_state(handle).unwrap_or(true) } /// Tri-state variant of [`Self::is_shutting_down_or_freed`] for callers /// that must distinguish a freed VM from a live-but-exiting one: `None` - /// when `vm` is gone (terminated worker), otherwise - /// `Some(is_shutting_down)`. Because `WebWorker::shutdown()` unregisters - /// the VM *before* setting `is_shutting_down`, `Some(true)` can only be - /// observed for VMs that are never freed (the main VM during - /// `global_exit`). - pub fn live_shutting_down_state(vm: *mut VirtualMachine) -> Option { - Self::with_live_vm(vm, |vm| vm.is_shutting_down()) + /// when the handle's registration is gone (terminated worker, or a new VM + /// reusing the freed address), otherwise `Some(is_shutting_down)`. + /// Because `WebWorker::shutdown()` unregisters the VM *before* setting + /// `is_shutting_down`, `Some(true)` can only be observed for VMs that are + /// never freed (the main VM during `global_exit`). + pub fn live_shutting_down_state(handle: VmHandle) -> Option { + Self::with_live_vm(handle, |vm| vm.is_shutting_down()) } /// `ref_concurrently`/`unref_concurrently` variants of - /// [`Self::try_enqueue_task_concurrent`]: no-ops when the VM is gone (a - /// freed loop has no liveness counter left to balance). - pub fn try_ref_concurrently(vm: *mut VirtualMachine) { - let _ = Self::with_live_vm(vm, |vm| vm.event_loop_shared().ref_concurrently()); + /// [`Self::try_enqueue_task_concurrent`] for pointers captured by C++ — + /// no-ops when the VM is gone (a freed loop has no liveness counter left + /// to balance), with the same address-only residual as + /// [`Self::with_live_vm_addr_only`]. + pub fn try_ref_concurrently_addr_only(vm: *mut VirtualMachine) { + let _ = Self::with_live_vm_addr_only(vm, |vm| vm.event_loop_shared().ref_concurrently()); } - pub fn try_unref_concurrently(vm: *mut VirtualMachine) { - let _ = Self::with_live_vm(vm, |vm| vm.event_loop_shared().unref_concurrently()); + pub fn try_unref_concurrently_addr_only(vm: *mut VirtualMachine) { + let _ = Self::with_live_vm_addr_only(vm, |vm| vm.event_loop_shared().unref_concurrently()); } /// `cond` is `&Cell` (not `&mut bool`): the re-entrant diff --git a/src/jsc/WorkTask.rs b/src/jsc/WorkTask.rs index 6f5cffb9a6f..6c827aaebfe 100644 --- a/src/jsc/WorkTask.rs +++ b/src/jsc/WorkTask.rs @@ -4,7 +4,7 @@ use bun_threading::{IntrusiveWorkTask as _, WorkPoolTask, work_pool::WorkPool}; use crate::JSGlobalObject; use crate::debugger::AsyncTaskTracker; -use crate::event_loop::EventLoop; +use crate::event_loop::{EventLoop, LoopHandle}; use bun_ptr::BackRef; /// A generic task that runs work on a thread pool and executes a callback on the main JavaScript thread. @@ -34,9 +34,11 @@ pub trait WorkTaskContext: Sized { pub struct WorkTask { pub ctx: *mut Context, pub task: WorkPoolTask, - /// BACKREF — captured from the JS-thread VM at create time; the VM (and its - /// `EventLoop`) outlives every task scheduled on it. - pub event_loop: BackRef, + /// Captured from the JS-thread VM at create time. A handle (not a + /// reference): the owning worker VM can be freed by terminate() while + /// this task sits in the pool, so `on_finish` must go through the + /// registry-checked enqueue. + pub event_loop: LoopHandle, // allocator field dropped — global mimalloc (see PORTING.md §Allocators) pub global_this: BackRef, pub concurrent_task: ConcurrentTask, @@ -61,7 +63,7 @@ impl Taskable for WorkTask { impl WorkTask { pub fn create_on_js_thread(global_this: &JSGlobalObject, value: *mut Context) -> *mut Self { let vm = global_this.bun_vm().as_mut(); - let event_loop = BackRef::new(vm.event_loop_shared()); + let event_loop = vm.event_loop_shared().concurrent_handle(); let mut this = Box::new(Self { event_loop, ctx: value, @@ -136,9 +138,9 @@ impl WorkTask { .from(this_ptr, AutoDeinit::ManualDeinit), ); // `task` is the inline `concurrent_task` field of the live - // heap-allocated `*this`; `event_loop` was stored at init and may - // point into a worker VM freed by terminate() while the pool task + // heap-allocated `*this`; `event_loop` was captured at init and may + // denote a worker VM's loop freed by terminate() while the pool task // ran — checked enqueue only. - let _ = EventLoop::try_enqueue_task_concurrent(event_loop.as_ptr(), task); + let _ = EventLoop::try_enqueue_task_concurrent(event_loop, task); } } diff --git a/src/jsc/any_task_job.rs b/src/jsc/any_task_job.rs index 41b5ccbaf46..a02ae9bf113 100644 --- a/src/jsc/any_task_job.rs +++ b/src/jsc/any_task_job.rs @@ -48,10 +48,12 @@ pub trait AnyTaskJobCtx: Sized { /// `run_from_js` (or on `init` failure). `ctx` is `pub` so callers can read /// e.g. a `JSPromiseStrong` field after scheduling. pub struct AnyTaskJob { - vm: bun_ptr::BackRef, + /// Schedule-time handle — the owning worker VM may be freed by + /// terminate() while the pool task is in flight, so the pool-thread + /// completion goes through the registry-checked enqueue. + vm: crate::virtual_machine::VmHandle, /// Captured at `create` so `run_task` (pool thread) never reads a field - /// of `vm`, which may be a worker VM freed by terminate() while the pool - /// task was in flight. + /// of the VM, which may be freed while the pool task was in flight. global: *mut JSGlobalObject, task: WorkPoolTask, any_task: AnyTask, @@ -76,7 +78,7 @@ impl AnyTaskJob { /// (running `Drop for C`). The returned pointer is owned by the caller /// until handed to [`Self::schedule`]. pub fn create(global: &JSGlobalObject, ctx: C) -> JsResult<*mut Self> { - let vm = bun_ptr::BackRef::new(global.bun_vm()); + let vm = global.bun_vm().concurrent_handle(); let job = bun_core::heap::into_raw(Box::new(Self { vm, global: core::ptr::from_ref(global).cast_mut(), @@ -146,11 +148,11 @@ impl AnyTaskJob { let job = unsafe { &mut *Self::from_task_ptr(task) }; job.ctx.run(job.global); // `ConcurrentTask::create` heap-allocates a fresh task; the queue - // takes ownership of it (or frees it when the VM is gone). `vm` may - // be a worker VM freed by terminate() while the pool task ran — - // checked enqueue only, and no field reads through it on this thread. + // takes ownership of it (or frees it when the VM is gone). The VM may + // be a worker freed by terminate() while the pool task ran — checked + // enqueue only, and no field reads through it on this thread. let _ = VirtualMachine::try_enqueue_task_concurrent( - job.vm.as_ptr(), + job.vm, ConcurrentTask::create(job.any_task.task()), ); } @@ -162,7 +164,8 @@ impl AnyTaskJob { // SAFETY: `this` was produced by `heap::into_raw` in `create` and is // uniquely owned here (the `AnyTask` fires exactly once). let mut this = unsafe { bun_core::heap::take(this) }; - let vm = this.vm; + // JS thread — the job's VM is this thread's live VM. + let vm = this.vm.vm_on_owning_thread(); if vm.is_shutting_down() { return Ok(()); } diff --git a/src/jsc/event_loop.rs b/src/jsc/event_loop.rs index 44f4e53f384..f26f5a694a1 100644 --- a/src/jsc/event_loop.rs +++ b/src/jsc/event_loop.rs @@ -104,6 +104,12 @@ pub struct EventLoop { pub signal_handler: Option>, #[cfg(not(unix))] pub signal_handler: (), + + /// Registration generation from the live-VM registry, stamped by + /// `register_vm` / `register_extra_loop` (0 = not registered). Read on + /// the owning thread only, by [`EventLoop::concurrent_handle`] and the + /// `JsEventLoop::live_generation` dispatch. + pub(crate) live_generation: u64, } impl Default for EventLoop { @@ -130,6 +136,7 @@ impl Default for EventLoop { signal_handler: None, #[cfg(not(unix))] signal_handler: (), + live_generation: 0, } } } @@ -284,6 +291,48 @@ fn tick_queue_with_count( unsafe { __bun_tick_queue_with_count(el, vm, counter) } } +/// Schedule-time identity of a specific [`EventLoop`] for cross-thread +/// producers — the loop-keyed analogue of +/// [`crate::virtual_machine::VmHandle`], for producers that must deliver to +/// the exact loop they captured (regular vs macro, or the boxed spawnSync +/// loop) rather than "whichever loop the VM currently runs". +/// +/// Plain data (`Copy`, `Send`, `Sync`): holding one neither keeps the loop +/// alive nor permits dereferencing it; only +/// [`EventLoop::try_enqueue_task_concurrent`] resolves it, by verifying +/// `(addr, generation)` against the live-VM registry under its lock. +#[derive(Copy, Clone, Eq, PartialEq, Debug)] +pub struct LoopHandle { + addr: usize, + generation: u64, +} + +impl LoopHandle { + /// Handle that matches no loop, for struct fields initialised before the + /// owning loop is known. Checked entry points treat it as "loop gone". + pub const fn dangling() -> Self { + LoopHandle { + addr: 0, + generation: 0, + } + } + + /// Reassemble a handle from parts that were carried separately (the + /// `JsEventLoop` dispatch, where the erased loop pointer and the + /// generation travel as distinct values). No liveness is implied. + pub(crate) fn from_raw_parts(addr: usize, generation: u64) -> Self { + LoopHandle { addr, generation } + } + + pub(crate) fn addr(self) -> usize { + self.addr + } + + pub(crate) fn generation(self) -> u64 { + self.generation + } +} + /// RAII pairing for [`EventLoop::enter`] / [`EventLoop::exit`]. /// /// Holds the raw `*mut EventLoop` (not `&mut`) so re-entrant JS callbacks that @@ -1049,44 +1098,58 @@ impl EventLoop { self.wakeup(); } - /// Loop-pointer-keyed variant of + /// Schedule-time [`LoopHandle`] for this loop, for producers that deliver + /// a completion from another thread through + /// [`EventLoop::try_enqueue_task_concurrent`]. Call on the owning thread + /// (or anywhere `self` is provably live). + #[inline] + pub fn concurrent_handle(&self) -> LoopHandle { + LoopHandle { + addr: core::ptr::from_ref(self) as usize, + generation: self.live_generation, + } + } + + /// Loop-keyed variant of /// [`VirtualMachine::try_enqueue_task_concurrent`]: cross-thread enqueue /// that tolerates the target event loop (and the worker VM embedding it) - /// having been freed. For producers that captured `*mut EventLoop` / - /// `&EventLoop` at schedule time rather than the VM pointer. + /// having been freed. For producers that captured a specific loop at + /// schedule time ([`EventLoop::concurrent_handle`]) rather than the VM. /// - /// Returns `false` when the loop is gone; the task node is freed if - /// `auto_delete` and the payload is leaked (same as a task left undrained - /// in a terminated worker's queue). - // Deliberately takes `*mut` and is NOT `unsafe`: accepting a possibly - // dangling pointer is the function's contract, and no deref happens until - // the registry proves the pointee live (and holds off its free). - #[allow(clippy::not_unsafe_ptr_arg_deref)] + /// Returns `false` when the loop is gone — including when a new loop + /// reuses the dead loop's address (the generation comparison catches + /// that); the task node is freed if `auto_delete` and the payload is + /// leaked (same as a task left undrained in a terminated worker's queue). pub fn try_enqueue_task_concurrent( - loop_: *mut EventLoop, + handle: LoopHandle, task: core::ptr::NonNull, ) -> bool { use crate::virtual_machine::live_vm_registry; - if loop_.is_null() { + if handle.addr == 0 { discard_unqueued_concurrent_task(task); return false; } - // Fast path: loops embedded in the main-thread VM are never freed. - if live_vm_registry::is_main_vm_loop(loop_) { + // Fast path: loops embedded in the main-thread VM are never freed. (A + // stale pre-registration generation read inside only causes a + // spurious miss into the locked path below.) + if live_vm_registry::is_main_vm_loop(handle) { // SAFETY: main-thread VM loop, never freed; `enqueue_task_concurrent` // takes `&self` and is thread-safe. - unsafe { (*loop_).enqueue_task_concurrent(task) }; + unsafe { (*(handle.addr as *const EventLoop)).enqueue_task_concurrent(task) }; return true; } let reg = live_vm_registry::REGISTRY.lock(); - if !reg.iter().any(|e| e.loop_ == loop_ as usize) { + if !reg + .iter() + .any(|e| e.loop_ == handle.addr && e.generation == handle.generation) + { drop(reg); discard_unqueued_concurrent_task(task); return false; } // SAFETY: the loop is registered-live, and unregistration (which // happens-before any free) takes the same lock we hold. - unsafe { (*loop_).enqueue_task_concurrent(task) }; + unsafe { (*(handle.addr as *const EventLoop)).enqueue_task_concurrent(task) }; true } @@ -1443,8 +1506,17 @@ bun_event_loop::link_impl_JsEventLoop! { enqueue_task(task) => (*this).enqueue_task(task), // Checked: `EventLoopHandle`s are captured at schedule time and used // from producer threads (waiter thread, work pool, shell tasks) that - // can outlive a terminated worker's loop. - enqueue_task_concurrent(task) => { let _ = EventLoop::try_enqueue_task_concurrent(this, task); }, + // can outlive a terminated worker's loop. `generation` is the registration + // generation the handle captured alongside the pointer — `this` is + // NOT dereferenced here (it may be freed); the checked entry resolves + // `(addr, generation)` against the live-VM registry first. + enqueue_task_concurrent(task, generation) => { + let _ = EventLoop::try_enqueue_task_concurrent( + LoopHandle::from_raw_parts(this as usize, generation), + task, + ); + }, + live_generation() => (*this).live_generation, env() => (*this).vm_ref().transpiler.env, top_level_dir() => core::ptr::from_ref::<[u8]>((*this).vm_ref().top_level_dir()), create_null_delimited_env_map() => diff --git a/src/runtime/api/Archive.rs b/src/runtime/api/Archive.rs index cd444693e72..5b928f218ce 100644 --- a/src/runtime/api/Archive.rs +++ b/src/runtime/api/Archive.rs @@ -665,7 +665,9 @@ pub trait TaskContext: Send { pub struct AsyncTask { ctx: C, promise: JSPromiseStrong, - vm: *mut VirtualMachine, + /// Schedule-time handle — the owning worker VM may be freed by + /// terminate() while the pool task is in flight. + vm: bun_jsc::virtual_machine::VmHandle, task: WorkPoolTask, concurrent_task: ConcurrentTask, keep_alive: KeepAlive, @@ -677,11 +679,7 @@ impl Taskable for AsyncTask { impl AsyncTask { fn create(global: &JSGlobalObject, ctx: C) -> Result<*mut Self, bun_alloc::AllocError> { - // `bun_vm_ptr()` returns `*mut VirtualMachine` with write provenance; valid for - // process lifetime. Do NOT launder `bun_vm()` (a `&VirtualMachine`) through - // `*const _ as *mut _` — that derives a writeable pointer from a shared - // reference and is UB under Stacked Borrows. - let vm: *mut VirtualMachine = global.bun_vm_ptr(); + let vm = global.bun_vm().concurrent_handle(); let this = Box::new(AsyncTask { ctx, promise: JSPromiseStrong::init(global), @@ -729,7 +727,7 @@ impl AsyncTask { let this: *mut Self = unsafe { bun_core::from_field_ptr!(Self, task, work_task) }; // SAFETY: thread-pool has exclusive access to ctx until it enqueues the concurrent task. unsafe { (*this).ctx.run() }; - // `vm` was captured on the JS thread at `create` and may point at a + // `vm` was captured on the JS thread at `create` and may denote a // worker VM freed by terminate() while the pool task ran — checked // enqueue only. // SAFETY: `this` is the live pool-owned allocation; `concurrent_task` diff --git a/src/runtime/api/JSBundler.rs b/src/runtime/api/JSBundler.rs index 40610f0d62b..2ebca5b535a 100644 --- a/src/runtime/api/JSBundler.rs +++ b/src/runtime/api/JSBundler.rs @@ -1467,11 +1467,11 @@ pub mod js_bundler { .r#loop() .expect("BundleV2.linker.loop must be set before plugins run"); match &mut *any_loop.as_ptr() { - bun_event_loop::AnyEventLoop::Js { owner } => { - owner.enqueue_task_concurrent(ConcurrentTask::from_callback( - ctx.as_mut_ptr(), - on_notify_defer_raw, - )); + bun_event_loop::AnyEventLoop::Js { owner, generation } => { + owner.enqueue_task_concurrent( + ConcurrentTask::from_callback(ctx.as_mut_ptr(), on_notify_defer_raw), + *generation, + ); } bun_event_loop::AnyEventLoop::Mini(mini) => { // `mini.enqueueTaskConcurrentWithExtraCtx( diff --git a/src/runtime/api/js_bundle_completion_task.rs b/src/runtime/api/js_bundle_completion_task.rs index 56094492aaa..df712b152df 100644 --- a/src/runtime/api/js_bundle_completion_task.rs +++ b/src/runtime/api/js_bundle_completion_task.rs @@ -59,9 +59,11 @@ pub struct JSBundleCompletionTask { // `unsafe impl Send` below for the thread-affinity constraint this imposes. pub ref_count: RefCount, pub config: JSBundlerConfig, - // BACKREF — the JS-thread `EventLoop` outlives every completion task; safe - // `Deref` so call sites read `self.jsc_event_loop.enqueue_task_concurrent(..)`. - pub jsc_event_loop: BackRef, + /// Schedule-time handle of the JS-thread `EventLoop` — the Bun.build + /// caller's VM may be a worker freed by terminate() while the bundle + /// runs, so the bundle-thread completion goes through the + /// registry-checked enqueue. + pub jsc_event_loop: jsc::event_loop::LoopHandle, pub task: AnyTask, pub global_this: BackRef, pub promise: jsc::JSPromiseStrong, @@ -123,9 +125,9 @@ pub(crate) fn create_and_schedule_completion_task( let completion = bun_core::heap::into_raw(Box::new(JSBundleCompletionTask { ref_count: RefCount::init(), config, - // `event_loop` is the live JS-thread loop (caller derives it from - // `vm.event_loop()`); never null once `Bun.build` is reachable. - jsc_event_loop: BackRef::from(core::ptr::NonNull::new(event_loop).expect("event_loop")), + // SAFETY: `event_loop` is the live JS-thread loop (caller derives it + // from `vm.event_loop()`); never null once `Bun.build` is reachable. + jsc_event_loop: unsafe { (*event_loop).concurrent_handle() }, task: AnyTask::default(), global_this: BackRef::new(global_this), promise: jsc::JSPromiseStrong::default(), @@ -789,7 +791,7 @@ static COMPLETION_VTABLE: dispatch::CompletionDispatch = dispatch::CompletionDis // SAFETY: `task` is a fresh heap-allocated non-null `ConcurrentTaskItem` // passed through from the bundler vtable; the queue takes ownership. let _ = jsc::event_loop::EventLoop::try_enqueue_task_concurrent( - from_completion_handle(c).jsc_event_loop.as_ptr(), + from_completion_handle(c).jsc_event_loop, // SAFETY: non-null per the vtable contract above. unsafe { core::ptr::NonNull::new_unchecked(task) }, ); @@ -995,7 +997,7 @@ impl CompletionStruct for JSBundleCompletionTask { // `ConcurrentTask::create` heap-allocates a fresh task; the queue // takes ownership of it (or frees it when the VM is gone). let _ = jsc::event_loop::EventLoop::try_enqueue_task_concurrent( - self.jsc_event_loop.as_ptr(), + self.jsc_event_loop, jsc::ConcurrentTask::create(self.task.task()), ); } diff --git a/src/runtime/bake/DevServer.rs b/src/runtime/bake/DevServer.rs index 930ca31e707..36f1d4d61dd 100644 --- a/src/runtime/bake/DevServer.rs +++ b/src/runtime/bake/DevServer.rs @@ -322,6 +322,10 @@ pub struct DevServer { /// is the JSC_BORROW guarantee: vm is valid for DevServer's entire /// lifetime. pub vm: bun_ptr::BackRef, + /// Schedule-time handle for `vm`, for the watcher-thread event enqueue + /// (the one access that must tolerate the VM being gone — nothing + /// structurally pins the dev server to the immortal main VM). + pub vm_handle: bun_jsc::virtual_machine::VmHandle, /// May be `None` if not attached to an HTTP server yet. When no server is /// available, functions taking in requests and responses are unavailable. /// However, a lot of testing in this mode is missing, so it may hit assertions. @@ -546,6 +550,7 @@ pub fn init(options: Options) -> JsResult> { w!(magic, Magic::Valid); w!(root, Box::from(options.root.as_bytes())); w!(vm, bun_ptr::BackRef::new(options.vm)); + w!(vm_handle, options.vm.concurrent_handle()); w!(server, None); w!(directory_watchers, DirectoryWatchStore::default()); w!(server_fetch_function_callback, jsc::StrongOptional::empty()); @@ -1081,6 +1086,7 @@ impl Drop for DevServer { inspector_server_id: _, configuration_hash_key: _, vm: _, + vm_handle: _, server: _, router: _, route_bundles: _, diff --git a/src/runtime/bake/DevServer/memory_cost.rs b/src/runtime/bake/DevServer/memory_cost.rs index 30c0265a71e..bcc8d71fe72 100644 --- a/src/runtime/bake/DevServer/memory_cost.rs +++ b/src/runtime/bake/DevServer/memory_cost.rs @@ -44,6 +44,7 @@ pub(crate) fn memory_cost_detailed(dev: &DevServer) -> MemoryCost { inspector_server_id: _, configuration_hash_key: _, vm: _, + vm_handle: _, server: _, router: _, route_bundles: _, diff --git a/src/runtime/bake/dev_server/mod.rs b/src/runtime/bake/dev_server/mod.rs index 0648fd3ffbe..c38efdde95f 100644 --- a/src/runtime/bake/dev_server/mod.rs +++ b/src/runtime/bake/dev_server/mod.rs @@ -960,9 +960,9 @@ impl WatcherAtomics { // a worker freed by terminate() (nothing structurally pins the // dev server to the main VM). // SAFETY: `owner` BACKREF is valid (DevServer-owned field reads only). - let vm_ptr = unsafe { (*ev_ref.owner).vm.as_ptr() }; + let vm_handle = unsafe { (*ev_ref.owner).vm_handle }; let _ = bun_jsc::virtual_machine::VirtualMachine::try_enqueue_task_concurrent( - vm_ptr, + vm_handle, core::ptr::NonNull::from(&mut ev_ref.concurrent_task), ); } diff --git a/src/runtime/crypto/PasswordObject.rs b/src/runtime/crypto/PasswordObject.rs index d6857add020..c2857a18120 100644 --- a/src/runtime/crypto/PasswordObject.rs +++ b/src/runtime/crypto/PasswordObject.rs @@ -9,7 +9,7 @@ use bun_jsc::{ }; // `bun_jsc::{AnyTask, ConcurrentTask, EventLoop}` are *modules* (re-exported from // `bun_event_loop`); pull the concrete types out by name. -use bun_jsc::event_loop::EventLoop; +use bun_jsc::event_loop::{EventLoop, LoopHandle}; // JSC-side ZigString carries `to_js` (the `bun_core::ZigString` repr-twin // lives in `bun_jsc::zig_string`); used for ASCII→JS conversions only. use bun_jsc::AnyTask::{AnyTask, JsResult as AnyTaskJsResult}; @@ -574,7 +574,7 @@ struct PasswordJob { op: Op, password: Box<[u8]>, promise: JSPromiseStrong, - event_loop: *mut EventLoop, + event_loop: LoopHandle, global: *const JSGlobalObject, r#ref: KeepAlive, task: WorkPoolTask, @@ -695,8 +695,10 @@ impl JSPasswordObject { op, password, promise, - // SAFETY: bun_vm() is non-null for a Bun-owned global; VM outlives the job. - event_loop: global_object.bun_vm().event_loop(), + event_loop: global_object + .bun_vm() + .event_loop_shared() + .concurrent_handle(), global: std::ptr::from_ref(global_object), r#ref: KeepAlive::default(), task: WorkPoolTask::default(), diff --git a/src/runtime/jsc_hooks.rs b/src/runtime/jsc_hooks.rs index a55ff61bd5e..de6ce19920c 100644 --- a/src/runtime/jsc_hooks.rs +++ b/src/runtime/jsc_hooks.rs @@ -402,6 +402,10 @@ unsafe fn init_runtime_state( t.resolver.opts.preserve_symlinks = preserve_symlinks; t.resolver.on_wake_package_manager = bun_resolver::install_types::WakeHandler { context: core::ptr::NonNull::new(ptr::addr_of_mut!((*vm).modules).cast()), + // Registry generation of `vm`, carried next to the + // context pointer so `on_wake_handler` can reject a + // wake targeting a freed (terminated worker) VM. + generation: (*vm).concurrent_handle().generation(), handler: Some(bun_jsc::async_module::Queue::on_wake_handler), on_dependency_error: Some( bun_jsc::async_module::Queue::on_dependency_error, diff --git a/src/runtime/napi/napi_body.rs b/src/runtime/napi/napi_body.rs index eb1260d84c9..d86b612f238 100644 --- a/src/runtime/napi/napi_body.rs +++ b/src/runtime/napi/napi_body.rs @@ -10,7 +10,7 @@ use bun_event_loop::ConcurrentTask::AutoDeinit; use bun_event_loop::{TaskTag, Taskable, task_tag}; use bun_io::KeepAlive; use bun_jsc::StringJsc; -use bun_jsc::event_loop::{ConcurrentTaskItem as ConcurrentTask, EventLoop}; +use bun_jsc::event_loop::{ConcurrentTaskItem as ConcurrentTask, EventLoop, LoopHandle}; use bun_jsc::virtual_machine::VirtualMachine; use bun_jsc::{ self as jsc, CallFrame, Debugger, GlobalRef, JSGlobalObject, JSPromiseStrong, JSValue, @@ -1767,8 +1767,10 @@ pub(super) enum AsyncWorkStatus { pub struct napi_async_work { pub task: WorkPoolTask, pub concurrent_task: ConcurrentTask, - // Note: BackRef — `enqueue_task` needs `&mut EventLoop`; reborrowed at use sites. - pub event_loop: bun_ptr::BackRef, + /// Schedule-time handle — the owning worker VM may be freed by + /// terminate() while this work sits in the pool, so the pool-thread + /// completion goes through the registry-checked enqueue. + pub event_loop: LoopHandle, pub global: GlobalRef, // JSC_BORROW (lives for vm lifetime) pub env: NapiEnvRef, pub execute: napi_async_execute_callback, @@ -1800,10 +1802,7 @@ impl napi_async_work { // SAFETY: env outlives the async work; clone bumps the C++ refcount. env: unsafe { NapiEnvRef::clone_from_raw(env.as_mut_ptr()) }, execute, - // SAFETY: bun_vm() never null for a Bun-owned global. - // SAFETY: `event_loop()` is the live JS-thread loop (non-null, - // stable address) and outlives every napi_async_work. - event_loop: unsafe { bun_ptr::BackRef::from_raw(global.bun_vm().event_loop()) }, + event_loop: global.bun_vm().event_loop_shared().concurrent_handle(), complete, data, status: AtomicU32::new(AsyncWorkStatus::Pending as u32), @@ -1850,7 +1849,7 @@ impl napi_async_work { // owning VM may be a worker freed by terminate() while this // work sat in the pool. let _ = EventLoop::try_enqueue_task_concurrent( - self.event_loop.as_ptr(), + self.event_loop, core::ptr::NonNull::from( self.concurrent_task .from(self_ptr, AutoDeinit::ManualDeinit), @@ -1867,7 +1866,7 @@ impl napi_async_work { // queue takes ownership of its `next` link. Checked: the owning VM // may be a worker freed by terminate() while the work ran. let _ = EventLoop::try_enqueue_task_concurrent( - self.event_loop.as_ptr(), + self.event_loop, core::ptr::NonNull::from( self.concurrent_task .from(self_ptr, AutoDeinit::ManualDeinit), @@ -2429,6 +2428,9 @@ pub struct ThreadSafeFunction { // Note: BackRef — `enqueue_task`/`drain_microtasks` need `&mut // EventLoop`; reborrowed at use sites (single JS thread). pub event_loop: bun_ptr::BackRef, + /// Schedule-time handle for `event_loop`, for the addon-thread dispatch + /// (the one access that must tolerate the loop being gone). + pub loop_handle: LoopHandle, pub tracker: Debugger::AsyncTaskTracker, pub env: NapiEnvRef, @@ -2718,7 +2720,7 @@ impl ThreadSafeFunction { // Checked: threadsafe functions are called from arbitrary // addon threads, which can outlive a terminated worker's loop. let _ = EventLoop::try_enqueue_task_concurrent( - self.event_loop.as_ptr(), + self.loop_handle, ConcurrentTask::create_from(self_ptr), ); } @@ -2856,8 +2858,9 @@ pub(super) extern "C" fn napi_create_threadsafe_function( let function = ThreadSafeFunction::new(ThreadSafeFunction { // SAFETY: `event_loop()` is the live JS-thread loop (non-null, stable - // address) and outlives every threadsafe function. + // address); JS-thread-only derefs. event_loop: unsafe { bun_ptr::BackRef::from_raw(vm.event_loop()) }, + loop_handle: vm.event_loop_shared().concurrent_handle(), // SAFETY: env is a live C++-owned napi_env. env: unsafe { NapiEnvRef::clone_from_raw(env.as_mut_ptr()) }, callback, @@ -4271,8 +4274,10 @@ impl NapiFinalizerTask { // Checked: scheduled from non-JS threads (addon threads, GC // helpers) that can outlive a terminated worker's VM. When the VM // is gone the finalizer box leaks, matching a task left undrained - // in the dead worker's queue. - let _ = VirtualMachine::try_enqueue_task_concurrent( + // in the dead worker's queue. Address-only: the VM identity comes + // from the napi env, which carries no schedule-time generation + // (napi env teardown is a tracked follow-up). + let _ = VirtualMachine::try_enqueue_task_concurrent_addr_only( core::ptr::from_ref(vm).cast_mut(), ConcurrentTask::create(Task::init(this)), ); diff --git a/src/runtime/node/node_fs.rs b/src/runtime/node/node_fs.rs index 03887e58eed..b801b59a4a7 100644 --- a/src/runtime/node/node_fs.rs +++ b/src/runtime/node/node_fs.rs @@ -1252,7 +1252,7 @@ mod _async_tasks { /// Captured at `create` so `work_pool_callback` never reads through /// `global_object` off-thread — the owning VM may be a worker freed /// by terminate() while the pool task was in flight. - pub vm: *mut VirtualMachine, + pub vm: bun_jsc::virtual_machine::VmHandle, pub task: WorkPoolTask, pub result: Maybe, pub r#ref: KeepAlive, @@ -1297,7 +1297,7 @@ mod _async_tasks { // niche-optimised; never construct an all-zero `Result` value. result: Err(sys::Error::default()), global_object: bun_ptr::BackRef::new(global_object), - vm: core::ptr::from_mut(vm), + vm: vm.concurrent_handle(), task: work_pool_task(Self::work_pool_callback), r#ref: KeepAlive::default(), tracker: AsyncTaskTracker::init(vm), @@ -1321,7 +1321,7 @@ mod _async_tasks { // `sys::Error::path` is `Box<[u8]>` boxed at the // `errno_sys_p` construction site, so no clone is needed — `node_fs` may drop. - // `this.vm` was captured at `create` and may point at a worker VM + // `this.vm` was captured at `create` and may denote a worker VM // freed by terminate() while the pool task ran — checked enqueue // only, and no reads through `global_object` on this thread. let _ = VirtualMachine::try_enqueue_task_concurrent( @@ -2187,7 +2187,7 @@ mod _async_tasks { /// Captured at `create` so pool-thread completion never reads through /// `global_object` off-thread (the owning VM may be a worker freed by /// terminate() mid-flight). - pub vm: *mut VirtualMachine, + pub vm: bun_jsc::virtual_machine::VmHandle, pub task: WorkPoolTask, pub r#ref: KeepAlive, pub tracker: AsyncTaskTracker, @@ -2383,7 +2383,7 @@ mod _async_tasks { args: FsArgument::into_thread_safe(args), has_result: AtomicBool::new(false), global_object: bun_ptr::BackRef::new(global_object), - vm: core::ptr::from_mut(vm), + vm: vm.concurrent_handle(), task: work_pool_task(Self::work_pool_callback), r#ref: KeepAlive::default(), tracker: AsyncTaskTracker::init(vm), @@ -2559,7 +2559,7 @@ mod _async_tasks { } } - // `self.vm` was captured at `create` and may point at a worker VM + // `self.vm` was captured at `create` and may denote a worker VM // freed by terminate() while subtasks ran — checked enqueue only, // and no reads through `global_object` on this thread. // `ConcurrentTask::create` heap-allocates a fresh task; the queue diff --git a/src/runtime/node/node_fs_stat_watcher.rs b/src/runtime/node/node_fs_stat_watcher.rs index dcf5ae13fcb..78811d55871 100644 --- a/src/runtime/node/node_fs_stat_watcher.rs +++ b/src/runtime/node/node_fs_stat_watcher.rs @@ -65,6 +65,9 @@ pub struct StatWatcherScheduler { // safe `&VirtualMachine` projection (Deref) at every read site; // `event_loop_shared()` / `enqueue_task_concurrent` take `&self`. vm: BackRef, + /// Schedule-time handle for `vm`, for the pool-thread completion enqueue + /// (the one access that must tolerate the VM being gone). + vm_handle: bun_jsc::virtual_machine::VmHandle, watchers: WatcherQueue, pub event_loop_timer: EventLoopTimer, @@ -189,6 +192,8 @@ impl StatWatcherScheduler { main_thread: thread::current().id(), // JSC_BORROW: `vm` is the live per-thread VM (never null). vm: BackRef::from(core::ptr::NonNull::new(vm).expect("vm")), + // JS thread; the current VM is the one `vm` points at. + vm_handle: VirtualMachine::get().concurrent_handle(), watchers: WatcherQueue::default(), event_loop_timer: EventLoopTimer::init_paused(EventLoopTimerTag::StatWatcherScheduler), ref_count: ThreadSafeRefCount::init(), @@ -330,12 +335,12 @@ impl StatWatcherScheduler { ctx: core::ptr::NonNull::new(holder_ptr.cast()), callback: update_timer, }; - // Checked: runs on the work-pool thread; `vm` may be a worker VM + // Checked: runs on the work-pool thread; the VM may be a worker // freed by terminate() while the restat ran. On failure reclaim // the holder here (plain `{ParentRef, AnyTask}` — no JSC state), // since `update_timer` will never run to take it. if !VirtualMachine::try_enqueue_task_concurrent( - (*this).vm.as_ptr(), + (*this).vm_handle, ConcurrentTask::create(Task::init(core::ptr::addr_of_mut!((*holder_ptr).task))), ) { drop(bun_core::heap::take(holder_ptr)); @@ -524,6 +529,9 @@ pub struct StatWatcher { // via `From` from `bun_vm_ptr()` so `as_ptr()` retains write // provenance for the one `rare_data()` (`&mut self`) call in `deinit`. ctx: BackRef, + /// Schedule-time handle for `ctx`, for the pool-thread completion enqueue + /// (the one access that must tolerate the VM being gone). + vm_handle: bun_jsc::virtual_machine::VmHandle, ref_count: ThreadSafeRefCount, @@ -686,7 +694,7 @@ impl StatWatcher { ) { // Called from the work-pool thread: `ctx` may point at a worker VM // freed by terminate() while the stat ran — checked enqueue only. - let _ = VirtualMachine::try_enqueue_task_concurrent(self.ctx.as_ptr(), task); + let _ = VirtualMachine::try_enqueue_task_concurrent(self.vm_handle, task); } /// Copy the last stat by value. @@ -1013,6 +1021,8 @@ impl StatWatcher { // JSC_BORROW: `vm` is the live per-thread VM (never null). `From` // preserves the FFI write provenance for the `rare_data()` call in `deinit`. ctx: BackRef::from(core::ptr::NonNull::new(vm).expect("vm")), + // JS thread; the current VM is the one `vm` points at. + vm_handle: VirtualMachine::get().concurrent_handle(), ref_count: ThreadSafeRefCount::init(), closed: AtomicBool::new(false), path: alloc_file_path, diff --git a/src/runtime/node/node_fs_watcher.rs b/src/runtime/node/node_fs_watcher.rs index 33d0e6c1a86..27c7ae13641 100644 --- a/src/runtime/node/node_fs_watcher.rs +++ b/src/runtime/node/node_fs_watcher.rs @@ -45,6 +45,9 @@ use super::win_watcher as path_watcher; pub struct FSWatcher { // codegen: jsc.Codegen.JSFSWatcher provides toJS/fromJS/fromJSDirect ctx: *mut VirtualMachine, + /// Schedule-time handle for `ctx`, for the watcher-thread completion + /// enqueue (the one access that must tolerate the VM being gone). + vm_handle: bun_jsc::virtual_machine::VmHandle, verbose: bool, mutex: Mutex, @@ -101,7 +104,7 @@ impl FSWatcher { pub fn enqueue_task_concurrent(&self, task: core::ptr::NonNull) -> bool { // Called from watcher threads: `ctx` may point at a worker VM freed // by terminate() while an event was in flight — checked enqueue only. - VirtualMachine::try_enqueue_task_concurrent(self.ctx, task) + VirtualMachine::try_enqueue_task_concurrent(self.vm_handle, task) } /// `self`'s address as `*mut Self` for path-watcher / abort-signal / @@ -1083,6 +1086,7 @@ impl FSWatcher { let ctx = bun_core::heap::into_raw(Box::new(FSWatcher { ctx: vm, + vm_handle: vm_ref.concurrent_handle(), current_task: JsCell::new(FSWatchTask { ctx: None, ..Default::default() diff --git a/src/runtime/node/node_zlib_binding.rs b/src/runtime/node/node_zlib_binding.rs index d4281906ae7..c9029501846 100644 --- a/src/runtime/node/node_zlib_binding.rs +++ b/src/runtime/node/node_zlib_binding.rs @@ -250,9 +250,9 @@ pub(crate) trait CompressionStreamImpl: Sized + Taskable + 'static { fn poll_ref(&self) -> &JsCell; - /// Owning VM captured at construction — see the `vm` field docs on the - /// `Native*` structs. - fn vm(&self) -> &JsCell<*mut VirtualMachine>; + /// Owning VM's schedule-time handle, captured at construction — see the + /// `vm` field docs on the `Native*` structs. + fn vm(&self) -> &JsCell; fn this_value(&self) -> &JsCell; fn task(&self) -> &JsCell; fn write_in_progress(&self) -> &Cell; @@ -1008,7 +1008,7 @@ macro_rules! __impl_compression_stream { #[inline] fn global_this(&self) -> &::bun_jsc::JSGlobalObject { self.global_this.get() } #[inline] fn stream(&self) -> &::bun_jsc::JsCell { &self.stream } #[inline] fn poll_ref(&self) -> &::bun_jsc::JsCell<$crate::node::node_zlib_binding::CountedKeepAlive> { &self.poll_ref } - #[inline] fn vm(&self) -> &::bun_jsc::JsCell<*mut ::bun_jsc::virtual_machine::VirtualMachine> { &self.vm } + #[inline] fn vm(&self) -> &::bun_jsc::JsCell<::bun_jsc::virtual_machine::VmHandle> { &self.vm } #[inline] fn this_value(&self) -> &::bun_jsc::JsCell<::bun_jsc::StrongOptional> { &self.this_value } #[inline] fn task(&self) -> &::bun_jsc::JsCell<::bun_jsc::WorkPoolTask> { &self.task } #[inline] fn write_in_progress(&self) -> &::core::cell::Cell { &self.write_in_progress } diff --git a/src/runtime/node/zlib/NativeBrotli.rs b/src/runtime/node/zlib/NativeBrotli.rs index 1d880bd88fe..e6b8a380429 100644 --- a/src/runtime/node/zlib/NativeBrotli.rs +++ b/src/runtime/node/zlib/NativeBrotli.rs @@ -85,12 +85,12 @@ mod _impl { pub pending_reset: Cell, pub closed: Cell, pub task: JsCell, - /// Owning VM, captured at construction. Read on the work-pool thread - /// by `async_job_run` (happens-before via `WorkPool::schedule`, same - /// contract as `task`) so completion never reads through - /// `global_this` off-thread — the VM may be a worker freed by - /// terminate() while the job was in flight. - pub vm: JsCell<*mut bun_jsc::virtual_machine::VirtualMachine>, + /// Owning VM's schedule-time handle, captured at construction. Read + /// on the work-pool thread by `async_job_run` (happens-before via + /// `WorkPool::schedule`, same contract as `task`) so completion never + /// reads through `global_this` off-thread — the VM may be a worker + /// freed by terminate() while the job was in flight. + pub vm: JsCell, /// External-allocation footprint reported to the GC, fixed at /// construction. `mode` never changes after this (only `close()` sets /// it to `NONE`, on the JS thread), so the external state size is @@ -146,7 +146,7 @@ mod _impl { ref_count: Cell::new(1), // JSC_BORROW backref — the global outlives this m_ctx payload. global_this: bun_ptr::BackRef::new(global_this), - vm: JsCell::new(global_this.bun_vm_ptr()), + vm: JsCell::new(global_this.bun_vm().concurrent_handle()), stream: JsCell::new(stream), poll_ref: JsCell::new(CountedKeepAlive::default()), this_value: JsCell::new(StrongOptional::empty()), diff --git a/src/runtime/node/zlib/NativeZlib.rs b/src/runtime/node/zlib/NativeZlib.rs index d39f24be4b5..d8ea8dd3bab 100644 --- a/src/runtime/node/zlib/NativeZlib.rs +++ b/src/runtime/node/zlib/NativeZlib.rs @@ -50,12 +50,12 @@ mod _impl { pub pending_reset: Cell, pub closed: Cell, pub task: JsCell, - /// Owning VM, captured at construction. Read on the work-pool thread - /// by `async_job_run` (happens-before via `WorkPool::schedule`, same - /// contract as `task`) so completion never reads through - /// `global_this` off-thread — the VM may be a worker freed by - /// terminate() while the job was in flight. - pub vm: JsCell<*mut bun_jsc::virtual_machine::VirtualMachine>, + /// Owning VM's schedule-time handle, captured at construction. Read + /// on the work-pool thread by `async_job_run` (happens-before via + /// `WorkPool::schedule`, same contract as `task`) so completion never + /// reads through `global_this` off-thread — the VM may be a worker + /// freed by terminate() while the job was in flight. + pub vm: JsCell, } // write / runFromJSThread / writeSync / reset / close / setOnError / getOnError / @@ -100,7 +100,7 @@ mod _impl { ref_count: Cell::new(1), // JSC_BORROW backref — the global outlives this m_ctx payload. global_this: bun_ptr::BackRef::new(global), - vm: JsCell::new(global.bun_vm_ptr()), + vm: JsCell::new(global.bun_vm().concurrent_handle()), stream: JsCell::new(stream), poll_ref: JsCell::new(CountedKeepAlive::default()), this_value: JsCell::new(StrongOptional::empty()), diff --git a/src/runtime/node/zlib/NativeZstd.rs b/src/runtime/node/zlib/NativeZstd.rs index ba91097f457..db1b98c2a65 100644 --- a/src/runtime/node/zlib/NativeZstd.rs +++ b/src/runtime/node/zlib/NativeZstd.rs @@ -49,12 +49,12 @@ mod _impl { pub pending_reset: Cell, pub closed: Cell, pub task: JsCell, - /// Owning VM, captured at construction. Read on the work-pool thread - /// by `async_job_run` (happens-before via `WorkPool::schedule`, same - /// contract as `task`) so completion never reads through - /// `global_this` off-thread — the VM may be a worker freed by - /// terminate() while the job was in flight. - pub vm: JsCell<*mut bun_jsc::virtual_machine::VirtualMachine>, + /// Owning VM's schedule-time handle, captured at construction. Read + /// on the work-pool thread by `async_job_run` (happens-before via + /// `WorkPool::schedule`, same contract as `task`) so completion never + /// reads through `global_this` off-thread — the VM may be a worker + /// freed by terminate() while the job was in flight. + pub vm: JsCell, /// External-allocation footprint reported to the GC, fixed at /// construction. `mode` never changes after this (only `close()` sets /// it to `NONE`, on the JS thread), so the external state size is @@ -112,7 +112,7 @@ mod _impl { // JSC_BORROW — the JSGlobalObject outlives this payload (the C++ // wrapper is owned by that global's heap). global_this: bun_ptr::BackRef::new(global), - vm: JsCell::new(global.bun_vm_ptr()), + vm: JsCell::new(global.bun_vm().concurrent_handle()), stream: JsCell::new(stream), poll_ref: JsCell::new(CountedKeepAlive::default()), this_value: JsCell::new(StrongOptional::empty()), diff --git a/src/runtime/shell/builtin/yes.rs b/src/runtime/shell/builtin/yes.rs index f8627dc5cc0..e505cde2746 100644 --- a/src/runtime/shell/builtin/yes.rs +++ b/src/runtime/shell/builtin/yes.rs @@ -218,13 +218,13 @@ impl YesTask { // backrefs (single-threaded shell). unsafe { match (*this).evtloop { - EventLoopHandle::Js { owner } => { + EventLoopHandle::Js { owner, generation } => { owner.tick(); let ct = core::ptr::NonNull::from(match &mut (*this).concurrent_task { EventLoopTask::Js(ct) => ct.from(this, AutoDeinit::ManualDeinit), EventLoopTask::Mini(_) => unreachable!(), }); - owner.enqueue_task_concurrent(ct); + owner.enqueue_task_concurrent(ct, generation); } EventLoopHandle::Mini(mut mini) => { (*mini.loop_).tick(); diff --git a/src/runtime/shell/shell_body.rs b/src/runtime/shell/shell_body.rs index bbc72d130c0..8600570f770 100644 --- a/src/runtime/shell/shell_body.rs +++ b/src/runtime/shell/shell_body.rs @@ -277,20 +277,6 @@ impl<'a> GlobalJS<'a> { } } - #[inline] - pub fn enqueue_task_concurrent_wait_pid(self, task: *mut T) { - let vm = self - .global_this - .bun_vm_concurrently() - .cast_const() - .cast_mut(); - let concurrent = bun_event_loop::ConcurrentTask::create(bun_event_loop::Task::init(task)); - // Checked: callable from waiter/pool threads, which can outlive a - // terminated worker's VM. - let _ = - bun_jsc::virtual_machine::VirtualMachine::try_enqueue_task_concurrent(vm, concurrent); - } - #[inline] pub fn top_level_dir(self) -> &'a [u8] { bun_resolver::fs::FileSystem::get().top_level_dir diff --git a/src/runtime/webcore/FileSink.rs b/src/runtime/webcore/FileSink.rs index 970c5e5f500..39491013fa5 100644 --- a/src/runtime/webcore/FileSink.rs +++ b/src/runtime/webcore/FileSink.rs @@ -870,7 +870,7 @@ impl FileSink { return; } self.run_pending_later.has.set(true); - if let EventLoopHandle::Js { owner } = self.event_loop() { + if let EventLoopHandle::Js { owner, .. } = self.event_loop() { self.ref_(); // The type→tag // map lives in `crate::dispatch`; the resolved tag for diff --git a/src/runtime/webcore/blob/copy_file.rs b/src/runtime/webcore/blob/copy_file.rs index db997f21f3c..7cd5263d54d 100644 --- a/src/runtime/webcore/blob/copy_file.rs +++ b/src/runtime/webcore/blob/copy_file.rs @@ -996,6 +996,9 @@ pub struct CopyFileWindows { // a worker VM freed this loop — JS-thread paths `get()` it, the // concurrent path passes `as_ptr()` to the checked enqueue only. pub event_loop: bun_ptr::BackRef, + /// Schedule-time handle for `event_loop`, for the pool-thread mkdirp + /// completion (the one access that must tolerate the loop being gone). + pub loop_handle: jsc::event_loop::LoopHandle, pub size: SizeType, @@ -1302,6 +1305,7 @@ impl CopyFileWindows { // SAFETY: all-zero is a valid libuv::fs_t io_request: bun_core::ffi::zeroed::(), event_loop: bun_ptr::BackRef::new(event_loop), + loop_handle: event_loop.concurrent_handle(), mkdirp_if_not_exists, destination_mode, size: size_, @@ -1831,7 +1835,7 @@ fn on_mkdirp_complete_concurrent(ctx: *mut (), err_: bun_sys::Maybe<()>) { // Checked: the mkdirp completion runs on the work-pool thread; the // owning VM may be a worker freed by terminate() in the meantime. let _ = jsc::event_loop::EventLoop::try_enqueue_task_concurrent( - this.event_loop.as_ptr(), + this.loop_handle, jsc::ConcurrentTask::create(jsc::ManagedTask::ManagedTask::new::( this, call_erased, diff --git a/src/runtime/webcore/blob/write_file.rs b/src/runtime/webcore/blob/write_file.rs index cb2ac64f6ff..d7851528995 100644 --- a/src/runtime/webcore/blob/write_file.rs +++ b/src/runtime/webcore/blob/write_file.rs @@ -610,7 +610,11 @@ mod windows_impl { use bun_io::{self as aio, IntrusiveUvFs as _, KeepAlive}; // `bun_jsc::EventLoop`/`ManagedTask` are *modules* (namespace // re-exports); the structs live one level deeper. - use bun_jsc::{ConcurrentTask, ManagedTask::ManagedTask, event_loop::EventLoop}; + use bun_jsc::{ + ConcurrentTask, + ManagedTask::ManagedTask, + event_loop::{EventLoop, LoopHandle}, + }; use bun_sys::ReturnCodeExt as _; use bun_sys::windows::libuv as uv; @@ -627,6 +631,9 @@ mod windows_impl { pub err: Option, pub total_written: usize, pub event_loop: *mut EventLoop, + /// Schedule-time handle for `event_loop`, for the pool-thread mkdirp + /// completion (the one access that must tolerate the loop being gone). + pub loop_handle: LoopHandle, pub poll_ref: KeepAlive, pub owned_fd: bool, @@ -685,6 +692,8 @@ mod windows_impl { len: 0, }], event_loop, + // SAFETY: `event_loop` is the live JS-thread loop at create. + loop_handle: unsafe { (*event_loop).concurrent_handle() }, fd: -1, err: None, total_written: 0, @@ -1032,7 +1041,7 @@ mod windows_impl { // Checked: the mkdirp completion runs on the work-pool thread; // the owning VM may be a worker freed by terminate() meanwhile. let _ = EventLoop::try_enqueue_task_concurrent( - this.event_loop, + this.loop_handle, ConcurrentTask::create(ManagedTask::new::( this, Self::on_mkdirp_complete_task, diff --git a/src/runtime/webcore/fetch/FetchTasklet.rs b/src/runtime/webcore/fetch/FetchTasklet.rs index 413d0b549b8..8f84a01bb22 100644 --- a/src/runtime/webcore/fetch/FetchTasklet.rs +++ b/src/runtime/webcore/fetch/FetchTasklet.rs @@ -78,12 +78,13 @@ pub struct FetchTasklet { pub http: Option>>, pub result: HTTPClientResult<'static>, pub metadata: Option, - /// Owning VM, captured at `get()`. `BackRef` (not `&'static`): a worker + /// Owning VM, captured at `get()` as a schedule-time handle: a worker /// VM can be freed by terminate() while this tasklet is still referenced - /// from the HTTP thread, so holding a Rust reference would dangle. - /// JS-thread paths `get()` it (VM provably alive there); HTTP-thread - /// paths pass `as_ptr()` to the checked `VirtualMachine` accessors only. - pub javascript_vm: bun_ptr::BackRef, + /// from the HTTP thread, so holding a pointer alone could also pass an + /// address check after the allocation is reused. JS-thread paths use + /// `vm_on_owning_thread()` (VM provably alive there); HTTP-thread paths + /// go through the checked `VirtualMachine` accessors only. + pub javascript_vm: jsc::virtual_machine::VmHandle, pub global_this: GlobalRef, pub request_body: HTTPRequestBody, // ThreadSafeStreamBuffer is intrusively refcounted (`ref_count: AtomicU32`, @@ -397,11 +398,11 @@ impl FetchTasklet { return; } let self_ = Self::from_raw_ref(this); - // `javascript_vm` may point at a freed worker VM (terminated while + // `javascript_vm` may denote a freed worker VM (terminated while // this request was in flight on the HTTP thread) — only the checked - // accessors may touch it. - let vm_ptr = self_.javascript_vm.as_ptr(); - match VirtualMachine::live_shutting_down_state(vm_ptr) { + // accessors may resolve it. + let vm = self_.javascript_vm; + match VirtualMachine::live_shutting_down_state(vm) { Some(false) => { // this is really unlikely to happen, but can happen // lets make sure that we always call deinit from main thread @@ -410,7 +411,7 @@ impl FetchTasklet { // enqueue when the VM died between the check and the push — // in which case fall through to the dead-VM parking below). if VirtualMachine::try_enqueue_task_concurrent( - vm_ptr, + vm, ConcurrentTask::from_callback(this, FetchTasklet::deinit_callback), ) { return; @@ -862,10 +863,10 @@ impl FetchTasklet { let is_done = !self.result.has_more; // JS-thread path: the VM is this thread's own live VM. Copy the - // `BackRef` out so the borrow below doesn't pin `self`. - let vm = self.javascript_vm; + // handle out so the borrow below doesn't pin `self`. + let vm = self.javascript_vm.vm_on_owning_thread(); // vm is shutting down we cannot touch JS - if vm.get().is_shutting_down() { + if vm.is_shutting_down() { // The certificate will never be checked; release the parked // HTTP-thread socket instead of leaving it occupying an active // request slot until the idle timeout. @@ -1772,10 +1773,10 @@ impl FetchTasklet { ) -> Result<*mut FetchTasklet, BunError> { // `bun_vm()` is the live per-thread VM at `get()` time, but a worker // VM can be freed by terminate() before this tasklet dies — which is - // why the field is a `BackRef` and never dereferenced off the JS - // thread without the checked `VirtualMachine` accessors (see the + // why the field is a schedule-time handle, resolved off the JS thread + // only through the checked `VirtualMachine` accessors (see the // `javascript_vm` field doc). - let jsc_vm = bun_ptr::BackRef::new(global_this.bun_vm()); + let jsc_vm = global_this.bun_vm().concurrent_handle(); let mut fetch_tasklet = Box::new(FetchTasklet { sink: None, // `AsyncHTTP` has no `Default`/zero-init; defer the Box until @@ -2056,8 +2057,8 @@ impl FetchTasklet { pub(crate) fn on_write_request_data_drain(this: *mut FetchTasklet) { let this_ref = Self::from_raw_ref(this); // Checked: the fetching VM may be a worker freed by terminate(). - let vm_ptr = this_ref.javascript_vm.as_ptr(); - if VirtualMachine::is_shutting_down_or_freed(vm_ptr) { + let vm = this_ref.javascript_vm; + if VirtualMachine::is_shutting_down_or_freed(vm) { return; } // ref until the main thread callback is called @@ -2065,7 +2066,7 @@ impl FetchTasklet { // `from_callback` heap-allocates a fresh `ConcurrentTaskItem`; the queue // takes ownership of it. if !VirtualMachine::try_enqueue_task_concurrent( - vm_ptr, + vm, ConcurrentTask::from_callback(this, FetchTasklet::resume_request_data_stream), ) { // VM died between the check and the push; the callback will never @@ -2363,8 +2364,8 @@ impl FetchTasklet { // will deinit when done with the http client (when is_done = true) // Checked accessors only: the fetching VM may be a worker freed by // terminate() while this request was in flight. - let vm_ptr = task_ref.javascript_vm.as_ptr(); - let queued = !VirtualMachine::is_shutting_down_or_freed(vm_ptr) && { + let vm = task_ref.javascript_vm; + let queued = !VirtualMachine::is_shutting_down_or_freed(vm) && { // `ct` is the inline `concurrent_task` field of the heap tasklet; // the queue takes ownership of its `next` link. The embedded node // is untouched when the checked enqueue loses the race. @@ -2373,7 +2374,7 @@ impl FetchTasklet { .concurrent_task .from(task, AutoDeinit::ManualDeinit), ); - VirtualMachine::try_enqueue_task_concurrent(vm_ptr, ct) + VirtualMachine::try_enqueue_task_concurrent(vm, ct) }; if !queued { // VM teardown: the JS-thread side will never drain this buffer (its diff --git a/src/runtime/webcore/s3/client.rs b/src/runtime/webcore/s3/client.rs index 9f3778cdd3a..aa3364194c5 100644 --- a/src/runtime/webcore/s3/client.rs +++ b/src/runtime/webcore/s3/client.rs @@ -305,7 +305,7 @@ pub(crate) fn list_objects( callback_context, callback: s3_simple_request::Callback::ListObjects(callback), headers, - vm: Some(bun_ptr::BackRef::new(VirtualMachine::get())), + vm: Some(VirtualMachine::get().concurrent_handle()), response_buffer: MutableString::default(), result: bun_http::HTTPClientResult::default(), concurrent_task: Default::default(), @@ -343,12 +343,14 @@ pub(crate) fn list_objects( } else { None }; - let mut vm_ref = task.vm.expect("vm set at task creation"); - // SAFETY: `task.vm` is the live per-thread VM BackRef from - // `VirtualMachine::get()`; `get_mut` exclusivity holds — single-threaded - // dispatch on the JS thread, no other `&`/`&mut VirtualMachine` is live for - // this call's duration. - let vm = unsafe { vm_ref.get_mut() }; + // JS thread — the task's VM is this thread's live VM. `as_mut` + // exclusivity holds: single-threaded dispatch, no other + // `&`/`&mut VirtualMachine` is live for this call's duration. + let vm = task + .vm + .expect("vm set at task creation") + .vm_on_owning_thread() + .as_mut(); task.http.write(bun_http::AsyncHTTP::init( bun_http::Method::GET, @@ -1023,7 +1025,7 @@ pub(crate) fn download_stream( range: range.map(Vec::into_boxed_slice), headers, // `VirtualMachine::get()` returns the live per-thread VM singleton. - vm: Some(bun_ptr::BackRef::new(VirtualMachine::get())), + vm: Some(VirtualMachine::get().concurrent_handle()), has_schedule_callback: core::sync::atomic::AtomicBool::new(false), signal_store: Default::default(), signals: Default::default(), diff --git a/src/runtime/webcore/s3/download_stream.rs b/src/runtime/webcore/s3/download_stream.rs index eae3304d654..eea072a3913 100644 --- a/src/runtime/webcore/s3/download_stream.rs +++ b/src/runtime/webcore/s3/download_stream.rs @@ -21,7 +21,7 @@ pub struct S3HttpDownloadStreamingTask { pub http: core::mem::MaybeUninit>, /// JSC_BORROW: per-thread VM singleton, outlives every task. `None` only in /// the inert `Default` placeholder (overwritten before the task escapes). - pub vm: Option>, + pub vm: Option, pub sign_result: SignResult, pub headers: Headers, pub callback_context: NonNull<()>, @@ -341,13 +341,13 @@ impl S3HttpDownloadStreamingTask { let task = core::ptr::NonNull::from( self_.concurrent_task.from(this, AutoDeinit::ManualDeinit), ); - // `vm` was captured at task creation and may point at a worker VM + // `vm` was captured at task creation and may denote a worker VM // freed by terminate() while this request was in flight — checked // enqueue only. `task` is the inline `concurrent_task` field of // this heap request; the queue takes ownership of its `next` link // (and leaves it untouched when the VM is gone). let _ = VirtualMachine::try_enqueue_task_concurrent( - self_.vm.expect("vm set at task creation").as_ptr(), + self_.vm.expect("vm set at task creation"), task, ); } diff --git a/src/runtime/webcore/s3/simple_request.rs b/src/runtime/webcore/s3/simple_request.rs index 05046c7a454..a75cb342499 100644 --- a/src/runtime/webcore/s3/simple_request.rs +++ b/src/runtime/webcore/s3/simple_request.rs @@ -117,9 +117,9 @@ pub struct S3HttpSimpleTask { // `execute_simple_s3_request` before the task pointer escapes, so every later access (in // `http_callback` / `Drop`) may `assume_init`. pub http: core::mem::MaybeUninit>, - /// JSC_BORROW: per-thread VM singleton, outlives every task. `None` only in - /// the inert `Default` placeholder (overwritten before the task escapes). - pub vm: Option>, + /// Schedule-time handle of the owning VM. `None` only in the inert + /// `Default` placeholder (overwritten before the task escapes). + pub vm: Option, pub sign_result: SignResult, pub headers: Headers, pub callback_context: *mut c_void, @@ -480,13 +480,13 @@ impl S3HttpSimpleTask { this.concurrent_task .from(this_ptr, AutoDeinit::ManualDeinit), ); - // `vm` was captured at task creation and may point at a worker VM + // `vm` was captured at task creation and may denote a worker VM // freed by terminate() while this request was in flight — checked // enqueue only. `task` is the inline `concurrent_task` field of // this heap request; the queue takes ownership of its `next` link // (and leaves it untouched when the VM is gone). let _ = VirtualMachine::try_enqueue_task_concurrent( - this.vm.expect("vm set at task creation").as_ptr(), + this.vm.expect("vm set at task creation"), task, ); } @@ -628,7 +628,7 @@ pub(crate) fn execute_simple_s3_request( callback, range: options.range, headers, - vm: Some(bun_ptr::BackRef::new(VirtualMachine::get())), + vm: Some(VirtualMachine::get().concurrent_handle()), response_buffer: MutableString::default(), result: HTTPClientResult::default(), concurrent_task: ConcurrentTask::default(), diff --git a/src/spawn/process.rs b/src/spawn/process.rs index 7a5382817c1..541277c609c 100644 --- a/src/spawn/process.rs +++ b/src/spawn/process.rs @@ -1201,7 +1201,7 @@ pub mod waiter_thread_posix { remove = true; match T::event_loop(process_ref) { - EventLoopHandle::Js { owner } => { + EventLoopHandle::Js { owner, generation } => { let ct = ConcurrentTask::create(Task::new( T::TASK_TAG, ResultTask::::new(ResultTask { @@ -1211,7 +1211,11 @@ pub mod waiter_thread_posix { }) .cast(), )); - owner.enqueue_task_concurrent(ct); + // Checked dispatch: the waiter thread outlives + // worker VMs; `(owner, generation)` — captured + // at spawn time — is validated against the + // live-VM registry before any dereference. + owner.enqueue_task_concurrent(ct, generation); } EventLoopHandle::Mini(mut mini) => { let out = ResultTaskMini::::new(ResultTaskMini { diff --git a/test/js/web/workers/worker-terminate-lifetime.test.ts b/test/js/web/workers/worker-terminate-lifetime.test.ts index 8e90d844e94..77e49ed344c 100644 --- a/test/js/web/workers/worker-terminate-lifetime.test.ts +++ b/test/js/web/workers/worker-terminate-lifetime.test.ts @@ -120,6 +120,124 @@ test( timeout, ); +// Cross-thread completion delivery inside a worker goes through schedule-time +// VM/loop handles validated against the live-VM registry (address + +// generation). A broken handle capture (e.g. a zero generation) silently +// drops the completion instead of delivering it, so each producer class below +// would hang instead of resolving: the HTTP client thread (fetch), the +// process waiter thread (Bun.spawn exit), and the work pool (node:fs async, +// zlib native streams, Bun.password). +test( + "cross-thread completions are delivered to live worker VMs", + async () => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + ` + const workerCode = \` + const results = {}; + const server = Bun.serve({ port: 0, fetch: () => new Response("pong") }); + results.fetch = await (await fetch("http://127.0.0.1:" + server.port + "/")).text(); + server.stop(true); + const child = Bun.spawn({ cmd: [process.execPath, "-e", "process.exit(7)"] }); + results.spawnExit = await child.exited; + results.statIsFile = (await require("fs").promises.stat(process.execPath)).isFile(); + const zlib = require("zlib"); + const gz = await new Promise((resolve, reject) => + zlib.gzip(Buffer.from("hello"), (e, d) => (e ? reject(e) : resolve(d))), + ); + results.gunzip = (await new Promise((resolve, reject) => + zlib.gunzip(gz, (e, d) => (e ? reject(e) : resolve(d))), + )).toString(); + const hash = await Bun.password.hash("pw", { algorithm: "bcrypt", cost: 4 }); + results.password = await Bun.password.verify("pw", hash); + postMessage(results); + \`; + const worker = new Worker( + "data:text/javascript," + encodeURIComponent("(async () => {" + workerCode + "})()"), + ); + const results = await new Promise((resolve, reject) => { + worker.onmessage = e => resolve(e.data); + worker.onerror = e => reject(new Error(e.message)); + }); + worker.terminate(); + console.log(JSON.stringify(results)); + `, + ], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toBe(""); + expect(JSON.parse(stdout)).toEqual({ + fetch: "pong", + spawnExit: 7, + statIsFile: true, + gunzip: "hello", + password: true, + }); + expect(exitCode).toBe(0); + }, + timeout, +); + +// Same class as the fetch test below, for the process waiter thread: a worker +// that spawned a subprocess is terminated (VM freed) before the subprocess +// exits; the waiter thread then delivers the exit notification through the +// EventLoopHandle captured at spawn time, which the registry check must drop +// instead of enqueueing into the freed loop. +test( + "terminating a worker with a subprocess in flight drops the waiter-thread completion", + async () => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + ` + const workerCode = + "const child = Bun.spawn({ cmd: [process.execPath, \\"-e\\", \\"setTimeout(() => {}, 100000)\\"] });" + + "postMessage(child.pid);"; + for (let i = 0; i < 3; i++) { + const worker = new Worker("data:text/javascript," + encodeURIComponent(workerCode)); + const pid = await new Promise(resolve => (worker.onmessage = e => resolve(e.data))); + const closed = new Promise(resolve => worker.addEventListener("close", resolve, { once: true })); + worker.terminate(); + await closed; + // Give the worker thread time to finish shutdown() and free its VM. + await Bun.sleep(300); + // Reap the orphan; the waiter thread now delivers the exit + // notification keyed by the freed worker's event loop. Tolerate the + // child having already died with the worker. + try { + process.kill(pid); + } catch {} + await Bun.sleep(300); + } + // Leave room for an in-progress sanitizer report to abort the process + // before we exit cleanly. + await Bun.sleep(1000); + console.log("done"); + `, + ], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).not.toContain("AddressSanitizer"); + expect({ stdout, exitCode, signalCode: proc.signalCode }).toEqual({ + stdout: "done\n", + exitCode: 0, + signalCode: null, + }); + }, + timeout, +); + // Regression: a worker terminated while a fetch was in flight freed its // VirtualMachine (and the event loop embedded in it) while the HTTP client // thread still held a pointer to it; the completion callback then read the