Fix use-after-free when a worker is terminated with a fetch in flight

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<crate::event_loop::ConcurrentEnqueueGate> {
+        // 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`.

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;
@@ -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
@@ -1480,3 +1492,116 @@ 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 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 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()`
+/// is called with no other locks held.
+#[derive(bun_ptr::ThreadSafeRefCounted)]
+#[ref_count(destroy = Self::destroy)]
+pub struct ConcurrentEnqueueGate {
+    ref_count: bun_ptr::ThreadSafeRefCount<Self>,
+    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::<Self>::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::<Self>::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) };
+    }
+}

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
@@ -1250,6 +1259,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 +1333,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::<VirtualMachine>())` in
                 // `init`, NOT `Box::new` — dealloc the raw storage directly so

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<AsyncHTTP>` 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::<FetchTasklet>());
             true
         }