diff --git a/src/jsc/RuntimeTranspilerStore.rs b/src/jsc/RuntimeTranspilerStore.rs index b025baa3e846..32e6ace2d488 100644 --- a/src/jsc/RuntimeTranspilerStore.rs +++ b/src/jsc/RuntimeTranspilerStore.rs @@ -15,7 +15,7 @@ use bun_bundler::options::ModuleType; use bun_bundler::transpiler::{self as transpiler, AlreadyBundled, ParseOptions, Transpiler}; use bun_collections::HiveArrayFallback; use bun_core::{MutableString, String, strings}; -use bun_event_loop::{TaskTag, Taskable, task_tag}; +use bun_event_loop::{Task, TaskTag, Taskable, task_tag}; use bun_io::posix_event_loop::get_vm_ctx; use bun_io::{AllocatorType, KeepAlive}; use bun_js_printer::{self as js_printer, BufferPrinter, BufferWriter}; @@ -26,13 +26,13 @@ use bun_resolver::fs as Fs; use bun_resolver::node_fallbacks; use bun_resolver::package_json::{MacroMap as MacroRemap, PackageJSON}; use bun_sys::{self, Dir, Fd, FdExt as _, File, OpenDirOptions}; -use bun_threading::Guarded; use bun_threading::unbounded_queue::{self, UnboundedQueue}; use bun_threading::work_pool::{Task as WorkPoolTask, WorkPool}; +use bun_threading::{Futex, Guarded}; use bun_watcher::Watcher; use crate::async_module::AsyncModule; -use crate::event_loop::{ConcurrentTask, EventLoop}; +use crate::event_loop::{ConcurrentTaskItem, EventLoop}; use crate::hot_reloader::ImportWatcher; use crate::resolved_source::OwnedResolvedSource; use crate::resolved_source_tag::ResolvedSourceTag; @@ -200,6 +200,16 @@ pub fn set_break_point_on_first_line() -> bool { pub struct RuntimeTranspilerStore { pub(crate) generation_number: AtomicU32, + /// Jobs currently on the work pool (incremented in `schedule`, decremented + /// after `run()` on the pool thread). `wait_for_inflight_jobs` joins on it + /// before VM teardown frees the memory `run()` reads. + pub(crate) in_flight: AtomicU32, + /// Single embedded wakeup node for `dispatch_to_main_thread`, guarded by + /// `dispatch_task_enqueued`. Embedded rather than Box'd per dispatch so a + /// node still sitting in the concurrent queue at VM teardown dies with the + /// VM allocation instead of leaking. + pub(crate) dispatch_task: ConcurrentTaskItem, + pub(crate) dispatch_task_enqueued: AtomicBool, pub(crate) store: TranspilerJobStore, pub enabled: bool, pub(crate) queue: Queue, @@ -211,6 +221,9 @@ impl Default for RuntimeTranspilerStore { fn default() -> Self { Self { generation_number: AtomicU32::new(0), + in_flight: AtomicU32::new(0), + dispatch_task: ConcurrentTaskItem::default(), + dispatch_task_enqueued: AtomicBool::new(false), store: TranspilerJobStore::init(), enabled: true, queue: Queue::new(), @@ -229,6 +242,34 @@ impl RuntimeTranspilerStore { Self::default() } + /// Blocks until every `TranspilerJob` scheduled on the work pool for this + /// VM has finished `run()` (including its dispatch back to this thread). + /// Must run on the VM's own thread before teardown frees anything `run()` + /// reads: the `Transpiler`, `source_mappings`, the event loop, and the job + /// slots in this store itself (the first 64 are inline in the VM box, so a + /// queued job's `work_task` node lives in freed memory otherwise). No new + /// job can be scheduled while this blocks, since `schedule()` only runs on + /// this same thread. + /// + /// Bumps `generation_number` first so jobs still queued on the pool bail at + /// the top of `run()` instead of doing a full parse of a dead VM's import. + /// + /// Polls instead of a decrement-side `Futex::wake`: the caller frees the + /// VM allocation (and this counter) right after this returns, so the pool + /// thread's final access to the VM must be the `fetch_sub` itself; a wake + /// after the decrement could land on freed memory. + pub fn wait_for_inflight_jobs(&self) { + const RECHECK_INTERVAL_NS: u64 = 1_000_000; + let _ = self.generation_number.fetch_add(1, Ordering::Release); + loop { + let n = self.in_flight.load(Ordering::Acquire); + if n == 0 { + return; + } + let _ = Futex::wait(&self.in_flight, n, Some(RECHECK_INTERVAL_NS)); + } + } + // Note: takes `NonNull` rather than `&mut` for `event_loop`/`vm` // because `&mut self` already aliases `vm.transpiler_store` (this `Self` is // a field of `VirtualMachine`). Field-level derefs only. @@ -238,6 +279,9 @@ impl RuntimeTranspilerStore { global: &JSGlobalObject, vm: NonNull, ) { + // Clear before popping so a dispatch racing this drain re-enqueues the + // embedded wakeup node (pairs with the swap in `dispatch_to_main_thread`). + let _ = self.dispatch_task_enqueued.swap(false, Ordering::AcqRel); let batch = self.queue.pop_batch(); // SAFETY: `vm` is the live owning VM (caller is the JS-thread tick loop). let jsc_vm = unsafe { (*vm.as_ptr()).jsc_vm() }; @@ -486,16 +530,32 @@ impl TranspilerJob { fn dispatch_to_main_thread(&mut self) { let vm = self.vm; - // SAFETY: vm outlives the job (BACKREF — VM owns the store). + // SAFETY: vm outlives the job (BACKREF — VM teardown joins `in_flight` + // before freeing, and this runs before the job's decrement). 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)); + + // Wake the JS thread through the store-embedded node rather than a Box + // per dispatch: a Box'd node still sitting in the concurrent queue at + // VM teardown is unreachable once the VM allocation is freed. Only the + // false→true winner touches the node; the consumer unlinked it from the + // queue before clearing the flag in `run_from_js_thread`. + // SAFETY: leaf-field atomics/writes on the live store; see BACKREF note. + if !unsafe { &(*transpiler_store).dispatch_task_enqueued }.swap(true, Ordering::AcqRel) { + // SAFETY: `event_loop()` is the live self-pointer; the embedded + // node is exclusively owned by this winner until the consumer pops + // it (auto_delete=false so the tick never frees it). + unsafe { + (*transpiler_store).dispatch_task.task = Task::init(transpiler_store); + (&*(*vm).event_loop()).enqueue_task_concurrent(NonNull::new_unchecked( + ptr::addr_of_mut!((*transpiler_store).dispatch_task), + )); + } + } } fn run_from_js_thread(&mut self) -> JsResult<()> { @@ -559,6 +619,17 @@ impl TranspilerJob { // `EventLoopCtx` vtable; resolve it via the `get_vm_ctx` hook (registered by // `bun_runtime::init`). self.poll_ref.ref_(get_vm_ctx(AllocatorType::Js)); + let vm = self.vm; + // Paired with the decrement in `run_from_worker_thread`; VM teardown + // joins on it via `wait_for_inflight_jobs` before freeing what `run()` + // reads. SAFETY: `vm` is the live owning VM (caller is the JS thread); + // leaf atomic field only, no `&VirtualMachine` formed. + let _ = unsafe { + (*vm) + .transpiler_store + .in_flight + .fetch_add(1, Ordering::Relaxed) + }; WorkPool::schedule(&raw mut self.work_task); } @@ -568,7 +639,19 @@ impl TranspilerJob { // `transpile`; the WorkPool calls back with exactly that field, so // `from_field_ptr!` recovers the live heap `TranspilerJob` parent. let this = unsafe { &mut *bun_core::from_field_ptr!(TranspilerJob, work_task, work_task) }; + let vm = this.vm; this.run(); + // `run()`'s defer already dispatched the job back to the JS thread, so + // the slot may be recycled at any point now; only `vm` is touched below. + // SAFETY: the VM stays alive until `wait_for_inflight_jobs` observes + // this decrement (its Acquire load pairs with this Release), and this + // `fetch_sub` is the pool thread's final access to the VM allocation. + let _ = unsafe { + (*vm) + .transpiler_store + .in_flight + .fetch_sub(1, Ordering::Release) + }; } fn run(&mut self) { diff --git a/src/jsc/VirtualMachine.rs b/src/jsc/VirtualMachine.rs index 6c6925f80673..a3a962b65974 100644 --- a/src/jsc/VirtualMachine.rs +++ b/src/jsc/VirtualMachine.rs @@ -4399,6 +4399,13 @@ impl VirtualMachine { } /// Worker-thread teardown. pub fn destroy(&mut self) { + // Pool-thread `TranspilerJob::run()` reads `self.transpiler`, the event + // loop, `source_mappings`, and the job slot inside `transpiler_store` + // itself; join in-flight jobs before freeing any of it. The worker + // shutdown path already waits earlier, so this is normally a single + // Acquire load. + self.transpiler_store.wait_for_inflight_jobs(); + self.regular_event_loop.deinit(); self.macro_event_loop.deinit(); diff --git a/src/jsc/web_worker.rs b/src/jsc/web_worker.rs index 6e67c931d47c..413c4e1f3993 100644 --- a/src/jsc/web_worker.rs +++ b/src/jsc/web_worker.rs @@ -1350,6 +1350,15 @@ impl WebWorker { live_workers::mark_exited(); // ---- 5. Free worker-thread resources ------------------------------- + if !vm_ptr.is_null() { + // A TranspilerJob still on the work pool (terminate() racing an + // async import) reads the VM allocation throughout `run()` and + // wakes this thread's loop when it finishes; the job slot itself + // (and its WorkPool queue link) lives inside the VM box. Join it + // before the loop teardown below and the VM free in `destroy()`. + // SAFETY: vm_ptr valid (sole owner); leaf-field borrow only. + unsafe { &(*vm_ptr).transpiler_store }.wait_for_inflight_jobs(); + } if let Some(loop_) = loop_ { // SAFETY: loop owned by this thread's VM; no concurrent access. unsafe { (*loop_).internal_loop_data.jsc_vm = core::ptr::null_mut() }; diff --git a/test/js/web/workers/worker-terminate-lifetime.test.ts b/test/js/web/workers/worker-terminate-lifetime.test.ts index 374246033f82..68159e1f42a9 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 { describe, expect, test } from "bun:test"; -import { bunEnv, bunExe, isASAN, isDebug, tls } from "harness"; +import { bunEnv, bunExe, isASAN, isDebug, tempDir, tls } from "harness"; import { join } from "path"; // Worker VM startup/teardown is much slower under debug and/or ASAN; these @@ -121,6 +121,70 @@ test( timeout, ); +// UAF: a TranspilerJob for a worker's dynamic import() runs on the shared work +// pool and reads the worker's VirtualMachine allocation throughout run(): the +// Transpiler (defines table), test_isolation_enabled, source_mappings, and the +// job slot itself (the first 64 slots are an inline array in the VM box, so a +// queued job's WorkPool link is inside the allocation too). terminate() freed +// the VM mid-transpile. VM teardown now joins in-flight transpiler jobs first. +// Only ASAN reliably turns the stale reads into a crash. +// https://github.com/oven-sh/bun/issues/33936 +test.skipIf(!isASAN)( + "terminate() racing an in-flight dynamic import transpile does not UAF", + async () => { + // Big enough that the pool-thread transpile outlasts the terminated + // worker's teardown by a wide margin under debug+ASAN. + const big = Array.from( + { length: 4000 }, + (_, i) => `export function f${i}(a,b){ const x = a?.b ?? (b ||= ${i}); return x + ${i}; }`, + ).join("\n"); + using dir = tempDir("worker-terminate-transpile", { + "big.ts": big, + // "started" is posted after import() has scheduled the transpile on the + // work pool, so the parent's terminate() lands while it is in flight. + "racer-worker.js": ` + import("./big.ts").then(() => postMessage("done")).catch(() => {}); + postMessage("started"); + `, + // Untouched worker transpiling the same module keeps the process alive + // for the whole window in which the racer's orphaned pool-thread job + // reads the freed VM on broken builds. + "ref-worker.js": ` + import("./big.ts?ref").then(() => postMessage("done"), err => postMessage("error: " + err)); + `, + "main.js": ` + const racer = new Worker(new URL("./racer-worker.js", import.meta.url).href); + const terminated = new Promise((resolve, reject) => { + racer.onmessage = e => { if (e.data === "started") { racer.terminate(); resolve(); } }; + racer.onerror = e => reject(new Error("racer worker: " + e.message)); + }); + const ref = new Worker(new URL("./ref-worker.js", import.meta.url).href); + const refDone = new Promise((resolve, reject) => { + ref.onmessage = e => { e.data === "done" ? resolve() : reject(new Error("ref worker: " + e.data)); }; + ref.onerror = e => reject(new Error("ref worker: " + e.message)); + }); + await terminated; + await refDone; + ref.terminate(); + console.log("ok"); + `, + }); + 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("ok\n"); + expect(exitCode).toBe(0); + }, + timeout, +); + // Regression: the per-VM c-ares channel was destroyed in deinit_runtime_state // (RuntimeState drop) AFTER JSC teardown and RareData.file_polls drop. // ares_destroy() synchronously fires EDESTRUCTION query callbacks and socket-