Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
97 changes: 90 additions & 7 deletions src/jsc/RuntimeTranspilerStore.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand All @@ -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;
Expand Down Expand Up @@ -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.
Comment thread
robobun marked this conversation as resolved.
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.
Comment thread
robobun marked this conversation as resolved.
pub(crate) dispatch_task: ConcurrentTaskItem,
pub(crate) dispatch_task_enqueued: AtomicBool,
pub(crate) store: TranspilerJobStore,
pub enabled: bool,
pub(crate) queue: Queue,
Expand All @@ -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(),
Expand All @@ -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.
Comment thread
robobun marked this conversation as resolved.
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.
Expand All @@ -238,6 +279,9 @@ impl RuntimeTranspilerStore {
global: &JSGlobalObject,
vm: NonNull<VirtualMachine>,
) {
// Clear before popping so a dispatch racing this drain re-enqueues the
// embedded wakeup node (pairs with the swap in `dispatch_to_main_thread`).
Comment thread
robobun marked this conversation as resolved.
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() };
Expand Down Expand Up @@ -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<()> {
Expand Down Expand Up @@ -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);
}

Expand All @@ -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) {
Expand Down
7 changes: 7 additions & 0 deletions src/jsc/VirtualMachine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Comment thread
robobun marked this conversation as resolved.
self.transpiler_store.wait_for_inflight_jobs();

self.regular_event_loop.deinit();
self.macro_event_loop.deinit();

Expand Down
9 changes: 9 additions & 0 deletions src/jsc/web_worker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Comment thread
robobun marked this conversation as resolved.
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() };
Expand Down
66 changes: 65 additions & 1 deletion test/js/web/workers/worker-terminate-lifetime.test.ts
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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));
});
Comment thread
coderabbitai[bot] marked this conversation as resolved.
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-
Expand Down
Loading