From f2311f4659406444524abd3f0499e16c4d838a5f Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 10 Jun 2026 19:37:37 +0000 Subject: [PATCH 01/17] Don't enqueue to a terminated worker's freed event loop from other threads A worker's VirtualMachine (with its EventLoop and MPSC concurrent task queue embedded in it) is freed by WebWorker::shutdown() when the worker is terminated. Cross-thread producers that captured a raw pointer to the VM or its event loop at schedule time (the HTTP client thread for fetch/S3 completions, the work pool for fs/crypto/zlib/napi completions, watcher threads, napi addon threads, the process waiter thread) had no way to observe that free: a completion landing after terminate() read the freed VM and pushed into its freed queue. The corrupted queue memory then surfaced on whichever live worker reused it, as "Panic: invalid enum value" in EventLoop.tickQueueWithCount (Sentry BUN-2VPE, 546 events, Windows-dominant, always on a worker thread with workers_spawned + workers_terminated set). Fix: a process-global registry of live (VirtualMachine, EventLoop) addresses, mirroring the fence the C++ postTaskTo path already has with allScriptExecutionContextsMap. VMs register in VirtualMachine::init(), workers unregister at the top of shutdown() before anything is freed, and the boxed spawnSync loop registers/unregisters around its lifetime. Cross-thread producers now go through checked entry points (VirtualMachine::try_enqueue_task_concurrent / with_live_vm / is_shutting_down_or_freed, EventLoop::try_enqueue_task_concurrent) that hold the registry lock across the push, so teardown cannot free the VM mid-enqueue; the immortal main-thread VM takes a lock-free address- compare fast path. When the target is gone the task is dropped: the node is freed if auto_delete and the payload is left to leak, matching the fate of tasks already sitting in a terminated worker's never-drained queue. Converted producers: FetchTasklet (the shutting-down check on the HTTP thread raced the same free), S3 simple/download tasks, WorkTask, ConcurrentPromiseTask, AnyTaskJob (also stops reading vm.global on the pool thread), node:fs AsyncFSTask and readdir-recursive (now capture the VM at schedule instead of deriving it from the freed JSGlobalObject at completion), node:zlib native streams (same capture), napi async_work / ThreadSafeFunction / NapiFinalizerTask, fs watchers and stat watcher, Archive, Bun.password, RuntimeTranspilerStore, JSBundler completion, DevServer hot reload events, JSC deferred work scheduler and the concurrent keep-alive counters, and every EventLoopHandle user (shell tasks, the POSIX waiter thread) via the vtable arm. The regression test terminates a worker while a fetch is in flight and releases the response only after the worker VM is freed; on an unfixed ASAN build the HTTP thread trips heap-use-after-free in FetchTasklet::callback -> VirtualMachine::is_shutting_down deterministically. --- src/jsc/AsyncModule.rs | 7 +- src/jsc/ConcurrentPromiseTask.rs | 4 +- src/jsc/CppTask.rs | 7 +- src/jsc/JSCScheduler.rs | 23 +-- src/jsc/RuntimeTranspilerStore.rs | 24 ++- src/jsc/VirtualMachine.rs | 189 ++++++++++++++++++ src/jsc/WorkTask.rs | 6 +- src/jsc/any_task_job.rs | 20 +- src/jsc/event_loop.rs | 69 ++++++- src/jsc/web_worker.rs | 9 + src/runtime/api/Archive.rs | 8 +- src/runtime/api/js_bundle_completion_task.rs | 26 ++- src/runtime/bake/dev_server/mod.rs | 16 +- src/runtime/crypto/PasswordObject.rs | 16 +- src/runtime/napi/napi_body.rs | 42 ++-- src/runtime/node/node_fs.rs | 47 +++-- src/runtime/node/node_fs_stat_watcher.rs | 16 +- src/runtime/node/node_fs_watcher.rs | 8 +- src/runtime/node/node_zlib_binding.rs | 33 +-- src/runtime/node/zlib/NativeBrotli.rs | 7 + src/runtime/node/zlib/NativeZlib.rs | 7 + src/runtime/node/zlib/NativeZstd.rs | 7 + src/runtime/shell/shell_body.rs | 8 +- src/runtime/webcore/blob/copy_file.rs | 13 +- src/runtime/webcore/blob/write_file.rs | 15 +- src/runtime/webcore/fetch/FetchTasklet.rs | 93 +++++---- src/runtime/webcore/s3/download_stream.rs | 18 +- src/runtime/webcore/s3/simple_request.rs | 17 +- .../workers/worker-terminate-lifetime.test.ts | 71 +++++++ 29 files changed, 625 insertions(+), 201 deletions(-) diff --git a/src/jsc/AsyncModule.rs b/src/jsc/AsyncModule.rs index c2741524f6a5..3782d9fb7179 100644 --- a/src/jsc/AsyncModule.rs +++ b/src/jsc/AsyncModule.rs @@ -372,8 +372,11 @@ impl Queue { // `from_field_ptr!` is sound. S017 does not apply: that rule forbids // widening from a `&mut self`-derived pointer, but `ctx` is a raw // `*mut` carried from the original allocation. - let vm = unsafe { &mut *bun_core::from_field_ptr!(VirtualMachine, modules, queue) }; - vm.enqueue_task_concurrent(task); + let vm: *mut VirtualMachine = + 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); } pub fn on_poll(&mut self) { diff --git a/src/jsc/ConcurrentPromiseTask.rs b/src/jsc/ConcurrentPromiseTask.rs index 62aba68d025b..ec0b9a03f8f0 100644 --- a/src/jsc/ConcurrentPromiseTask.rs +++ b/src/jsc/ConcurrentPromiseTask.rs @@ -116,7 +116,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.enqueue_task_concurrent(task); + // `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); } /// Frees the heap allocation backing this task. diff --git a/src/jsc/CppTask.rs b/src/jsc/CppTask.rs index 130402469c25..3b67d2b7e25f 100644 --- a/src/jsc/CppTask.rs +++ b/src/jsc/CppTask.rs @@ -79,7 +79,9 @@ impl ConcurrentCppTask { // `opaque_ref` above proved it non-null and it has not yet been freed — `run` consumes it here. unsafe { EventLoopTaskNoContext::run(cpp_task) }; if let Some(vm) = maybe_vm { - vm.event_loop_shared().unref_concurrently(); + // 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()); } } } @@ -90,7 +92,8 @@ pub(crate) extern "C" fn ConcurrentCppTask__createAndRun(cpp_task: *mut EventLoo // `EventLoopTaskNoContext` is an `opaque_ffi!` ZST handle; `opaque_ref` is // the centralised non-null deref proof. C++ just handed it over. if let Some(vm) = EventLoopTaskNoContext::opaque_ref(cpp_task).get_vm() { - vm.event_loop_shared().ref_concurrently(); + // Checked for symmetry with the pool-thread unref in `run_owned`. + VirtualMachine::try_ref_concurrently(vm.as_ptr()); } WorkPool::schedule_new(ConcurrentCppTask { cpp_task, diff --git a/src/jsc/JSCScheduler.rs b/src/jsc/JSCScheduler.rs index e17cd9523656..be51d283d043 100644 --- a/src/jsc/JSCScheduler.rs +++ b/src/jsc/JSCScheduler.rs @@ -2,7 +2,7 @@ use core::ffi::c_int; use bun_event_loop::{ConcurrentTask::ConcurrentTask, TaskTag, Taskable, task_tag}; -use crate::event_loop::{EventLoop, JsTerminated}; +use crate::event_loop::JsTerminated; use crate::virtual_machine::VirtualMachine; bun_opaque::opaque_ffi! { @@ -38,30 +38,29 @@ impl JSCDeferredWorkTask { #[unsafe(no_mangle)] pub(crate) extern "C" fn Bun__eventLoop__incrementRefConcurrently( - jsc_vm: &VirtualMachine, + jsc_vm: *mut VirtualMachine, delta: c_int, ) { crate::mark_binding!(); - // C++ passes a non-null live `VirtualMachine*`; ABI-compatible with `&T`. - // `event_loop_shared()` is the safe accessor over the VM-owned EventLoop. - let event_loop: &EventLoop = jsc_vm.event_loop_shared(); + // Checked: called from JSC helper threads, which can outlive a + // terminated worker's VM (the counter of a freed loop needs no balancing). if delta > 0 { - event_loop.ref_concurrently(); + VirtualMachine::try_ref_concurrently(jsc_vm); } else { - event_loop.unref_concurrently(); + VirtualMachine::try_unref_concurrently(jsc_vm); } } #[unsafe(no_mangle)] pub(crate) extern "C" fn Bun__queueJSCDeferredWorkTaskConcurrently( - jsc_vm: &VirtualMachine, + jsc_vm: *mut VirtualMachine, task: *mut JSCDeferredWorkTask, ) { crate::mark_binding!(); - // C++ passes a non-null live `VirtualMachine*`; ABI-compatible with `&T`. - let loop_: &EventLoop = jsc_vm.event_loop_shared(); - // `create_from` heap-allocates with the auto-delete bit set. - loop_.enqueue_task_concurrent(ConcurrentTask::create_from(task)); + // 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)); } /// # Safety diff --git a/src/jsc/RuntimeTranspilerStore.rs b/src/jsc/RuntimeTranspilerStore.rs index 28057e87299b..3c4d674963c0 100644 --- a/src/jsc/RuntimeTranspilerStore.rs +++ b/src/jsc/RuntimeTranspilerStore.rs @@ -515,16 +515,22 @@ impl TranspilerJob { pub(crate) fn dispatch_to_main_thread(&mut self) { let vm = self.vm; - // SAFETY: vm outlives the job (BACKREF — VM owns the store). - let transpiler_store: *mut RuntimeTranspilerStore = - unsafe { ptr::addr_of_mut!((*vm).transpiler_store) }; let job = NonNull::from(&mut *self); - // SAFETY: queue is concurrent-safe (UnboundedQueue uses atomics). - unsafe { (*transpiler_store).queue.push(job) }; - // Another thread may free `self` at any time after .push, so we cannot use it any more. - // SAFETY: vm outlives the job; event_loop() returns the live self-pointer. - unsafe { &*(*vm).event_loop() } - .enqueue_task_concurrent(ConcurrentTask::create_from(transpiler_store)); + // Both the store's queue and the event loop live inside the VM + // allocation, which may be a worker VM freed by terminate() while + // this job ran on the pool — touch them only inside `with_live_vm` + // (the registry lock holds off the free). When the VM is gone the + // job is simply dropped on the floor; nothing will ever drain it. + let _ = crate::VirtualMachineRef::with_live_vm(vm, |vm| { + let transpiler_store: *mut RuntimeTranspilerStore = + ptr::addr_of!(vm.transpiler_store).cast_mut(); + // SAFETY: queue is concurrent-safe (UnboundedQueue uses atomics). + unsafe { (*transpiler_store).queue.push(job) }; + // Another thread may free `*job` at any time after .push, so we + // cannot use it any more. + vm.event_loop_shared() + .enqueue_task_concurrent(ConcurrentTask::create_from(transpiler_store)); + }); } pub(crate) fn run_from_js_thread(&mut self) -> JsResult<()> { diff --git a/src/jsc/VirtualMachine.rs b/src/jsc/VirtualMachine.rs index e847b6a66271..a745c7bff1fe 100644 --- a/src/jsc/VirtualMachine.rs +++ b/src/jsc/VirtualMachine.rs @@ -467,6 +467,110 @@ impl VMHolder { } } +/// Process-global registry of `(VirtualMachine, EventLoop)` addresses that +/// cross-thread producers may still enqueue to. +/// +/// Worker `VirtualMachine`s (and the `EventLoop`s embedded in them) are freed +/// by `WebWorker::shutdown()` while producers on other threads — the HTTP +/// client thread (fetch/S3 completions), the work pool (fs/crypto/zlib/napi +/// completions), watcher threads, napi addon threads — still hold raw +/// pointers captured when the work was scheduled. A push after the free +/// corrupts reused heap memory; the corrupted task queue then surfaces as +/// "invalid enum value" panics in `tickQueueWithCount` on whichever live +/// worker inherited the memory. +/// +/// This is the Rust-side analogue of the fence the C++ `postTaskTo` path +/// already has (`allScriptExecutionContextsMap` + its lock, see +/// `ScriptExecutionContext.cpp`): teardown removes the VM from the registry +/// under the same lock producers take to enqueue, so a producer either +/// observes the VM live (and the teardown path then waits for the lock before +/// freeing) or drops the task. +/// +/// Addresses are stored as `usize` — the registry never dereferences them. +/// The main-thread VM is registered but never unregistered (its allocation is +/// static-rooted and never freed), which enables the lock-free fast path in +/// the checked entry points 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 +/// no other locks. +pub(crate) mod live_vm_registry { + use super::VirtualMachine; + use crate::event_loop::EventLoop; + use bun_threading::Guarded; + + #[derive(Copy, Clone, Eq, PartialEq)] + pub(crate) struct Entry { + pub(crate) vm: usize, + pub(crate) loop_: usize, + } + + pub(crate) static REGISTRY: Guarded> = Guarded::new(Vec::new()); + + /// Register `vm` and both of its embedded event loops. Called once from + /// `VirtualMachine::init()` after `regular_event_loop`/`macro_event_loop` + /// are initialised. + pub(crate) fn register_vm(vm: *mut VirtualMachine) { + // SAFETY: `vm` is the freshly initialised allocation; `addr_of!` only + // projects field addresses, no reads. + let (regular, macro_) = unsafe { + ( + core::ptr::addr_of!((*vm).regular_event_loop), + core::ptr::addr_of!((*vm).macro_event_loop), + ) + }; + let mut reg = REGISTRY.lock(); + reg.push(Entry { + vm: vm as usize, + loop_: regular as usize, + }); + reg.push(Entry { + vm: vm as usize, + loop_: macro_ as usize, + }); + } + + /// Remove every entry for `vm`. Called from `WebWorker::shutdown()` before + /// anything the VM owns is freed; once this returns, no producer can be + /// inside a checked enqueue targeting `vm` (they would have to re-acquire + /// the lock and re-check). + pub(crate) fn unregister_vm(vm: *mut VirtualMachine) { + REGISTRY.lock().retain(|e| e.vm != vm as usize); + } + + /// Register a loop that lives outside the VM allocation (the boxed + /// spawnSync event loop). Removed with `unregister_loop` when the box is + /// freed. + pub(crate) fn register_extra_loop(vm: *mut VirtualMachine, loop_: *mut EventLoop) { + REGISTRY.lock().push(Entry { + vm: vm as usize, + loop_: loop_ as usize, + }); + } + + pub(crate) fn unregister_loop(loop_: *mut EventLoop) { + 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 { + 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 { + ( + core::ptr::addr_of!((*main).regular_event_loop), + core::ptr::addr_of!((*main).macro_event_loop), + ) + }; + core::ptr::eq(loop_, regular.cast_mut()) || core::ptr::eq(loop_, macro_.cast_mut()) + } +} + #[thread_local] pub static IS_BUNDLER_THREAD_FOR_BYTECODE_CACHE: Cell = Cell::new(false); #[thread_local] @@ -2114,6 +2218,11 @@ impl VirtualMachine { let _ = (*regular).tasks.ensure_unused_capacity(64); addr_of_mut!((*vm).event_loop).write(regular); + // Make this VM reachable for checked cross-thread enqueues. + // Worker VMs are unregistered in `WebWorker::shutdown()` before + // the allocation is freed; the main VM stays registered forever. + live_vm_registry::register_vm(vm); + // `source_mappings.map` is a sibling-field backref onto // `saved_source_map_table`. addr_of_mut!((*vm).saved_source_map_table) @@ -3571,6 +3680,86 @@ 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. + /// + /// 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` + /// (called before any free) also takes — so the VM cannot be freed while + /// `f` runs. `f` must therefore be short and lock-free: pushing to the + /// MPSC queue, `wakeup()`, reading a flag. + pub fn with_live_vm( + 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 })) + } + + /// 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`]. + /// + /// 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 + /// intentionally not touched — equivalent to a task left undrained in a + /// 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( + vm: *mut VirtualMachine, + task: core::ptr::NonNull, + ) -> bool { + match Self::with_live_vm(vm, |vm| { + vm.event_loop_shared().enqueue_task_concurrent(task); + }) { + Some(()) => true, + None => { + crate::event_loop::discard_unqueued_concurrent_task(task); + false + } + } + } + + /// 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::with_live_vm(vm, |vm| vm.is_shutting_down()).unwrap_or(true) + } + + /// `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()); + } + + pub fn try_unref_concurrently(vm: *mut VirtualMachine) { + let _ = Self::with_live_vm(vm, |vm| vm.event_loop_shared().unref_concurrently()); + } + /// `cond` is `&Cell` (not `&mut bool`): the re-entrant /// `tick()/auto_tick()` calls run JS that flips the flag through an /// independently-captured handle, so the read must not be `noalias`. diff --git a/src/jsc/WorkTask.rs b/src/jsc/WorkTask.rs index aa90ad9373a0..6f5cffb9a6f3 100644 --- a/src/jsc/WorkTask.rs +++ b/src/jsc/WorkTask.rs @@ -136,7 +136,9 @@ impl WorkTask { .from(this_ptr, AutoDeinit::ManualDeinit), ); // `task` is the inline `concurrent_task` field of the live - // heap-allocated `*this`; `event_loop` is the JS-thread loop stored at init. - event_loop.enqueue_task_concurrent(task); + // heap-allocated `*this`; `event_loop` was stored at init and 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); } } diff --git a/src/jsc/any_task_job.rs b/src/jsc/any_task_job.rs index 870232d0fae8..41b5ccbaf461 100644 --- a/src/jsc/any_task_job.rs +++ b/src/jsc/any_task_job.rs @@ -49,6 +49,10 @@ pub trait AnyTaskJobCtx: Sized { /// e.g. a `JSPromiseStrong` field after scheduling. pub struct AnyTaskJob { vm: bun_ptr::BackRef, + /// 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. + global: *mut JSGlobalObject, task: WorkPoolTask, any_task: AnyTask, poll: KeepAlive, @@ -75,6 +79,7 @@ impl AnyTaskJob { let vm = bun_ptr::BackRef::new(global.bun_vm()); let job = bun_core::heap::into_raw(Box::new(Self { vm, + global: core::ptr::from_ref(global).cast_mut(), task: WorkPoolTask { node: Default::default(), callback: Self::run_task, @@ -139,12 +144,15 @@ impl AnyTaskJob { // in `create`; `task` points to `Self.task` and the job is live until // `run_from_js` reclaims it. let job = unsafe { &mut *Self::from_task_ptr(task) }; - let vm = job.vm; - job.ctx.run(vm.global); - // `ConcurrentTask::create` heap-allocates a fresh task; the queue takes - // ownership of it. - vm.event_loop_shared() - .enqueue_task_concurrent(ConcurrentTask::create(job.any_task.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. + let _ = VirtualMachine::try_enqueue_task_concurrent( + job.vm.as_ptr(), + ConcurrentTask::create(job.any_task.task()), + ); } /// `AnyTask` callback — runs ON the JS thread. Reclaims the heap diff --git a/src/jsc/event_loop.rs b/src/jsc/event_loop.rs index eefced1e6374..1227b4e5f354 100644 --- a/src/jsc/event_loop.rs +++ b/src/jsc/event_loop.rs @@ -1049,6 +1049,43 @@ impl EventLoop { self.wakeup(); } + /// Loop-pointer-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. + /// + /// 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). + pub fn try_enqueue_task_concurrent( + loop_: *mut EventLoop, + task: core::ptr::NonNull, + ) -> bool { + use crate::virtual_machine::live_vm_registry; + if loop_.is_null() { + 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_) { + // SAFETY: main-thread VM loop, never freed; `enqueue_task_concurrent` + // takes `&self` and is thread-safe. + unsafe { (*loop_).enqueue_task_concurrent(task) }; + return true; + } + let reg = live_vm_registry::REGISTRY.lock(); + if !reg.iter().any(|e| e.loop_ == loop_ as usize) { + 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) }; + true + } + pub fn ref_concurrently(&self) { let _ = self.concurrent_ref.fetch_add(1, Ordering::SeqCst); self.wakeup(); @@ -1223,6 +1260,25 @@ impl EventLoop { } } +/// Free a `ConcurrentTaskItem` that was built for a cross-thread enqueue +/// whose target turned out to be freed (terminated worker). The node was +/// never linked into any queue, so the producer still owns it: `auto_delete` +/// nodes are reclaimed here, struct-embedded nodes stay with their owner. +/// The task payload is deliberately not run or freed — dispatching it would +/// touch the dead VM, and its tag-specific teardown can only run on the +/// (gone) JS thread. This matches the fate of tasks that made it into the +/// queue moments earlier: a terminated worker's queue is never drained. +pub fn discard_unqueued_concurrent_task(task: core::ptr::NonNull) { + // SAFETY: never linked (see doc); `auto_delete` nodes come from + // `ConcurrentTask::new` (`heap::into_raw`), so reclaiming the box here is + // the producer exercising its ownership. + unsafe { + if task.as_ref().auto_delete() { + drop(bun_core::heap::take(task.as_ptr())); + } + } +} + /// Testing API to expose event loop state #[bun_jsc::host_fn] pub fn get_active_tasks(global_object: &JSGlobalObject, _frame: &CallFrame) -> JsResult { @@ -1381,7 +1437,10 @@ bun_event_loop::link_impl_JsEventLoop! { enter() => (*this).enter(), exit() => (*this).exit(), enqueue_task(task) => (*this).enqueue_task(task), - enqueue_task_concurrent(task) => (*this).enqueue_task_concurrent(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); }, 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() => @@ -1431,11 +1490,17 @@ pub(crate) fn __bun_spawn_sync_create_event_loop(vm: *mut (), uws_loop: *mut uws { let _ = uws_loop; } - bun_core::heap::into_raw(el).cast() + let raw = bun_core::heap::into_raw(el); + // The boxed spawnSync loop is a cross-thread enqueue target (e.g. the + // POSIX waiter thread posts exit events to it) but lives outside the VM + // allocation — register it so checked enqueues can find it. + crate::virtual_machine::live_vm_registry::register_extra_loop(std::ptr::from_mut(vm), raw); + raw.cast() } #[unsafe(no_mangle)] pub(crate) fn __bun_spawn_sync_destroy_event_loop(el: *mut ()) { + crate::virtual_machine::live_vm_registry::unregister_loop(el.cast::()); // SAFETY: paired with `heap::alloc` in `__bun_spawn_sync_create_event_loop`. drop(unsafe { bun_core::heap::take(el.cast::()) }); } diff --git a/src/jsc/web_worker.rs b/src/jsc/web_worker.rs index ceb045b8f5e6..845c5a1c3f69 100644 --- a/src/jsc/web_worker.rs +++ b/src/jsc/web_worker.rs @@ -1207,6 +1207,15 @@ impl WebWorker { // vm_lock held; this is the unpublish point. let vm_ptr = self.vm.replace(core::ptr::null_mut()); self.vm_lock.unlock(); + // Unregister from the live-VM registry so checked cross-thread + // producers (HTTP client thread, work pool, watcher threads) observe + // the VM as gone instead of pushing into memory freed in step 5. Must + // precede every free below; a producer inside a checked enqueue holds + // the registry lock, so returning from this call also means no such + // producer is still touching the VM. + if !vm_ptr.is_null() { + crate::virtual_machine::live_vm_registry::unregister_vm(vm_ptr); + } let mut loop_: Option<*mut bun_uws::Loop> = None; if !vm_ptr.is_null() { // SAFETY: vm_ptr was published under vm_lock; sole owner now. diff --git a/src/runtime/api/Archive.rs b/src/runtime/api/Archive.rs index f51fc4b3f2b6..cd444693e721 100644 --- a/src/runtime/api/Archive.rs +++ b/src/runtime/api/Archive.rs @@ -729,12 +729,16 @@ 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() }; - // SAFETY: vm points to the live owning VM; concurrent_task is intrusive on the same allocation. + // `vm` was captured on the JS thread at `create` and may point at a + // worker VM freed by terminate() while the pool task ran — checked + // enqueue only. + // SAFETY: `this` is the live pool-owned allocation; `concurrent_task` + // is intrusive on it. unsafe { let ct = core::ptr::NonNull::from( (*this).concurrent_task.from(this, AutoDeinit::ManualDeinit), ); - (*(*this).vm).enqueue_task_concurrent(ct); + let _ = VirtualMachine::try_enqueue_task_concurrent((*this).vm, ct); } } diff --git a/src/runtime/api/js_bundle_completion_task.rs b/src/runtime/api/js_bundle_completion_task.rs index eb829f098247..56094492aaa9 100644 --- a/src/runtime/api/js_bundle_completion_task.rs +++ b/src/runtime/api/js_bundle_completion_task.rs @@ -784,14 +784,15 @@ fn from_completion_handle<'a>(c: NonNull) -> &'a JSBundleCo static COMPLETION_VTABLE: dispatch::CompletionDispatch = dispatch::CompletionDispatch { result_is_err: |c| matches!(from_completion_handle(c).result, BundleV2Result::Err(_)), enqueue_task_concurrent: |c, task| { - // `jsc_event_loop` is a `BackRef` — safe Deref. + // Checked: runs on the bundle thread; the Bun.build caller's VM may + // be a worker freed by terminate() while the bundle ran. // SAFETY: `task` is a fresh heap-allocated non-null `ConcurrentTaskItem` // passed through from the bundler vtable; the queue takes ownership. - unsafe { - from_completion_handle(c) - .jsc_event_loop - .enqueue_task_concurrent(core::ptr::NonNull::new_unchecked(task)) - } + let _ = jsc::event_loop::EventLoop::try_enqueue_task_concurrent( + from_completion_handle(c).jsc_event_loop.as_ptr(), + // SAFETY: non-null per the vtable contract above. + unsafe { core::ptr::NonNull::new_unchecked(task) }, + ); }, }; @@ -989,11 +990,14 @@ impl CompletionStruct for JSBundleCompletionTask { } fn complete_on_bundle_thread(&mut self) { - // `jsc_event_loop` is a `BackRef` — safe Deref. - // `ConcurrentTask::create` heap-allocates a fresh task; the - // queue takes ownership of it. - self.jsc_event_loop - .enqueue_task_concurrent(jsc::ConcurrentTask::create(self.task.task())); + // Checked: runs on the bundle thread; the Bun.build caller's VM may + // be a worker freed by terminate() while the bundle ran. + // `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(), + jsc::ConcurrentTask::create(self.task.task()), + ); } fn set_result(&mut self, result: BundleV2Result) { self.result = result; diff --git a/src/runtime/bake/dev_server/mod.rs b/src/runtime/bake/dev_server/mod.rs index b4037a778e62..0648fd3ffbe4 100644 --- a/src/runtime/bake/dev_server/mod.rs +++ b/src/runtime/bake/dev_server/mod.rs @@ -956,13 +956,15 @@ impl WatcherAtomics { task: bun_event_loop::Task::init(ev), ..Default::default() }; - // SAFETY: `owner` BACKREF is valid; `vm` is a `BackRef` (safe - // Deref); `event_loop` points at a sibling field of `VirtualMachine`. - unsafe { - (*(&(*ev_ref.owner).vm).event_loop).enqueue_task_concurrent( - core::ptr::NonNull::from(&mut ev_ref.concurrent_task), - ); - } + // Checked: runs on the watcher thread; the owning VM could be + // 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 _ = bun_jsc::virtual_machine::VirtualMachine::try_enqueue_task_concurrent( + vm_ptr, + core::ptr::NonNull::from(&mut ev_ref.concurrent_task), + ); } NextEvent::WAITING => { diff --git a/src/runtime/crypto/PasswordObject.rs b/src/runtime/crypto/PasswordObject.rs index 950cc59d114f..d6857add0203 100644 --- a/src/runtime/crypto/PasswordObject.rs +++ b/src/runtime/crypto/PasswordObject.rs @@ -610,14 +610,14 @@ impl PasswordJob { unsafe { (*result).task = AnyTask::from_typed(result, PasswordResult::::run_from_js_erased); } - // SAFETY: `event_loop` was stored from the JS-thread VM and outlives the - // job; ownership of `result` transfers to the event loop here. `task` is - // an intrusive field at a stable address. - unsafe { - (*self.event_loop).enqueue_task_concurrent(ConcurrentTask::create_from( - core::ptr::addr_of_mut!((*result).task), - )); - } + // `event_loop` was stored from the JS-thread VM at schedule time and + // may point into a worker VM freed by terminate() while the pool task + // ran — checked enqueue only. `task` is an intrusive field at a + // stable address; on a dead VM the result box leaks (same as a task + // left undrained in the dead worker's queue). + // SAFETY: `result` is the live heap allocation built above. + let ct = ConcurrentTask::create_from(unsafe { core::ptr::addr_of_mut!((*result).task) }); + let _ = EventLoop::try_enqueue_task_concurrent(self.event_loop, ct); // `self: Box` drops here; Drop runs secure_zero on password (+op). } } diff --git a/src/runtime/napi/napi_body.rs b/src/runtime/napi/napi_body.rs index 600d9dcfac17..eb1260d84c9a 100644 --- a/src/runtime/napi/napi_body.rs +++ b/src/runtime/napi/napi_body.rs @@ -1846,12 +1846,16 @@ impl napi_async_work { ) { if state == AsyncWorkStatus::Cancelled as u32 { // `concurrent_task` is the live inline field of this heap work; - // the queue takes ownership of its `next` link. - self.event_loop - .enqueue_task_concurrent(core::ptr::NonNull::from( + // the queue takes ownership of its `next` link. Checked: the + // 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(), + core::ptr::NonNull::from( self.concurrent_task .from(self_ptr, AutoDeinit::ManualDeinit), - )); + ), + ); return; } } @@ -1860,12 +1864,15 @@ impl napi_async_work { .store(AsyncWorkStatus::Completed as u32, Ordering::SeqCst); // `concurrent_task` is the live inline field of this heap work; the - // queue takes ownership of its `next` link. - self.event_loop - .enqueue_task_concurrent(core::ptr::NonNull::from( + // 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(), + core::ptr::NonNull::from( self.concurrent_task .from(self_ptr, AutoDeinit::ManualDeinit), - )); + ), + ); } pub fn cancel(&mut self) -> bool { @@ -2708,8 +2715,12 @@ impl ThreadSafeFunction { match prev { x if x == DispatchState::Idle as u8 => { let self_ptr: *mut Self = self; - self.event_loop - .enqueue_task_concurrent(ConcurrentTask::create_from(self_ptr)); + // 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(), + ConcurrentTask::create_from(self_ptr), + ); } x if x == DispatchState::Running as u8 => { // it will check if it has more work to do @@ -4256,10 +4267,15 @@ impl NapiFinalizerTask { let is_main_thread = VirtualMachine::get_or_null().is_some(); if !is_main_thread { - // TODO(@heimskr): do we need to handle the case where the vm is shutting down? let this = bun_core::heap::into_raw(self); - vm.event_loop_ref() - .enqueue_task_concurrent(ConcurrentTask::create(Task::init(this))); + // 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( + core::ptr::from_ref(vm).cast_mut(), + ConcurrentTask::create(Task::init(this)), + ); return; } diff --git a/src/runtime/node/node_fs.rs b/src/runtime/node/node_fs.rs index 09caf024833b..56915a0ad27b 100644 --- a/src/runtime/node/node_fs.rs +++ b/src/runtime/node/node_fs.rs @@ -1249,6 +1249,10 @@ mod _async_tasks { /// Wrapped in [`ThreadSafe`] so the paired `unprotect()` runs on drop. pub args: ThreadSafe, pub global_object: bun_ptr::BackRef, + /// 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 task: WorkPoolTask, pub result: Maybe, pub r#ref: KeepAlive, @@ -1293,6 +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), task: work_pool_task(Self::work_pool_callback), r#ref: KeepAlive::default(), tracker: AsyncTaskTracker::init(vm), @@ -1316,17 +1321,13 @@ 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. - // `bun_vm_concurrently()` skips the JS-thread debug assert and is the - // documented accessor for off-thread (work-pool) callers; the - // event-loop's concurrent queue is MPSC-safe. - let vm = this.global_object().bun_vm_concurrently(); - // SAFETY: VirtualMachine and its event loop are process-static - // (LIFETIMES.tsv); the concurrent queue is MPSC-safe. - unsafe { - (*(*vm).event_loop()).enqueue_task_concurrent(ConcurrentTask::create_from( - std::ptr::from_mut::(this), - )); - } + // `this.vm` was captured at `create` and may point at 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( + this.vm, + ConcurrentTask::create_from(std::ptr::from_mut::(this)), + ); } pub fn run_from_js_thread(&mut self) -> Result<(), bun_jsc::JsTerminated> { @@ -2183,6 +2184,10 @@ mod _async_tasks { /// Wrapped in [`ThreadSafe`] so the paired `unprotect()` runs on drop. pub args: ThreadSafe, pub global_object: bun_ptr::BackRef, + /// 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 task: WorkPoolTask, pub r#ref: KeepAlive, pub tracker: AsyncTaskTracker, @@ -2378,6 +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), task: work_pool_task(Self::work_pool_callback), r#ref: KeepAlive::default(), tracker: AsyncTaskTracker::init(vm), @@ -2553,16 +2559,15 @@ mod _async_tasks { } } - // `bun_vm_concurrently()` skips the JS-thread debug assert and is the - // documented accessor for off-thread (work-pool) callers. - // SAFETY: `bun_vm_concurrently()` returns the process-singleton VM; - // sole `&mut` borrow at this point on the work-pool thread. - let vm = unsafe { &mut *self.global_object().bun_vm_concurrently() }; - // `ConcurrentTask::create` heap-allocates a fresh task; the - // queue takes ownership of it. - vm.enqueue_task_concurrent(ConcurrentTask::create(Task::init(std::ptr::from_mut::< - Self, - >(self)))); + // `self.vm` was captured at `create` and may point at 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 + // takes ownership of it (or frees it when the VM is gone). + let _ = VirtualMachine::try_enqueue_task_concurrent( + self.vm, + ConcurrentTask::create(Task::init(std::ptr::from_mut::(self))), + ); } fn clear_result_list(&mut self) { diff --git a/src/runtime/node/node_fs_stat_watcher.rs b/src/runtime/node/node_fs_stat_watcher.rs index c0cb7885dbe1..6b85dd39f564 100644 --- a/src/runtime/node/node_fs_stat_watcher.rs +++ b/src/runtime/node/node_fs_stat_watcher.rs @@ -330,12 +330,12 @@ impl StatWatcherScheduler { ctx: core::ptr::NonNull::new(holder_ptr.cast()), callback: update_timer, }; - (*this) - .vm - .event_loop_shared() - .enqueue_task_concurrent(ConcurrentTask::create(Task::init( - core::ptr::addr_of_mut!((*holder_ptr).task), - ))); + // Checked: runs on the work-pool thread; `vm` may be a worker VM + // freed by terminate() while the restat ran. + let _ = VirtualMachine::try_enqueue_task_concurrent( + (*this).vm.as_ptr(), + ConcurrentTask::create(Task::init(core::ptr::addr_of_mut!((*holder_ptr).task))), + ); } } @@ -680,7 +680,9 @@ impl StatWatcher { &self, task: NonNull, ) { - self.ctx.event_loop_shared().enqueue_task_concurrent(task); + // 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); } /// Copy the last stat by value. diff --git a/src/runtime/node/node_fs_watcher.rs b/src/runtime/node/node_fs_watcher.rs index 0b30191b0395..54754a022e3a 100644 --- a/src/runtime/node/node_fs_watcher.rs +++ b/src/runtime/node/node_fs_watcher.rs @@ -98,11 +98,9 @@ impl FSWatcher { /// the caller releases ownership of; the concurrent queue takes ownership /// and frees it on the JS thread after dispatch. pub fn enqueue_task_concurrent(&self, task: core::ptr::NonNull) { - // `vm()` is the BACKREF accessor; `event_loop_shared()` is the audited - // safe `&EventLoop` accessor. `enqueue_task_concurrent` is the - // documented cross-thread entry point and only touches the lock-free - // queue. - self.vm().event_loop_shared().enqueue_task_concurrent(task); + // Called from watcher threads: `ctx` may point at a worker VM freed + // by terminate() while an event was in flight — checked enqueue only. + let _ = VirtualMachine::try_enqueue_task_concurrent(self.ctx, task); } /// `self`'s address as `*mut Self` for path-watcher / abort-signal / diff --git a/src/runtime/node/node_zlib_binding.rs b/src/runtime/node/node_zlib_binding.rs index 63b7c71be943..d4281906ae75 100644 --- a/src/runtime/node/node_zlib_binding.rs +++ b/src/runtime/node/node_zlib_binding.rs @@ -249,6 +249,10 @@ 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>; fn this_value(&self) -> &JsCell; fn task(&self) -> &JsCell; fn write_in_progress(&self) -> &Cell; @@ -469,25 +473,21 @@ impl CompressionStream { // `ref_()` in `write()`); bodies use the `&self` accessor surface // (R-2). `ParentRef` Deref collapses the per-site raw deref. let this_ref = ParentRef::from(NonNull::new(this).expect("async_job_run: this")); - let global_this: &JSGlobalObject = this_ref.global_this(); - // `bun_vm_concurrently()` is the thread-safe accessor (skips the - // JS-thread debug assert; same backing pointer as `bun_vm()`). - // BACKREF — `bun_vm_concurrently()` never returns null for a Bun-owned - // global; wrap once so the `event_loop()` read below is safe Deref. - let vm = ParentRef::from( - NonNull::new(global_this.bun_vm_concurrently()).expect("bun_vm_concurrently"), - ); this_ref.stream().with_mut(|s| s.do_work()); - // SAFETY: `event_loop()` is a self-pointer into a live VM; the - // `enqueue_task_concurrent` body only touches the lock-free - // `concurrent_tasks` queue (thread-safe). `this` is the heap-allocated - // `m_ctx` payload — the matching `ref()` in `write()` keeps it alive - // until `run_from_js_thread` runs and calls `deref()`. - unsafe { - (*vm.event_loop()).enqueue_task_concurrent(ConcurrentTask::create(Task::init(this))); - } + // The owning VM was captured at construction and may be a worker VM + // freed by terminate() while this job ran — checked enqueue only, and + // no reads through `global_this` on this thread. `this` is the + // heap-allocated `m_ctx` payload — the matching `ref()` in `write()` + // keeps it alive until `run_from_js_thread` runs and calls `deref()`. + // Shared read of the init-immutable `vm` slot; happens-before via + // `WorkPool::schedule` (same contract as the `task`/`stream` cells). + let vm = *this_ref.vm().get(); + let _ = VirtualMachine::try_enqueue_task_concurrent( + vm, + ConcurrentTask::create(Task::init(this)), + ); } /// Dispatched from `dispatch.rs` when the worker-thread `do_work()` posts @@ -1008,6 +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 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 9aee4a90d5dd..1d880bd88fe4 100644 --- a/src/runtime/node/zlib/NativeBrotli.rs +++ b/src/runtime/node/zlib/NativeBrotli.rs @@ -85,6 +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>, /// 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 @@ -140,6 +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()), 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 f452a45204ed..d39f24be4b50 100644 --- a/src/runtime/node/zlib/NativeZlib.rs +++ b/src/runtime/node/zlib/NativeZlib.rs @@ -50,6 +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>, } // write / runFromJSThread / writeSync / reset / close / setOnError / getOnError / @@ -94,6 +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()), 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 2e6227071738..ba91097f4576 100644 --- a/src/runtime/node/zlib/NativeZstd.rs +++ b/src/runtime/node/zlib/NativeZstd.rs @@ -49,6 +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>, /// 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 @@ -106,6 +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()), stream: JsCell::new(stream), poll_ref: JsCell::new(CountedKeepAlive::default()), this_value: JsCell::new(StrongOptional::empty()), diff --git a/src/runtime/shell/shell_body.rs b/src/runtime/shell/shell_body.rs index 858fb9769f0a..bbc72d130c07 100644 --- a/src/runtime/shell/shell_body.rs +++ b/src/runtime/shell/shell_body.rs @@ -279,16 +279,16 @@ impl<'a> GlobalJS<'a> { #[inline] pub fn enqueue_task_concurrent_wait_pid(self, task: *mut T) { - // SAFETY: bun_vm_concurrently() returns a valid &VirtualMachine; we need &mut for the - // intrusive concurrent queue push (which is itself thread-safe). The VM outlives the call. 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)); - // SAFETY: see above — `vm` is a live VM pointer. - unsafe { &mut *vm }.enqueue_task_concurrent(concurrent); + // 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] diff --git a/src/runtime/webcore/blob/copy_file.rs b/src/runtime/webcore/blob/copy_file.rs index 831c84a0bee4..6492fba1a57b 100644 --- a/src/runtime/webcore/blob/copy_file.rs +++ b/src/runtime/webcore/blob/copy_file.rs @@ -1831,10 +1831,15 @@ fn on_mkdirp_complete_concurrent(ctx: *mut (), err_: bun_sys::Maybe<()>) { unsafe { (*this).on_mkdirp_complete() }; Ok(()) } - this.event_loop - .enqueue_task_concurrent(jsc::ConcurrentTask::create( - jsc::ManagedTask::ManagedTask::new::(this, call_erased), - )); + // 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( + core::ptr::from_ref(this.event_loop).cast_mut(), + 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 f49196680e5a..cb2ac64f6ff8 100644 --- a/src/runtime/webcore/blob/write_file.rs +++ b/src/runtime/webcore/blob/write_file.rs @@ -1029,12 +1029,15 @@ mod windows_impl { bun_sys::Result::Err(e) => Some(e), bun_sys::Result::Ok(()) => None, }; - // SAFETY: event_loop is the VM-owned EventLoop with process lifetime. - unsafe { - (*this.event_loop).enqueue_task_concurrent(ConcurrentTask::create( - ManagedTask::new::(this, Self::on_mkdirp_complete_task), - )); - } + // 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, + ConcurrentTask::create(ManagedTask::new::( + this, + Self::on_mkdirp_complete_task, + )), + ); } extern "C" fn on_write_complete(req: *mut uv::fs_t) { diff --git a/src/runtime/webcore/fetch/FetchTasklet.rs b/src/runtime/webcore/fetch/FetchTasklet.rs index dbd9ba71d0d1..27a793cfedc5 100644 --- a/src/runtime/webcore/fetch/FetchTasklet.rs +++ b/src/runtime/webcore/fetch/FetchTasklet.rs @@ -294,19 +294,6 @@ impl FetchTasklet { unsafe { &*this } } - /// Enqueue a concurrent task on the JS-thread event loop. - /// - /// Centralises the `(*vm.event_loop()).enqueue_task_concurrent(..)` raw - /// deref. `event_loop()` returns a self-ptr into the VirtualMachine that - /// is valid for the VM's lifetime; `enqueue_task_concurrent` takes `&self` - /// and is thread-safe (lock-free MPSC push). `task` is a live - /// `ConcurrentTaskItem` that the queue takes ownership of via its - /// intrusive `next` link. - #[inline] - fn enqueue_concurrent(vm: &VirtualMachine, task: core::ptr::NonNull) { - vm.event_loop_shared().enqueue_task_concurrent(task); - } - /// Wrap a borrowed body chunk in a `StreamResult::Temporary*` for /// synchronous delivery to `ByteStream::on_data`. /// @@ -392,23 +379,29 @@ impl FetchTasklet { return; } let self_ = Self::from_raw_ref(this); - if self_.javascript_vm.is_shutting_down() { - // SAFETY: last ref; exclusive access. `deinit()` would run - // `clear_data()` + `Drop` for the JSC `Strong`/`Weak` fields, which - // reach into the VM's HandleSet from this (HTTP) thread — not - // thread-safe. Reclaim only the Rust-side boxes; the HandleSet is - // freed wholesale by `destructOnExit`. - unsafe { FetchTasklet::dealloc_for_shutdown(this) }; - return; + // `javascript_vm` may point at 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 = core::ptr::from_ref(self_.javascript_vm).cast_mut(); + if !VirtualMachine::is_shutting_down_or_freed(vm_ptr) { + // this is really unlikely to happen, but can happen + // lets make sure that we always call deinit from main thread + // `from_callback` heap-allocates a fresh `ConcurrentTaskItem`; the + // queue takes ownership of it (freed by the checked enqueue when + // the VM died between the check above and the push). + if VirtualMachine::try_enqueue_task_concurrent( + vm_ptr, + ConcurrentTask::from_callback(this, FetchTasklet::deinit_callback), + ) { + return; + } } - // this is really unlikely to happen, but can happen - // lets make sure that we always call deinit from main thread - // `from_callback` heap-allocates a fresh `ConcurrentTaskItem`; the queue - // takes ownership of it. - Self::enqueue_concurrent( - self_.javascript_vm, - ConcurrentTask::from_callback(this, FetchTasklet::deinit_callback), - ); + // SAFETY: last ref; exclusive access. `deinit()` would run + // `clear_data()` + `Drop` for the JSC `Strong`/`Weak` fields, which + // reach into the VM's HandleSet from this (HTTP) thread — not + // thread-safe. Reclaim only the Rust-side boxes; the HandleSet is + // freed wholesale by `destructOnExit`. + unsafe { FetchTasklet::dealloc_for_shutdown(this) }; } // ConcurrentTask::from_callback takes `fn(*mut T) -> bun_event_loop::JsResult<()>` @@ -1982,17 +1975,23 @@ impl FetchTasklet { /// This is ALWAYS called from the http thread and we cannot touch the buffer here because is locked pub(crate) fn on_write_request_data_drain(this: *mut FetchTasklet) { let this_ref = Self::from_raw_ref(this); - if this_ref.javascript_vm.is_shutting_down() { + // Checked: the fetching VM may be a worker freed by terminate(). + let vm_ptr = core::ptr::from_ref(this_ref.javascript_vm).cast_mut(); + if VirtualMachine::is_shutting_down_or_freed(vm_ptr) { return; } // ref until the main thread callback is called this_ref.ref_(); // `from_callback` heap-allocates a fresh `ConcurrentTaskItem`; the queue // takes ownership of it. - Self::enqueue_concurrent( - this_ref.javascript_vm, + if !VirtualMachine::try_enqueue_task_concurrent( + vm_ptr, ConcurrentTask::from_callback(this, FetchTasklet::resume_request_data_stream), - ); + ) { + // VM died between the check and the push; the callback will never + // run, so balance the ref taken above. + FetchTasklet::deref_from_thread(this); + } } /// This is ALWAYS called from the main thread @@ -2282,7 +2281,21 @@ impl FetchTasklet { } } // will deinit when done with the http client (when is_done = true) - if task_ref.javascript_vm.is_shutting_down() { + // Checked accessors only: the fetching VM may be a worker freed by + // terminate() while this request was in flight. + let vm_ptr = core::ptr::from_ref(task_ref.javascript_vm).cast_mut(); + let queued = !VirtualMachine::is_shutting_down_or_freed(vm_ptr) && { + // `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. + let ct = core::ptr::NonNull::from( + task_ref + .concurrent_task + .from(task, AutoDeinit::ManualDeinit), + ); + VirtualMachine::try_enqueue_task_concurrent(vm_ptr, ct) + }; + if !queued { // VM teardown: the JS-thread side will never drain this buffer (its // on_progress_update bails the same way), so free the body bytes now. task_ref.scheduled_response_buffer = MutableString::default(); @@ -2294,8 +2307,8 @@ impl FetchTasklet { http::http_thread().schedule_shutdown(http_); } } - // We won the `has_schedule_callback` CAS above but are not - // enqueueing the on_progress_update task; undo the flag so a later + // We won the `has_schedule_callback` CAS above but did not + // enqueue the on_progress_update task; undo the flag so a later // (final) callback can re-enter this branch instead of taking the // already-scheduled early return. task_ref @@ -2314,14 +2327,6 @@ impl FetchTasklet { } return; } - let ct = core::ptr::NonNull::from( - task_ref - .concurrent_task - .from(task, AutoDeinit::ManualDeinit), - ); - // `ct` is the inline `concurrent_task` field of the heap tasklet; the - // queue takes ownership of its `next` link. - Self::enqueue_concurrent(task_ref.javascript_vm, ct); task_ref.mutex.unlock(); // we are done with the http client so we can deref our side diff --git a/src/runtime/webcore/s3/download_stream.rs b/src/runtime/webcore/s3/download_stream.rs index 17e6b5b33aa4..eae3304d654a 100644 --- a/src/runtime/webcore/s3/download_stream.rs +++ b/src/runtime/webcore/s3/download_stream.rs @@ -341,15 +341,15 @@ impl S3HttpDownloadStreamingTask { let task = core::ptr::NonNull::from( self_.concurrent_task.from(this, AutoDeinit::ManualDeinit), ); - // `vm` is the live per-thread VM BackRef captured at task creation; event_loop - // is initialized for the request's lifetime and enqueue is thread-safe (`&self`). - // `task` is the inline `concurrent_task` field of this heap request; - // the queue takes ownership of its `next` link. - self_ - .vm - .expect("vm set at task creation") - .event_loop_shared() - .enqueue_task_concurrent(task); + // `vm` was captured at task creation and may point at 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(), + task, + ); } } } diff --git a/src/runtime/webcore/s3/simple_request.rs b/src/runtime/webcore/s3/simple_request.rs index 986715dbe109..05046c7a4543 100644 --- a/src/runtime/webcore/s3/simple_request.rs +++ b/src/runtime/webcore/s3/simple_request.rs @@ -480,14 +480,15 @@ impl S3HttpSimpleTask { this.concurrent_task .from(this_ptr, AutoDeinit::ManualDeinit), ); - // `vm` is the live per-thread VM BackRef captured at task creation; event_loop - // is set during VM init and outlives this task. `enqueue_task_concurrent` is `&self`. - // `task` is the inline `concurrent_task` field of this heap request; - // the queue takes ownership of its `next` link. - this.vm - .expect("vm set at task creation") - .event_loop_shared() - .enqueue_task_concurrent(task); + // `vm` was captured at task creation and may point at 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(), + task, + ); } } } diff --git a/test/js/web/workers/worker-terminate-lifetime.test.ts b/test/js/web/workers/worker-terminate-lifetime.test.ts index b938d02fc470..7948effdaddb 100644 --- a/test/js/web/workers/worker-terminate-lifetime.test.ts +++ b/test/js/web/workers/worker-terminate-lifetime.test.ts @@ -119,3 +119,74 @@ test( }, 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 +// freed VM (heap-use-after-free in FetchTasklet::callback → +// VirtualMachine::is_shutting_down) and pushed into the freed concurrent +// queue. The corrupted queue surfaced in the wild as "Panic: invalid enum +// value" in EventLoop.tickQueueWithCount on worker threads. Same class: any +// cross-thread producer (work pool, watcher threads, napi) completing after +// worker.terminate(). ASAN-only: without a sanitizer the stale write is +// silent, so this test can only prove the bug on ASAN builds (the gate and +// the asan CI lanes). +test.skipIf(!isASAN)( + "terminating a worker with a fetch in flight does not touch the freed VM", + async () => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + ` + let releaseResponse = null; + let requestArrived = null; + const server = Bun.serve({ + port: 0, + async fetch(req) { + requestArrived(); + await new Promise(resolve => (releaseResponse = resolve)); + return new Response("x".repeat(1024)); + }, + }); + const url = "http://127.0.0.1:" + server.port + "/"; + const workerCode = + "fetch(" + JSON.stringify(url) + ").then(r => r.text()).catch(() => {});" + + "postMessage('fetching');"; + for (let i = 0; i < 3; i++) { + const arrived = new Promise(resolve => (requestArrived = resolve)); + const worker = new Worker("data:text/javascript," + encodeURIComponent(workerCode)); + await new Promise(resolve => (worker.onmessage = resolve)); + // The request is now in flight on the HTTP client thread. + await arrived; + 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); + // The HTTP thread now delivers the response to the freed VM. + releaseResponse(); + await Bun.sleep(300); + } + // Leave room for an in-progress sanitizer report to abort the process + // before we exit cleanly. + await Bun.sleep(3000); + console.log("done"); + server.stop(true); + `, + ], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect({ stdout, exitCode, signalCode: proc.signalCode }).toEqual({ + stdout: "done\n", + exitCode: 0, + signalCode: null, + }); + void stderr; + }, + timeout, +); From c618e4c4b818c18bd8eb7910180d3376b84ebbfa Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 10 Jun 2026 19:53:11 +0000 Subject: [PATCH 02/17] Allow not_unsafe_ptr_arg_deref on the checked enqueue entry points Accepting a possibly dangling pointer is these functions' contract; no dereference happens until the live-VM registry proves the pointee alive and holds off its free. Same pattern as FetchTasklet::deref_from_thread. --- src/jsc/VirtualMachine.rs | 4 ++++ src/jsc/event_loop.rs | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/src/jsc/VirtualMachine.rs b/src/jsc/VirtualMachine.rs index a745c7bff1fe..ea0bf01782dc 100644 --- a/src/jsc/VirtualMachine.rs +++ b/src/jsc/VirtualMachine.rs @@ -3689,6 +3689,10 @@ impl VirtualMachine { /// (called before any free) also takes — so the VM cannot be freed while /// `f` runs. `f` must therefore be short and lock-free: pushing to the /// MPSC queue, `wakeup()`, reading a flag. + // 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 fn with_live_vm( vm: *mut VirtualMachine, f: impl FnOnce(&VirtualMachine) -> R, diff --git a/src/jsc/event_loop.rs b/src/jsc/event_loop.rs index 1227b4e5f354..6650581d9b4f 100644 --- a/src/jsc/event_loop.rs +++ b/src/jsc/event_loop.rs @@ -1058,6 +1058,10 @@ impl EventLoop { /// 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)] pub fn try_enqueue_task_concurrent( loop_: *mut EventLoop, task: core::ptr::NonNull, From 007f64bbd3754dd039f394ce6edc25c7953292e1 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 10 Jun 2026 20:16:10 +0000 Subject: [PATCH 03/17] Address review: dangling-reference fields, failure-path cleanup, visibility - FetchTasklet.javascript_vm and CopyFileWindows.event_loop become BackRef instead of Rust references (same pattern as the S3 tasks): a worker VM can be freed while these structs are still referenced from the HTTP/pool threads, so holding a reference would dangle. JS-thread paths get() them; cross-thread paths only pass as_ptr() to the checked accessors. Resolves the struct's own TODO in copy_file.rs. - with_live_vm becomes pub(crate): the closure receives &VirtualMachine on a non-JS thread and must stick to the documented thread-safe subset, so don't expose it outside the crate. - StatWatcherScheduler frees its timer-update Holder when the checked enqueue is rejected (plain ParentRef + AnyTask, safe off-thread). - FSWatchTaskPosix::enqueue reclaims the cloned task box and balances the pending-activity ref when the checked enqueue is rejected (both are watcher-thread-safe: plain heap entries, atomic counter). - Document the residual address-reuse (ABA) window on the registry: pre-existing, strictly narrower than the bug this fixes, and closing it needs schedule-time generation tokens in every producer struct (follow-up). - Regression test asserts stderr contains no AddressSanitizer report so failures surface the report text. --- src/jsc/VirtualMachine.rs | 17 ++++- src/runtime/node/node_fs_stat_watcher.rs | 10 ++- src/runtime/node/node_fs_watcher.rs | 21 +++++-- src/runtime/webcore/blob/copy_file.rs | 63 +++++++++---------- src/runtime/webcore/fetch/FetchTasklet.rs | 21 ++++--- .../workers/worker-terminate-lifetime.test.ts | 4 +- 6 files changed, 86 insertions(+), 50 deletions(-) diff --git a/src/jsc/VirtualMachine.rs b/src/jsc/VirtualMachine.rs index ea0bf01782dc..fbdc49334dd0 100644 --- a/src/jsc/VirtualMachine.rs +++ b/src/jsc/VirtualMachine.rs @@ -491,6 +491,16 @@ 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. +/// /// 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 /// no other locks. @@ -3688,12 +3698,15 @@ impl VirtualMachine { /// `f` runs under the [`live_vm_registry`] lock, which `unregister_vm` /// (called before any free) also takes — so the VM cannot be freed while /// `f` runs. `f` must therefore be short and lock-free: pushing to the - /// MPSC queue, `wakeup()`, reading a flag. + /// MPSC queue, `wakeup()`, reading a flag. The `&VirtualMachine` handed to + /// `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`. // 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 fn with_live_vm( + pub(crate) fn with_live_vm( vm: *mut VirtualMachine, f: impl FnOnce(&VirtualMachine) -> R, ) -> Option { diff --git a/src/runtime/node/node_fs_stat_watcher.rs b/src/runtime/node/node_fs_stat_watcher.rs index 6b85dd39f564..dcf5ae13fcb7 100644 --- a/src/runtime/node/node_fs_stat_watcher.rs +++ b/src/runtime/node/node_fs_stat_watcher.rs @@ -331,11 +331,15 @@ impl StatWatcherScheduler { callback: update_timer, }; // Checked: runs on the work-pool thread; `vm` may be a worker VM - // freed by terminate() while the restat ran. - let _ = VirtualMachine::try_enqueue_task_concurrent( + // 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(), ConcurrentTask::create(Task::init(core::ptr::addr_of_mut!((*holder_ptr).task))), - ); + ) { + drop(bun_core::heap::take(holder_ptr)); + } } } diff --git a/src/runtime/node/node_fs_watcher.rs b/src/runtime/node/node_fs_watcher.rs index 54754a022e3a..33d0e6c1a86a 100644 --- a/src/runtime/node/node_fs_watcher.rs +++ b/src/runtime/node/node_fs_watcher.rs @@ -97,10 +97,11 @@ impl FSWatcher { /// `task` must point to a live heap-allocated `ConcurrentTask` node that /// the caller releases ownership of; the concurrent queue takes ownership /// and frees it on the JS thread after dispatch. - pub fn enqueue_task_concurrent(&self, task: core::ptr::NonNull) { + #[must_use] + 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. - let _ = VirtualMachine::try_enqueue_task_concurrent(self.ctx, task); + VirtualMachine::try_enqueue_task_concurrent(self.ctx, task) } /// `self`'s address as `*mut Self` for path-watcher / abort-signal / @@ -228,10 +229,22 @@ impl FSWatchTaskPosix { // until the JS thread drains and `heap::take`s it in `dispatch`. unsafe { (*that).concurrent_task.task = Task::init(that); - self.ctx() + if !self + .ctx() .enqueue_task_concurrent(core::ptr::NonNull::new_unchecked( core::ptr::addr_of_mut!((*that).concurrent_task), - )); + )) + { + // VM gone: `dispatch`/`run` will never consume the clone. + // Reclaim it here (entries own plain heap bytes, no JSC + // state) and balance the `ref_task()` above — both the + // entry frees and `pending_activity_count` are safe from + // this thread (atomic counter, mutex-guarded flag). + let mut clone = bun_core::heap::take(that); + clone.clean_entries(); + drop(clone); + self.ctx().unref_task(); + } } return; } diff --git a/src/runtime/webcore/blob/copy_file.rs b/src/runtime/webcore/blob/copy_file.rs index 6492fba1a57b..db997f21f3c1 100644 --- a/src/runtime/webcore/blob/copy_file.rs +++ b/src/runtime/webcore/blob/copy_file.rs @@ -983,7 +983,7 @@ impl TryWith { // ─────────────────────────────────────────────────────────────────────────── #[cfg(windows)] -pub struct CopyFileWindows<'a> { +pub struct CopyFileWindows { pub destination_file_store: StoreRef, pub source_file_store: StoreRef, @@ -991,10 +991,11 @@ pub struct CopyFileWindows<'a> { pub promise: jsc::JSPromiseStrong, pub mkdirp_if_not_exists: bool, pub destination_mode: Option, - // per LIFETIMES.tsv: JSC_BORROW → &jsc::EventLoop - // TODO(refactor): lifetime — heap-allocated and re-entered from libuv callbacks; - // likely should be *const jsc::EventLoop. - pub event_loop: &'a jsc::event_loop::EventLoop, + // BACKREF (not `&'a`): heap-allocated and re-entered from libuv + // callbacks, and the mkdirp completion can arrive on the work pool after + // 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, pub size: SizeType, @@ -1042,7 +1043,7 @@ impl Default for ReadWriteLoop { // borrow checker can see `self.read_write_loop` / `self.io_request` / `self.event_loop` // as disjoint field accesses through a single `&mut self`. #[cfg(windows)] -impl<'a> CopyFileWindows<'a> { +impl CopyFileWindows { fn read_write_loop_start(&mut self) -> bun_sys::Result<()> { self.read_write_loop.read_buf.reserve_exact(64 * 1024); @@ -1058,7 +1059,7 @@ impl<'a> CopyFileWindows<'a> { base: self.read_write_loop.read_buf.as_mut_ptr(), }; let source_fd = self.read_write_loop.source_fd; - let loop_ = self.event_loop.uv_loop(); + let loop_ = self.event_loop.get().uv_loop(); // This io_request is used for both reading and writing. // For now, we don't start reading the next chunk until @@ -1172,7 +1173,7 @@ extern "C" fn on_read(req: *mut libuv::fs_t) { // `read_buf` (len set above), and `on_write` is a valid `uv_fs_cb`. let rc2 = unsafe { libuv::uv_fs_write( - event_loop.uv_loop(), + event_loop.get().uv_loop(), &mut this.io_request, destination_fd.uv(), core::ptr::from_mut(&mut this.read_write_loop.uv_buf), @@ -1237,7 +1238,7 @@ extern "C" fn on_write(req: *mut libuv::fs_t) { // `on_write` is a valid `uv_fs_cb`. let rc2 = unsafe { libuv::uv_fs_write( - this.event_loop.uv_loop(), + this.event_loop.get().uv_loop(), &mut this.io_request, destination_fd.uv(), core::ptr::from_mut(&mut this.read_write_loop.uv_buf), @@ -1267,9 +1268,9 @@ extern "C" fn on_write(req: *mut libuv::fs_t) { } #[cfg(windows)] -impl<'a> CopyFileWindows<'a> { +impl CopyFileWindows { pub fn on_read_write_loop_complete(&mut self) { - self.event_loop.unref_concurrently(); + self.event_loop.get().unref_concurrently(); if let Some(err) = self.err.take() { self.throw(err); @@ -1280,14 +1281,14 @@ impl<'a> CopyFileWindows<'a> { self.on_complete(written); } - pub fn new(init: CopyFileWindows<'a>) -> Box> { + pub fn new(init: CopyFileWindows) -> Box { Box::new(init) } pub fn init( destination_file_store: StoreRef, source_file_store: StoreRef, - event_loop: &'a jsc::event_loop::EventLoop, + event_loop: &jsc::event_loop::EventLoop, mkdirp_if_not_exists: bool, size_: SizeType, destination_mode: Option, @@ -1300,7 +1301,7 @@ impl<'a> CopyFileWindows<'a> { promise: jsc::JSPromiseStrong::init(global), // SAFETY: all-zero is a valid libuv::fs_t io_request: bun_core::ffi::zeroed::(), - event_loop, + event_loop: bun_ptr::BackRef::new(event_loop), mkdirp_if_not_exists, destination_mode, size: size_, @@ -1401,7 +1402,7 @@ impl<'a> CopyFileWindows<'a> { self.throw(err); } bun_sys::Result::Ok(()) => { - self.event_loop.ref_concurrently(); + self.event_loop.get().ref_concurrently(); } } } @@ -1522,7 +1523,7 @@ impl<'a> CopyFileWindows<'a> { } } }; - let loop_ = self.event_loop.uv_loop(); + let loop_ = self.event_loop.get().uv_loop(); self.io_request.data = this_ptr; // SAFETY: FFI — `loop_` is the live VM uv loop, `io_request` is owned by `self`, @@ -1553,11 +1554,11 @@ impl<'a> CopyFileWindows<'a> { }); return; } - self.event_loop.ref_concurrently(); + self.event_loop.get().ref_concurrently(); } pub fn throw(&mut self, err: bun_sys::Error) { - let global_this = self.event_loop.global_ref(); + let global_this = self.event_loop.get().global_ref(); // `swap()` returns a `&mut JSPromise` into a GC-owned cell (not into // `self`), but its lifetime is elided to `&mut self`. Decay to a raw pointer so // borrowck doesn't tie it to `self` across `destroy` below. @@ -1566,9 +1567,7 @@ impl<'a> CopyFileWindows<'a> { // SAFETY: VM-owned event loop is valid for the process lifetime; `enter_scope` // calls enter() now and exit() on drop. - let _guard = unsafe { - jsc::event_loop::EventLoop::enter_scope(self.event_loop as *const _ as *mut _) - }; + let _guard = unsafe { jsc::event_loop::EventLoop::enter_scope(self.event_loop.as_ptr()) }; // SAFETY: self was heap-allocated in init(); destroy reclaims and drops it. self is not accessed afterward. unsafe { Self::destroy(core::ptr::from_mut(self)) }; // `promise` points to a GC-owned `JSPromise` cell, not into `self`; valid after `destroy`. @@ -1604,7 +1603,7 @@ impl<'a> CopyFileWindows<'a> { .path() .slice_z(&mut pathbuf) .as_ptr(); - let loop_ = self.event_loop.uv_loop(); + let loop_ = self.event_loop.get().uv_loop(); self.io_request.deinit(); // SAFETY: all-zero is a valid libuv::fs_t self.io_request = bun_core::ffi::zeroed::(); @@ -1637,7 +1636,7 @@ impl<'a> CopyFileWindows<'a> { self.throw(err); return; } - self.event_loop.ref_concurrently(); + self.event_loop.get().ref_concurrently(); return; } } @@ -1646,15 +1645,13 @@ impl<'a> CopyFileWindows<'a> { } fn resolve_promise(&mut self, written: usize) { - let global_this = self.event_loop.global_ref(); + let global_this = self.event_loop.get().global_ref(); // see `throw` — re-type the GC cell via the ZST opaque deref so it // outlives `destroy(self)` for borrowck. let promise = JSPromise::opaque_mut(self.promise.swap()); // SAFETY: VM-owned event loop is valid for the process lifetime; `enter_scope` // calls enter() now and exit() on drop. - let _guard = unsafe { - jsc::event_loop::EventLoop::enter_scope(self.event_loop as *const _ as *mut _) - }; + let _guard = unsafe { jsc::event_loop::EventLoop::enter_scope(self.event_loop.as_ptr()) }; // SAFETY: self was heap-allocated in init(); destroy reclaims and drops it. self is not accessed afterward. unsafe { Self::destroy(core::ptr::from_mut(self)) }; @@ -1715,7 +1712,7 @@ impl<'a> CopyFileWindows<'a> { .unwrap_or(path_slice) as *const [u8] }; - self.event_loop.ref_concurrently(); + self.event_loop.get().ref_concurrently(); node_fs::async_::AsyncMkdirp::new(node_fs::async_::AsyncMkdirp { completion: on_mkdirp_complete_concurrent, completion_ctx: core::ptr::from_mut(self).cast::<()>(), @@ -1726,7 +1723,7 @@ impl<'a> CopyFileWindows<'a> { } fn on_mkdirp_complete(&mut self) { - self.event_loop.unref_concurrently(); + self.event_loop.get().unref_concurrently(); if let Some(err) = self.err.take() { // `bun_sys::Error.path` is an owned `Box<[u8]>` and is dropped with @@ -1747,7 +1744,7 @@ extern "C" fn on_copy_file(req: *mut libuv::fs_t) { debug_assert!(core::ptr::addr_of_mut!(this.io_request) == req); let event_loop = this.event_loop; - event_loop.unref_concurrently(); + event_loop.get().unref_concurrently(); let rc = this.io_request.result; bun_sys::syslog!("uv_fs_copyfile() = {}", rc); @@ -1795,7 +1792,7 @@ extern "C" fn on_chmod(req: *mut libuv::fs_t) { debug_assert!(core::ptr::addr_of_mut!(this.io_request) == req); let event_loop = this.event_loop; - event_loop.unref_concurrently(); + event_loop.get().unref_concurrently(); let rc = this.io_request.result; if let Some(errno) = rc.err_enum_e() { @@ -1824,7 +1821,7 @@ fn on_mkdirp_complete_concurrent(ctx: *mut (), err_: bun_sys::Maybe<()>) { }; // `bun_event_loop::JsResult` carries the low-tier `ErasedJsError`; shim the // callback signature to match `ManagedTask::new`'s `fn(*mut T) -> JsResult<()>`. - fn call_erased(this: *mut CopyFileWindows<'_>) -> bun_event_loop::JsResult<()> { + fn call_erased(this: *mut CopyFileWindows) -> bun_event_loop::JsResult<()> { // SAFETY: `this` is the heap-allocated `CopyFileWindows` passed to // `ManagedTask::new` below; `on_mkdirp_complete` may free it via `throw`, so we // do not touch `this` afterward. @@ -1834,7 +1831,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( - core::ptr::from_ref(this.event_loop).cast_mut(), + this.event_loop.as_ptr(), jsc::ConcurrentTask::create(jsc::ManagedTask::ManagedTask::new::( this, call_erased, diff --git a/src/runtime/webcore/fetch/FetchTasklet.rs b/src/runtime/webcore/fetch/FetchTasklet.rs index 27a793cfedc5..a46b3cfac6c8 100644 --- a/src/runtime/webcore/fetch/FetchTasklet.rs +++ b/src/runtime/webcore/fetch/FetchTasklet.rs @@ -65,7 +65,12 @@ pub struct FetchTasklet { pub http: Option>>, pub result: HTTPClientResult<'static>, pub metadata: Option, - pub javascript_vm: &'static VirtualMachine, + /// Owning VM, captured at `get()`. `BackRef` (not `&'static`): 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, pub global_this: GlobalRef, pub request_body: HTTPRequestBody, // ThreadSafeStreamBuffer is intrusively refcounted (`ref_count: AtomicU32`, @@ -382,7 +387,7 @@ impl FetchTasklet { // `javascript_vm` may point at 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 = core::ptr::from_ref(self_.javascript_vm).cast_mut(); + let vm_ptr = self_.javascript_vm.as_ptr(); if !VirtualMachine::is_shutting_down_or_freed(vm_ptr) { // this is really unlikely to happen, but can happen // lets make sure that we always call deinit from main thread @@ -786,9 +791,11 @@ impl FetchTasklet { self.has_schedule_callback.store(false, Ordering::Relaxed); 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; // vm is shutting down we cannot touch JS - if vm.is_shutting_down() { + if vm.get().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. @@ -1652,7 +1659,7 @@ impl FetchTasklet { http_.enable_response_body_streaming(); } // we should not keep the process alive if we are ignoring the body - let _ = self.javascript_vm; + let _ = &self.javascript_vm; self.poll_ref.unref(bun_io::js_vm_ctx()); // clean any remaining references self.clear_stream_cancel_handler(); @@ -1695,7 +1702,7 @@ impl FetchTasklet { ) -> Result<*mut FetchTasklet, BunError> { // SAFETY: bun_vm() returns the FFI `*mut VirtualMachine`; the VM outlives // this tasklet (process-lifetime singleton on the JS thread). - let jsc_vm: &'static VirtualMachine = global_this.bun_vm(); + let jsc_vm = bun_ptr::BackRef::new(global_this.bun_vm()); let mut fetch_tasklet = Box::new(FetchTasklet { sink: None, // `AsyncHTTP` has no `Default`/zero-init; defer the Box until @@ -1976,7 +1983,7 @@ 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 = core::ptr::from_ref(this_ref.javascript_vm).cast_mut(); + let vm_ptr = this_ref.javascript_vm.as_ptr(); if VirtualMachine::is_shutting_down_or_freed(vm_ptr) { return; } @@ -2283,7 +2290,7 @@ 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 = core::ptr::from_ref(task_ref.javascript_vm).cast_mut(); + let vm_ptr = task_ref.javascript_vm.as_ptr(); let queued = !VirtualMachine::is_shutting_down_or_freed(vm_ptr) && { // `ct` is the inline `concurrent_task` field of the heap tasklet; // the queue takes ownership of its `next` link. The embedded node diff --git a/test/js/web/workers/worker-terminate-lifetime.test.ts b/test/js/web/workers/worker-terminate-lifetime.test.ts index 7948effdaddb..8e90d844e94a 100644 --- a/test/js/web/workers/worker-terminate-lifetime.test.ts +++ b/test/js/web/workers/worker-terminate-lifetime.test.ts @@ -181,12 +181,14 @@ test.skipIf(!isASAN)( }); const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + // Check stderr first: on failure the sanitizer report is the useful + // output, and asserting on it surfaces the report text in the diff. + expect(stderr).not.toContain("AddressSanitizer"); expect({ stdout, exitCode, signalCode: proc.signalCode }).toEqual({ stdout: "done\n", exitCode: 0, signalCode: null, }); - void stderr; }, timeout, ); From 64bafc19200ac03162d38d17218224dae71a3d72 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 10 Jun 2026 20:39:34 +0000 Subject: [PATCH 04/17] Don't deinit a dead worker's FetchTasklet against its freed JSC heap When the HTTP thread dropped the last tasklet reference after a worker was terminated, the previous code parked the box via dealloc_for_shutdown, whose global_exit drain runs deinit() on the main thread: that drops the tasklet's Strong/Weak handles into the dead worker VM's freed HandleSet (invisible to ASAN because the JSC heap is bmalloc-backed). Split the two cases with VirtualMachine::live_shutting_down_state: a VM observed registered and shutting down can only be the never-freed main VM (workers unregister before setting the flag), so the park-for-deinit path stays for process exit; a freed worker's tasklet is parked permanently in a reachable static instead, so nothing ever touches its dead JSC state and LSan-enabled CI lanes don't report it. Also rewrites the stale comments flagged in review that still described dealloc_for_shutdown as reclaiming Rust-side boxes on the HTTP thread. --- src/jsc/VirtualMachine.rs | 13 ++++- src/runtime/webcore/fetch/FetchTasklet.rs | 68 ++++++++++++++++------- 2 files changed, 61 insertions(+), 20 deletions(-) diff --git a/src/jsc/VirtualMachine.rs b/src/jsc/VirtualMachine.rs index fbdc49334dd0..38c567f49665 100644 --- a/src/jsc/VirtualMachine.rs +++ b/src/jsc/VirtualMachine.rs @@ -3763,7 +3763,18 @@ impl VirtualMachine { /// `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::with_live_vm(vm, |vm| vm.is_shutting_down()).unwrap_or(true) + Self::live_shutting_down_state(vm).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()) } /// `ref_concurrently`/`unref_concurrently` variants of diff --git a/src/runtime/webcore/fetch/FetchTasklet.rs b/src/runtime/webcore/fetch/FetchTasklet.rs index a46b3cfac6c8..db3d3786a13d 100644 --- a/src/runtime/webcore/fetch/FetchTasklet.rs +++ b/src/runtime/webcore/fetch/FetchTasklet.rs @@ -23,6 +23,16 @@ use bun_jsc::{ }; use bun_sys::FdExt; use bun_threading::Mutex; + +/// `FetchTasklet` boxes whose owning worker VM was freed (terminated) while +/// the HTTP thread held the last reference. They can never be reclaimed: +/// `deinit()` drops JSC `Strong`/`Weak` handles into the dead VM's freed +/// HandleSet, on any thread, at any time — including the `global_exit` drain +/// that `dealloc_for_shutdown` parks into. Kept reachable here so +/// LSan-enabled CI lanes don't report them as leaks; growth is bounded by +/// in-flight requests that lose a terminate race. +static DEAD_VM_TASKLETS: bun_threading::Guarded> = + bun_threading::Guarded::new(Vec::new()); use bun_url::URL as ZigURL; use crate::api::bun_x509 as X509; @@ -388,25 +398,40 @@ impl FetchTasklet { // this request was in flight on the HTTP thread) — only the checked // accessors may touch it. let vm_ptr = self_.javascript_vm.as_ptr(); - if !VirtualMachine::is_shutting_down_or_freed(vm_ptr) { - // this is really unlikely to happen, but can happen - // lets make sure that we always call deinit from main thread - // `from_callback` heap-allocates a fresh `ConcurrentTaskItem`; the - // queue takes ownership of it (freed by the checked enqueue when - // the VM died between the check above and the push). - if VirtualMachine::try_enqueue_task_concurrent( - vm_ptr, - ConcurrentTask::from_callback(this, FetchTasklet::deinit_callback), - ) { + match VirtualMachine::live_shutting_down_state(vm_ptr) { + Some(false) => { + // this is really unlikely to happen, but can happen + // lets make sure that we always call deinit from main thread + // `from_callback` heap-allocates a fresh `ConcurrentTaskItem`; + // the queue takes ownership of it (freed by the checked + // 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, + ConcurrentTask::from_callback(this, FetchTasklet::deinit_callback), + ) { + return; + } + } + Some(true) => { + // Only the never-freed main VM can be observed registered and + // shutting down (process exit) — see + // `live_shutting_down_state`. Park the intact box for the + // `global_exit` drain, which runs `deinit()` on the JS thread + // while the JSC VM is still alive. + // SAFETY: last ref; exclusive access. + unsafe { FetchTasklet::dealloc_for_shutdown(this) }; return; } + None => {} } - // SAFETY: last ref; exclusive access. `deinit()` would run - // `clear_data()` + `Drop` for the JSC `Strong`/`Weak` fields, which - // reach into the VM's HandleSet from this (HTTP) thread — not - // thread-safe. Reclaim only the Rust-side boxes; the HandleSet is - // freed wholesale by `destructOnExit`. - unsafe { FetchTasklet::dealloc_for_shutdown(this) }; + // The owning worker VM is gone. Nothing may ever touch this tasklet's + // JSC state again: `deinit()` (directly or via the global_exit drain) + // would drop the `Strong`/`Weak` fields into the dead VM's freed + // HandleSet. Park the box forever instead — kept reachable so + // LSan-enabled CI lanes don't report it; bounded by requests that + // lose the terminate race. + DEAD_VM_TASKLETS.lock().push(this as usize); } // ConcurrentTask::from_callback takes `fn(*mut T) -> bun_event_loop::JsResult<()>` @@ -503,7 +528,10 @@ impl FetchTasklet { drop(boxed); } - /// Last-ref reclaim from the HTTP thread once the VM has begun shutdown. + /// Last-ref reclaim from the HTTP thread once the (never-freed) owning VM + /// has begun process exit. Not used for freed worker VMs — those park in + /// [`DEAD_VM_TASKLETS`] instead, because the drain below would `deinit()` + /// against a dead JSC heap. /// /// Neither `clear_data()` nor dropping the box is safe here: /// * the JSC `Strong`/`Weak` fields touch the VM's HandleSet/WeakSet on @@ -2325,8 +2353,10 @@ impl FetchTasklet { if is_done { // No on_progress_update will ever run for this final result, so // release the JS-side ref it would have dropped, then the - // HTTP-side ref. The 1→0 transition runs `dealloc_for_shutdown` - // (Rust boxes only — JSC handles are leaked to destructOnExit). + // HTTP-side ref. The 1→0 transition parks the box: for the + // exiting main VM via `dealloc_for_shutdown` (deinit on the JS + // thread from the global_exit drain), for a freed worker VM in + // `DEAD_VM_TASKLETS` (never touched again). // SAFETY: `task` is the live heap tasklet; both refs held. FetchTasklet::deref_from_thread(task); // SAFETY: second ref still held until this 1→0 transition. From 575acf4e5c2d019bdbc6354f696cd1155e913278 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 10 Jun 2026 21:26:47 +0000 Subject: [PATCH 05/17] Make cross-thread VM field reads atomic and shrink dead-VM tasklet parking - VirtualMachine::init now registers in the live-VM registry as its final step, so an init_runtime_state error cannot leave a stale entry. - is_shutting_down becomes AtomicBool and the event_loop selector becomes AtomicPtr: the checked cross-thread enqueue helpers read both from producer threads while the JS thread writes them. - FetchTasklet frees its plain-heap buffers (response/header/URL) on the HTTP thread before parking in DEAD_VM_TASKLETS; only the handles whose teardown would touch the dead JSC heap stay parked. --- src/jsc/VirtualMachine.rs | 67 ++++++++++++++++------- src/jsc/event_loop.rs | 7 ++- src/jsc/web_worker.rs | 2 +- src/runtime/cli/test/parallel/runner.rs | 2 +- src/runtime/cli/test_command.rs | 8 +-- src/runtime/jsc_hooks.rs | 4 +- src/runtime/node/node_fs.rs | 4 +- src/runtime/webcore/fetch/FetchTasklet.rs | 50 ++++++++++++++--- 8 files changed, 103 insertions(+), 41 deletions(-) diff --git a/src/jsc/VirtualMachine.rs b/src/jsc/VirtualMachine.rs index 38c567f49665..71356123cf84 100644 --- a/src/jsc/VirtualMachine.rs +++ b/src/jsc/VirtualMachine.rs @@ -208,7 +208,10 @@ pub struct VirtualMachine { pub hide_bun_stackframes: bool, pub is_printing_plugin: bool, - pub is_shutting_down: bool, + /// Atomic because the checked cross-thread helpers + /// ([`Self::live_shutting_down_state`]) read it from producer threads + /// while the JS thread writes it. Only ever flips `false` → `true`. + pub is_shutting_down: core::sync::atomic::AtomicBool, /// Set once `on_exit()` has finished draining `RareData::cleanup_hooks`. /// After this point the cleanup-hook list is never iterated again, so /// pushing to it (e.g. from a deferred N-API finalizer scheduled during @@ -277,7 +280,12 @@ pub struct VirtualMachine { pub overridden_performance_now: Option, pub macro_event_loop: EventLoop, pub regular_event_loop: EventLoop, - pub event_loop: *mut EventLoop, // BORROW_FIELD — points at sibling regular_event_loop/macro_event_loop + /// BORROW_FIELD — points at sibling `regular_event_loop`/`macro_event_loop` + /// (or the boxed spawnSync loop). Written only by the JS thread (init, + /// macro-mode swap, spawnSync swap); atomic because the checked + /// 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, pub ref_strings: crate::ref_string::Map, pub ref_strings_mutex: bun_threading::Mutex, @@ -517,9 +525,9 @@ pub(crate) mod live_vm_registry { pub(crate) static REGISTRY: Guarded> = Guarded::new(Vec::new()); - /// Register `vm` and both of its embedded event loops. Called once from - /// `VirtualMachine::init()` after `regular_event_loop`/`macro_event_loop` - /// are initialised. + /// Register `vm` and both of its embedded event loops. 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. @@ -841,8 +849,12 @@ impl VirtualMachine { /// short-lived `&mut *p` at the use site instead, mirroring [`Self::get`]. #[inline(always)] pub fn event_loop(&self) -> *mut EventLoop { - // self-pointer to regular_event_loop or macro_event_loop - self.event_loop + // self-pointer to regular_event_loop or macro_event_loop (or the + // boxed spawnSync loop). Acquire pairs with the Release stores so a + // cross-thread reader that observes a freshly-swapped-in loop also + // observes its initialization; same-thread readers are ordered by + // program order regardless. + self.event_loop.load(core::sync::atomic::Ordering::Acquire) } /// Safe `&mut EventLoop` accessor — the [`JsCell`] escape hatch applied to @@ -857,7 +869,7 @@ impl VirtualMachine { pub fn event_loop_mut(&self) -> &mut EventLoop { // SAFETY: `event_loop` points at a sibling field of this VM; non-null // after `init()`; single-JS-thread invariant per `unsafe impl Sync`. - unsafe { &mut *self.event_loop } + unsafe { &mut *self.event_loop() } } /// Safe `&EventLoop` accessor — shared variant of [`Self::event_loop_mut`]. @@ -866,7 +878,7 @@ impl VirtualMachine { #[inline(always)] pub fn event_loop_shared(&self) -> &EventLoop { // SAFETY: see `event_loop_mut`. - unsafe { &*self.event_loop } + unsafe { &*self.event_loop() } } /// Alias for [`Self::event_loop_mut`]. Kept for callers migrated on the @@ -923,7 +935,7 @@ impl VirtualMachine { pub fn enter_event_loop_scope(&self) -> crate::event_loop::EventLoopEnterGuard { // SAFETY: `self.event_loop` is the live VM-owned event-loop pointer and // remains valid for the VM (and thus the guard's) lifetime. - unsafe { EventLoop::enter_scope(self.event_loop) } + unsafe { EventLoop::enter_scope(self.event_loop()) } } /// Safe shared-reference accessor for the process-lifetime dotenv loader @@ -1098,6 +1110,13 @@ impl VirtualMachine { pub fn is_shutting_down(&self) -> bool { self.is_shutting_down + .load(core::sync::atomic::Ordering::Acquire) + } + + /// One-way flip; see the field doc for why it is atomic. + pub fn set_shutting_down(&self) { + self.is_shutting_down + .store(true, core::sync::atomic::Ordering::Release); } pub fn has_run_cleanup_hooks(&self) -> bool { @@ -1106,7 +1125,7 @@ impl VirtualMachine { /// Exported to C++ as `Bun__VM__scriptExecutionStatus` via virtual_machine_exports.rs. pub fn script_execution_status(&self) -> crate::ScriptExecutionStatus { - if self.is_shutting_down { + if self.is_shutting_down() { return crate::ScriptExecutionStatus::Stopped; } @@ -1272,7 +1291,10 @@ impl VirtualMachine { .fs .use_alternate_source_cache = true; self.macro_mode = true; - self.event_loop = &raw mut self.macro_event_loop; + self.event_loop.store( + &raw mut self.macro_event_loop, + core::sync::atomic::Ordering::Release, + ); bun_analytics::features::macros.fetch_add(1, core::sync::atomic::Ordering::Relaxed); self.transpiler_store.enabled = false; } @@ -1285,7 +1307,10 @@ impl VirtualMachine { .fs .use_alternate_source_cache = false; self.macro_mode = false; - self.event_loop = &raw mut self.regular_event_loop; + self.event_loop.store( + &raw mut self.regular_event_loop, + core::sync::atomic::Ordering::Release, + ); self.transpiler_store.enabled = true; } @@ -1594,7 +1619,7 @@ impl VirtualMachine { } ExitHandler::dispatch_on_exit(self); - self.is_shutting_down = true; + self.set_shutting_down(); // Make sure we run new cleanup hooks introduced by running cleanup // hooks. @@ -2226,12 +2251,7 @@ impl VirtualMachine { let regular = addr_of_mut!((*vm).regular_event_loop); (*regular).virtual_machine = NonNull::new(vm); let _ = (*regular).tasks.ensure_unused_capacity(64); - addr_of_mut!((*vm).event_loop).write(regular); - - // Make this VM reachable for checked cross-thread enqueues. - // Worker VMs are unregistered in `WebWorker::shutdown()` before - // the allocation is freed; the main VM stays registered forever. - live_vm_registry::register_vm(vm); + addr_of_mut!((*vm).event_loop).write(core::sync::atomic::AtomicPtr::new(regular)); // `source_mappings.map` is a sibling-field backref onto // `saved_source_map_table`. @@ -2321,6 +2341,13 @@ impl VirtualMachine { IS_SMOL_MODE.store(true, core::sync::atomic::Ordering::Relaxed); } + // Make this VM reachable for checked cross-thread enqueues. Last step + // of `init` so the fallible ones above (`init_runtime_state`) cannot + // leave a stale entry behind on an `Err` return. Worker VMs are + // unregistered in `WebWorker::shutdown()` before the allocation is + // freed; the main VM stays registered forever. + live_vm_registry::register_vm(vm); + Ok(vm) } diff --git a/src/jsc/event_loop.rs b/src/jsc/event_loop.rs index 6650581d9b4f..44f4e53f384a 100644 --- a/src/jsc/event_loop.rs +++ b/src/jsc/event_loop.rs @@ -1541,8 +1541,11 @@ pub(crate) fn __bun_spawn_sync_vm_set_event_loop_handle( #[unsafe(no_mangle)] pub(crate) fn __bun_spawn_sync_vm_set_event_loop(vm: *mut (), el: *mut ()) { // `el` is its previous `event_loop` pointer (a `*mut EventLoop` into - // `regular_event_loop`/`macro_event_loop`). - vm_from_ptr(vm).event_loop = el.cast::(); + // `regular_event_loop`/`macro_event_loop`). Release pairs with the + // Acquire load in `VirtualMachine::event_loop()`. + vm_from_ptr(vm) + .event_loop + .store(el.cast::(), Ordering::Release); } #[unsafe(no_mangle)] diff --git a/src/jsc/web_worker.rs b/src/jsc/web_worker.rs index 845c5a1c3f69..8a7eb5e64b65 100644 --- a/src/jsc/web_worker.rs +++ b/src/jsc/web_worker.rs @@ -1233,7 +1233,7 @@ impl WebWorker { // clear it so process.on('exit') handlers can run. teardownJSCVM // re-sets it for the JSC VM teardown. vm.jsc_vm().clear_has_termination_request(); - vm.is_shutting_down = true; + vm.set_shutting_down(); vm.on_exit(); if let Some(hooks) = runtime_hooks() { (hooks.cron_clear_all_teardown)(vm); diff --git a/src/runtime/cli/test/parallel/runner.rs b/src/runtime/cli/test/parallel/runner.rs index d264444c0314..4e5b11f9a15e 100644 --- a/src/runtime/cli/test/parallel/runner.rs +++ b/src/runtime/cli/test/parallel/runner.rs @@ -709,7 +709,7 @@ pub fn run_as_worker( // Mirror TestCommand::exec's exit path so BUN_DESTRUCT_VM_ON_EXIT teardown // (lastChanceToFinalize) runs; bypassing it leaks JSC-owned native state. vm_ref.exit_handler.exit_code = 0; - vm_ref.is_shutting_down = true; + vm_ref.set_shutting_down(); vm_ref.run_with_api_lock(|| { // SAFETY: caller guarantees `vm` is a valid live VM pointer for the worker's lifetime. unsafe { (*vm).global_exit() } diff --git a/src/runtime/cli/test_command.rs b/src/runtime/cli/test_command.rs index 150537464980..47d612619e47 100644 --- a/src/runtime/cli/test_command.rs +++ b/src/runtime/cli/test_command.rs @@ -2307,7 +2307,7 @@ impl TestCommand { ); } vm.exit_handler.exit_code = 1; - vm.is_shutting_down = true; + vm.set_shutting_down(); let vm_ptr: *mut VirtualMachine = vm; // SAFETY: `vm_ptr` reborrows the live `&mut VirtualMachine`; // `run_with_api_lock` takes `&self` only and `global_exit()` @@ -2404,7 +2404,7 @@ impl TestCommand { ); } vm.exit_handler.exit_code = 1; - vm.is_shutting_down = true; + vm.set_shutting_down(); let vm_ptr: *mut VirtualMachine = vm; // SAFETY: `vm_ptr` reborrows the live `&mut VirtualMachine`; // `run_with_api_lock` takes `&self` only and `global_exit()` @@ -2939,7 +2939,7 @@ impl TestCommand { { vm.exit_handler.exit_code = 1; } - vm.is_shutting_down = true; + vm.set_shutting_down(); // Release `bun:test` GC roots before `global_exit()` so // `destructOnExit()`'s `collectNow()` can reach the closures they pin // (preload hooks, per-file describe/test callbacks). Clear `RUNNER` @@ -3185,7 +3185,7 @@ impl TestCommand { reporter.write_junit_report_if_needed(); vm.exit_handler.exit_code = 1; - vm.is_shutting_down = true; + vm.set_shutting_down(); // `global_exit()` diverges, so the `exit_file()` defer // above never fires. Release the active file's // `Strong`s and the preload-hook scope here so diff --git a/src/runtime/jsc_hooks.rs b/src/runtime/jsc_hooks.rs index e7eac1d2afab..a55ff61bd5e1 100644 --- a/src/runtime/jsc_hooks.rs +++ b/src/runtime/jsc_hooks.rs @@ -856,7 +856,7 @@ unsafe fn auto_tick(vm: *mut VirtualMachine) { // `VirtualMachine`, so holding `&mut EventLoop` while also touching VM // siblings would alias. Dereference per-field via the raw `vm` ptr. // SAFETY: per fn contract — `vm` is the live per-thread VM. - let el: *mut bun_jsc::event_loop::EventLoop = unsafe { &*vm }.event_loop; + let el: *mut bun_jsc::event_loop::EventLoop = unsafe { &*vm }.event_loop(); // SAFETY: `el` is the live per-thread event loop (field of `*vm`). let loop_ = unsafe { (*el).usockets_loop() }; @@ -1008,7 +1008,7 @@ unsafe fn auto_tick(vm: *mut VirtualMachine) { unsafe fn auto_tick_active(vm: *mut VirtualMachine) { // Note: reshaped for borrowck — see `auto_tick` above. // SAFETY: per fn contract — `vm` is the live per-thread VM. - let el: *mut bun_jsc::event_loop::EventLoop = unsafe { &*vm }.event_loop; + let el: *mut bun_jsc::event_loop::EventLoop = unsafe { &*vm }.event_loop(); // SAFETY: `el` is the live per-thread event loop (field of `*vm`). let loop_ = unsafe { (*el).usockets_loop() }; diff --git a/src/runtime/node/node_fs.rs b/src/runtime/node/node_fs.rs index 56915a0ad27b..03887e58eedd 100644 --- a/src/runtime/node/node_fs.rs +++ b/src/runtime/node/node_fs.rs @@ -1603,8 +1603,8 @@ mod _async_tasks { // Sentinel — overwritten by `finish_concurrently` (gated by the // `has_result` CAS) before any read on the JS thread. result: core::cell::Cell::new(Ok(())), - // `vm.event_loop` is the live per-thread `jsc::EventLoop` field. - evtloop: EventLoopHandle::init(vm.event_loop.cast()), + // `vm.event_loop()` is the live per-thread `jsc::EventLoop`. + evtloop: EventLoopHandle::init(vm.event_loop().cast()), task: work_pool_task(Self::work_pool_callback), r#ref: KeepAlive::default(), tracker: AsyncTaskTracker::init(vm), diff --git a/src/runtime/webcore/fetch/FetchTasklet.rs b/src/runtime/webcore/fetch/FetchTasklet.rs index db3d3786a13d..e9f8fa8d7103 100644 --- a/src/runtime/webcore/fetch/FetchTasklet.rs +++ b/src/runtime/webcore/fetch/FetchTasklet.rs @@ -25,12 +25,15 @@ use bun_sys::FdExt; use bun_threading::Mutex; /// `FetchTasklet` boxes whose owning worker VM was freed (terminated) while -/// the HTTP thread held the last reference. They can never be reclaimed: -/// `deinit()` drops JSC `Strong`/`Weak` handles into the dead VM's freed -/// HandleSet, on any thread, at any time — including the `global_exit` drain -/// that `dealloc_for_shutdown` parks into. Kept reachable here so -/// LSan-enabled CI lanes don't report them as leaks; growth is bounded by -/// in-flight requests that lose a terminate race. +/// the HTTP thread held the last reference. They can never be fully +/// reclaimed: `deinit()` drops JSC `Strong`/`Weak` handles into the dead +/// VM's freed HandleSet, on any thread, at any time — including the +/// `global_exit` drain that `dealloc_for_shutdown` parks into. +/// [`FetchTasklet::free_native_data_for_dead_vm`] reclaims the plain-heap +/// buffers first, so what parks here is the struct itself plus the handles +/// only the (gone) JS thread could release. Kept reachable so LSan-enabled +/// CI lanes don't report them as leaks; growth is bounded by in-flight +/// requests that lose a terminate race. static DEAD_VM_TASKLETS: bun_threading::Guarded> = bun_threading::Guarded::new(Vec::new()); use bun_url::URL as ZigURL; @@ -428,12 +431,41 @@ impl FetchTasklet { // The owning worker VM is gone. Nothing may ever touch this tasklet's // JSC state again: `deinit()` (directly or via the global_exit drain) // would drop the `Strong`/`Weak` fields into the dead VM's freed - // HandleSet. Park the box forever instead — kept reachable so - // LSan-enabled CI lanes don't report it; bounded by requests that - // lose the terminate race. + // HandleSet. Reclaim the plain-heap buffers, then park the box + // forever — kept reachable so LSan-enabled CI lanes don't report it; + // bounded by requests that lose the terminate race. + // SAFETY: ref_count == 0 — this thread holds the only access. + unsafe { (*this).free_native_data_for_dead_vm() }; DEAD_VM_TASKLETS.lock().push(this as usize); } + /// Reclaim, on the HTTP thread, the plain-heap allocations of a tasklet + /// about to park in [`DEAD_VM_TASKLETS`]. The last reference dropping + /// means the HTTP client is done with this request, so nothing writes + /// into these buffers anymore; the parked `http` mirror keeps dangling + /// views into them, but a parked tasklet is never read again. + /// + /// Deliberately skipped — everything whose teardown needs the (gone) JS + /// thread: the JSC handles (`response`, `promise`, `readable_stream_ref`, + /// `abort_reason`, `check_server_identity`, `signal`, `native_response`), + /// `sink` / `request_body_streaming_buffer` (their teardown drops stored + /// JS callbacks), `request_body` (blob teardown can release WTF strings — + /// see the cross-thread string hazard note near `Response::init`), and + /// the `http` box itself (`AsyncHTTP::clear_data` / `Drop` release + /// `ZigStringSlice`s with the same hazard). + fn free_native_data_for_dead_vm(&mut self) { + // Aliases `response_buffer` (freed below); drop the view first. + self.result.body = None; + drop(self.result.certificate_info.take()); + drop(self.result.metadata.take()); + drop(self.metadata.take()); + self.request_headers = Headers::default(); + self.url_proxy_buffer = Box::default(); + self.hostname = None; + self.response_buffer = MutableString::default(); + self.scheduled_response_buffer = MutableString::default(); + } + // ConcurrentTask::from_callback takes `fn(*mut T) -> bun_event_loop::JsResult<()>` // (cycle-broken erased error). fn deinit_callback(this: *mut FetchTasklet) -> ElJsResult<()> { From 3ca7b27e2861586f08982c85adb5ddc86da4a3da Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 10 Jun 2026 21:48:38 +0000 Subject: [PATCH 06/17] Publish MAIN_THREAD_VM only once the main VM is fully initialized Also close the Sendfile request-body fd before parking a dead-VM FetchTasklet (fds are process-wide, unlike the parked heap bytes), and fix the stale lifetime comment at the javascript_vm BackRef creation. --- src/jsc/VirtualMachine.rs | 16 ++++++++++++--- src/runtime/webcore/fetch/FetchTasklet.rs | 25 +++++++++++++++++------ 2 files changed, 32 insertions(+), 9 deletions(-) diff --git a/src/jsc/VirtualMachine.rs b/src/jsc/VirtualMachine.rs index 71356123cf84..c498b9a0f36c 100644 --- a/src/jsc/VirtualMachine.rs +++ b/src/jsc/VirtualMachine.rs @@ -2151,9 +2151,11 @@ impl VirtualMachine { p.cast() }; VM.set(Some(vm)); - if opts.is_main_thread { - MAIN_THREAD_VM.store(vm, core::sync::atomic::Ordering::Release); - } + // NOTE: `MAIN_THREAD_VM` is deliberately NOT published here — it is + // the lock-free liveness fast path in `with_live_vm` / + // `get_main_thread_vm`, which dereference it from other threads, so + // it must not point at this still-zeroed allocation. Published at the + // end of `init`, next to `register_vm`. // ConsoleObject is self-referential (buffers + adapters) — allocate // stable storage and init in place. @@ -2346,6 +2348,14 @@ impl VirtualMachine { // leave a stale entry behind on an `Err` return. Worker VMs are // unregistered in `WebWorker::shutdown()` before the allocation is // freed; the main VM stays registered forever. + // + // `MAIN_THREAD_VM` is published only now, for the same reason: + // `with_live_vm` / `get_main_thread_vm` dereference it from other + // threads without the registry lock, so it must never point at a + // partially initialized VM. + if opts.is_main_thread { + MAIN_THREAD_VM.store(vm, core::sync::atomic::Ordering::Release); + } live_vm_registry::register_vm(vm); Ok(vm) diff --git a/src/runtime/webcore/fetch/FetchTasklet.rs b/src/runtime/webcore/fetch/FetchTasklet.rs index e9f8fa8d7103..413d0b549b81 100644 --- a/src/runtime/webcore/fetch/FetchTasklet.rs +++ b/src/runtime/webcore/fetch/FetchTasklet.rs @@ -449,11 +449,21 @@ impl FetchTasklet { /// thread: the JSC handles (`response`, `promise`, `readable_stream_ref`, /// `abort_reason`, `check_server_identity`, `signal`, `native_response`), /// `sink` / `request_body_streaming_buffer` (their teardown drops stored - /// JS callbacks), `request_body` (blob teardown can release WTF strings — - /// see the cross-thread string hazard note near `Response::init`), and - /// the `http` box itself (`AsyncHTTP::clear_data` / `Drop` release - /// `ZigStringSlice`s with the same hazard). + /// JS callbacks), the blob/stream arms of `request_body` (blob teardown + /// can release WTF strings — see the cross-thread string hazard note near + /// `Response::init`), and the `http` box itself (`AsyncHTTP::clear_data` + /// / `Drop` release `ZigStringSlice`s with the same hazard). fn free_native_data_for_dead_vm(&mut self) { + // The `Sendfile` arm owns only an fd — native teardown, mirroring + // `HTTPRequestBody::detach`. An fd is a process-wide resource that + // would otherwise leak toward `EMFILE` under worker churn. + if let HTTPRequestBody::Sendfile(sendfile) = &mut self.request_body { + if sendfile.offset.max(sendfile.remain) > 0 { + sendfile.fd.close(); + } + sendfile.offset = 0; + sendfile.remain = 0; + } // Aliases `response_buffer` (freed below); drop the view first. self.result.body = None; drop(self.result.certificate_info.take()); @@ -1760,8 +1770,11 @@ impl FetchTasklet { fetch_options: FetchOptions, promise: jsc::JSPromiseStrong, ) -> Result<*mut FetchTasklet, BunError> { - // SAFETY: bun_vm() returns the FFI `*mut VirtualMachine`; the VM outlives - // this tasklet (process-lifetime singleton on the JS thread). + // `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 + // `javascript_vm` field doc). let jsc_vm = bun_ptr::BackRef::new(global_this.bun_vm()); let mut fetch_tasklet = Box::new(FetchTasklet { sink: None, From 0cf613260e183723d3cd05e83973e8f566380280 Mon Sep 17 00:00:00 2001 From: robobun Date: Fri, 12 Jun 2026 03:23:07 -0700 Subject: [PATCH 07/17] Carry a generation token with cross-thread VM handles (#32082) Fixes #32073. Stacked on #32071 (base branch is that PR's branch; retargets to main when it lands). ### Problem The live-VM registry from #32071 keys liveness by address only. Its known residual, documented in the registry's doc comment: if a new VM is allocated at a dead worker VM's address, a stale producer that captured the old pointer passes the check and its completion is delivered to the new VM instead of being dropped. A registry-side counter alone cannot close this, because producers only bring an address to the lookup; the token has to be captured at schedule time and carried by the producer. ### Fix - Every registration in `live_vm_registry` is stamped with a process-unique generation (`AtomicU64`, starts at 1; 0 is the never-matches value of placeholder handles). The generation is also stamped into `VirtualMachine.live_generation` and each `EventLoop.live_generation` so handles can be built without the lock. `enable_macro_mode` re-stamps the macro loop it recreates. - New `Copy` handle types: `VmHandle { addr, generation }` (from `VirtualMachine::concurrent_handle()`) and `LoopHandle` (from `EventLoop::concurrent_handle()`, for producers that must deliver to the exact captured loop: regular vs macro, or the boxed spawnSync loop, which gets its own generation in `register_extra_loop`). - The checked entry points (`VirtualMachine::try_enqueue_task_concurrent`, `EventLoop::try_enqueue_task_concurrent`, `with_live_vm`, `is_shutting_down_or_freed`, `try_ref/unref_concurrently`) take the handle and verify `(addr, generation)` under the registry lock. The lock-free main-VM fast paths compare the generation too; a stale pre-registration read only causes a spurious miss into the locked path. - Every Rust producer that stored `*mut VirtualMachine` / `BackRef` for completion delivery now stores the handle: FetchTasklet, S3 simple/list/download tasks, WorkTask, ConcurrentPromiseTask, AnyTaskJob, TranspilerJob dispatch, node:fs async tasks, zlib native streams (NativeZlib/NativeBrotli/NativeZstd), napi async_work and TSFN dispatch, FSWatcher/StatWatcher, PasswordObject, Archive async tasks, DevServer watcher events, `Bun.build` completion, and blob copy_file/write_file on Windows. - `EventLoopHandle::Js` / `AnyEventLoop::Js` (bun_event_loop) gain a `generation` field captured at construction, and the `JsEventLoop::enqueue_task_concurrent` dispatch carries it, covering the process waiter thread, shell tasks, and the bundler plugin/parse-task completions in one place. `EventLoopHandle::from_tag_ptr` re-reads the generation under its existing still-live contract. - The package manager's `WakeHandler` carries the generation next to its context pointer so the auto-install wake (`AsyncModule::on_wake_handler`) can reassemble the handle. Structs whose VM backref is also used on the JS thread (FSWatcher, StatWatcher/scheduler, DevServer, TSFN) keep that pointer and add the handle for the one cross-thread access; single-purpose fields were converted outright. `GlobalJS::enqueue_task_concurrent_wait_pid` (shell) is deleted: it had no callers and computed the VM from the global object at call time on non-JS threads, which cannot carry a schedule-time handle. ### Remaining address-only checks (explicitly named `*_addr_only`) Pointers captured by C++ cannot carry a generation without C++-side plumbing, so `JSVMClientData::bunVM` (JSCScheduler: `Bun__queueJSCDeferredWorkTaskConcurrently`, `Bun__eventLoop__incrementRefConcurrently`), `EventLoopTaskNoContext` (webcrypto CppTask ref/unref), and the napi finalizer's env-derived VM stay address-checked, which is exactly #32071's behavior for them. They are named `*_addr_only` so the residual is greppable. TSFN acceptance-after-death and napi env teardown remain tracked in #15964 / #30286. ### Verification - `bun bd test test/js/web/workers/worker-terminate-lifetime.test.ts`: 6/6 pass under the ASAN debug build, including #32071's fetch-in-flight ASAN test and two new tests: - "cross-thread completions are delivered to live worker VMs" exercises fetch, Bun.spawn exit, node:fs, zlib, and Bun.password inside a worker; a broken generation capture silently drops the completion, so each would hang. - "terminating a worker with a subprocess in flight drops the waiter-thread completion" covers the waiter-thread path (EventLoopHandle generation dispatch) against a freed worker loop. - Debug-build suites for the converted producers pass: spawn, password, zlib (2 failures are container-speed artifacts: the tests' data generation alone takes 40s under this ASAN build vs their 15s cap), Bun.build API, fs.watch (2 failures are run-as-root artifacts, identical on release main), worker.test.ts (2 failures are a pre-existing 1s hardcoded budget; timed identically on the base branch build). - `cargo check -p bun_bin` on linux-x64, windows-x64 (covers the copy_file/write_file changes), darwin-arm64; clippy clean on all touched crates. The ABA mis-delivery itself is not deterministically reproducible from JS (it requires the allocator to reuse a dead VM's exact address for a new VM while a stale producer is in flight), so there is no fail-before test for it; the new tests pin the delivery and drop behavior of the handle plumbing on both sides. --- src/bundler/ParseTask.rs | 3 +- src/bundler/ServerComponentParseTask.rs | 3 +- src/bundler/bundle_v2.rs | 10 +- src/event_loop/AnyEventLoop.rs | 109 ++++-- src/event_loop/lib.rs | 3 +- src/install/PackageManager.rs | 2 +- src/install_types/resolver_hooks.rs | 10 +- src/jsc/AsyncModule.rs | 9 +- src/jsc/ConcurrentPromiseTask.rs | 24 +- src/jsc/CppTask.rs | 8 +- src/jsc/JSCScheduler.rs | 13 +- src/jsc/RuntimeTranspilerStore.rs | 7 +- src/jsc/VirtualMachine.rs | 329 ++++++++++++++---- src/jsc/WorkTask.rs | 18 +- src/jsc/any_task_job.rs | 21 +- src/jsc/event_loop.rs | 110 +++++- src/runtime/api/Archive.rs | 12 +- src/runtime/api/JSBundler.rs | 10 +- src/runtime/api/js_bundle_completion_task.rs | 18 +- src/runtime/bake/DevServer.rs | 6 + src/runtime/bake/DevServer/memory_cost.rs | 1 + src/runtime/bake/dev_server/mod.rs | 4 +- src/runtime/crypto/PasswordObject.rs | 10 +- src/runtime/jsc_hooks.rs | 4 + src/runtime/napi/napi_body.rs | 31 +- src/runtime/node/node_fs.rs | 12 +- src/runtime/node/node_fs_stat_watcher.rs | 16 +- src/runtime/node/node_fs_watcher.rs | 6 +- src/runtime/node/node_zlib_binding.rs | 8 +- src/runtime/node/zlib/NativeBrotli.rs | 14 +- src/runtime/node/zlib/NativeZlib.rs | 14 +- src/runtime/node/zlib/NativeZstd.rs | 14 +- src/runtime/shell/builtin/yes.rs | 4 +- src/runtime/shell/shell_body.rs | 14 - src/runtime/webcore/FileSink.rs | 2 +- src/runtime/webcore/blob/copy_file.rs | 6 +- src/runtime/webcore/blob/write_file.rs | 13 +- src/runtime/webcore/fetch/FetchTasklet.rs | 45 +-- src/runtime/webcore/s3/client.rs | 18 +- src/runtime/webcore/s3/download_stream.rs | 6 +- src/runtime/webcore/s3/simple_request.rs | 12 +- src/spawn/process.rs | 8 +- .../workers/worker-terminate-lifetime.test.ts | 118 +++++++ 43 files changed, 801 insertions(+), 304 deletions(-) diff --git a/src/bundler/ParseTask.rs b/src/bundler/ParseTask.rs index 67402efd94a7..e58ba5156a95 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 e8a51b602cdb..cde720b35cd1 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 a622920a50b2..b56abdff2d2e 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 98b75524e59f..1c7df0ad7ac1 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 8a9a5dab8640..1c5e4517b96a 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 2159986104a5..60d326c8ae42 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 b1aea8524e70..7ec97e0ef4aa 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 3782d9fb7179..a09f51859a5c 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 ec0b9a03f8f0..b70e9e5a9c6e 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 3b67d2b7e25f..d6d4bdd5cd25 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 be51d283d043..1926a97c3996 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 3c4d674963c0..d6370caf5f93 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 c498b9a0f36c..47583a8473e4 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 6f5cffb9a6f3..6c827aaebfee 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 41b5ccbaf461..a02ae9bf113c 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 44f4e53f384a..f26f5a694a1a 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 cd444693e721..5b928f218ce3 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 40610f0d62b5..2ebca5b535af 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 56094492aaa9..df712b152df3 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 930ca31e7073..36f1d4d61dd6 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 30c0265a71e1..bcc8d71fe722 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 0648fd3ffbe4..c38efdde95f5 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 d6857add0203..c2857a18120d 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 a55ff61bd5e1..de6ce19920c7 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 eb1260d84c9a..d86b612f2381 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 03887e58eedd..b801b59a4a7b 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 dcf5ae13fcb7..78811d558712 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 33d0e6c1a86a..27c7ae136411 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 d4281906ae75..c90295018463 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 1d880bd88fe4..e6b8a380429c 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 d39f24be4b50..d8ea8dd3babf 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 ba91097f4576..db1b98c2a650 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 f8627dc5cc06..e505cde2746e 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 bbc72d130c07..8600570f7709 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 970c5e5f5000..39491013fa51 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 db997f21f3c1..7cd5263d54d0 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 cb2ac64f6ff8..d78515289950 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 413d0b549b81..8f84a01bb220 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 9f3778cdd3a3..aa3364194c58 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 eae3304d654a..eea072a39132 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 05046c7a4543..a75cb342499f 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 7a5382817c1e..541277c609c9 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 8e90d844e94a..77e49ed344c4 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 From c417173848bbdf32162361e333f22c9c453e0cb8 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 12 Jun 2026 11:04:30 +0000 Subject: [PATCH 08/17] Make VmHandle the only cross-thread VM identity, including C++ captures Every C++ site that stored a bare bunVM pointer for later concurrent use now captures the VM's registry generation next to it (JSVMClientData, Zig::GlobalObject, EventLoopTaskNoContext, NapiEnv) and passes both back across the ABI, where VmHandle::from_raw_parts reassembles the handle. The address-only fallback entry points are deleted. The generation is stamped early in VirtualMachine::init (before Zig__GlobalObject__create and init_runtime_state) instead of at registration, so creation-time captures see the real value; registration stays the final init step. This also fixes the resolver WakeHandler capturing generation 0, which made the checked enqueue drop every auto-install wake. NapiFinalizerTask::schedule no longer dereferences the env's global on non-JS threads; it uses the env's creation-time handle capture. --- src/jsc/CppTask.rs | 40 ++--- src/jsc/JSCScheduler.rs | 23 ++- src/jsc/VirtualMachine.rs | 166 ++++++++---------- src/jsc/bindings/BunClientData.cpp | 3 +- src/jsc/bindings/BunClientData.h | 6 +- src/jsc/bindings/BunDebugger.cpp | 6 +- src/jsc/bindings/EventLoopTaskNoContext.cpp | 5 + src/jsc/bindings/EventLoopTaskNoContext.h | 4 + src/jsc/bindings/JSCTaskScheduler.cpp | 17 +- src/jsc/bindings/ScriptExecutionContext.cpp | 8 +- src/jsc/bindings/ZigGlobalObject.cpp | 5 +- src/jsc/bindings/ZigGlobalObject.h | 5 + src/jsc/bindings/napi.cpp | 10 ++ src/jsc/bindings/napi.h | 9 + src/jsc/bindings/webcore/BroadcastChannel.cpp | 6 +- src/jsc/bindings/webcore/MessagePort.cpp | 6 +- src/jsc/virtual_machine_exports.rs | 10 ++ src/runtime/bake/BakeGlobalObject.cpp | 3 +- src/runtime/napi/napi_body.rs | 45 +++-- src/runtime/webview/ChromeBackend.cpp | 5 +- src/runtime/webview/WebKitBackend.cpp | 5 +- 21 files changed, 223 insertions(+), 164 deletions(-) diff --git a/src/jsc/CppTask.rs b/src/jsc/CppTask.rs index d6d4bdd5cd25..8e654d02a13f 100644 --- a/src/jsc/CppTask.rs +++ b/src/jsc/CppTask.rs @@ -1,5 +1,4 @@ -use core::ptr::NonNull; - +use crate::virtual_machine::VmHandle; use crate::{JSGlobalObject, JsResult, VirtualMachineRef as VirtualMachine}; use bun_event_loop::{TaskTag, Taskable, task_tag}; use bun_threading::work_pool::{Task as WorkPoolTask, WorkPool}; @@ -10,6 +9,9 @@ unsafe extern "C" { safe fn Bun__EventLoopTaskNoContext__createdInBunVm( task: &EventLoopTaskNoContext, ) -> *mut VirtualMachine; + safe fn Bun__EventLoopTaskNoContext__createdInBunVmGeneration( + task: &EventLoopTaskNoContext, + ) -> u64; } bun_opaque::opaque_ffi! { @@ -47,13 +49,15 @@ impl EventLoopTaskNoContext { unsafe { Bun__EventLoopTaskNoContext__performTask(this) } } - /// Get the VM that created this task. `VirtualMachine` is process-lifetime - /// (PORTING.md §Global mutable state), so a [`BackRef`] is the right - /// non-owning handle: callers project `&VirtualMachine` via `Deref` and - /// route mutation through the VM's safe interior accessors (e.g. - /// `event_loop_shared()`). - pub fn get_vm(&self) -> Option> { - NonNull::new(Bun__EventLoopTaskNoContext__createdInBunVm(self)).map(bun_ptr::BackRef::from) + /// Schedule-time [`VmHandle`] of the VM that created this task, captured + /// by the C++ constructor (`EventLoopTaskNoContext`) next to the `bunVM` + /// pointer. The creating VM may be a worker freed by terminate() — all + /// use goes through the checked `VirtualMachine` entry points. + pub fn vm_handle(&self) -> VmHandle { + VmHandle::from_raw_parts( + Bun__EventLoopTaskNoContext__createdInBunVm(self) as usize, + Bun__EventLoopTaskNoContext__createdInBunVmGeneration(self), + ) } } @@ -73,18 +77,14 @@ impl ConcurrentCppTask { let cpp_task = self.cpp_task; // `EventLoopTaskNoContext` is an `opaque_ffi!` ZST handle; `opaque_ref` // is the centralised non-null deref proof. Valid until `run` consumes it. - let maybe_vm = EventLoopTaskNoContext::opaque_ref(cpp_task).get_vm(); + let vm_handle = EventLoopTaskNoContext::opaque_ref(cpp_task).vm_handle(); drop(self); // SAFETY: `cpp_task` is the valid C++ handle stored by `ConcurrentCppTask__createAndRun`; // `opaque_ref` above proved it non-null and it has not yet been freed — `run` consumes it here. 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. Address-only: - // the pointer was captured by C++ (`EventLoopTaskNoContext`) and - // carries no generation. - VirtualMachine::try_unref_concurrently_addr_only(vm.as_ptr()); - } + // 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_handle); } } @@ -93,10 +93,8 @@ pub(crate) extern "C" fn ConcurrentCppTask__createAndRun(cpp_task: *mut EventLoo crate::mark_binding!(); // `EventLoopTaskNoContext` is an `opaque_ffi!` ZST handle; `opaque_ref` is // 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_addr_only(vm.as_ptr()); - } + // Checked for symmetry with the pool-thread unref in `run_owned`. + VirtualMachine::try_ref_concurrently(EventLoopTaskNoContext::opaque_ref(cpp_task).vm_handle()); WorkPool::schedule_new(ConcurrentCppTask { cpp_task, workpool_task: WorkPoolTask::default(), diff --git a/src/jsc/JSCScheduler.rs b/src/jsc/JSCScheduler.rs index 1926a97c3996..8adfee86f0a5 100644 --- a/src/jsc/JSCScheduler.rs +++ b/src/jsc/JSCScheduler.rs @@ -40,16 +40,19 @@ impl JSCDeferredWorkTask { pub(crate) extern "C" fn Bun__eventLoop__incrementRefConcurrently( jsc_vm: *mut VirtualMachine, delta: c_int, + generation: u64, ) { 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. + // terminated worker's VM (the counter of a freed loop needs no + // balancing). C++ captured `(bunVM, generation)` at creation time + // (`JSVMClientData` / `Zig::GlobalObject`), so the reassembled handle + // rejects a new VM reusing the freed address. + let handle = crate::virtual_machine::VmHandle::from_raw_parts(jsc_vm as usize, generation); if delta > 0 { - VirtualMachine::try_ref_concurrently_addr_only(jsc_vm); + VirtualMachine::try_ref_concurrently(handle); } else { - VirtualMachine::try_unref_concurrently_addr_only(jsc_vm); + VirtualMachine::try_unref_concurrently(handle); } } @@ -57,15 +60,17 @@ pub(crate) extern "C" fn Bun__eventLoop__incrementRefConcurrently( pub(crate) extern "C" fn Bun__queueJSCDeferredWorkTaskConcurrently( jsc_vm: *mut VirtualMachine, task: *mut JSCDeferredWorkTask, + generation: u64, ) { crate::mark_binding!(); // 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). - // Address-only: the pointer was captured by C++ (`JSVMClientData::bunVM`) - // and carries no generation. - let _ = VirtualMachine::try_enqueue_task_concurrent_addr_only( - jsc_vm, + // C++ captured `(bunVM, generation)` at creation time + // (`JSVMClientData`), 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(jsc_vm as usize, generation), ConcurrentTask::create_from(task), ); } diff --git a/src/jsc/VirtualMachine.rs b/src/jsc/VirtualMachine.rs index 9a40e1ec747d..dfdbd4cd74e2 100644 --- a/src/jsc/VirtualMachine.rs +++ b/src/jsc/VirtualMachine.rs @@ -513,10 +513,12 @@ impl VMHolder { /// 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. +/// C++ producers participate the same way: the creation-time captures of +/// `bunVM` (`JSVMClientData`, `Zig::GlobalObject`, `EventLoopTaskNoContext`, +/// `NapiEnv`) store the generation next to the pointer (via +/// `Bun__getVmGeneration`) and pass both back across the ABI, where +/// [`VmHandle::from_raw_parts`](super::VmHandle::from_raw_parts) reassembles +/// the handle. /// /// 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 @@ -548,25 +550,39 @@ pub(crate) mod live_vm_registry { 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 + /// Mint this VM's process-unique generation and stamp it 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) { + /// `concurrent_handle()` can build handles without the lock and the C++ + /// side (`Bun__getVmGeneration`) can capture it during global creation. + /// Called once from `VirtualMachine::init()` before + /// `Zig__GlobalObject__create`; until `register_vm` publishes the entry, + /// the stamped `(addr, generation)` pair matches nothing. + pub(crate) fn stamp_generation(vm: *mut VirtualMachine) { 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 { + unsafe { (*vm).live_generation.store(generation, Ordering::Relaxed); (*vm).regular_event_loop.live_generation = generation; (*vm).macro_event_loop.live_generation = generation; + } + } + + /// Register `vm` and both of its embedded event loops under the + /// generation stamped by [`stamp_generation`]. 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, and `live_generation` is atomic. + let (regular, macro_, generation) = unsafe { ( core::ptr::addr_of!((*vm).regular_event_loop), core::ptr::addr_of!((*vm).macro_event_loop), + (*vm).live_generation.load(Ordering::Relaxed), ) }; + debug_assert!(generation != 0, "register_vm before stamp_generation"); let mut reg = REGISTRY.lock(); reg.push(Entry { vm: vm as usize, @@ -664,11 +680,14 @@ impl VmHandle { } } - /// 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 { + /// Reassemble a handle whose parts were carried separately, because the + /// carrier cannot name this type: the package-manager `WakeHandler` (its + /// context pointer and generation travel as distinct fields in a low-tier + /// crate) and the C++ `(bunVM, generation)` captures that come back + /// across the ABI (`JSVMClientData`, `EventLoopTaskNoContext`, + /// `NapiEnv`). No liveness is implied — the checked entry points verify + /// the pair against the registry on every use. + pub fn from_raw_parts(addr: usize, generation: u64) -> Self { VmHandle { addr, generation } } @@ -2358,6 +2377,15 @@ impl VirtualMachine { let _ = (*regular).tasks.ensure_unused_capacity(64); addr_of_mut!((*vm).event_loop).write(core::sync::atomic::AtomicPtr::new(regular)); + // Stamp the VM's process-unique generation before + // `Zig__GlobalObject__create` below: the C++ side captures + // `(bunVM, generation)` pairs during global creation + // (`JSVMClientData`, `Zig::GlobalObject`, via + // `Bun__getVmGeneration`). The registration itself still happens + // as the final step of `init` — an unregistered `(addr, + // generation)` pair matches nothing in the checked entry points. + live_vm_registry::stamp_generation(vm); + // `source_mappings.map` is a sibling-field backref onto // `saved_source_map_table`. addr_of_mut!((*vm).saved_source_map_table) @@ -3843,38 +3871,48 @@ impl VirtualMachine { } } - /// 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, + /// 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` + /// (called before any free) also takes — so the VM cannot be freed while + /// `f` runs. `f` must therefore be short and lock-free: pushing to the + /// MPSC queue, `wakeup()`, reading a flag. The `&VirtualMachine` handed to + /// `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 { + let VmHandle { addr, generation } = handle; 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. + // an address + generation match proves liveness without the lock. A + // stale (pre-`stamp_generation`) 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 + if generation + == 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)) + .any(|e| e.vm == addr && e.generation == generation) { return None; } @@ -3884,43 +3922,6 @@ impl VirtualMachine { 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` - /// (called before any free) also takes — so the VM cannot be freed while - /// `f` runs. `f` must therefore be short and lock-free: pushing to the - /// MPSC queue, `wakeup()`, reading a flag. The `&VirtualMachine` handed to - /// `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_addr_only( - vm: *mut VirtualMachine, - f: impl FnOnce(&VirtualMachine) -> R, - ) -> Option { - Self::with_live_vm_impl(vm as usize, None, f) - } - /// 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 @@ -3948,23 +3949,6 @@ impl VirtualMachine { } } - /// 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_addr_only(vm, |vm| { - vm.event_loop_shared().enqueue_task_concurrent(task); - }) { - Some(()) => true, - None => { - crate::event_loop::discard_unqueued_concurrent_task(task); - false - } - } - } - /// 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 / @@ -3986,16 +3970,14 @@ impl VirtualMachine { } /// `ref_concurrently`/`unref_concurrently` variants of - /// [`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()); + /// [`Self::try_enqueue_task_concurrent`] — no-ops when the handle's VM is + /// gone (a freed loop has no liveness counter left to balance). + pub fn try_ref_concurrently(handle: VmHandle) { + let _ = Self::with_live_vm(handle, |vm| vm.event_loop_shared().ref_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()); + pub fn try_unref_concurrently(handle: VmHandle) { + let _ = Self::with_live_vm(handle, |vm| vm.event_loop_shared().unref_concurrently()); } /// `cond` is `&Cell` (not `&mut bool`): the re-entrant diff --git a/src/jsc/bindings/BunClientData.cpp b/src/jsc/bindings/BunClientData.cpp index a80a81f2e9dd..9556a19ac5b1 100644 --- a/src/jsc/bindings/BunClientData.cpp +++ b/src/jsc/bindings/BunClientData.cpp @@ -102,11 +102,12 @@ JSVMClientData::~JSVMClientData() m_normalWorld = nullptr; } -void JSVMClientData::create(VM* vm, void* bunVM) +void JSVMClientData::create(VM* vm, void* bunVM, uint64_t bunVMGeneration) { auto provider = WebCore::createBuiltinsSourceProvider(); JSVMClientData* clientData = new JSVMClientData(*vm, provider); clientData->bunVM = bunVM; + clientData->bunVMGeneration = bunVMGeneration; vm->deferredWorkTimer->onAddPendingWork = [clientData](Ref&& ticket, JSC::DeferredWorkTimer::WorkType kind) -> void { Bun::JSCTaskScheduler::onAddPendingWork(clientData, WTF::move(ticket), kind); }; diff --git a/src/jsc/bindings/BunClientData.h b/src/jsc/bindings/BunClientData.h index 39830bb53845..3134f7bafc02 100644 --- a/src/jsc/bindings/BunClientData.h +++ b/src/jsc/bindings/BunClientData.h @@ -90,7 +90,7 @@ class JSVMClientData : public JSC::VM::ClientData { virtual ~JSVMClientData(); - static void create(JSC::VM*, void*); + static void create(JSC::VM*, void*, uint64_t bunVMGeneration); JSHeapData& heapData() { return *m_heapData; } BunBuiltinNames& builtinNames() { return m_builtinNames; } @@ -120,6 +120,10 @@ class JSVMClientData : public JSC::VM::ClientData { } void* bunVM; + // Schedule-time generation of `bunVM` (see Rust `live_vm_registry`), + // captured at VM creation and passed back across the ABI so checked + // concurrent entry points can reject a new VM reusing a freed address. + uint64_t bunVMGeneration; Bun::JSCTaskScheduler deferredWorkTimer; // Backing storage for Bun::IsolatedModuleCache (see IsolatedModuleCache.h). diff --git a/src/jsc/bindings/BunDebugger.cpp b/src/jsc/bindings/BunDebugger.cpp index 07de46c89962..ada064e2b0ee 100644 --- a/src/jsc/bindings/BunDebugger.cpp +++ b/src/jsc/bindings/BunDebugger.cpp @@ -18,7 +18,7 @@ #include "InspectorHTTPServerAgent.h" extern "C" void Bun__tickWhilePaused(bool*); -extern "C" void Bun__eventLoop__incrementRefConcurrently(void* bunVM, int delta); +extern "C" void Bun__eventLoop__incrementRefConcurrently(void* bunVM, int delta, uint64_t bunVMGeneration); namespace Bun { using namespace JSC; @@ -116,7 +116,7 @@ class BunInspectorConnection : public Inspector::FrontendChannel { this->status = ConnectionStatus::Connected; auto* globalObject = context.jsGlobalObject(); if (this->unrefOnDisconnect) { - Bun__eventLoop__incrementRefConcurrently(static_cast(globalObject)->bunVM(), 1); + Bun__eventLoop__incrementRefConcurrently(static_cast(globalObject)->bunVM(), 1, static_cast(globalObject)->bunVMGeneration()); } globalObject->setInspectable(true); auto& inspector = globalObject->inspectorDebuggable(); @@ -202,7 +202,7 @@ class BunInspectorConnection : public Inspector::FrontendChannel { if (connection->unrefOnDisconnect) { connection->unrefOnDisconnect = false; - Bun__eventLoop__incrementRefConcurrently(static_cast(context.jsGlobalObject())->bunVM(), -1); + Bun__eventLoop__incrementRefConcurrently(static_cast(context.jsGlobalObject())->bunVM(), -1, static_cast(context.jsGlobalObject())->bunVMGeneration()); } }); } diff --git a/src/jsc/bindings/EventLoopTaskNoContext.cpp b/src/jsc/bindings/EventLoopTaskNoContext.cpp index 6f1f3d367ad9..44cb09e071dc 100644 --- a/src/jsc/bindings/EventLoopTaskNoContext.cpp +++ b/src/jsc/bindings/EventLoopTaskNoContext.cpp @@ -12,4 +12,9 @@ extern "C" void* Bun__EventLoopTaskNoContext__createdInBunVm(const EventLoopTask return task->createdInBunVm(); } +extern "C" uint64_t Bun__EventLoopTaskNoContext__createdInBunVmGeneration(const EventLoopTaskNoContext* task) +{ + return task->createdInBunVmGeneration(); +} + } // namespace Bun diff --git a/src/jsc/bindings/EventLoopTaskNoContext.h b/src/jsc/bindings/EventLoopTaskNoContext.h index fede33f2603c..7f4bad115078 100644 --- a/src/jsc/bindings/EventLoopTaskNoContext.h +++ b/src/jsc/bindings/EventLoopTaskNoContext.h @@ -12,6 +12,7 @@ class EventLoopTaskNoContext { public: EventLoopTaskNoContext(JSC::JSGlobalObject* globalObject, Function&& task) : m_createdInBunVm(defaultGlobalObject(globalObject)->bunVM()) + , m_createdInBunVmGeneration(defaultGlobalObject(globalObject)->bunVMGeneration()) , m_task(WTF::move(task)) { } @@ -23,13 +24,16 @@ class EventLoopTaskNoContext { } void* createdInBunVm() const { return m_createdInBunVm; } + uint64_t createdInBunVmGeneration() const { return m_createdInBunVmGeneration; } private: void* m_createdInBunVm; + uint64_t m_createdInBunVmGeneration; Function m_task; }; extern "C" void Bun__EventLoopTaskNoContext__performTask(EventLoopTaskNoContext* task); extern "C" void* Bun__EventLoopTaskNoContext__createdInBunVm(const EventLoopTaskNoContext* task); +extern "C" uint64_t Bun__EventLoopTaskNoContext__createdInBunVmGeneration(const EventLoopTaskNoContext* task); } // namespace Bun diff --git a/src/jsc/bindings/JSCTaskScheduler.cpp b/src/jsc/bindings/JSCTaskScheduler.cpp index 171b5c4edc19..d95aa4a737ff 100644 --- a/src/jsc/bindings/JSCTaskScheduler.cpp +++ b/src/jsc/bindings/JSCTaskScheduler.cpp @@ -10,8 +10,8 @@ using TicketData = JSC::DeferredWorkTimer::TicketData; namespace Bun { using namespace JSC; -extern "C" void Bun__queueJSCDeferredWorkTaskConcurrently(void* bunVM, void* task); -extern "C" void Bun__eventLoop__incrementRefConcurrently(void* bunVM, int delta); +extern "C" void Bun__queueJSCDeferredWorkTaskConcurrently(void* bunVM, void* task, uint64_t bunVMGeneration); +extern "C" void Bun__eventLoop__incrementRefConcurrently(void* bunVM, int delta, uint64_t bunVMGeneration); class JSCDeferredWorkTask { public: @@ -42,7 +42,7 @@ void JSCTaskScheduler::onAddPendingWork(WebCore::JSVMClientData* clientData, Ref auto& scheduler = clientData->deferredWorkTimer; Locker holder { scheduler.m_lock }; if (kind == DeferredWorkTimer::WorkType::ImminentlyScheduled) { - Bun__eventLoop__incrementRefConcurrently(clientData->bunVM, 1); + Bun__eventLoop__incrementRefConcurrently(clientData->bunVM, 1, clientData->bunVMGeneration); scheduler.m_pendingTicketsKeepingEventLoopAlive.add(WTF::move(ticket)); } else { scheduler.m_pendingTicketsOther.add(WTF::move(ticket)); @@ -51,12 +51,13 @@ void JSCTaskScheduler::onAddPendingWork(WebCore::JSVMClientData* clientData, Ref void JSCTaskScheduler::onScheduleWorkSoon(WebCore::JSVMClientData* clientData, Ticket ticket, Task&& task) { auto* job = new JSCDeferredWorkTask(*ticket, WTF::move(task)); - Bun__queueJSCDeferredWorkTaskConcurrently(clientData->bunVM, job); + Bun__queueJSCDeferredWorkTaskConcurrently(clientData->bunVM, job, clientData->bunVMGeneration); } void JSCTaskScheduler::onCancelPendingWork(WebCore::JSVMClientData* clientData, Ticket ticket) { auto* bunVM = clientData->bunVM; + auto bunVMGeneration = clientData->bunVMGeneration; auto& scheduler = clientData->deferredWorkTimer; Locker holder { scheduler.m_lock }; @@ -67,7 +68,7 @@ void JSCTaskScheduler::onCancelPendingWork(WebCore::JSVMClientData* clientData, if (isKeepingEventLoopAlive) { holder.unlockEarly(); - Bun__eventLoop__incrementRefConcurrently(bunVM, -1); + Bun__eventLoop__incrementRefConcurrently(bunVM, -1, bunVMGeneration); } else { scheduler.m_pendingTicketsOther.removeIf([ticket](auto pendingTicket) { return pendingTicket.ptr() == ticket; @@ -75,14 +76,14 @@ void JSCTaskScheduler::onCancelPendingWork(WebCore::JSVMClientData* clientData, } } -static void runPendingWork(void* bunVM, Bun::JSCTaskScheduler& scheduler, JSCDeferredWorkTask* job) +static void runPendingWork(void* bunVM, uint64_t bunVMGeneration, Bun::JSCTaskScheduler& scheduler, JSCDeferredWorkTask* job) { Locker holder { scheduler.m_lock }; auto pendingTicket = scheduler.m_pendingTicketsKeepingEventLoopAlive.take(job->ticket); if (!pendingTicket) { pendingTicket = scheduler.m_pendingTicketsOther.take(job->ticket); } else { - Bun__eventLoop__incrementRefConcurrently(bunVM, -1); + Bun__eventLoop__incrementRefConcurrently(bunVM, -1, bunVMGeneration); } holder.unlockEarly(); @@ -98,7 +99,7 @@ extern "C" void Bun__runDeferredWork(Bun::JSCDeferredWorkTask* job) auto& vm = job->vm(); auto clientData = WebCore::clientData(vm); - runPendingWork(clientData->bunVM, clientData->deferredWorkTimer, job); + runPendingWork(clientData->bunVM, clientData->bunVMGeneration, clientData->deferredWorkTimer, job); } } diff --git a/src/jsc/bindings/ScriptExecutionContext.cpp b/src/jsc/bindings/ScriptExecutionContext.cpp index 66a581369c58..4501efc17691 100644 --- a/src/jsc/bindings/ScriptExecutionContext.cpp +++ b/src/jsc/bindings/ScriptExecutionContext.cpp @@ -76,15 +76,17 @@ JSGlobalObject* ScriptExecutionContext::globalObject() return m_globalObject; } -extern "C" void Bun__eventLoop__incrementRefConcurrently(void* bunVM, int delta); +extern "C" void Bun__eventLoop__incrementRefConcurrently(void* bunVM, int delta, uint64_t bunVMGeneration); void ScriptExecutionContext::refEventLoop() { - Bun__eventLoop__incrementRefConcurrently(WebCore::clientData(vm())->bunVM, 1); + auto* clientData = WebCore::clientData(vm()); + Bun__eventLoop__incrementRefConcurrently(clientData->bunVM, 1, clientData->bunVMGeneration); } void ScriptExecutionContext::unrefEventLoop() { - Bun__eventLoop__incrementRefConcurrently(WebCore::clientData(vm())->bunVM, -1); + auto* clientData = WebCore::clientData(vm()); + Bun__eventLoop__incrementRefConcurrently(clientData->bunVM, -1, clientData->bunVMGeneration); } ScriptExecutionContext::~ScriptExecutionContext() diff --git a/src/jsc/bindings/ZigGlobalObject.cpp b/src/jsc/bindings/ZigGlobalObject.cpp index 456898e72aaf..343d25f859e6 100644 --- a/src/jsc/bindings/ZigGlobalObject.cpp +++ b/src/jsc/bindings/ZigGlobalObject.cpp @@ -352,6 +352,7 @@ extern "C" void JSCInitialize(const char* envp[], size_t envc, void (*onCrash)(c } extern "C" void* Bun__getVM(); +extern "C" uint64_t Bun__getVmGeneration(); extern "C" void Bun__setDefaultGlobalObject(Zig::GlobalObject* globalObject); @@ -479,7 +480,7 @@ extern "C" JSC::JSGlobalObject* Zig__GlobalObject__create(void* console_client, // Every JS VM's RunLoop should use Bun's RunLoop implementation ASSERT(vmPtr->runLoop().kind() == WTF::RunLoop::Kind::Bun); - WebCore::JSVMClientData::create(&vm, Bun__getVM()); + WebCore::JSVMClientData::create(&vm, Bun__getVM(), Bun__getVmGeneration()); const auto createGlobalObject = [&]() -> Zig::GlobalObject* { if (executionContextId == std::numeric_limits::max() || executionContextId > 1) [[unlikely]] { @@ -974,6 +975,7 @@ const JSC::GlobalObjectMethodTable& EvalGlobalObject::globalObjectMethodTable() GlobalObject::GlobalObject(JSC::VM& vm, JSC::Structure* structure, const JSC::GlobalObjectMethodTable* methodTable) : Base(vm, structure, methodTable) , m_bunVM(Bun__getVM()) + , m_bunVMGeneration(Bun__getVmGeneration()) , m_constructors(makeUnique()) , m_world(static_cast(vm.clientData)->normalWorld()) , m_worldIsNormal(true) @@ -989,6 +991,7 @@ GlobalObject::GlobalObject(JSC::VM& vm, JSC::Structure* structure, const JSC::Gl GlobalObject::GlobalObject(JSC::VM& vm, JSC::Structure* structure, WebCore::ScriptExecutionContextIdentifier contextId, const JSC::GlobalObjectMethodTable* methodTable) : Base(vm, structure, methodTable) , m_bunVM(Bun__getVM()) + , m_bunVMGeneration(Bun__getVmGeneration()) , m_constructors(makeUnique()) , m_world(static_cast(vm.clientData)->normalWorld()) , m_worldIsNormal(true) diff --git a/src/jsc/bindings/ZigGlobalObject.h b/src/jsc/bindings/ZigGlobalObject.h index eae5d1cc5a6f..dbdc1897d638 100644 --- a/src/jsc/bindings/ZigGlobalObject.h +++ b/src/jsc/bindings/ZigGlobalObject.h @@ -107,6 +107,10 @@ class GlobalObject : public Bun::GlobalScope { public: // Move this to the front for better cache locality. void* m_bunVM; + // Schedule-time generation of `m_bunVM` (see Rust `live_vm_registry`); + // captured at construction, passed back across the ABI by concurrent + // producers so checked entry points can reject a freed-and-reused VM. + uint64_t m_bunVMGeneration; bool isShuttingDown() const { @@ -350,6 +354,7 @@ class GlobalObject : public Bun::GlobalScope { void visitGeneratedLazyClasses(GlobalObject*, Visitor&); ALWAYS_INLINE void* bunVM() const { return m_bunVM; } + ALWAYS_INLINE uint64_t bunVMGeneration() const { return m_bunVMGeneration; } #if OS(WINDOWS) uv_loop_t* uvLoop() const { diff --git a/src/jsc/bindings/napi.cpp b/src/jsc/bindings/napi.cpp index 8fef5dcfa30f..0d6b4768f287 100644 --- a/src/jsc/bindings/napi.cpp +++ b/src/jsc/bindings/napi.cpp @@ -3017,6 +3017,16 @@ extern "C" uint32_t napi_internal_get_version(napi_env env) return env->napiModule().nm_version; } +extern "C" void* NapiEnv__bunVM(napi_env env) +{ + return env->bunVM(); +} + +extern "C" uint64_t NapiEnv__bunVMGeneration(napi_env env) +{ + return env->bunVMGeneration(); +} + extern "C" JSGlobalObject* NapiEnv__globalObject(napi_env env) { return env->globalObject(); diff --git a/src/jsc/bindings/napi.h b/src/jsc/bindings/napi.h index aa305eae35c3..37138ace6277 100644 --- a/src/jsc/bindings/napi.h +++ b/src/jsc/bindings/napi.h @@ -178,6 +178,8 @@ struct NapiEnv : public WTF::RefCounted { : m_globalObject(globalObject) , m_napiModule(napiModule) , m_vm(JSC::getVM(globalObject)) + , m_bunVM(globalObject->bunVM()) + , m_bunVMGeneration(globalObject->bunVMGeneration()) { napi_internal_register_cleanup_zig(this); } @@ -380,6 +382,11 @@ struct NapiEnv : public WTF::RefCounted { } inline Zig::GlobalObject* globalObject() const { return m_globalObject; } + // Schedule-time `(bunVM, generation)` capture (see Rust + // `live_vm_registry`): lets non-JS threads holding a NapiEnv ref build a + // checked VM handle without dereferencing the (possibly freed) global. + inline void* bunVM() const { return m_bunVM; } + inline uint64_t bunVMGeneration() const { return m_bunVMGeneration; } // `bun test --isolate` creates a fresh Zig::GlobalObject per file and // gcUnprotect()s the previous one. NapiEnv outlives its owning global — // GC-enqueued NapiFinalizerTasks hold a Ref and run on the event @@ -498,6 +505,8 @@ struct NapiEnv : public WTF::RefCounted { WTF::ListHashSet m_finalizers; bool m_isFinishingFinalizers = false; JSC::VM& m_vm; + void* m_bunVM; + uint64_t m_bunVMGeneration; Napi::HookSet m_cleanupHooks; JSC::Strong m_pendingException; size_t m_cleanupHookCounter = 0; diff --git a/src/jsc/bindings/webcore/BroadcastChannel.cpp b/src/jsc/bindings/webcore/BroadcastChannel.cpp index c88bb970df10..6ebf00d0e653 100644 --- a/src/jsc/bindings/webcore/BroadcastChannel.cpp +++ b/src/jsc/bindings/webcore/BroadcastChannel.cpp @@ -33,7 +33,7 @@ #include "SerializedScriptValue.h" #include -extern "C" void Bun__eventLoop__incrementRefConcurrently(void* bunVM, int delta); +extern "C" void Bun__eventLoop__incrementRefConcurrently(void* bunVM, int delta, uint64_t bunVMGeneration); namespace WebCore { @@ -131,7 +131,7 @@ void BroadcastChannel::jsRef(JSGlobalObject* lexicalGlobalObject) { if (!m_hasRef) { m_hasRef = true; - Bun__eventLoop__incrementRefConcurrently(WebCore::clientData(lexicalGlobalObject->vm())->bunVM, 1); + Bun__eventLoop__incrementRefConcurrently(WebCore::clientData(lexicalGlobalObject->vm())->bunVM, 1, WebCore::clientData(lexicalGlobalObject->vm())->bunVMGeneration); } } @@ -139,7 +139,7 @@ void BroadcastChannel::jsUnref(JSGlobalObject* lexicalGlobalObject) { if (m_hasRef) { m_hasRef = false; - Bun__eventLoop__incrementRefConcurrently(WebCore::clientData(lexicalGlobalObject->vm())->bunVM, -1); + Bun__eventLoop__incrementRefConcurrently(WebCore::clientData(lexicalGlobalObject->vm())->bunVM, -1, WebCore::clientData(lexicalGlobalObject->vm())->bunVMGeneration); } } diff --git a/src/jsc/bindings/webcore/MessagePort.cpp b/src/jsc/bindings/webcore/MessagePort.cpp index f152366907f2..b6d6f0a46060 100644 --- a/src/jsc/bindings/webcore/MessagePort.cpp +++ b/src/jsc/bindings/webcore/MessagePort.cpp @@ -36,7 +36,7 @@ #include "WebCoreOpaqueRoot.h" #include -extern "C" void Bun__eventLoop__incrementRefConcurrently(void* bunVM, int delta); +extern "C" void Bun__eventLoop__incrementRefConcurrently(void* bunVM, int delta, uint64_t bunVMGeneration); namespace WebCore { @@ -341,7 +341,7 @@ void MessagePort::jsRef(JSGlobalObject* lexicalGlobalObject) if (!m_hasRef) { m_hasRef = true; ref(); - Bun__eventLoop__incrementRefConcurrently(WebCore::clientData(lexicalGlobalObject->vm())->bunVM, 1); + Bun__eventLoop__incrementRefConcurrently(WebCore::clientData(lexicalGlobalObject->vm())->bunVM, 1, WebCore::clientData(lexicalGlobalObject->vm())->bunVMGeneration); } } @@ -350,7 +350,7 @@ void MessagePort::jsUnref(JSGlobalObject* lexicalGlobalObject) if (m_hasRef) { m_hasRef = false; deref(); - Bun__eventLoop__incrementRefConcurrently(WebCore::clientData(lexicalGlobalObject->vm())->bunVM, -1); + Bun__eventLoop__incrementRefConcurrently(WebCore::clientData(lexicalGlobalObject->vm())->bunVM, -1, WebCore::clientData(lexicalGlobalObject->vm())->bunVMGeneration); } } diff --git a/src/jsc/virtual_machine_exports.rs b/src/jsc/virtual_machine_exports.rs index cacc80863c1d..ed7ee22ef53a 100644 --- a/src/jsc/virtual_machine_exports.rs +++ b/src/jsc/virtual_machine_exports.rs @@ -31,6 +31,16 @@ pub fn get_vm() -> *mut VirtualMachine { VirtualMachine::get_mut_ptr() } +/// The current thread's VM generation (see `live_vm_registry`), for C++ call +/// sites that capture `(bunVM, generation)` pairs at creation time and later +/// reassemble a `VmHandle` for the checked concurrent entry points. +// HOST_EXPORT(Bun__getVmGeneration, c) +pub fn get_vm_generation() -> u64 { + VirtualMachine::get() + .live_generation + .load(core::sync::atomic::Ordering::Relaxed) +} + /// Caller must check for termination exception // HOST_EXPORT(Bun__drainMicrotasks, c) pub fn drain_microtasks() { diff --git a/src/runtime/bake/BakeGlobalObject.cpp b/src/runtime/bake/BakeGlobalObject.cpp index b86cfa16c340..e94fba43a6d5 100644 --- a/src/runtime/bake/BakeGlobalObject.cpp +++ b/src/runtime/bake/BakeGlobalObject.cpp @@ -197,6 +197,7 @@ JSC::Structure* GlobalObject::createStructure(JSC::VM& vm) struct BunVirtualMachine; extern "C" BunVirtualMachine* Bun__getVM(); +extern "C" uint64_t Bun__getVmGeneration(); const JSC::GlobalObjectMethodTable& GlobalObject::globalObjectMethodTable() { @@ -248,7 +249,7 @@ extern "C" GlobalObject* BakeCreateProdGlobal(void* console) vm.heap.acquireAccess(); JSC::JSLockHolder locker(vm); BunVirtualMachine* bunVM = Bun__getVM(); - WebCore::JSVMClientData::create(&vm, bunVM); + WebCore::JSVMClientData::create(&vm, bunVM, Bun__getVmGeneration()); JSC::Structure* structure = Bake::GlobalObject::createStructure(vm); Bake::GlobalObject* global = Bake::GlobalObject::create( diff --git a/src/runtime/napi/napi_body.rs b/src/runtime/napi/napi_body.rs index d86b612f2381..930c8d6163d2 100644 --- a/src/runtime/napi/napi_body.rs +++ b/src/runtime/napi/napi_body.rs @@ -108,6 +108,8 @@ bun_opaque::opaque_ffi! { unsafe extern "C" { fn NapiEnv__globalObject(env: *mut NapiEnv) -> *mut JSGlobalObject; + fn NapiEnv__bunVM(env: *mut NapiEnv) -> *mut core::ffi::c_void; + fn NapiEnv__bunVMGeneration(env: *mut NapiEnv) -> u64; fn NapiEnv__getAndClearPendingException(env: *mut NapiEnv, out: *mut JSValue) -> bool; fn napi_internal_get_version(env: *mut NapiEnv) -> u32; fn NapiEnv__deref(env: *mut NapiEnv); @@ -121,6 +123,20 @@ impl NapiEnv { unsafe { &*NapiEnv__globalObject(self.as_mut_ptr()) } } + /// Schedule-time [`VmHandle`] of the VM that owns this env, captured by + /// the C++ `NapiEnv` constructor. Safe to read from any thread holding an + /// env ref (plain data owned by the refcounted env); all VM access goes + /// through the checked `VirtualMachine` entry points. + pub fn vm_handle(&self) -> bun_jsc::virtual_machine::VmHandle { + // SAFETY: env is non-null; C++ side reads POD members only. + unsafe { + bun_jsc::virtual_machine::VmHandle::from_raw_parts( + NapiEnv__bunVM(self.as_mut_ptr()) as usize, + NapiEnv__bunVMGeneration(self.as_mut_ptr()), + ) + } + } + /// Convert err to an extern napi_status, and store the error code in env so that it can be /// accessed by napi_get_last_error_info pub fn set_last_error(self_: Option<&Self>, err: NapiStatus) -> napi_status { @@ -4258,32 +4274,33 @@ impl NapiFinalizerTask { } pub fn schedule(self: Box) { - // SAFETY: env is valid (held by NapiEnvRef). - let global_this = unsafe { &*self.finalizer.env.get() }.to_js(); - - // Inline of `JSGlobalObject::try_bun_vm` (the full impl lives in the - // gated `JSGlobalObject.rs`): the VM pointer is fetched unconditionally - // from C++; "main thread" is determined by whether the thread-local VM - // holder is populated. - // SAFETY: `bun_vm()` returns a valid `*mut VirtualMachine` for this global. - let vm: &VirtualMachine = global_this.bun_vm(); + // "Main thread" here means the thread owning some VM: the thread-local + // VM holder is populated. On any other thread nothing VM-owned may be + // dereferenced — the env's creation-time `(bunVM, generation)` capture + // is the only VM identity used. let is_main_thread = VirtualMachine::get_or_null().is_some(); if !is_main_thread { + // SAFETY: env is valid (held by NapiEnvRef); `vm_handle` reads + // POD members only. + let vm_handle = unsafe { &*self.finalizer.env.get() }.vm_handle(); let this = bun_core::heap::into_raw(self); // 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. 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(), + // in the dead worker's queue. + let _ = VirtualMachine::try_enqueue_task_concurrent( + vm_handle, ConcurrentTask::create(Task::init(this)), ); return; } + // SAFETY: env is valid (held by NapiEnvRef); on the owning thread the + // global and its VM are alive. + let global_this = unsafe { &*self.finalizer.env.get() }.to_js(); + let vm: &VirtualMachine = global_this.bun_vm(); + if vm.is_shutting_down() { if vm.has_run_cleanup_hooks() { // `on_exit()` already drained cleanup hooks; we are inside the diff --git a/src/runtime/webview/ChromeBackend.cpp b/src/runtime/webview/ChromeBackend.cpp index 6562810592bd..934418fa457d 100644 --- a/src/runtime/webview/ChromeBackend.cpp +++ b/src/runtime/webview/ChromeBackend.cpp @@ -99,7 +99,7 @@ extern "C" int32_t Bun__Chrome__ensure(Zig::GlobalObject*, const char* userDataD bool stdoutInherit, bool stderrInherit); extern "C" void* Blob__fromBytesWithType(JSC::JSGlobalObject*, const uint8_t* ptr, size_t len, const char* mime); extern "C" JSC::EncodedJSValue SYSV_ABI Blob__create(Zig::GlobalObject*, void* impl); -extern "C" void Bun__eventLoop__incrementRefConcurrently(void* bunVM, int delta); +extern "C" void Bun__eventLoop__incrementRefConcurrently(void* bunVM, int delta, uint64_t bunVMGeneration); extern "C" void Bun__EventLoop__enter(Zig::GlobalObject*); extern "C" void Bun__EventLoop__exit(Zig::GlobalObject*); extern "C" void Bun__EventLoop__runCallback2(JSGlobalObject*, EncodedJSValue cb, @@ -1307,7 +1307,8 @@ void Transport::updateKeepAlive() if (want == m_sockRefd || !m_global) return; m_sockRefd = want; Bun__eventLoop__incrementRefConcurrently( - WebCore::clientData(m_global->vm())->bunVM, want ? 1 : -1); + WebCore::clientData(m_global->vm())->bunVM, want ? 1 : -1, + WebCore::clientData(m_global->vm())->bunVMGeneration); // WebSocket mode: close the connection when the last view is gone. // We're connected to the USER'S Chrome — keeping the WS open after diff --git a/src/runtime/webview/WebKitBackend.cpp b/src/runtime/webview/WebKitBackend.cpp index cd961d74c420..349777ae9c4c 100644 --- a/src/runtime/webview/WebKitBackend.cpp +++ b/src/runtime/webview/WebKitBackend.cpp @@ -44,7 +44,7 @@ extern "C" int32_t Bun__WebViewHost__ensure(Zig::GlobalObject*, bool stdoutInher extern "C" void* Blob__fromMmapWithType(JSC::JSGlobalObject*, uint8_t* ptr, size_t len, const char* mime); extern "C" JSC::EncodedJSValue SYSV_ABI Blob__create(Zig::GlobalObject*, void* impl); extern "C" JSC::EncodedJSValue JSBuffer__fromMmap(Zig::GlobalObject*, void* ptr, size_t length); -extern "C" void Bun__eventLoop__incrementRefConcurrently(void* bunVM, int delta); +extern "C" void Bun__eventLoop__incrementRefConcurrently(void* bunVM, int delta, uint64_t bunVMGeneration); // Bracket the whole onData batch. exit() drains microtasks when outermost, // so all the promise reactions from this batch run before we return to usockets. extern "C" void Bun__EventLoop__enter(Zig::GlobalObject*); @@ -124,7 +124,8 @@ void HostClient::updateKeepAlive() if (want == sockRefd || !global) return; sockRefd = want; Bun__eventLoop__incrementRefConcurrently( - WebCore::clientData(global->vm())->bunVM, want ? 1 : -1); + WebCore::clientData(global->vm())->bunVM, want ? 1 : -1, + WebCore::clientData(global->vm())->bunVMGeneration); } bool HostClient::ensureSpawned(Zig::GlobalObject* zig, bool stdoutInherit, bool stderrInherit) From 950d3c9a960423811c1ebc41364d8548f6023e6f Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 12 Jun 2026 11:35:10 +0000 Subject: [PATCH 09/17] Remove a no-op borrow of the fetch tasklet's VM handle --- src/runtime/webcore/fetch/FetchTasklet.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/runtime/webcore/fetch/FetchTasklet.rs b/src/runtime/webcore/fetch/FetchTasklet.rs index cf99c1670e4a..a6e8413e8405 100644 --- a/src/runtime/webcore/fetch/FetchTasklet.rs +++ b/src/runtime/webcore/fetch/FetchTasklet.rs @@ -1741,7 +1741,6 @@ impl FetchTasklet { http_.enable_response_body_streaming(); } // we should not keep the process alive if we are ignoring the body - let _ = &self.javascript_vm; self.poll_ref.unref(bun_io::js_vm_ctx()); // clean any remaining references self.clear_stream_cancel_handler(); From 26fe7339c94cf7d4c575d857b2f3c626a1d51d67 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 12 Jun 2026 11:52:56 +0000 Subject: [PATCH 10/17] Load the positive-delivery worker from a file instead of a data: URL The worker body for the cross-thread completion test encoded to an 1837-byte data: URL, which fails to resolve on macOS with NameTooLong: the resolver's specifier length gate (1.5x MAX_PATH_BYTES, 1536 on macOS vs 6144 on Linux) runs before the data: scheme is recognized. Both macOS CI lanes failed on it; a real file avoids the limit entirely. --- .../workers/worker-terminate-lifetime.test.ts | 61 ++++++++++--------- 1 file changed, 33 insertions(+), 28 deletions(-) diff --git a/test/js/web/workers/worker-terminate-lifetime.test.ts b/test/js/web/workers/worker-terminate-lifetime.test.ts index 77e49ed344c4..340089a9362f 100644 --- a/test/js/web/workers/worker-terminate-lifetime.test.ts +++ b/test/js/web/workers/worker-terminate-lifetime.test.ts @@ -1,5 +1,5 @@ import { expect, test } from "bun:test"; -import { bunEnv, bunExe, isASAN, isDebug } from "harness"; +import { bunEnv, bunExe, isASAN, isDebug, tempDir } from "harness"; // Worker VM startup/teardown is much slower under debug and/or ASAN; these // tests spawn many workers, so scale iteration counts and timeouts down. @@ -130,33 +130,34 @@ test( 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 + "})()"), + // The worker body lives in a real file: as a data: URL it exceeds + // macOS's 1024-byte path-resolution limit (NameTooLong). + using dir = tempDir("worker-live-delivery", { + "worker.ts": ` + import { promises as fsPromises } from "node:fs"; + import zlib from "node:zlib"; + + const results: Record = {}; + 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 fsPromises.stat(process.execPath)).isFile(); + const gz: Buffer = 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); + `, + "main.ts": ` + const worker = new Worker(new URL("./worker.ts", import.meta.url).href); const results = await new Promise((resolve, reject) => { worker.onmessage = e => resolve(e.data); worker.onerror = e => reject(new Error(e.message)); @@ -164,8 +165,12 @@ test( worker.terminate(); console.log(JSON.stringify(results)); `, - ], + }); + + await using proc = Bun.spawn({ + cmd: [bunExe(), "main.ts"], env: bunEnv, + cwd: String(dir), stdout: "pipe", stderr: "pipe", }); From 6f4c56a2b75ccaa4a076d73b6bd8a260e74407a2 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 12 Jun 2026 12:25:17 +0000 Subject: [PATCH 11/17] Sync stale VM-lifetime comments with the handle semantics The S3 download-stream task's vm field kept the old 'per-thread VM singleton, outlives every task' doc after the field became a VmHandle; use the same wording as S3HttpSimpleTask. The node:fs AsyncFSTask global_object() doc still advertised the removed off-thread bun_vm_concurrently() caller, and the readdir-recursive accessor lost its last caller entirely, so it is deleted. --- src/runtime/node/node_fs.rs | 18 +++--------------- src/runtime/webcore/s3/download_stream.rs | 4 ++-- 2 files changed, 5 insertions(+), 17 deletions(-) diff --git a/src/runtime/node/node_fs.rs b/src/runtime/node/node_fs.rs index b801b59a4a7b..3a74eeaceea5 100644 --- a/src/runtime/node/node_fs.rs +++ b/src/runtime/node/node_fs.rs @@ -1276,8 +1276,9 @@ mod _async_tasks { /// Deref the raw `global_object` pointer. /// /// Invariant: set from a live `&JSGlobalObject` in `create()` and never - /// null; the JSC global outlives every task (JSC_BORROW per LIFETIMES.tsv). - /// Safe to call from the work-pool thread for `bun_vm_concurrently()`. + /// null. JS-thread only: the work-pool completion goes through the + /// captured `vm` handle instead, because the owning worker VM (and its + /// global) may be freed by terminate() while the task is in flight. #[inline] pub fn global_object(&self) -> &JSGlobalObject { self.global_object.get() @@ -2310,19 +2311,6 @@ mod _async_tasks { Box::new(init) } - /// Borrow the owning `JSGlobalObject`. - /// - /// SAFETY: `global_object` is set from a live `&JSGlobalObject` in - /// `create()` (never null) and the JSC_BORROW invariant (LIFETIMES.tsv) - /// guarantees the global outlives every task it spawns. The pointee is a - /// pinned JSC heap object; `bun_vm_concurrently()` is the only method we - /// call off-thread and it reads init-immutable state, so a shared borrow - /// is sound from both the JS thread and the work pool. - #[inline] - pub fn global_object(&self) -> &JSGlobalObject { - self.global_object.get() - } - /// Free `root_path` — paired with the NUL-terminated duplication in /// `create()`. Idempotent (empty `Box` after first call). fn free_root_path(&mut self) { diff --git a/src/runtime/webcore/s3/download_stream.rs b/src/runtime/webcore/s3/download_stream.rs index eea072a39132..506e8271b01a 100644 --- a/src/runtime/webcore/s3/download_stream.rs +++ b/src/runtime/webcore/s3/download_stream.rs @@ -19,8 +19,8 @@ pub struct S3HttpDownloadStreamingTask { // `MaybeUninit` because `AsyncHTTP` contains non-null references, so // `mem::zeroed()` can't be used here (mirrors `S3HttpSimpleTask`). 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). + /// 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, From a300ceb646581ddca1a8f8268c093ccb4176ec69 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 12 Jun 2026 12:57:18 +0000 Subject: [PATCH 12/17] Fix two more stale lifetime comments and ungroup the parked-tasklet static StatWatcherScheduler.vm still cited the enqueue_task_concurrent path that now goes through vm_handle; FSWatcher::enqueue_task_concurrent's doc only described the success path of the now-fallible enqueue. Also move DEAD_VM_TASKLETS below the import block it was splitting in FetchTasklet.rs. --- src/runtime/node/node_fs_stat_watcher.rs | 6 +++--- src/runtime/node/node_fs_watcher.rs | 6 ++++-- src/runtime/webcore/fetch/FetchTasklet.rs | 26 +++++++++++------------ 3 files changed, 20 insertions(+), 18 deletions(-) diff --git a/src/runtime/node/node_fs_stat_watcher.rs b/src/runtime/node/node_fs_stat_watcher.rs index 78811d558712..505e49093fa4 100644 --- a/src/runtime/node/node_fs_stat_watcher.rs +++ b/src/runtime/node/node_fs_stat_watcher.rs @@ -61,9 +61,9 @@ pub struct StatWatcherScheduler { is_shutdown: AtomicBool, task: WorkPoolTask, main_thread: ThreadId, - // JSC_BORROW per LIFETIMES.tsv — VM outlives the scheduler. `BackRef` gives - // safe `&VirtualMachine` projection (Deref) at every read site; - // `event_loop_shared()` / `enqueue_task_concurrent` take `&self`. + // JS-thread reads only (`timer_callback` via `vm()`), where the VM + // driving the timer is necessarily live. The pool-thread completion + // enqueue does not touch this field; it goes through `vm_handle` below. vm: BackRef, /// Schedule-time handle for `vm`, for the pool-thread completion enqueue /// (the one access that must tolerate the VM being gone). diff --git a/src/runtime/node/node_fs_watcher.rs b/src/runtime/node/node_fs_watcher.rs index 27c7ae136411..5febd9fad9e6 100644 --- a/src/runtime/node/node_fs_watcher.rs +++ b/src/runtime/node/node_fs_watcher.rs @@ -98,8 +98,10 @@ impl FSWatcher { } /// `task` must point to a live heap-allocated `ConcurrentTask` node that - /// the caller releases ownership of; the concurrent queue takes ownership - /// and frees it on the JS thread after dispatch. + /// the caller releases ownership of. On `true` the concurrent queue takes + /// ownership and frees it on the JS thread after dispatch; on `false` + /// (VM already gone) the node was never linked and the payload never + /// runs, so the caller reclaims it (see `FSWatchTaskPosix::enqueue`). #[must_use] pub fn enqueue_task_concurrent(&self, task: core::ptr::NonNull) -> bool { // Called from watcher threads: `ctx` may point at a worker VM freed diff --git a/src/runtime/webcore/fetch/FetchTasklet.rs b/src/runtime/webcore/fetch/FetchTasklet.rs index a6e8413e8405..8addb257ae1c 100644 --- a/src/runtime/webcore/fetch/FetchTasklet.rs +++ b/src/runtime/webcore/fetch/FetchTasklet.rs @@ -23,19 +23,6 @@ use bun_jsc::{ }; use bun_sys::FdExt; use bun_threading::Mutex; - -/// `FetchTasklet` boxes whose owning worker VM was freed (terminated) while -/// the HTTP thread held the last reference. They can never be fully -/// reclaimed: `deinit()` drops JSC `Strong`/`Weak` handles into the dead -/// VM's freed HandleSet, on any thread, at any time — including the -/// `global_exit` drain that `dealloc_for_shutdown` parks into. -/// [`FetchTasklet::free_native_data_for_dead_vm`] reclaims the plain-heap -/// buffers first, so what parks here is the struct itself plus the handles -/// only the (gone) JS thread could release. Kept reachable so LSan-enabled -/// CI lanes don't report them as leaks; growth is bounded by in-flight -/// requests that lose a terminate race. -static DEAD_VM_TASKLETS: bun_threading::Guarded> = - bun_threading::Guarded::new(Vec::new()); use bun_url::URL as ZigURL; use crate::api::bun_x509 as X509; @@ -56,6 +43,19 @@ type ElJsResult = bun_event_loop::JsResult; use boringssl::c::{X509_free, d2i_X509}; +/// `FetchTasklet` boxes whose owning worker VM was freed (terminated) while +/// the HTTP thread held the last reference. They can never be fully +/// reclaimed: `deinit()` drops JSC `Strong`/`Weak` handles into the dead +/// VM's freed HandleSet, on any thread, at any time, including the +/// `global_exit` drain that `dealloc_for_shutdown` parks into. +/// [`FetchTasklet::free_native_data_for_dead_vm`] reclaims the plain-heap +/// buffers first, so what parks here is the struct itself plus the handles +/// only the (gone) JS thread could release. Kept reachable so LSan-enabled +/// CI lanes don't report them as leaks; growth is bounded by in-flight +/// requests that lose a terminate race. +static DEAD_VM_TASKLETS: bun_threading::Guarded> = + bun_threading::Guarded::new(Vec::new()); + // ConcurrentTask::from() needs `Taskable`; tag is declared in bun_event_loop // but the impl lives next to the type (cycle-break). impl Taskable for FetchTasklet { From 26aa62e3d3eb5789b1282713191b99aba1b32111 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 12 Jun 2026 13:51:12 +0000 Subject: [PATCH 13/17] ci: retrigger From 1192b8253a32721e3ef8e82f5b2fb1b8a97f5a0d Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 10 Jul 2026 19:20:23 +0000 Subject: [PATCH 14/17] ci: re-run checks From d1c2a20b02bd03bcf6f9e4fcbcbb7abfa1359830 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 10 Jul 2026 19:54:36 +0000 Subject: [PATCH 15/17] Correct stale doc claims about MAIN_THREAD_VM publish, event_loop targets, and generation stamping Bun__setDefaultGlobalObject publishes MAIN_THREAD_VM during Zig__GlobalObject__create (after the lock-free readers' fields are initialized), so the end-of-init store is a re-store, not the first publish. vm.event_loop only ever points at the two embedded sibling loops (spawnSync swaps event_loop_handle, not this field). Generation stamping moved from register_vm to stamp_generation in c417173848; update the field docs that still cited register_vm. --- src/jsc/VirtualMachine.rs | 38 +++++++++++++++++++++++--------------- src/jsc/event_loop.rs | 5 +++-- 2 files changed, 26 insertions(+), 17 deletions(-) diff --git a/src/jsc/VirtualMachine.rs b/src/jsc/VirtualMachine.rs index f681c08f7f9e..f15914d63a82 100644 --- a/src/jsc/VirtualMachine.rs +++ b/src/jsc/VirtualMachine.rs @@ -280,16 +280,18 @@ pub struct VirtualMachine { pub overridden_performance_now: Option, pub macro_event_loop: EventLoop, pub regular_event_loop: EventLoop, - /// BORROW_FIELD — points at sibling `regular_event_loop`/`macro_event_loop` - /// (or the boxed spawnSync loop). Written only by the JS thread (init, - /// macro-mode swap, spawnSync swap); atomic because the checked + /// BORROW_FIELD — points at one of the sibling embedded loops + /// (`regular_event_loop`/`macro_event_loop`), never the boxed spawnSync + /// loop (spawnSync swaps only `event_loop_handle`). Written only by the + /// JS thread (init, macro-mode swaps); atomic because the checked /// 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. + /// Registration generation from [`live_vm_registry`], stamped once by + /// `stamp_generation` early in `init` (0 = not yet stamped). 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, @@ -541,7 +543,8 @@ pub(crate) mod live_vm_registry { /// 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`). + /// handle (zero-initialised VM allocations carry it until + /// `stamp_generation`). static NEXT_GENERATION: AtomicU64 = AtomicU64::new(1); fn mint_generation() -> u64 { @@ -1402,7 +1405,7 @@ impl VirtualMachine { 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 + // `stamp_generation` stamped; restore it (same address, same // registration — the registry entry is untouched). self.macro_event_loop.live_generation = self .live_generation @@ -2285,8 +2288,12 @@ impl VirtualMachine { // NOTE: `MAIN_THREAD_VM` is deliberately NOT published here — it is // the lock-free liveness fast path in `with_live_vm` / // `get_main_thread_vm`, which dereference it from other threads, so - // it must not point at this still-zeroed allocation. Published at the - // end of `init`, next to `register_vm`. + // it must not point at this still-zeroed allocation. The first + // publish is `Bun__setDefaultGlobalObject` during + // `Zig__GlobalObject__create`, by which point the fields those + // readers touch (`event_loop`, `live_generation`, + // `event_loop_handle`) are initialized; `init` re-stores it at the + // end, next to `register_vm`. // ConsoleObject is self-referential (buffers + adapters) — allocate // stable storage and init in place. @@ -2486,10 +2493,11 @@ impl VirtualMachine { // unregistered in `WebWorker::shutdown()` before the allocation is // freed; the main VM stays registered forever. // - // `MAIN_THREAD_VM` is published only now, for the same reason: - // `with_live_vm` / `get_main_thread_vm` dereference it from other - // threads without the registry lock, so it must never point at a - // partially initialized VM. + // Re-store `MAIN_THREAD_VM` next to the registration it pairs with. + // The first publish happened in `Bun__setDefaultGlobalObject` during + // `Zig__GlobalObject__create`, after the fields the lock-free + // readers touch were initialized; this store is the explicit + // end-of-init anchor. if opts.is_main_thread { MAIN_THREAD_VM.store(vm, core::sync::atomic::Ordering::Release); } diff --git a/src/jsc/event_loop.rs b/src/jsc/event_loop.rs index 590ad38c3493..ebdd3dc5d1e3 100644 --- a/src/jsc/event_loop.rs +++ b/src/jsc/event_loop.rs @@ -108,8 +108,9 @@ pub struct EventLoop { 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 + /// `stamp_generation` (embedded VM loops) / `register_extra_loop` (boxed + /// spawnSync loops); 0 = not stamped. Read on the owning thread only, by + /// [`EventLoop::concurrent_handle`] and the /// `JsEventLoop::live_generation` dispatch. pub(crate) live_generation: u64, } From 7f785818671d31a0d4a16375f44052b771d54721 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 10 Jul 2026 20:25:45 +0000 Subject: [PATCH 16/17] Fix three more doc comments contradicted by the generation capture The event_loop() accessor body still mentioned the boxed spawnSync loop after d1c2a20b02 corrected the field doc; EventLoopHandle::init and from_tag_ptr's Safety doc still claimed the constructor does not dereference the pointer, but both read the loop's registration generation at construction. --- src/event_loop/AnyEventLoop.rs | 19 ++++++++++--------- src/jsc/VirtualMachine.rs | 10 +++++----- 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/src/event_loop/AnyEventLoop.rs b/src/event_loop/AnyEventLoop.rs index 1c7df0ad7ac1..f35337497adb 100644 --- a/src/event_loop/AnyEventLoop.rs +++ b/src/event_loop/AnyEventLoop.rs @@ -393,11 +393,12 @@ impl EventLoopHandle { /// bun_runtime since it must call `vm.eventLoop()`.) /// /// `js_event_loop` is a live erased `*mut jsc::EventLoop` whose owner - /// outlives every dispatch through the returned handle. The pointer is not - /// dereferenced here — it's stored opaquely in [`JsEventLoop`] and only - /// dereferenced at dispatch sites. A null pointer is a documented sentinel + /// outlives every dispatch through the returned handle. The pointer is + /// stored opaquely in [`JsEventLoop`]; the generation capture reads it + /// once here, while it is live per this contract, and dispatch sites + /// dereference it afterwards. A null pointer is a documented sentinel /// for "never dispatched" placeholders (e.g. struct field initialisers - /// that are overwritten before use). + /// that are overwritten before use) and skips the generation read. #[inline] pub fn init(js_event_loop: *mut ()) -> EventLoopHandle { let owner = jsc_event_loop_handle(js_event_loop); @@ -457,11 +458,11 @@ impl EventLoopHandle { /// /// # Safety /// `(tag, ptr)` must have been produced by [`into_tag_ptr`] on a still-live - /// event loop. The constructor itself only stores the opaque pointer, but - /// dispatch through the resulting handle dereferences it — this fn is the - /// last place the precondition can be discharged. (NOT eligible for - /// `unsafe-fn-narrow`: the invariant is caller-provided, not internally - /// guarded.) + /// event loop. The constructor reads the loop's registration generation + /// (via `js_loop_generation`) and dispatch through the resulting handle + /// dereferences it — this fn is the last place the precondition can be + /// discharged. (NOT eligible for `unsafe-fn-narrow`: the invariant is + /// caller-provided, not internally guarded.) #[inline] pub unsafe fn from_tag_ptr( tag: core::ffi::c_char, diff --git a/src/jsc/VirtualMachine.rs b/src/jsc/VirtualMachine.rs index f15914d63a82..746186de1bb3 100644 --- a/src/jsc/VirtualMachine.rs +++ b/src/jsc/VirtualMachine.rs @@ -980,11 +980,11 @@ impl VirtualMachine { /// short-lived `&mut *p` at the use site instead, mirroring [`Self::get`]. #[inline(always)] pub fn event_loop(&self) -> *mut EventLoop { - // self-pointer to regular_event_loop or macro_event_loop (or the - // boxed spawnSync loop). Acquire pairs with the Release stores so a - // cross-thread reader that observes a freshly-swapped-in loop also - // observes its initialization; same-thread readers are ordered by - // program order regardless. + // self-pointer to regular_event_loop or macro_event_loop. Acquire + // pairs with the Release stores so a cross-thread reader that + // observes a freshly-swapped-in loop also observes its + // initialization; same-thread readers are ordered by program order + // regardless. self.event_loop.load(core::sync::atomic::Ordering::Acquire) } From 76cb8a3e03667047a14b58afd8c9f76e1c849d2d Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 10 Jul 2026 20:54:28 +0000 Subject: [PATCH 17/17] Qualify the link_impl_JsEventLoop header for the cross-thread enqueue arm Every other arm dereferences this on the JS thread; the checked enqueue_task_concurrent arm runs on producer threads and never dereferences this, which the block header previously contradicted. --- src/jsc/event_loop.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/jsc/event_loop.rs b/src/jsc/event_loop.rs index ebdd3dc5d1e3..4c75603d30ad 100644 --- a/src/jsc/event_loop.rs +++ b/src/jsc/event_loop.rs @@ -1403,8 +1403,10 @@ fn el_ref<'a>(owner: *mut ()) -> &'a mut EventLoop { } // `this: *mut EventLoop` — owner was erased from a live `*mut EventLoop` in -// `__bun_js_event_loop_current` / `EventLoopHandle::js`. All calls run on the -// JS thread. +// `__bun_js_event_loop_current` / `EventLoopHandle::js`. All calls except +// `enqueue_task_concurrent` run on the JS thread with `this` live; that one +// is the checked cross-thread entry and never dereferences `this` (see its +// comment). bun_event_loop::link_impl_JsEventLoop! { Jsc for EventLoop => |this| { // Reads the EventLoop's own `uws_loop` field; on