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
20 changes: 17 additions & 3 deletions src/bundler/BundleThread.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,12 @@ pub struct BundleThread<C: Node> {
/// The trait accessors keep the generic `BundleThread<C>`
/// 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`).
Expand All @@ -74,9 +80,8 @@ pub trait CompletionStruct: Node + Send + 'static {
/// `FileMap` layout stays in T6.
fn file_map(&mut self) -> Option<NonNull<FileMap>>;
/// 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`,
Expand Down Expand Up @@ -221,6 +226,15 @@ impl<C: CompletionStruct> BundleThread<C> {
// 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;
}
Comment thread
robobun marked this conversation as resolved.
// 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
Expand Down
2 changes: 1 addition & 1 deletion src/bundler/bundle_v2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
39 changes: 39 additions & 0 deletions src/jsc/JSGlobalObject.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<crate::event_loop::ConcurrentTaskItem>,
) -> bool {
ScriptExecutionContext__postConcurrentTask(self.0, task.as_ptr().cast::<c_void>())
}

/// Returns `None` if the context referred to by `self` no longer exists.
pub fn global_object(self) -> Option<GlobalRef> {
// FFI call returns a valid pointer or null; the JSGlobalObject is owned
Expand All @@ -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))
}
}
28 changes: 28 additions & 0 deletions src/jsc/bindings/ScriptExecutionContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down
20 changes: 20 additions & 0 deletions src/jsc/virtual_machine_exports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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!();
Expand Down
12 changes: 8 additions & 4 deletions src/runtime/api/JSBundler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 {
Expand Down
108 changes: 90 additions & 18 deletions src/runtime/api/js_bundle_completion_task.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -58,14 +59,27 @@ pub struct JSBundleCompletionTask {
// `unsafe impl Send` below for the thread-affinity constraint this imposes.
pub ref_count: RefCount<Self>,
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<EventLoop>,
pub task: AnyTask,
pub global_this: BackRef<JSGlobalObject>,
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,

Expand Down Expand Up @@ -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.
}
}
Expand All @@ -115,21 +139,43 @@ pub(crate) fn create_and_schedule_completion_task(
config: JSBundlerConfig,
plugins: Option<NonNull<Plugin>>,
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,
Expand Down Expand Up @@ -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::<bun_dotenv::Loader>() };

let result = match to_executable(
Expand Down Expand Up @@ -761,7 +807,12 @@ fn from_completion_handle<'a>(c: NonNull<Bv2OpaqueCompletion>) -> &'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<EventLoop>` — 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 {
Expand All @@ -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`
Expand Down Expand Up @@ -980,11 +1035,30 @@ impl CompletionStruct for JSBundleCompletionTask {
}

fn complete_on_bundle_thread(&mut self) {
// `jsc_event_loop` is a `BackRef<EventLoop>` — 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;
Expand Down Expand Up @@ -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::<bun_dotenv::Loader<'static>>();
let t = Transpiler::init(bump, log, opts, Some(env))?;
let transpiler: &'a mut Transpiler<'a> = bump.alloc(t);
Expand Down
3 changes: 1 addition & 2 deletions src/runtime/server/HTMLBundle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down
Loading
Loading