diff --git a/src/bundler/BundleThread.rs b/src/bundler/BundleThread.rs index 18c41f02c3ee..2504b008caf0 100644 --- a/src/bundler/BundleThread.rs +++ b/src/bundler/BundleThread.rs @@ -57,6 +57,12 @@ pub struct BundleThread { /// The trait accessors keep the generic `BundleThread` /// layout-agnostic. The concrete impl lives in T6 (`bun_bundler_jsc`). pub trait CompletionStruct: Node + Send + 'static { + /// Whether the JS context that scheduled this build is still alive. When + /// `false` the bundle thread skips the build, since + /// `complete_on_bundle_thread` would discard the result anyway. + fn is_owner_alive(&self) -> bool { + true + } /// `bump` is the per-build mimalloc heap that backs `transpiler`, so the /// two share lifetime `'a` (option fields like `optimize_imports: &'a /// StringSet` borrow from `bump`). @@ -74,9 +80,8 @@ pub trait CompletionStruct: Node + Send + 'static { /// `FileMap` layout stays in T6. fn file_map(&mut self) -> Option>; /// Returns a §Dispatch handle (erased owner + `&'static` vtable) the impl - /// provides, so the bundler can read `result == .err` / - /// `jsc_event_loop.enqueueTaskConcurrent` without naming the concrete - /// struct. + /// provides, so the bundler can read `result == .err` / post plugin tasks + /// to the JS event loop without naming the concrete struct. fn as_js_bundle_completion_task(&mut self) -> dispatch::CompletionHandle; /// `Transpiler<'a>` has borrow-carrying fields (`arena: &'a Arena`, @@ -221,6 +226,15 @@ impl BundleThread { // SAFETY: queue stores non-null *mut C pushed via enqueue(); owner keeps it alive // until complete_on_bundle_thread() signals completion. let completion = unsafe { &mut *completion }; + if !completion.is_owner_alive() { + // The worker that scheduled this build has begun shutdown; + // skip the build since its result would be dropped anyway. + // `complete_on_bundle_thread` observes the same dead + // context and discards the post. + completion.complete_on_bundle_thread(); + has_bundled = true; + continue; + } // SAFETY: `generation` is only read/written on this (bundle) thread. let generation = unsafe { (*instance).generation }; // `panic = "abort"` → a Rust panic on this thread enters the diff --git a/src/bundler/bundle_v2.rs b/src/bundler/bundle_v2.rs index e1a2f1fda10b..a2e7987c9a53 100644 --- a/src/bundler/bundle_v2.rs +++ b/src/bundler/bundle_v2.rs @@ -1501,7 +1501,7 @@ pub mod bv2_impl { ) { debug_assert!(self.plugins.is_some()); if let Some(completion) = self.completion { - // From Bun.build — `completion.jsc_event_loop.enqueueTaskConcurrent(task)`. + // From Bun.build — post to the originating context's JS loop. completion.enqueue_task_concurrent(task); return; } diff --git a/src/jsc/JSGlobalObject.rs b/src/jsc/JSGlobalObject.rs index d80cdb520570..38ec9b0de115 100644 --- a/src/jsc/JSGlobalObject.rs +++ b/src/jsc/JSGlobalObject.rs @@ -1567,6 +1567,22 @@ unsafe extern "C" { } impl ScriptExecutionContextIdentifier { + /// Enqueue a heap-allocated [`ConcurrentTaskItem`] onto this context's JS + /// event loop from any thread. Serializes with worker termination on the + /// C++ contexts-map lock (the same lock `markTerminating()` takes before + /// the worker's VM is freed), so this is safe to call without holding any + /// pointer into the target VM. + /// + /// Returns `true` if the task was enqueued (ownership transferred). + /// Returns `false` if the context is gone or terminating; the caller + /// retains ownership of `task` and must free it. + pub fn post_concurrent_task( + self, + task: core::ptr::NonNull, + ) -> bool { + ScriptExecutionContext__postConcurrentTask(self.0, task.as_ptr().cast::()) + } + /// Returns `None` if the context referred to by `self` no longer exists. pub fn global_object(self) -> Option { // FFI call returns a valid pointer or null; the JSGlobalObject is owned @@ -1587,9 +1603,32 @@ impl ScriptExecutionContextIdentifier { pub fn valid(self) -> bool { self.global_object().is_some() } + + /// `true` if the context still exists and has not been marked terminating. + /// Serializes with `markTerminating()` on the C++ contexts-map lock. + pub fn is_alive(self) -> bool { + ScriptExecutionContext__isAlive(self.0) + } } unsafe extern "C" { // safe: by-value `u32` in, raw nullable pointer out (caller checks before deref). safe fn ScriptExecutionContextIdentifier__getGlobalObject(id: u32) -> *mut JSGlobalObject; + // safe: by-value `u32` in, opaque pointer C++ never dereferences (it only + // passes it back to Rust under the contexts-map lock); bool out. + safe fn ScriptExecutionContext__postConcurrentTask(id: u32, task: *mut c_void) -> bool; + // safe: by-value `u32` in; bool out. + safe fn ScriptExecutionContext__isAlive(id: u32) -> bool; + // safe: `&JSGlobalObject` is a live opaque handle; returns the context's id. + safe fn ScriptExecutionContextIdentifier__forGlobalObject(global: &JSGlobalObject) -> u32; +} + +impl JSGlobalObject { + /// The stable identifier for this global's `ScriptExecutionContext`. + /// Safe to capture for later use from another thread via + /// [`ScriptExecutionContextIdentifier::post_concurrent_task`]. + #[inline] + pub fn script_execution_context_identifier(&self) -> ScriptExecutionContextIdentifier { + ScriptExecutionContextIdentifier(ScriptExecutionContextIdentifier__forGlobalObject(self)) + } } diff --git a/src/jsc/bindings/ScriptExecutionContext.cpp b/src/jsc/bindings/ScriptExecutionContext.cpp index 60f4ca58b9ef..d01774afa70b 100644 --- a/src/jsc/bindings/ScriptExecutionContext.cpp +++ b/src/jsc/bindings/ScriptExecutionContext.cpp @@ -302,6 +302,34 @@ extern "C" JSC::JSGlobalObject* ScriptExecutionContextIdentifier__getGlobalObjec return context->globalObject(); } +// Checks under the contexts-map lock whether the context still exists and has +// not been marked terminating. Used by off-thread work (bundle thread) that +// borrows worker-owned resources, to skip a queued unit of work whose owning +// worker has already begun shutdown. +extern "C" bool ScriptExecutionContext__isAlive(ScriptExecutionContextIdentifier id) +{ + Locker locker { allScriptExecutionContextsMapLock }; + auto* context = allScriptExecutionContextsMap().get(id); + return context && !context->isTerminating(); +} + +extern "C" void Bun__EventLoop__enqueueConcurrentTask(JSC::JSGlobalObject*, void* task); + +// postTaskTo() for a Rust-allocated ConcurrentTask. Returns true if the task +// was enqueued (ownership transferred to the target context's concurrent +// queue); false if the context is gone or terminating, in which case the +// caller retains ownership. The map lock is held across the enqueue so this +// serializes with markTerminating() the same way postTaskTo() does. +extern "C" bool ScriptExecutionContext__postConcurrentTask(ScriptExecutionContextIdentifier id, void* task) +{ + Locker locker { allScriptExecutionContextsMapLock }; + auto* context = allScriptExecutionContextsMap().get(id); + if (!context || context->isTerminating()) + return false; + Bun__EventLoop__enqueueConcurrentTask(context->globalObject(), task); + return true; +} + extern "C" void ScriptExecutionContext__markTerminating(JSC::JSGlobalObject* globalObject) { if (auto* context = defaultGlobalObject(globalObject)->scriptExecutionContext()) diff --git a/src/jsc/virtual_machine_exports.rs b/src/jsc/virtual_machine_exports.rs index 89258ef3d574..b153fc448fb4 100644 --- a/src/jsc/virtual_machine_exports.rs +++ b/src/jsc/virtual_machine_exports.rs @@ -141,6 +141,26 @@ pub fn queue_task_concurrently(global: &JSGlobalObject, task: *mut crate::cpp_ta } } +/// Called from `ScriptExecutionContext__postConcurrentTask` with the contexts +/// map lock held, so `global`'s VM and its `EventLoop` are guaranteed live for +/// the duration of this call (worker `markTerminating()` serializes on the +/// same lock before any dealloc). `task` is a heap `ConcurrentTaskItem` the +/// caller has already allocated. +// HOST_EXPORT(Bun__EventLoop__enqueueConcurrentTask, c) +#[allow(clippy::not_unsafe_ptr_arg_deref)] +pub fn event_loop_enqueue_concurrent_task( + global: &JSGlobalObject, + task: *mut crate::event_loop::ConcurrentTaskItem, +) { + crate::mark_binding!(); + // SAFETY: see fn doc — VM/EventLoop live under the held contexts-map lock; + // `task` is a non-null heap allocation handed over from the Rust caller. + unsafe { + (*(*global.bun_vm_concurrently()).event_loop()) + .enqueue_task_concurrent(core::ptr::NonNull::new_unchecked(task)); + } +} + // HOST_EXPORT(Bun__handleRejectedPromise, c) pub fn handle_rejected_promise(global: &JSGlobalObject, promise: &mut JSPromise) { crate::mark_binding!(); diff --git a/src/runtime/api/JSBundler.rs b/src/runtime/api/JSBundler.rs index abf14975491d..32aa713bf522 100644 --- a/src/runtime/api/JSBundler.rs +++ b/src/runtime/api/JSBundler.rs @@ -1339,8 +1339,6 @@ pub mod js_bundler { let mut plugins: Option<*mut Plugin> = None; let config = Config::from_js(global_this, arguments[0], &mut plugins)?; - let event_loop = vm.event_loop(); - // `BundleV2.generateFromJavaScript` — the completion-task struct lives in // `crate::api::js_bundle_completion_task` (bun_runtime owns it because its // fields name `Config`/`Plugin`/`HTMLBundle::Route`; lower-tier crates @@ -1350,9 +1348,15 @@ pub mod js_bundler { config, plugins.and_then(core::ptr::NonNull::new), global_this, - event_loop, ) - .map_err(|_| JsError::OutOfMemory)?; + .map_err(|_| { + // `Bun.build` owns its plugin (unlike `HTMLBundle::Route`, which + // borrows the server's); release on the one fallible early return. + if let Some(p) = plugins { + Plugin::destroy(p); + } + JsError::OutOfMemory + })?; // SAFETY: `completion` is the freshly-boxed allocation returned above; // sole owner on the JS thread until enqueued task runs. unsafe { diff --git a/src/runtime/api/js_bundle_completion_task.rs b/src/runtime/api/js_bundle_completion_task.rs index 911d20f849d2..c0e9a7c22e44 100644 --- a/src/runtime/api/js_bundle_completion_task.rs +++ b/src/runtime/api/js_bundle_completion_task.rs @@ -26,6 +26,7 @@ use bun_io::KeepAlive; use bun_jsc::AnyTask::AnyTask; use bun_jsc::WorkPool; use bun_jsc::event_loop::EventLoop; +use bun_jsc::js_global_object::ScriptExecutionContextIdentifier; use bun_jsc::{self as jsc, JSGlobalObject, JSPromise, JSValue}; use bun_options_types::WindowsOptions; use bun_options_types::schema::api; @@ -58,14 +59,27 @@ pub struct JSBundleCompletionTask { // `unsafe impl Send` below for the thread-affinity constraint this imposes. pub ref_count: RefCount, pub config: JSBundlerConfig, - // BACKREF — the JS-thread `EventLoop` outlives every completion task; safe - // `Deref` so call sites read `self.jsc_event_loop.enqueue_task_concurrent(..)`. + /// Stable identifier for the originating `ScriptExecutionContext`. The + /// bundle thread posts the completion via this id (under the C++ + /// contexts-map lock), so it never holds a pointer into a worker VM that + /// can be freed mid-bundle. + pub context_id: ScriptExecutionContextIdentifier, + /// Direct event-loop backref for mid-build plugin dispatches + /// (`COMPLETION_VTABLE.enqueue_task_concurrent`). Dropping a plugin + /// `onLoad`/`onResolve` post when the worker is terminating would leave + /// `graph.pending_items` unbalanced and hang the process-wide + /// `BundleThread` inside `wait_for_parse`, so that path keeps the pre-PR + /// direct enqueue (and its pre-existing UAF on termination). pub jsc_event_loop: BackRef, pub task: AnyTask, pub global_this: BackRef, pub promise: jsc::JSPromiseStrong, pub poll_ref: KeepAlive, + /// Task-owned env loader + map cloned from the VM at creation time, so the + /// bundle thread never dereferences worker-lifetime memory. Freed in + /// `deinit` (loader first; it borrows `env_map`). pub env: *mut bun_dotenv::Loader<'static>, + env_map: *mut bun_dotenv::Map, pub log: bun_ast::Log, pub cancelled: bool, @@ -96,6 +110,16 @@ impl JSBundleCompletionTask { // last-ref drop is the only place that releases it. Plugin::destroy(plugin.as_ptr()); } + // `env`/`env_map` are null only when `complete_on_bundle_thread`'s + // dead-owner branch already freed them. + if !boxed.env.is_null() { + // SAFETY: `heap::into_raw`'d in `create_and_schedule_completion_task`; + // sole owner. Loader borrows `env_map`, so drop it first. + unsafe { + drop(bun_core::heap::take(boxed.env)); + drop(bun_core::heap::take(boxed.env_map)); + } + } // Owned fields (`config`, `log`, `result`, `promise`) drop with the Box. } } @@ -115,21 +139,43 @@ pub(crate) fn create_and_schedule_completion_task( config: JSBundlerConfig, plugins: Option>, global_this: &JSGlobalObject, - event_loop: *mut EventLoop, ) -> crate::Result<*mut JSBundleCompletionTask> { let vm = global_this.bun_vm_ptr(); - let env = global_this.bun_vm().transpiler.env; + // Snapshot the env map now, on the JS thread, so the bundle thread never + // dereferences worker-owned memory (a worker terminated mid-bundle frees + // its VM's loader). `did_load_process` is set so `run_env_loader`'s + // `load_process()` early-returns instead of re-walking OS environ and + // clobbering JS-set values that differ from it. The `plugins` argument is + // owned for `Bun.build` but borrowed for `HTMLBundle::Route`, so on OOM + // release is the caller's responsibility. + let env_map = bun_core::heap::into_raw(Box::new( + global_this + .bun_vm() + .env_loader() + .map + .clone_with_allocator()?, + )); + let env = { + // SAFETY: `env_map` is a fresh heap allocation owned by this task; the + // `'static` borrow is the lifetime erasure for the task-lifetime map. + let mut l = bun_dotenv::Loader::init(unsafe { &mut *env_map }); + l.did_load_process = true; + bun_core::heap::into_raw(Box::new(l)) + }; + // SAFETY: `event_loop()` is non-null once `Bun.build` is reachable. + let jsc_event_loop = + BackRef::from(unsafe { NonNull::new_unchecked(global_this.bun_vm().event_loop()) }); let completion = bun_core::heap::into_raw(Box::new(JSBundleCompletionTask { ref_count: RefCount::init(), config, - // `event_loop` is the live JS-thread loop (caller derives it from - // `vm.event_loop()`); never null once `Bun.build` is reachable. - jsc_event_loop: BackRef::from(core::ptr::NonNull::new(event_loop).expect("event_loop")), + context_id: global_this.script_execution_context_identifier(), + jsc_event_loop, task: AnyTask::default(), global_this: BackRef::new(global_this), promise: jsc::JSPromiseStrong::default(), poll_ref: KeepAlive::init(), env, + env_map, log: bun_ast::Log::init(), cancelled: false, html_build_task: None, @@ -379,8 +425,8 @@ impl JSBundleCompletionTask { flags |= StandaloneFlags::DISABLE_AUTOLOAD_PACKAGE_JSON; } - // SAFETY: `self.env` is the per-VM `DotEnv.Loader` stashed at - // construction; valid for the lifetime of the VirtualMachine. + // SAFETY: `self.env` is the task-owned loader heap-allocated at + // construction; valid until `deinit`. let env = unsafe { &mut *self.env.cast::() }; let result = match to_executable( @@ -761,7 +807,12 @@ fn from_completion_handle<'a>(c: NonNull) -> &'a JSBundleCo static COMPLETION_VTABLE: dispatch::CompletionDispatch = dispatch::CompletionDispatch { result_is_err: |c| matches!(from_completion_handle(c).result, BundleV2Result::Err(_)), enqueue_task_concurrent: |c, task| { - // `jsc_event_loop` is a `BackRef` — safe Deref. + // This path carries mid-build plugin `onLoad`/`onResolve` dispatches + // (via `enqueue_on_js_loop_for_plugins`). Silently dropping one when + // the owning worker is terminating would strand `graph.pending_items` + // and hang the process-wide `BundleThread` inside `wait_for_parse`, so + // it stays on the direct enqueue (unchanged from main) rather than the + // `post_concurrent_task` path used by `complete_on_bundle_thread`. // SAFETY: `task` is a fresh heap-allocated non-null `ConcurrentTaskItem` // passed through from the bundler vtable; the queue takes ownership. unsafe { @@ -784,6 +835,10 @@ unsafe impl bun_threading::Linked for JSBundleCompletionTask { } impl CompletionStruct for JSBundleCompletionTask { + fn is_owner_alive(&self) -> bool { + self.context_id.is_alive() + } + /// Port of `JSBundleCompletionTask.configureBundler` — the post-init half /// (everything after `transpiler.* = try Transpiler.init(...)`). /// `Transpiler::init` itself is called by `create_and_configure_transpiler` @@ -980,11 +1035,30 @@ impl CompletionStruct for JSBundleCompletionTask { } fn complete_on_bundle_thread(&mut self) { - // `jsc_event_loop` is a `BackRef` — safe Deref. - // `ConcurrentTask::create` heap-allocates a fresh task; the - // queue takes ownership of it. - self.jsc_event_loop - .enqueue_task_concurrent(jsc::ConcurrentTask::create(self.task.task())); + // Post by stable context id: `post_concurrent_task` serializes with + // `markTerminating()` on the C++ contexts-map lock and only + // dereferences the target VM while that lock is held. + let task = jsc::ConcurrentTask::create(self.task.task()); + if !self.context_id.post_concurrent_task(task) { + // Target context is gone or terminating. `on_complete_anytask` + // will never run; reclaim the `ConcurrentTaskItem` and the large + // task-owned payloads here. The box itself (and its JSC handle + // `promise` / C++ `plugins`, which point into the dead VM) cannot + // be released off the JS thread; that residual is bounded. + // SAFETY: `post_concurrent_task` returned false so ownership was + // not transferred; `task` was `ConcurrentTask::create`-allocated. + drop(unsafe { bun_core::heap::take(task.as_ptr()) }); + self.result = BundleV2Result::Pending; + self.log = bun_ast::Log::init(); + let env = core::mem::replace(&mut self.env, ptr::null_mut()); + let env_map = core::mem::replace(&mut self.env_map, ptr::null_mut()); + // SAFETY: both are the non-null `heap::into_raw`'d allocations + // from `create_and_schedule_completion_task`; sole owner. + unsafe { + drop(bun_core::heap::take(env)); + drop(bun_core::heap::take(env_map)); + } + } } fn set_result(&mut self, result: BundleV2Result) { self.result = result; @@ -1050,9 +1124,7 @@ impl CompletionStruct for JSBundleCompletionTask { }; let log: *mut bun_ast::Log = &raw mut self.log; - // SAFETY: `self.env` is the per-VM dotenv loader stashed at - // construction; cast erases `'_` (bun_dotenv::Loader is invariant on - // its arena lifetime, but `Transpiler::init` only stores the pointer). + // `self.env` is the task-owned loader heap-allocated at construction. let env = self.env.cast::>(); let t = Transpiler::init(bump, log, opts, Some(env))?; let transpiler: &'a mut Transpiler<'a> = bump.alloc(t); diff --git a/src/runtime/server/HTMLBundle.rs b/src/runtime/server/HTMLBundle.rs index 937785d53a27..28de23af4537 100644 --- a/src/runtime/server/HTMLBundle.rs +++ b/src/runtime/server/HTMLBundle.rs @@ -498,8 +498,7 @@ impl Route { } config.source_map = bundler_options::SourceMapOption::Linked; - let completion_task = - create_and_schedule_completion_task(config, plugins, global, vm.event_loop())?; + let completion_task = create_and_schedule_completion_task(config, plugins, global)?; // SAFETY: `completion_task` is the freshly-boxed allocation (refcount==1); sole owner. unsafe { (*completion_task).started_at_ns = diff --git a/test/bundler/bun-build-api.test.ts b/test/bundler/bun-build-api.test.ts index 2b7fa720b654..e2858c581e6c 100644 --- a/test/bundler/bun-build-api.test.ts +++ b/test/bundler/bun-build-api.test.ts @@ -1513,3 +1513,68 @@ test("Bun.build can be called thousands of times in one process without crashing expect(stdout.trim()).toBe("OK 400"); expect(exitCode).toBe(0); }, 180_000); + +// Regression: terminating a worker with an in-flight Bun.build() would crash +// the whole process. The bundler runs on its own thread and posted the +// completion back into the terminated worker's freed EventLoop/VM +// (JSBundleCompletionTask::complete_on_bundle_thread -> enqueue_task_concurrent +// -> heap-use-after-free, or SIGSEGV on release builds). A second queued +// build then dereferenced the worker's freed env loader in Transpiler::init. +// Under ASAN (the gate's default build) this was deterministic; on release it +// SIGSEGV'd in a few rounds. +test( + "terminating a worker mid-Bun.build() does not crash the process", + async () => { + // ASAN catches the UAF on the first round; release needs the terminate to + // land mid-bundle, so scale the module graph / round count by build speed. + const slow = isDebug || isASAN; + const modules = slow ? 100 : 400; + const rounds = slow ? 4 : 12; + + const files: Record = {}; + const lines: string[] = []; + for (let i = 0; i < modules; i++) { + files[`m${i}.ts`] = + `export const v${i}: number = ${i};\n` + `export function f${i}(x: number) { return x + ${i}; }\n`; + lines.push(`import * as m${i} from "./m${i}.ts"; globalThis.k${i} = m${i};`); + } + files["entry.ts"] = lines.join("\n"); + files["run.ts"] = ` + const { Worker } = require("node:worker_threads"); + const entry = process.argv[2]; + const src = \`const { parentPort, workerData } = require("node:worker_threads"); + const lanes = (n, f) => { for (let i = 0; i < n; i++) (async () => { for (;;) { try { await f(i); } catch {} } })(); }; + lanes(2, () => Bun.build({ entrypoints: [workerData.entry], target: "bun", minify: true })); + parentPort.postMessage("up");\`; + for (let r = 0; r < ${rounds}; r++) { + const w = new Worker(src, { eval: true, workerData: { entry } }); + await new Promise((res, rej) => { w.once("message", res); w.once("error", rej); }); + await Bun.sleep(60 + (r * 41) % 220); + await w.terminate(); + } + console.log("OK " + ${rounds}); + `; + using dir = tempDir("bun-build-worker-terminate", files); + + await using proc = Bun.spawn({ + cmd: [bunExe(), join(String(dir), "run.ts"), join(String(dir), "entry.ts")], + env: { + ...bunEnv, + // Pre-existing bounded worker-termination leaks on main (per-worker + // node:fs Binding, WebWorker box) would abort the subprocess under + // CI's detect_leaks=1. This test covers the UAF -> crash; the unfixed + // bug aborts with a heap-use-after-free report regardless. + ASAN_OPTIONS: [bunEnv.ASAN_OPTIONS, "detect_leaks=0"].filter(Boolean).join(":"), + }, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect({ stdout: stdout.trim(), stderr, exitCode }).toEqual({ + stdout: "OK " + rounds, + stderr: "", + exitCode: 0, + }); + }, + isDebug || isASAN ? 120_000 : 45_000, +);