From 5e251dca1ba9f27960d7c28bf13f9807885aeaec Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 31 Jul 2026 16:31:09 +0000 Subject: [PATCH 1/7] fetch: fence HTTP-thread FetchTasklet callbacks against worker VM dealloc A worker's VirtualMachine is raw-dealloc'd by WebWorker::shutdown while the shared HTTP client thread can still deliver FetchTasklet callbacks for that worker's in-flight requests. Those callbacks dereferenced javascript_vm (a lifetime-erased &'static VirtualMachine) to read is_shutting_down and to enqueue_task_concurrent, which is a heap-use-after-free on the freed VM storage and crashes the whole process. Add a per-VM Arc that tasklets clone at creation. The three HTTP-thread entry points (callback, deref_from_thread, on_write_request_data_drain) now read the shutdown flag from the Arc and hold a reader count across the VM dereference. WebWorker::shutdown marks the signal early and spins on the reader count before freeing the VM, so every in-flight callback either observes shutting_down and takes the early-return path or completes its enqueue before the dealloc. Tasklets orphaned by a dead worker VM are leaked rather than parked in the process-exit reclaim list, since the parked deinit() would touch JSC handles in freed storage; the large body buffers are released before the leak. --- src/jsc/VirtualMachine.rs | 115 ++++++++++++++++++++++ src/jsc/web_worker.rs | 13 +++ src/runtime/webcore/fetch/FetchTasklet.rs | 32 ++++-- 3 files changed, 154 insertions(+), 6 deletions(-) diff --git a/src/jsc/VirtualMachine.rs b/src/jsc/VirtualMachine.rs index e64145537196..7631555f97af 100644 --- a/src/jsc/VirtualMachine.rs +++ b/src/jsc/VirtualMachine.rs @@ -69,6 +69,95 @@ pub type ExceptionList = Vec; // VirtualMachine struct (file-level @This()) // ────────────────────────────────────────────────────────────────────────── +/// Per-VM shutdown flag readable from threads that may outlive the VM. +/// +/// A worker's [`WebWorker::shutdown`] raw-`dealloc`s the `VirtualMachine` +/// while the shared HTTP client thread can still hold tasklets that captured +/// `&'static VirtualMachine`. Those tasklets clone this `Arc` at creation and +/// use it, instead of the VM reference, to decide whether the VM is still +/// accepting work. The reader count lets shutdown wait out any callback that +/// loaded `shutting_down == false` and is mid-enqueue, so the VM is never +/// freed underneath an `enqueue_task_concurrent`. +/// +/// [`WebWorker::shutdown`]: crate::web_worker::WebWorker::shutdown +pub struct CrossThreadShutdownSignal { + shutting_down: core::sync::atomic::AtomicBool, + readers: core::sync::atomic::AtomicUsize, + /// `true` for the process-lifetime main-thread VM. Main-thread shutdown + /// already parks the HTTP thread via `bun_http::shutdown_for_exit()` and + /// then drains parked tasklets before `destructOnExit`; worker shutdown has + /// no such drain, so a tasklet orphaned by a dead worker VM is leaked + /// instead of parked (its JSC handles point into freed storage). + is_main_thread: bool, +} + +impl CrossThreadShutdownSignal { + fn new(is_main_thread: bool) -> std::sync::Arc { + std::sync::Arc::new(Self { + shutting_down: core::sync::atomic::AtomicBool::new(false), + readers: core::sync::atomic::AtomicUsize::new(0), + is_main_thread, + }) + } + + #[inline] + pub fn is_shutting_down(&self) -> bool { + self.shutting_down.load(core::sync::atomic::Ordering::Acquire) + } + + #[inline] + pub fn is_main_thread(&self) -> bool { + self.is_main_thread + } + + /// Mark the VM as shutting down for cross-thread readers. Idempotent. + pub fn mark_shutting_down(&self) { + self.shutting_down + .store(true, core::sync::atomic::Ordering::SeqCst); + } + + /// Spin until every cross-thread reader that entered via + /// [`Self::try_begin_vm_read`] has called [`Self::end_vm_read`]. Must be + /// preceded by [`Self::mark_shutting_down`] so new readers bail instead of + /// re-entering; otherwise this can spin indefinitely. + pub fn wait_for_readers(&self) { + debug_assert!(self.is_shutting_down()); + while self.readers.load(core::sync::atomic::Ordering::SeqCst) > 0 { + std::hint::spin_loop(); + } + } + + /// Enter a read-side critical section in which the caller may dereference + /// the `VirtualMachine` that owns this signal. Returns `false` if shutdown + /// has begun; in that case the caller must not touch the VM and no + /// matching [`Self::end_vm_read`] is owed. On `true` the caller must pair + /// with exactly one [`Self::end_vm_read`]. + /// + /// SeqCst on both the increment and the flag load, together with the + /// SeqCst store + load in [`Self::mark_shutting_down`] / + /// [`Self::wait_for_readers`], establishes a total order: a reader that + /// observes `shutting_down == false` incremented `readers` before the + /// writer loaded it as zero, so the writer spins until the reader exits. + #[inline] + #[must_use] + pub fn try_begin_vm_read(&self) -> bool { + self.readers + .fetch_add(1, core::sync::atomic::Ordering::SeqCst); + if self.shutting_down.load(core::sync::atomic::Ordering::SeqCst) { + self.readers + .fetch_sub(1, core::sync::atomic::Ordering::SeqCst); + return false; + } + true + } + + #[inline] + pub fn end_vm_read(&self) { + self.readers + .fetch_sub(1, core::sync::atomic::Ordering::SeqCst); + } +} + #[derive(Default)] pub struct EntryPointResult { pub value: crate::strong::Optional, // jsc.Strong.Optional @@ -207,6 +296,15 @@ pub struct VirtualMachine { pub(crate) hide_bun_stackframes: bool, pub is_shutting_down: bool, + /// Arc'd mirror of [`Self::is_shutting_down`] for readers on other + /// threads. The HTTP client thread is shared across every worker and may + /// deliver a `FetchTasklet` callback after a worker's `shutdown()` has + /// `dealloc`'d this struct, so those callbacks read the flag (and fence + /// against the dealloc) through this handle instead of `&VirtualMachine`. + /// `Some` from `init()` through `destroy()`; `Option` only so `destroy()` + /// can release this VM's strong ref (the box is raw-`dealloc`'d without + /// running field `Drop`s). + pub cross_thread_shutdown: Option>, /// 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 @@ -979,6 +1077,16 @@ impl VirtualMachine { self.is_shutting_down } + /// The per-VM [`CrossThreadShutdownSignal`]. `Some` from `init()` until + /// `destroy()`; every path that can call `fetch()` (and so clone the + /// handle) runs between those two points. + #[inline] + pub fn cross_thread_shutdown(&self) -> &std::sync::Arc { + self.cross_thread_shutdown + .as_ref() + .expect("cross_thread_shutdown is Some from init() to destroy()") + } + pub fn has_run_cleanup_hooks(&self) -> bool { self.has_run_cleanup_hooks } @@ -1488,6 +1596,7 @@ impl VirtualMachine { } self.is_shutting_down = true; + self.cross_thread_shutdown().mark_shutting_down(); // Make sure we run new cleanup hooks introduced by running cleanup // hooks. @@ -2110,6 +2219,8 @@ impl VirtualMachine { addr_of_mut!((*vm).resolved_path_dups).write(Vec::new()); addr_of_mut!((*vm).macros).write(Default::default()); addr_of_mut!((*vm).macro_entry_points).write(Default::default()); + addr_of_mut!((*vm).cross_thread_shutdown) + .write(Some(CrossThreadShutdownSignal::new(opts.is_main_thread))); addr_of_mut!((*vm).auto_killer).write(Default::default()); addr_of_mut!((*vm).commonjs_custom_extensions).write(Default::default()); addr_of_mut!((*vm).entry_point).write(Default::default()); @@ -4463,6 +4574,10 @@ impl VirtualMachine { // proxy strings; `ProxyEnvStorage: Default` so take()+drop suffices. drop(core::mem::take(&mut self.proxy_env_storage)); + // Release this VM's strong ref; in-flight tasklets on the HTTP thread + // may still hold clones past the raw dealloc below. + drop(self.cross_thread_shutdown.take()); + // The VM box is `dealloc`'d raw by the worker (see `web_worker.rs` // section 5) so field `Drop`s never run; reclaim the boxed // `ModuleLoader` payloads explicitly. `eval_source.contents` may be diff --git a/src/jsc/web_worker.rs b/src/jsc/web_worker.rs index 6e67c931d47c..469d024e73b3 100644 --- a/src/jsc/web_worker.rs +++ b/src/jsc/web_worker.rs @@ -1248,6 +1248,10 @@ impl WebWorker { // re-sets it for the JSC VM teardown. vm.jsc_vm().clear_has_termination_request(); vm.is_shutting_down = true; + // Publish shutdown to HTTP-thread FetchTasklet callbacks before + // on_exit()'s user JS runs, so fewer of them enter the reader + // fence. on_exit() re-stores it (idempotent). + vm.cross_thread_shutdown().mark_shutting_down(); vm.on_exit(); if let Some(hooks) = runtime_hooks() { (hooks.cron_clear_all_teardown)(vm); @@ -1302,6 +1306,15 @@ impl WebWorker { // or observes m_isShuttingDown under m_lock and drops. Idempotent; // teardownJSCVM sets it again. Bun__JSCTaskScheduler__markShuttingDown(vm.global()); + // Fence the HTTP client thread: any FetchTasklet callback that + // loaded `shutting_down == false` and is about to push to / wake + // this VM's event loop has incremented the reader count. Spin + // until they exit, so the drain below sees every such task and + // `enqueue_task_concurrent` never touches the storage freed in + // step 5. The main-thread `global_exit()` path parks the whole + // HTTP thread via `bun_http::shutdown_for_exit()` instead; that is + // process-global and cannot be used per-worker. + vm.cross_thread_shutdown().wait_for_readers(); // Reclaim queued CppTasks (the per-worker stdio/messaging // MessagePort drain tasks that can be in self.tasks mid-tick when // terminate() lands, and any Worker dispatchExit close task from a diff --git a/src/runtime/webcore/fetch/FetchTasklet.rs b/src/runtime/webcore/fetch/FetchTasklet.rs index 93c274dc1369..9bc8d2887a0d 100644 --- a/src/runtime/webcore/fetch/FetchTasklet.rs +++ b/src/runtime/webcore/fetch/FetchTasklet.rs @@ -17,7 +17,7 @@ use bun_http::{ }; use bun_io::KeepAlive; use bun_jsc::debugger::AsyncTaskTracker; -use bun_jsc::virtual_machine::VirtualMachine; +use bun_jsc::virtual_machine::{CrossThreadShutdownSignal, VirtualMachine}; use bun_jsc::{ self as jsc, GlobalRef, JSGlobalObject, JSValue, JsResult, StringJsc, StrongOptional, }; @@ -68,6 +68,13 @@ pub struct FetchTasklet { pub(crate) result: HTTPClientResult<'static>, pub(crate) metadata: Option, pub(crate) javascript_vm: &'static VirtualMachine, + /// Clone of `javascript_vm.cross_thread_shutdown`. The HTTP-thread + /// callbacks below read the shutdown state and fence the + /// `enqueue_task_concurrent` against `WebWorker::shutdown` through this + /// handle: `javascript_vm` is a lifetime-erased reference that dangles + /// once a worker has `dealloc`'d its VM, so it may only be dereferenced + /// inside a successful `try_begin_vm_read` section. + pub(crate) vm_shutdown_signal: std::sync::Arc, pub global_this: GlobalRef, pub(crate) request_body: HTTPRequestBody, // ThreadSafeStreamBuffer is intrusively refcounted (`ref_count: AtomicU32`, @@ -397,7 +404,7 @@ impl FetchTasklet { return; } let self_ = Self::from_raw_ref(this); - if self_.javascript_vm.is_shutting_down() { + if !self_.vm_shutdown_signal.try_begin_vm_read() { // SAFETY: last ref; exclusive access. `deinit()` would run // `clear_data()` + `Drop` for the JSC `Strong`/`Weak` fields, which // reach into the VM's StrongRootBlock list / WeakSet from this @@ -414,6 +421,7 @@ impl FetchTasklet { self_.javascript_vm, ConcurrentTask::from_callback(this, FetchTasklet::deinit_callback), ); + self_.vm_shutdown_signal.end_vm_read(); } // ConcurrentTask::from_callback takes `fn(*mut T) -> bun_event_loop::JsResult<()>` @@ -518,19 +526,28 @@ impl FetchTasklet { /// (`on_response_finalize`) registered against `this`, so freeing the /// box before `destructOnExit` sweeps the Response is a UAF. /// - /// Park the intact box on the JS thread via + /// Main-thread VM: park the intact box on the JS thread via /// `bun_http::defer_shutdown_reclaim`; the drain runs from /// `global_exit()` after the HTTP thread has parked but before /// `destructOnExit`, so `deinit()` there can release every handle on the /// right thread and the Weak is cleared before its referent is finalized. /// + /// Worker VM: there is no per-worker drain point (`shutdown_for_exit` is + /// process-global) and by the time this runs the worker's JSC heap is, or + /// is about to be, freed, so the parked `deinit()` would dereference dead + /// handles. Leak the box instead; the large buffers were already released + /// by the caller. + /// /// SAFETY: `this` must be the last reference (ref_count == 0) and have /// been allocated via heap::alloc. unsafe fn dealloc_for_shutdown(this: *mut FetchTasklet) { bun_output::scoped_log!(FetchTasklet, "deallocForShutdown"); // SAFETY: caller contract — `this` is live with ref_count == 0. unsafe { (*this).ref_count.assert_no_refs() }; - http::defer_shutdown_reclaim(this.cast(), FetchTasklet::deinit_erased); + // SAFETY: caller contract — `this` is live with ref_count == 0. + if unsafe { (*this).vm_shutdown_signal.is_main_thread() } { + http::defer_shutdown_reclaim(this.cast(), FetchTasklet::deinit_erased); + } } unsafe fn deinit_erased(this: *mut c_void) { @@ -2027,6 +2044,7 @@ impl FetchTasklet { result: HTTPClientResult::default(), metadata: None, javascript_vm: jsc_vm, + vm_shutdown_signal: std::sync::Arc::clone(jsc_vm.cross_thread_shutdown()), global_this: GlobalRef::from(global_this), request_body: fetch_options.body, request_body_streaming_buffer: None, @@ -2271,7 +2289,7 @@ impl FetchTasklet { /// This is ALWAYS called from the http thread and we cannot touch the buffer here because is locked 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() { + if !this_ref.vm_shutdown_signal.try_begin_vm_read() { return; } // ref until the main thread callback is called @@ -2282,6 +2300,7 @@ impl FetchTasklet { this_ref.javascript_vm, ConcurrentTask::from_callback(this, FetchTasklet::resume_request_data_stream), ); + this_ref.vm_shutdown_signal.end_vm_read(); } /// This is ALWAYS called from the main thread @@ -2649,7 +2668,7 @@ impl FetchTasklet { } } // will deinit when done with the http client (when is_done = true) - if task_ref.javascript_vm.is_shutting_down() { + if !task_ref.vm_shutdown_signal.try_begin_vm_read() { // 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(); @@ -2689,6 +2708,7 @@ impl FetchTasklet { // `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.vm_shutdown_signal.end_vm_read(); task_ref.mutex.unlock(); // we are done with the http client so we can deref our side From dfd394ae706a727349f7fd8a70e23df26bb48384 Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Fri, 31 Jul 2026 16:33:57 +0000 Subject: [PATCH 2/7] [autofix.ci] apply automated fixes --- src/jsc/VirtualMachine.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/jsc/VirtualMachine.rs b/src/jsc/VirtualMachine.rs index 7631555f97af..3ba904b30ee1 100644 --- a/src/jsc/VirtualMachine.rs +++ b/src/jsc/VirtualMachine.rs @@ -102,7 +102,8 @@ impl CrossThreadShutdownSignal { #[inline] pub fn is_shutting_down(&self) -> bool { - self.shutting_down.load(core::sync::atomic::Ordering::Acquire) + self.shutting_down + .load(core::sync::atomic::Ordering::Acquire) } #[inline] @@ -143,7 +144,10 @@ impl CrossThreadShutdownSignal { pub fn try_begin_vm_read(&self) -> bool { self.readers .fetch_add(1, core::sync::atomic::Ordering::SeqCst); - if self.shutting_down.load(core::sync::atomic::Ordering::SeqCst) { + if self + .shutting_down + .load(core::sync::atomic::Ordering::SeqCst) + { self.readers .fetch_sub(1, core::sync::atomic::Ordering::SeqCst); return false; From bdb19154d7fab0f61ddeb48226ca0bc84f6151b2 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 31 Jul 2026 16:37:42 +0000 Subject: [PATCH 3/7] tighten doc comments per comment-cop --- src/jsc/VirtualMachine.rs | 57 ++++++----------------- src/jsc/web_worker.rs | 14 ++---- src/runtime/webcore/fetch/FetchTasklet.rs | 16 ++----- 3 files changed, 22 insertions(+), 65 deletions(-) diff --git a/src/jsc/VirtualMachine.rs b/src/jsc/VirtualMachine.rs index 3ba904b30ee1..b67df023b784 100644 --- a/src/jsc/VirtualMachine.rs +++ b/src/jsc/VirtualMachine.rs @@ -69,25 +69,17 @@ pub type ExceptionList = Vec; // VirtualMachine struct (file-level @This()) // ────────────────────────────────────────────────────────────────────────── -/// Per-VM shutdown flag readable from threads that may outlive the VM. -/// -/// A worker's [`WebWorker::shutdown`] raw-`dealloc`s the `VirtualMachine` -/// while the shared HTTP client thread can still hold tasklets that captured -/// `&'static VirtualMachine`. Those tasklets clone this `Arc` at creation and -/// use it, instead of the VM reference, to decide whether the VM is still -/// accepting work. The reader count lets shutdown wait out any callback that -/// loaded `shutting_down == false` and is mid-enqueue, so the VM is never -/// freed underneath an `enqueue_task_concurrent`. +/// Arc'd per-VM shutdown flag + reader fence for threads that may outlive the +/// VM. A worker's [`WebWorker::shutdown`] raw-`dealloc`s the `VirtualMachine` +/// while the shared HTTP client thread can still be delivering `FetchTasklet` +/// callbacks that dereference it; those callbacks hold a clone of this Arc +/// and bracket every VM access with [`Self::try_begin_vm_read`] / +/// [`Self::end_vm_read`] so shutdown can spin the readers out first. /// /// [`WebWorker::shutdown`]: crate::web_worker::WebWorker::shutdown pub struct CrossThreadShutdownSignal { shutting_down: core::sync::atomic::AtomicBool, readers: core::sync::atomic::AtomicUsize, - /// `true` for the process-lifetime main-thread VM. Main-thread shutdown - /// already parks the HTTP thread via `bun_http::shutdown_for_exit()` and - /// then drains parked tasklets before `destructOnExit`; worker shutdown has - /// no such drain, so a tasklet orphaned by a dead worker VM is leaked - /// instead of parked (its JSC handles point into freed storage). is_main_thread: bool, } @@ -111,16 +103,11 @@ impl CrossThreadShutdownSignal { self.is_main_thread } - /// Mark the VM as shutting down for cross-thread readers. Idempotent. pub fn mark_shutting_down(&self) { self.shutting_down .store(true, core::sync::atomic::Ordering::SeqCst); } - /// Spin until every cross-thread reader that entered via - /// [`Self::try_begin_vm_read`] has called [`Self::end_vm_read`]. Must be - /// preceded by [`Self::mark_shutting_down`] so new readers bail instead of - /// re-entering; otherwise this can spin indefinitely. pub fn wait_for_readers(&self) { debug_assert!(self.is_shutting_down()); while self.readers.load(core::sync::atomic::Ordering::SeqCst) > 0 { @@ -128,17 +115,12 @@ impl CrossThreadShutdownSignal { } } - /// Enter a read-side critical section in which the caller may dereference - /// the `VirtualMachine` that owns this signal. Returns `false` if shutdown - /// has begun; in that case the caller must not touch the VM and no - /// matching [`Self::end_vm_read`] is owed. On `true` the caller must pair - /// with exactly one [`Self::end_vm_read`]. - /// - /// SeqCst on both the increment and the flag load, together with the - /// SeqCst store + load in [`Self::mark_shutting_down`] / - /// [`Self::wait_for_readers`], establishes a total order: a reader that - /// observes `shutting_down == false` incremented `readers` before the - /// writer loaded it as zero, so the writer spins until the reader exits. + /// On `true` the caller may dereference the owning VM until the paired + /// [`Self::end_vm_read`]; on `false` the VM is (or is about to be) freed + /// and no `end_vm_read` is owed. SeqCst on all four ops (this increment + + /// flag load, and the writer's flag store + reader-count load) is the + /// Dekker-style fence that keeps "reader saw `false`" ordered before + /// "writer saw `readers == 0`". #[inline] #[must_use] pub fn try_begin_vm_read(&self) -> bool { @@ -300,14 +282,8 @@ pub struct VirtualMachine { pub(crate) hide_bun_stackframes: bool, pub is_shutting_down: bool, - /// Arc'd mirror of [`Self::is_shutting_down`] for readers on other - /// threads. The HTTP client thread is shared across every worker and may - /// deliver a `FetchTasklet` callback after a worker's `shutdown()` has - /// `dealloc`'d this struct, so those callbacks read the flag (and fence - /// against the dealloc) through this handle instead of `&VirtualMachine`. - /// `Some` from `init()` through `destroy()`; `Option` only so `destroy()` - /// can release this VM's strong ref (the box is raw-`dealloc`'d without - /// running field `Drop`s). + /// See [`CrossThreadShutdownSignal`]. `Option` only so `destroy()` can + /// release the strong ref (the box is raw-`dealloc`'d, no field `Drop`s). pub cross_thread_shutdown: Option>, /// Set once `on_exit()` has finished draining `RareData::cleanup_hooks`. /// After this point the cleanup-hook list is never iterated again, so @@ -1081,9 +1057,6 @@ impl VirtualMachine { self.is_shutting_down } - /// The per-VM [`CrossThreadShutdownSignal`]. `Some` from `init()` until - /// `destroy()`; every path that can call `fetch()` (and so clone the - /// handle) runs between those two points. #[inline] pub fn cross_thread_shutdown(&self) -> &std::sync::Arc { self.cross_thread_shutdown @@ -4578,8 +4551,6 @@ impl VirtualMachine { // proxy strings; `ProxyEnvStorage: Default` so take()+drop suffices. drop(core::mem::take(&mut self.proxy_env_storage)); - // Release this VM's strong ref; in-flight tasklets on the HTTP thread - // may still hold clones past the raw dealloc below. drop(self.cross_thread_shutdown.take()); // The VM box is `dealloc`'d raw by the worker (see `web_worker.rs` diff --git a/src/jsc/web_worker.rs b/src/jsc/web_worker.rs index 469d024e73b3..6960878d512b 100644 --- a/src/jsc/web_worker.rs +++ b/src/jsc/web_worker.rs @@ -1248,9 +1248,6 @@ impl WebWorker { // re-sets it for the JSC VM teardown. vm.jsc_vm().clear_has_termination_request(); vm.is_shutting_down = true; - // Publish shutdown to HTTP-thread FetchTasklet callbacks before - // on_exit()'s user JS runs, so fewer of them enter the reader - // fence. on_exit() re-stores it (idempotent). vm.cross_thread_shutdown().mark_shutting_down(); vm.on_exit(); if let Some(hooks) = runtime_hooks() { @@ -1306,14 +1303,9 @@ impl WebWorker { // or observes m_isShuttingDown under m_lock and drops. Idempotent; // teardownJSCVM sets it again. Bun__JSCTaskScheduler__markShuttingDown(vm.global()); - // Fence the HTTP client thread: any FetchTasklet callback that - // loaded `shutting_down == false` and is about to push to / wake - // this VM's event loop has incremented the reader count. Spin - // until they exit, so the drain below sees every such task and - // `enqueue_task_concurrent` never touches the storage freed in - // step 5. The main-thread `global_exit()` path parks the whole - // HTTP thread via `bun_http::shutdown_for_exit()` instead; that is - // process-global and cannot be used per-worker. + // Per-worker fence for HTTP-thread FetchTasklet callbacks (the + // main-thread equivalent is `bun_http::shutdown_for_exit()`, a + // process-global one-shot). See `CrossThreadShutdownSignal`. vm.cross_thread_shutdown().wait_for_readers(); // Reclaim queued CppTasks (the per-worker stdio/messaging // MessagePort drain tasks that can be in self.tasks mid-tick when diff --git a/src/runtime/webcore/fetch/FetchTasklet.rs b/src/runtime/webcore/fetch/FetchTasklet.rs index 9bc8d2887a0d..5552fc496db0 100644 --- a/src/runtime/webcore/fetch/FetchTasklet.rs +++ b/src/runtime/webcore/fetch/FetchTasklet.rs @@ -68,12 +68,8 @@ pub struct FetchTasklet { pub(crate) result: HTTPClientResult<'static>, pub(crate) metadata: Option, pub(crate) javascript_vm: &'static VirtualMachine, - /// Clone of `javascript_vm.cross_thread_shutdown`. The HTTP-thread - /// callbacks below read the shutdown state and fence the - /// `enqueue_task_concurrent` against `WebWorker::shutdown` through this - /// handle: `javascript_vm` is a lifetime-erased reference that dangles - /// once a worker has `dealloc`'d its VM, so it may only be dereferenced - /// inside a successful `try_begin_vm_read` section. + /// `javascript_vm` dangles once a worker VM is `dealloc`'d; HTTP-thread + /// callbacks fence every VM deref through this (see the struct doc). pub(crate) vm_shutdown_signal: std::sync::Arc, pub global_this: GlobalRef, pub(crate) request_body: HTTPRequestBody, @@ -532,11 +528,9 @@ impl FetchTasklet { /// `destructOnExit`, so `deinit()` there can release every handle on the /// right thread and the Weak is cleared before its referent is finalized. /// - /// Worker VM: there is no per-worker drain point (`shutdown_for_exit` is - /// process-global) and by the time this runs the worker's JSC heap is, or - /// is about to be, freed, so the parked `deinit()` would dereference dead - /// handles. Leak the box instead; the large buffers were already released - /// by the caller. + /// Worker VM: no such drain exists and the JSC heap is (about to be) + /// freed, so a parked `deinit()` would dereference dead handles. Leak the + /// box; the large buffers were already released by the caller. /// /// SAFETY: `this` must be the last reference (ref_count == 0) and have /// been allocated via heap::alloc. From aef2e91efed40d2f29024d672bf34778c3737392 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 31 Jul 2026 16:41:12 +0000 Subject: [PATCH 4/7] deref_from_thread: snapshot signal/vm into locals before enqueue The enqueued deinit_callback may free the tasklet before this function returns, so reading self_.vm_shutdown_signal after the enqueue is a use-after-free and could leave the reader count unbalanced. --- src/runtime/webcore/fetch/FetchTasklet.rs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/runtime/webcore/fetch/FetchTasklet.rs b/src/runtime/webcore/fetch/FetchTasklet.rs index 5552fc496db0..3cd580f27b4b 100644 --- a/src/runtime/webcore/fetch/FetchTasklet.rs +++ b/src/runtime/webcore/fetch/FetchTasklet.rs @@ -400,7 +400,11 @@ impl FetchTasklet { return; } let self_ = Self::from_raw_ref(this); - if !self_.vm_shutdown_signal.try_begin_vm_read() { + // The enqueued `deinit_callback` may free `this` before we return, so + // snapshot what's needed past the enqueue into locals now. + let signal = std::sync::Arc::clone(&self_.vm_shutdown_signal); + let vm = self_.javascript_vm; + if !signal.try_begin_vm_read() { // SAFETY: last ref; exclusive access. `deinit()` would run // `clear_data()` + `Drop` for the JSC `Strong`/`Weak` fields, which // reach into the VM's StrongRootBlock list / WeakSet from this @@ -414,10 +418,10 @@ impl FetchTasklet { // `from_callback` heap-allocates a fresh `ConcurrentTaskItem`; the queue // takes ownership of it. Self::enqueue_concurrent( - self_.javascript_vm, + vm, ConcurrentTask::from_callback(this, FetchTasklet::deinit_callback), ); - self_.vm_shutdown_signal.end_vm_read(); + signal.end_vm_read(); } // ConcurrentTask::from_callback takes `fn(*mut T) -> bun_event_loop::JsResult<()>` From 5fbe35b07cbc1be26d88fdcd880e0673ba36da50 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 3 Aug 2026 10:16:43 +0000 Subject: [PATCH 5/7] test: cover all four worker shutdown doors (terminate/process.exit/throw/reject) for the fetch UAF A worker can reach WebWorker::shutdown via four doors: parent terminate(), worker process.exit(), an uncaught throw, or an unhandled rejection. All four raw-dealloc the VirtualMachine while the shared HTTP thread may still be inside FetchTasklet::callback reading it; the fence in shutdown() is door-agnostic by construction. The test matrix runs the fetch-in-flight repro once per door so the fence is proven against each, not only terminate(). --- .../workers/worker-terminate-lifetime.test.ts | 97 +++++++++++++++++++ 1 file changed, 97 insertions(+) diff --git a/test/js/web/workers/worker-terminate-lifetime.test.ts b/test/js/web/workers/worker-terminate-lifetime.test.ts index 6b9f7c1b3366..7cf36753a3da 100644 --- a/test/js/web/workers/worker-terminate-lifetime.test.ts +++ b/test/js/web/workers/worker-terminate-lifetime.test.ts @@ -455,3 +455,100 @@ test.skipIf(!isDebug)( }, 120_000, ); + +// Regression: FetchTasklet holds a lifetime-erased &'static VirtualMachine and +// the shared HTTP client thread read it (is_shutting_down / +// enqueue_task_concurrent) after WebWorker::shutdown had dealloc'd the worker's +// VM storage, taking the whole process down (SIGSEGV on release, ASAN +// heap-use-after-free on debug). All four shutdown doors funnel through the +// same WebWorker::shutdown, so the fence is door-agnostic; the test matrix +// proves it. ASAN-gated: the read is one byte from freed memory, which +// release builds can survive. +describe.skipIf(!isASAN)( + "worker shutdown with fetch() in flight does not read the freed worker VM from the HTTP thread", + () => { + // workerExit is inlined into the worker body; parentAction replaces + // terminate() when the worker ends itself. + const doors: { door: string; workerExit: string; parentAction: string }[] = [ + { door: "terminate()", workerExit: "", parentAction: "await w.terminate();" }, + { door: "process.exit()", workerExit: "setTimeout(() => process.exit(0), d.T);", parentAction: "" }, + { door: "uncaught throw", workerExit: "setTimeout(() => { throw new Error('boom'); }, d.T);", parentAction: "" }, + { + door: "unhandled rejection", + workerExit: "setTimeout(() => Promise.reject(new Error('boom')), d.T);", + parentAction: "", + }, + ]; + for (const { door, workerExit, parentAction } of doors) { + test.concurrent( + door, + async () => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + ` + const { Worker } = require("node:worker_threads"); + const server = Bun.serve({ + hostname: "127.0.0.1", + port: 0, + fetch(req) { + if (new URL(req.url).pathname === "/health") return new Response("ok"); + // Long trickle so HTTP-thread callbacks for this request keep + // arriving past the worker's VM dealloc. + const enc = new TextEncoder(); + return new Response(new ReadableStream({ async start(c) { + for (let i = 0; i < 200; i++) { c.enqueue(enc.encode("chunk" + i + "\\n")); await Bun.sleep(2); } + c.close(); + } })); + }, + }); + const base = "http://127.0.0.1:" + server.port; + // 10 lanes of back-to-back fetches, mixed body consumption: half + // buffer the whole body, half read one chunk and release the + // reader so the stream is still draining when the worker exits. + const src = + 'const { parentPort, workerData: d } = require("node:worker_threads");' + + 'async function lane(l) { for (let i = 0; ; i++) { try {' + + ' const r = await fetch(d.base + "/slow?l=" + l + "&i=" + i);' + + ' if (i & 1) { const rd = r.body.getReader(); await rd.read(); rd.releaseLock(); }' + + ' else await r.arrayBuffer(); } catch {} } }' + + 'for (let l = 0; l < 10; l++) lane(l);' + + 'parentPort.postMessage("up");' + + ${JSON.stringify(workerExit)}; + for (let r = 0; r < ${rounds * 2}; r++) { + const T = 60 + ((r * 37) % 200); + const w = new Worker(src, { eval: true, workerData: { base, T } }); + w.on("error", () => {}); + const exited = new Promise(res => w.once("exit", res)); + await new Promise(res => w.once("message", res)); + ${parentAction ? `await Bun.sleep(T); ${parentAction}` : ""} + await exited; + // Keep-alive pool must stay healthy across the shutdown. + const t = await fetch(base + "/health").then(x => x.text()); + if (t !== "ok") throw new Error("pool unhealthy after round " + r); + } + server.stop(true); + console.log("survived"); + `, + ], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + + 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 part. + expect(stderr).toBe(""); + expect(stdout).toBe("survived\n"); + expect(exitCode).toBe(0); + }, + timeout, + ); + } + }, +); From 060a660f23fdccdf086ed29da69f9142db9640cb Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Mon, 3 Aug 2026 10:25:57 +0000 Subject: [PATCH 6/7] [autofix.ci] apply automated fixes --- test/js/web/workers/worker-terminate-lifetime.test.ts | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/test/js/web/workers/worker-terminate-lifetime.test.ts b/test/js/web/workers/worker-terminate-lifetime.test.ts index 7cf36753a3da..5aa536b2a92e 100644 --- a/test/js/web/workers/worker-terminate-lifetime.test.ts +++ b/test/js/web/workers/worker-terminate-lifetime.test.ts @@ -537,11 +537,7 @@ describe.skipIf(!isASAN)( stderr: "pipe", }); - const [stdout, stderr, exitCode] = await Promise.all([ - proc.stdout.text(), - proc.stderr.text(), - proc.exited, - ]); + 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 part. expect(stderr).toBe(""); expect(stdout).toBe("survived\n"); From dfd74de05f1a27f5f9df72892d737b36fa6c1ca2 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 3 Aug 2026 11:01:15 +0000 Subject: [PATCH 7/7] test: wire worker error/exit to reject the ready-wait Matches the file's existing ready(w) pattern so a worker that fails before posting 'up' rejects with a useful message instead of hanging to the outer timeout. The error-swallowing handler for the throw/reject doors moves to after the ready-wait. --- test/js/web/workers/worker-terminate-lifetime.test.ts | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/test/js/web/workers/worker-terminate-lifetime.test.ts b/test/js/web/workers/worker-terminate-lifetime.test.ts index 5aa536b2a92e..d74fb937a183 100644 --- a/test/js/web/workers/worker-terminate-lifetime.test.ts +++ b/test/js/web/workers/worker-terminate-lifetime.test.ts @@ -516,12 +516,19 @@ describe.skipIf(!isASAN)( 'for (let l = 0; l < 10; l++) lane(l);' + 'parentPort.postMessage("up");' + ${JSON.stringify(workerExit)}; + function ready(w) { + return new Promise((res, rej) => { + w.once("message", res); + w.once("error", rej); + w.once("exit", c => rej(new Error("worker exited " + c + " before ready"))); + }); + } for (let r = 0; r < ${rounds * 2}; r++) { const T = 60 + ((r * 37) % 200); const w = new Worker(src, { eval: true, workerData: { base, T } }); + await ready(w); w.on("error", () => {}); const exited = new Promise(res => w.once("exit", res)); - await new Promise(res => w.once("message", res)); ${parentAction ? `await Bun.sleep(T); ${parentAction}` : ""} await exited; // Keep-alive pool must stay healthy across the shutdown.