From 2eba4ff32968e1d30a59561601e2ad14f328593f Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Tue, 2 Jun 2026 03:39:21 +0000 Subject: [PATCH 1/6] Fix use-after-free when a worker is terminated with a fetch in flight MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The HTTP client thread's FetchTasklet callbacks dereference a raw VirtualMachine backref (the is_shutting_down read, the concurrent-task enqueue, and the loop wakeup), but WebWorker::shutdown frees the worker's VirtualMachine allocation with no synchronization against those callbacks. A progress update landing after the dealloc read freed memory and pushed onto the freed concurrent queue — a segfault on the queue-head atomic swap. Zig has the same logical race but frees worker VMs by destroying a private mimalloc heap, which typically leaves the pages mapped and masks it; the Rust port frees through the global allocator, so the race is a hard UAF. Fix: a refcounted ConcurrentEnqueueGate (mutex + vm_alive flag) shared between the VM and every FetchTasklet, so it outlives both sides. The HTTP thread brackets every VM access with enter()/exit(); worker shutdown close()s the gate — synchronizing with any in-flight gated section — then drains the concurrent queue (releasing parked tasklet refs while JSC is still alive) before invalidating and freeing the VM. A closed gate is handled exactly like is_shutting_down, without touching the VM. --- src/jsc/VirtualMachine.rs | 25 ++++ src/jsc/event_loop.rs | 111 +++++++++++++++++- src/jsc/web_worker.rs | 21 ++++ src/runtime/webcore/fetch/FetchTasklet.rs | 78 +++++++++++- .../workers/worker-terminate-lifetime.test.ts | 86 +++++++++++++- 5 files changed, 318 insertions(+), 3 deletions(-) diff --git a/src/jsc/VirtualMachine.rs b/src/jsc/VirtualMachine.rs index e847b6a66271..980d93915169 100644 --- a/src/jsc/VirtualMachine.rs +++ b/src/jsc/VirtualMachine.rs @@ -278,6 +278,13 @@ pub struct VirtualMachine { 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 + /// PORT NOTE (Rust-only): refcounted gate shared with cross-thread + /// concurrent-task producers (e.g. `FetchTasklet` on the HTTP client + /// thread) so they can serialize their VM accesses against teardown of + /// this allocation. Allocated in `init()` (one ref owned by the VM); + /// `WebWorker::shutdown` `close()`s it before freeing the VM and drops + /// the VM's ref. See [`crate::event_loop::ConcurrentEnqueueGate`]. + pub concurrent_enqueue_gate: *mut crate::event_loop::ConcurrentEnqueueGate, pub ref_strings: crate::ref_string::Map, pub ref_strings_mutex: bun_threading::Mutex, @@ -755,6 +762,22 @@ impl VirtualMachine { unsafe { &*self.event_loop } } + /// Take a counted ref on this VM's [`ConcurrentEnqueueGate`] for a + /// cross-thread producer that holds a backref to this VM. Must be called + /// on the JS thread while the VM is alive (i.e. where the backref itself + /// is created); release with [`event_loop::ConcurrentEnqueueGate::deref`]. + pub fn retain_concurrent_enqueue_gate( + &self, + ) -> core::ptr::NonNull { + // SAFETY: written once in `init()` from `ConcurrentEnqueueGate::new()` + // (never null) and freed only after the VM's ref drops, so it is live + // for the VM lifetime. + let gate = unsafe { &*self.concurrent_enqueue_gate }; + gate.ref_(); + // `concurrent_enqueue_gate` is non-null per the SAFETY note above. + core::ptr::NonNull::new(self.concurrent_enqueue_gate).unwrap() + } + /// Alias for [`Self::event_loop_mut`]. Kept for callers migrated on the /// `runtime-hostfn-safe` branch; both names funnel into the single audited /// `unsafe` deref above. @@ -2113,6 +2136,8 @@ impl VirtualMachine { (*regular).virtual_machine = NonNull::new(vm); let _ = (*regular).tasks.ensure_unused_capacity(64); addr_of_mut!((*vm).event_loop).write(regular); + addr_of_mut!((*vm).concurrent_enqueue_gate) + .write(crate::event_loop::ConcurrentEnqueueGate::new()); // `source_mappings.map` is a sibling-field backref onto // `saved_source_map_table`. diff --git a/src/jsc/event_loop.rs b/src/jsc/event_loop.rs index eefced1e6374..0b9d22c11855 100644 --- a/src/jsc/event_loop.rs +++ b/src/jsc/event_loop.rs @@ -10,7 +10,7 @@ //! poll deadline). See PORTING.md §Dispatch. use core::ptr::NonNull; -use core::sync::atomic::{AtomicI32, AtomicPtr, Ordering}; +use core::sync::atomic::{AtomicBool, AtomicI32, AtomicPtr, Ordering}; use bun_io::{self as Async, Waker}; use bun_uws as uws; @@ -1480,3 +1480,112 @@ pub(crate) fn __bun_spawn_sync_vm_set_event_loop(vm: *mut (), el: *mut ()) { pub(crate) fn __bun_spawn_sync_vm_swap_suppress_microtask_drain(vm: *mut (), v: bool) -> bool { vm_from_ptr(vm).suppress_microtask_drain.replace(v) } + +// ────────────────────────────────────────────────────────────────────────── +// ConcurrentEnqueueGate — VM-teardown guard for cross-thread producers +// ────────────────────────────────────────────────────────────────────────── + +/// Serializes cross-thread producers of `EventLoop.concurrent_tasks` against +/// teardown of the owning `VirtualMachine` allocation. +/// +/// PORT NOTE (Rust-only; no Zig counterpart): worker teardown frees the +/// `VirtualMachine` allocation (`WebWorker::shutdown`), but the HTTP client +/// thread may still hold a `FetchTasklet` whose `javascript_vm` backref points +/// at it — its result callback reads `vm.is_shutting_down` and pushes onto +/// `vm.regular_event_loop.concurrent_tasks` (then wakes the VM's uws loop), +/// all of which is a use-after-free once the worker thread has dealloc'd the +/// VM. Zig has the same logical race but frees the worker VM by destroying a +/// private mimalloc heap, which typically leaves the pages mapped and masks +/// it; the Rust port frees through the global allocator, so the race is a +/// hard segfault on the queue's atomic head swap. +/// +/// The gate is a standalone refcounted allocation so it strictly outlives +/// both sides: the VM holds one ref (`VirtualMachine.concurrent_enqueue_gate`, +/// released when the worker frees the VM allocation) and every cross-thread +/// producer holding a VM backref holds one (e.g. `FetchTasklet`, released +/// with the tasklet). Producers bracket every touch of the VM with +/// `enter()`/`exit()`; teardown calls `close()` exactly once, before invali- +/// dating the VM. Because `close()` takes the same mutex, it blocks until any +/// in-flight gated section has finished, and every later `enter()` returns +/// `false` — so after `close()` returns, no producer is inside the VM and +/// none can get back in. Teardown can then drain `concurrent_tasks` (the +/// drain observes every push that won the race) and free the allocation. +/// +/// Lock ordering: the gate is a leaf lock — producers may take it while +/// holding their own state lock (e.g. `FetchTasklet.mutex`), and `close()` +/// is called with no other locks held. +#[derive(bun_ptr::ThreadSafeRefCounted)] +#[ref_count(destroy = Self::destroy)] +pub struct ConcurrentEnqueueGate { + ref_count: bun_ptr::ThreadSafeRefCount, + mutex: bun_threading::Mutex, + /// Guarded by `mutex`; atomic only so the type is `Sync` (every access + /// happens with the mutex held, hence `Relaxed`). + vm_alive: AtomicBool, +} + +impl ConcurrentEnqueueGate { + /// Allocate an open gate with one ref (the VM's). + pub fn new() -> *mut Self { + bun_core::heap::into_raw(Box::new(Self { + ref_count: bun_ptr::ThreadSafeRefCount::init(), + mutex: bun_threading::Mutex::new(), + vm_alive: AtomicBool::new(true), + })) + } + + pub fn ref_(&self) { + // SAFETY: `self` is live; `ref_` only touches the interior-mutable + // atomic `ref_count` field. + unsafe { + bun_ptr::ThreadSafeRefCount::::ref_(core::ptr::from_ref(self).cast_mut()); + } + } + + /// Drop one ref; frees the gate on the last one. + /// + /// Takes a raw pointer (not `&self`) because the call may drop the last + /// ref and free the allocation. + #[allow(clippy::not_unsafe_ptr_arg_deref)] + pub fn deref(this: *mut Self) { + // SAFETY: caller holds a ref, so `this` is live until this decrement. + unsafe { bun_ptr::ThreadSafeRefCount::::deref(this) }; + } + + /// Enter the gated section. Returns `true` with the gate lock HELD iff + /// the VM is still alive — the caller may touch the VM and must call + /// [`exit`](Self::exit) afterwards. Returns `false` (lock released) once + /// [`close`](Self::close) has run; the caller must not touch the VM. + #[must_use] + pub fn enter(&self) -> bool { + self.mutex.lock(); + if self.vm_alive.load(Ordering::Relaxed) { + return true; + } + self.mutex.unlock(); + false + } + + /// Leave a gated section previously entered via a successful + /// [`enter`](Self::enter). + pub fn exit(&self) { + self.mutex.unlock(); + } + + /// Mark the VM dead. Blocks until any in-flight gated section exits; + /// afterwards every `enter()` fails. Called by VM teardown exactly once, + /// before the VM allocation is invalidated. + pub fn close(&self) { + self.mutex.lock(); + self.vm_alive.store(false, Ordering::Relaxed); + self.mutex.unlock(); + } + + /// `#[ref_count(destroy)]` hook — last ref dropped. + unsafe fn destroy(this: *mut Self) { + // SAFETY: refcount hit zero; `this` came from `heap::into_raw` in + // `new()`, so reclaiming the box is exclusive. + unsafe { bun_core::heap::destroy(this) }; + } +} + diff --git a/src/jsc/web_worker.rs b/src/jsc/web_worker.rs index ceb045b8f5e6..f0c796901614 100644 --- a/src/jsc/web_worker.rs +++ b/src/jsc/web_worker.rs @@ -1250,6 +1250,20 @@ impl WebWorker { // is step 3 below). rare.close_all_socket_groups(unsafe { &*vm_ptr }); } + // Cut off cross-thread concurrent-task producers (the HTTP client + // thread's `FetchTasklet` callbacks) BEFORE the VM is invalidated + // below: `close()` synchronizes with any in-flight gated enqueue + // and makes every later one observe the gate closed instead of + // touching this VM (see `ConcurrentEnqueueGate`). Then release + // queued-but-never-run tasks while JSC is still alive — e.g. a + // parked `FetchTasklet` progress task owns the JS-side tasklet + // ref, and dropping it may run `deinit` → JSC `Strong`/`Weak` + // teardown. No gated producer can enqueue after `close()`, so + // this drain observes every task that won the race. + // SAFETY: `concurrent_enqueue_gate` is set in `init()` and the + // VM's ref is dropped only at the dealloc below, so it is live. + unsafe { &*vm.concurrent_enqueue_gate }.close(); + vm.event_loop_mut().release_queued_tasks_for_shutdown(); exit_code = i32::from(vm.exit_handler.exit_code); global_object = Some(vm.global); } @@ -1310,6 +1324,13 @@ impl WebWorker { if let Some(log) = (*vm_ptr).log.take() { bun_core::heap::destroy(log.as_ptr()); } + // Drop the VM's ref on the (already closed) enqueue gate. + // Producers that still hold refs (in-flight fetches) keep the + // gate box alive past the VM dealloc below; that is the point. + jsc::event_loop::ConcurrentEnqueueGate::deref(core::mem::replace( + &mut (*vm_ptr).concurrent_enqueue_gate, + core::ptr::null_mut(), + )); virtual_machine::VMHolder::set_vm(None); // The VM was `alloc_zeroed(Layout::())` in // `init`, NOT `Box::new` — dealloc the raw storage directly so diff --git a/src/runtime/webcore/fetch/FetchTasklet.rs b/src/runtime/webcore/fetch/FetchTasklet.rs index dbd9ba71d0d1..f2914b259797 100644 --- a/src/runtime/webcore/fetch/FetchTasklet.rs +++ b/src/runtime/webcore/fetch/FetchTasklet.rs @@ -17,6 +17,7 @@ use bun_http::{ }; use bun_io::KeepAlive; use bun_jsc::debugger::AsyncTaskTracker; +use bun_jsc::event_loop::ConcurrentEnqueueGate; use bun_jsc::virtual_machine::VirtualMachine; use bun_jsc::{ self as jsc, GlobalRef, JSGlobalObject, JSValue, JsResult, StringJsc, StrongOptional, @@ -66,6 +67,14 @@ pub struct FetchTasklet { pub result: HTTPClientResult<'static>, pub metadata: Option, pub javascript_vm: &'static VirtualMachine, + /// Counted ref on `javascript_vm`'s [`ConcurrentEnqueueGate`], taken in + /// [`get`](Self::get) and released in [`deinit`](Self::deinit). Every + /// HTTP-thread dereference of `javascript_vm` must happen inside + /// `enter()`/`exit()` on this gate: worker teardown frees the + /// `VirtualMachine` allocation while this tasklet may still be in flight + /// on the HTTP thread, and the gate (a separate allocation that outlives + /// both sides) is what makes that race safe. + pub vm_gate: core::ptr::NonNull, pub global_this: GlobalRef, pub request_body: HTTPRequestBody, // ThreadSafeStreamBuffer is intrusively refcounted (`ref_count: AtomicU32`, @@ -302,11 +311,24 @@ impl FetchTasklet { /// and is thread-safe (lock-free MPSC push). `task` is a live /// `ConcurrentTaskItem` that the queue takes ownership of via its /// intrusive `next` link. + /// + /// HTTP-thread callers must hold `vm_gate` (see [`Self::vm_gate_ref`]) — + /// "the VM's lifetime" ends while the request is still in flight when a + /// worker is terminated, and the gate is what excludes that teardown. #[inline] fn enqueue_concurrent(vm: &VirtualMachine, task: core::ptr::NonNull) { vm.event_loop_shared().enqueue_task_concurrent(task); } + /// The [`ConcurrentEnqueueGate`] this tasklet holds a counted ref on (see + /// the `vm_gate` field doc). Valid from `get()` until `deinit()`. + #[inline] + fn vm_gate_ref(&self) -> &ConcurrentEnqueueGate { + // SAFETY: `vm_gate` is the counted ref taken in `get()`; live until + // `deinit()` releases it. + unsafe { self.vm_gate.as_ref() } + } + /// Wrap a borrowed body chunk in a `StreamResult::Temporary*` for /// synchronous delivery to `ByteStream::on_data`. /// @@ -392,7 +414,29 @@ impl FetchTasklet { return; } let self_ = Self::from_raw_ref(this); + // Take a temp ref on the gate for this gated section: the enqueued + // `deinit_callback` may run on the JS thread (and release the + // tasklet's own gate ref via `deinit`) before `exit()` below. + let gate_ptr = self_.vm_gate.as_ptr(); + // SAFETY: the tasklet's counted gate ref is still held here (released + // only in `deinit`, which cannot have run — we hold the last tasklet + // ref), so `gate_ptr` is live. + let gate = unsafe { &*gate_ptr }; + gate.ref_(); + // A closed gate means the owning worker's VM has been (or is being) + // freed — same disposition as `is_shutting_down`, except the flag + // itself must not be read. Entering the gate is what makes the + // `is_shutting_down` read and the enqueue safe against teardown. + if !gate.enter() { + ConcurrentEnqueueGate::deref(gate_ptr); + // SAFETY: last ref; exclusive access. See the shutdown comment + // below — same reclaim path. + unsafe { FetchTasklet::dealloc_for_shutdown(this) }; + return; + } if self_.javascript_vm.is_shutting_down() { + gate.exit(); + ConcurrentEnqueueGate::deref(gate_ptr); // 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 @@ -409,6 +453,8 @@ impl FetchTasklet { self_.javascript_vm, ConcurrentTask::from_callback(this, FetchTasklet::deinit_callback), ); + gate.exit(); + ConcurrentEnqueueGate::deref(gate_ptr); } // ConcurrentTask::from_callback takes `fn(*mut T) -> bun_event_loop::JsResult<()>` @@ -501,6 +547,10 @@ impl FetchTasklet { // SAFETY: this was allocated via heap::alloc in `get()`; ref_count == 0 so exclusive let mut boxed = unsafe { bun_core::heap::take(this) }; boxed.clear_data(); + // Release the counted gate ref taken in `get()`. `deinit` runs exactly + // once per tasklet (directly, via `deinit_callback`, or on the parked + // box from `shutdown_for_exit`'s reclaim drain), so this balances. + ConcurrentEnqueueGate::deref(boxed.vm_gate.as_ptr()); // self.http: Option> dropped here automatically drop(boxed); } @@ -1711,6 +1761,8 @@ impl FetchTasklet { result: HTTPClientResult::default(), metadata: None, javascript_vm: jsc_vm, + // JS thread, VM alive — the one place a gate ref may be taken. + vm_gate: jsc_vm.retain_concurrent_enqueue_gate(), global_this: GlobalRef::from(global_this), request_body: fetch_options.body, request_body_streaming_buffer: None, @@ -1982,7 +2034,14 @@ 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); + // Gate every VM access on this (HTTP) thread against worker teardown + // freeing the VM allocation; a closed gate means "VM is gone", same + // disposition as `is_shutting_down`. + if !this_ref.vm_gate_ref().enter() { + return; + } if this_ref.javascript_vm.is_shutting_down() { + this_ref.vm_gate_ref().exit(); return; } // ref until the main thread callback is called @@ -1993,6 +2052,10 @@ impl FetchTasklet { this_ref.javascript_vm, ConcurrentTask::from_callback(this, FetchTasklet::resume_request_data_stream), ); + // The HTTP side still holds its own tasklet ref (the request is in + // flight), so `this`/the gate field stay valid across the enqueue + // even if the JS thread consumes the task immediately. + this_ref.vm_gate_ref().exit(); } /// This is ALWAYS called from the main thread @@ -2282,7 +2345,16 @@ impl FetchTasklet { } } // will deinit when done with the http client (when is_done = true) - if task_ref.javascript_vm.is_shutting_down() { + // Gate the VM accesses below (the `is_shutting_down` read and the + // enqueue + loop wakeup) against worker teardown freeing the VM + // allocation. Lock order: `task_ref.mutex` (held) → gate (leaf). + // A closed gate means the VM is gone — same disposition as + // `is_shutting_down`, except the flag itself must not be read. + let vm_gone = !task_ref.vm_gate_ref().enter(); + if vm_gone || task_ref.javascript_vm.is_shutting_down() { + if !vm_gone { + task_ref.vm_gate_ref().exit(); + } // 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(); @@ -2322,6 +2394,10 @@ 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); + // The JS thread can't free the tasklet while we hold `task_ref.mutex` + // (`on_progress_update` locks it first), so exiting the gate here is + // safe even though the task above is already visible to the consumer. + task_ref.vm_gate_ref().exit(); task_ref.mutex.unlock(); // we are done with the http client so we can deref our side diff --git a/test/js/web/workers/worker-terminate-lifetime.test.ts b/test/js/web/workers/worker-terminate-lifetime.test.ts index b938d02fc470..31a5b52180f3 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. @@ -82,6 +82,90 @@ test( timeout, ); +// Regression: terminating a worker while a fetch() it started was still in +// flight freed the worker's VirtualMachine while the HTTP client thread still +// held a FetchTasklet backref to it. The next progress callback read +// `is_shutting_down` from the freed allocation and pushed onto the freed +// concurrent task queue — a segfault on the queue-head atomic swap in release +// builds, a deterministic heap-use-after-free under ASAN. +test( + "terminating a worker with an in-flight fetch does not UAF the worker VM", + async () => { + using dir = tempDir("worker-fetch-terminate", { + "main.js": ` + // Drip body chunks forever so each worker's fetch stays in flight and + // keeps generating HTTP-thread progress callbacks after terminate(). + const server = Bun.serve({ + port: 0, + idleTimeout: 0, + fetch() { + let timer; + return new Response( + new ReadableStream({ + start(controller) { + timer = setInterval(() => { + try { + controller.enqueue(new Uint8Array(4096)); + } catch { + clearInterval(timer); + } + }, 1); + }, + cancel() { + clearInterval(timer); + }, + }), + ); + }, + }); + + for (let i = 0; i < ${slow ? 8 : 16}; i++) { + const worker = new Worker(new URL("./worker.js", import.meta.url).href); + const inFlight = new Promise(resolve => { + worker.onmessage = resolve; + }); + worker.postMessage(server.port); + await inFlight; + // The worker has processed the response headers; the body is still + // streaming. Terminate mid-stream: the worker VM is torn down while + // the HTTP thread keeps delivering chunks for its tasklet — and the + // streams of workers terminated in previous iterations keep + // dripping into their freed VMs throughout the loop. + worker.terminate(); + } + // Force-close the dripping connections so every leaked request also + // delivers its final result callback for a long-dead worker VM, then + // give those callbacks a moment to land (they are internal HTTP-thread + // events for dead workers — there is nothing observable to await). + server.stop(true); + await Bun.sleep(50); + console.log("done"); + `, + "worker.js": ` + self.onmessage = async e => { + const res = await fetch("http://127.0.0.1:" + e.data + "/"); + // Deliberately do not consume res.body — the server keeps dripping + // and every chunk is an HTTP-thread callback targeting this VM. + postMessage("in-flight"); + }; + `, + }); + await using proc = Bun.spawn({ + cmd: [bunExe(), "main.js"], + env: bunEnv, + cwd: String(dir), + stdout: "pipe", + stderr: "pipe", + }); + + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toBe(""); + expect(stdout).toBe("done\n"); + expect(exitCode).toBe(0); + }, + timeout, +); + // Regression: WebWorker__dispatchExit deref'd the C++ Worker on the worker // thread; if that was the last ref, ~Worker → ~EventTarget ran there and // EventListenerMap::releaseAssertOrSetThreadUID tripped because the listener From b53da0477400d412b0f4bcbc0d36fa1bc55cd342 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Tue, 2 Jun 2026 06:33:29 +0000 Subject: [PATCH 2/6] Document the worker-shutdown drain caller in the release-task contracts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The worker-shutdown caller of release_queued_tasks_for_shutdown runs with the HTTP daemon still live (the closed ConcurrentEnqueueGate, not a parked daemon, is what makes the drain complete), so the per-tag release fns own only the queued entry's counted ref — exclusivity exists only on the 1→0 transition. Update the two callee docs and the callback gate-exit note, and run the worker-terminate regression fixture with BUN_DESTRUCT_VM_ON_EXIT=1 so the process-exit reclaim paths are exercised on every lane (ASAN CI already sets it). --- src/jsc/event_loop.rs | 26 ++++++++++++++----- src/runtime/dispatch.rs | 26 +++++++++++-------- src/runtime/webcore/fetch/FetchTasklet.rs | 10 ++++--- .../workers/worker-terminate-lifetime.test.ts | 6 ++++- 4 files changed, 46 insertions(+), 22 deletions(-) diff --git a/src/jsc/event_loop.rs b/src/jsc/event_loop.rs index 0b9d22c11855..8185c780798f 100644 --- a/src/jsc/event_loop.rs +++ b/src/jsc/event_loop.rs @@ -776,13 +776,25 @@ impl EventLoop { } /// Release queued-but-never-run tasks that own a ref the dispatch path - /// would have dropped. Called from `global_exit` after `shutdown_for_exit` - /// (HTTP daemon parked, no further cross-thread posts) and before - /// `destructOnExit` (JSC still live, so `FetchTasklet::deinit` can drop - /// its `Strong`/`Weak` handles). Re-runs `drop_concurrent_cpp_tasks` first - /// so any task the HTTP thread posted after the earlier drain — its - /// `is_shutting_down()` read is non-atomic and can lag — is forwarded into - /// `self.tasks` for the per-tag release below. + /// would have dropped. Two callers, both pre-JSC-teardown (JSC still + /// live, so `FetchTasklet::deinit` can drop its `Strong`/`Weak` handles): + /// + /// * `global_exit`, after `shutdown_for_exit` — the HTTP daemon is + /// parked, so no further cross-thread posts can land and released + /// entries are the sole remaining refs. + /// * `WebWorker::shutdown`, after `ConcurrentEnqueueGate::close()` — + /// here the HTTP daemon is still RUNNING; the closed gate guarantees + /// no further gated post can land (so this drain is complete), but a + /// racing fetch callback may still hold its own tasklet ref + /// concurrently. Per-tag release fns must therefore not assume + /// exclusive ownership of the pointee — only that the queued entry + /// carries one counted ref (released via atomic refcount; exclusivity + /// exists only on a 1→0 transition). + /// + /// Re-runs `drop_concurrent_cpp_tasks` first so any task the HTTP thread + /// posted after the earlier drain — its `is_shutting_down()` read is + /// non-atomic and can lag — is forwarded into `self.tasks` for the + /// per-tag release below. /// /// `ManagedTask` entries are deliberately re-queued rather than freed: /// owners (e.g. `SendQueue.close_next_tick` / `after_close_task`) keep raw diff --git a/src/runtime/dispatch.rs b/src/runtime/dispatch.rs index cd79d08ba66f..952b970e3d96 100644 --- a/src/runtime/dispatch.rs +++ b/src/runtime/dispatch.rs @@ -1139,12 +1139,13 @@ pub(crate) unsafe fn __bun_tick_queue_with_count( /// `__bun_release_task_at_shutdown` body — declared `extern "Rust"` in /// `bun_jsc::event_loop`. Called from `release_queued_tasks_for_shutdown` on -/// the JS thread for every queued task that will never be dispatched (the JS -/// thread is past `global_exit`'s `is_shutting_down` flip and the loop will -/// not tick again), after the HTTP daemon has parked and before -/// `destructOnExit`. Releases the boxes and JSC handles the dispatch path -/// would have dropped. Tags not yet listed leak their box at exit; add them -/// as LSan surfaces them. +/// the JS thread for every queued task that will never be dispatched (the +/// loop will not tick again): from `global_exit` after the HTTP daemon has +/// parked, and from `WebWorker::shutdown` while the HTTP daemon is still +/// running (see `release_queued_tasks_for_shutdown`'s doc). Both run before +/// the caller's JSC teardown. Releases the boxes and JSC handles the dispatch +/// path would have dropped. Tags not yet listed leak their box at exit; add +/// them as LSan surfaces them. #[unsafe(no_mangle)] pub(crate) fn __bun_release_task_at_shutdown(task: bun_event_loop::Task) -> bool { use bun_event_loop::task_tag; @@ -1153,12 +1154,15 @@ pub(crate) fn __bun_release_task_at_shutdown(task: bun_event_loop::Task) -> bool // posted this entry, then deref'd its own +1 if final; the JS-side // +1 it expected `on_progress_update` to drop is the one we release // here. Runs on the JS thread, so the plain `deref` (→ `deinit` on - // 1→0) is the right teardown path; the HTTP daemon is already - // parked (`shutdown_for_exit` precedes `destroy`), so the - // `Box` and any `metadata` it owns are exclusively ours. + // 1→0) is the right teardown path. On the worker-shutdown caller the + // HTTP thread may still hold its own tasklet ref concurrently — the + // queued entry only represents one counted ref, so release it with + // the atomic `deref`; exclusive access (and thus `deinit`'s + // single-threaded teardown) exists only on the 1→0 transition, which + // requires every HTTP-side ref to have already been dropped. task_tag::FetchTasklet => { - // SAFETY: `task.ptr` is the live heap `FetchTasklet`; HTTP daemon is - // already parked so we hold the sole reference. + // SAFETY: `task.ptr` is the live heap `FetchTasklet` (the queued + // entry's counted ref keeps it alive until this decrement). FetchTasklet::deref(task.ptr.cast::()); true } diff --git a/src/runtime/webcore/fetch/FetchTasklet.rs b/src/runtime/webcore/fetch/FetchTasklet.rs index f2914b259797..08eb7a046db4 100644 --- a/src/runtime/webcore/fetch/FetchTasklet.rs +++ b/src/runtime/webcore/fetch/FetchTasklet.rs @@ -2394,9 +2394,13 @@ 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); - // The JS thread can't free the tasklet while we hold `task_ref.mutex` - // (`on_progress_update` locks it first), so exiting the gate here is - // safe even though the task above is already visible to the consumer. + // The JS thread can't free the tasklet while we hold `task_ref.mutex`: + // `on_progress_update` locks it first, and the other consumer of this + // task (`__bun_release_task_at_shutdown` from the worker-shutdown + // drain) doesn't lock it but drops only the entry's counted ref — our + // HTTP-side ref keeps the count ≥ 1 until after `mutex.unlock()` + // below. So exiting the gate here is safe even though the task above + // is already visible to consumers. task_ref.vm_gate_ref().exit(); task_ref.mutex.unlock(); diff --git a/test/js/web/workers/worker-terminate-lifetime.test.ts b/test/js/web/workers/worker-terminate-lifetime.test.ts index 31a5b52180f3..bc86ee733ed6 100644 --- a/test/js/web/workers/worker-terminate-lifetime.test.ts +++ b/test/js/web/workers/worker-terminate-lifetime.test.ts @@ -152,7 +152,11 @@ test( }); await using proc = Bun.spawn({ cmd: [bunExe(), "main.js"], - env: bunEnv, + // Destruct-on-exit (what ASAN CI runs with) additionally walks the + // process-exit reclaim paths: dead-worker tasklets must not be parked + // in the exit reclaim list, whose drain tears down JSC handles — for a + // worker tasklet those handles died with the worker's JSC heap. + env: { ...bunEnv, BUN_DESTRUCT_VM_ON_EXIT: "1" }, cwd: String(dir), stdout: "pipe", stderr: "pipe", From 5eeacc9a8aae020b282b02d85a9f09ea4e2ebc74 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Tue, 2 Jun 2026 07:11:00 +0000 Subject: [PATCH 3/6] test: describe the park-at-exit disposition the destruct-on-exit run actually exercises --- src/jsc/event_loop.rs | 17 +++++++++++------ .../workers/worker-terminate-lifetime.test.ts | 8 +++++--- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/src/jsc/event_loop.rs b/src/jsc/event_loop.rs index 8185c780798f..4aedf4b7823a 100644 --- a/src/jsc/event_loop.rs +++ b/src/jsc/event_loop.rs @@ -1513,15 +1513,20 @@ pub(crate) fn __bun_spawn_sync_vm_swap_suppress_microtask_drain(vm: *mut (), v: /// /// The gate is a standalone refcounted allocation so it strictly outlives /// both sides: the VM holds one ref (`VirtualMachine.concurrent_enqueue_gate`, -/// released when the worker frees the VM allocation) and every cross-thread -/// producer holding a VM backref holds one (e.g. `FetchTasklet`, released -/// with the tasklet). Producers bracket every touch of the VM with +/// released when the worker frees the VM allocation) and each participating +/// cross-thread producer holds one for as long as it keeps a VM backref. +/// `FetchTasklet` (the producer behind the observed crash) is the only +/// participant so far; the S3 HTTP tasks (`S3HttpSimpleTask`, +/// `S3HttpDownloadStreamingTask`) carry the same kind of backref and still +/// enqueue ungated — the identical take-ref/bracket/reclaim pattern applies +/// to them. Producers bracket every touch of the VM with /// `enter()`/`exit()`; teardown calls `close()` exactly once, before invali- /// dating the VM. Because `close()` takes the same mutex, it blocks until any /// in-flight gated section has finished, and every later `enter()` returns -/// `false` — so after `close()` returns, no producer is inside the VM and -/// none can get back in. Teardown can then drain `concurrent_tasks` (the -/// drain observes every push that won the race) and free the allocation. +/// `false` — so after `close()` returns, no gated producer is inside the VM +/// and none can get back in. Teardown can then drain `concurrent_tasks` (the +/// drain observes every gated push that won the race) and free the +/// allocation. /// /// Lock ordering: the gate is a leaf lock — producers may take it while /// holding their own state lock (e.g. `FetchTasklet.mutex`), and `close()` diff --git a/test/js/web/workers/worker-terminate-lifetime.test.ts b/test/js/web/workers/worker-terminate-lifetime.test.ts index bc86ee733ed6..0538035057e6 100644 --- a/test/js/web/workers/worker-terminate-lifetime.test.ts +++ b/test/js/web/workers/worker-terminate-lifetime.test.ts @@ -153,9 +153,11 @@ test( await using proc = Bun.spawn({ cmd: [bunExe(), "main.js"], // Destruct-on-exit (what ASAN CI runs with) additionally walks the - // process-exit reclaim paths: dead-worker tasklets must not be parked - // in the exit reclaim list, whose drain tears down JSC handles — for a - // worker tasklet those handles died with the worker's JSC heap. + // process-exit reclaim paths: dead-worker tasklets sit parked in the + // process-global exit reclaim list, and the exit drain then runs their + // `deinit` — including JSC-handle teardown against worker heaps that + // died with `teardownJSCVM` (bmalloc-backed, so ASAN alone cannot + // flag it). The fixture must stay crash-free through that whole path. env: { ...bunEnv, BUN_DESTRUCT_VM_ON_EXIT: "1" }, cwd: String(dir), stdout: "pipe", From c0758312f90863f3b4c477fc03d0a517d7dcc4e4 Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Sun, 7 Jun 2026 01:41:10 +0000 Subject: [PATCH 4/6] [autofix.ci] apply automated fixes --- src/jsc/event_loop.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/jsc/event_loop.rs b/src/jsc/event_loop.rs index 4aedf4b7823a..06efb8f9fd4b 100644 --- a/src/jsc/event_loop.rs +++ b/src/jsc/event_loop.rs @@ -1605,4 +1605,3 @@ impl ConcurrentEnqueueGate { unsafe { bun_core::heap::destroy(this) }; } } - From daae0987253ce3e1bb69898b4ed9caf8e8cb3c06 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 7 Jun 2026 01:52:20 +0000 Subject: [PATCH 5/6] test: reject the in-flight signal on worker error/messageerror/close --- test/js/web/workers/worker-terminate-lifetime.test.ts | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/test/js/web/workers/worker-terminate-lifetime.test.ts b/test/js/web/workers/worker-terminate-lifetime.test.ts index 0538035057e6..f108aed262da 100644 --- a/test/js/web/workers/worker-terminate-lifetime.test.ts +++ b/test/js/web/workers/worker-terminate-lifetime.test.ts @@ -121,8 +121,14 @@ test( for (let i = 0; i < ${slow ? 8 : 16}; i++) { const worker = new Worker(new URL("./worker.js", import.meta.url).href); - const inFlight = new Promise(resolve => { - worker.onmessage = resolve; + // Reject on every failure event so a broken worker fails the test + // immediately instead of hanging it until the timeout. + const { promise: inFlight, resolve: markInFlight, reject: failInFlight } = Promise.withResolvers(); + worker.onmessage = markInFlight; + worker.onmessageerror = () => failInFlight(new Error("worker message failed to deserialize")); + worker.onerror = e => failInFlight(new Error("worker error: " + (e?.message ?? e))); + worker.addEventListener("close", () => failInFlight(new Error("worker closed before signaling in-flight fetch")), { + once: true, }); worker.postMessage(server.port); await inFlight; From 7c583fbec36f2134640162cde1624b0830a60084 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 7 Jun 2026 02:15:16 +0000 Subject: [PATCH 6/6] Scope the worker-shutdown SAFETY comment to the vm_lock reader class --- src/jsc/web_worker.rs | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/jsc/web_worker.rs b/src/jsc/web_worker.rs index f0c796901614..bf9ff779f312 100644 --- a/src/jsc/web_worker.rs +++ b/src/jsc/web_worker.rs @@ -1217,8 +1217,17 @@ impl WebWorker { let mut exit_code: i32 = 0; let mut global_object: Option<*const JSGlobalObject> = None; if !vm_ptr.is_null() { - // SAFETY: vm_ptr valid; unpublished above under vm_lock, so no - // other thread can dereference it now — `&mut` is exclusive. + // SAFETY: vm_ptr valid; unpublished above under vm_lock, so the + // vm_lock-guarded readers (notify_need_termination / + // terminate_all_and_wait) can no longer reach it. The HTTP client + // thread may still dereference it through a `FetchTasklet`'s + // gated `javascript_vm` backref until the `close()` below + // returns; those gated sections touch only the lock-free + // `concurrent_tasks` queue, the uws loop, and the plain + // `is_shutting_down` bool — whose racy read against the store + // below is long-standing and tolerated (a stale `false` just + // means one more task lands in the queue for the drain; the gate, + // not the flag, is the teardown cutoff). let vm = unsafe { &mut *vm_ptr }; // terminate() set the JSC termination flag to interrupt running JS; // clear it so process.on('exit') handlers can run. teardownJSCVM