diff --git a/src/bun_core/env_var.rs b/src/bun_core/env_var.rs index eb14e27b6a99..1c5082acfb23 100644 --- a/src/bun_core/env_var.rs +++ b/src/bun_core/env_var.rs @@ -206,10 +206,10 @@ pub mod feature_flag { // Run the full VM teardown when the main thread exits (workers always do). // The CI runner turns it on for LeakSanitizer-validated files on ASAN. new_feature_flag!(pub BUN_DESTRUCT_VM_ON_EXIT, "BUN_DESTRUCT_VM_ON_EXIT", {}); - // Test suite only, builds with debug assertions: a worker VM's handle makes - // cross-thread completions wait for its close, so each producer's "refused" - // release path runs deterministically (bun_jsc::vm_handle::refusal_gate). - new_feature_flag!(pub BUN_DEBUG_TEST_WORKER_REFUSAL_GATE, "BUN_DEBUG_TEST_WORKER_REFUSAL_GATE", {}); + // Test suite only, builds with debug assertions: a worker VM holds every + // cross-thread completion until its teardown is waiting, so the "arrived + // during teardown" paths run deterministically (bun_jsc::vm_handle::test_gate). + new_feature_flag!(pub BUN_DEBUG_TEST_WORKER_TEARDOWN_GATE, "BUN_DEBUG_TEST_WORKER_TEARDOWN_GATE", {}); // Disable "nativeDependencies" new_feature_flag!(pub BUN_FEATURE_FLAG_DISABLE_NATIVE_DEPENDENCY_LINKER, "BUN_FEATURE_FLAG_DISABLE_NATIVE_DEPENDENCY_LINKER", {}); diff --git a/src/bundler/bundle_v2.rs b/src/bundler/bundle_v2.rs index a2a31fd5e974..f9be9d28d01a 100644 --- a/src/bundler/bundle_v2.rs +++ b/src/bundler/bundle_v2.rs @@ -1531,7 +1531,7 @@ pub mod bv2_impl { ) { debug_assert!(self.plugins.is_some()); if let Some(completion) = self.completion { - // From Bun.build — the completion posts it to its VM (`loop_handle.post_task` via the vtable). + // From Bun.build — the completion posts it to its VM (through its ticket, via the vtable). completion.enqueue_task_concurrent(task); return; } diff --git a/src/event_loop/AnyEventLoop.rs b/src/event_loop/AnyEventLoop.rs index d5978e8bb1a7..d237e2f42331 100644 --- a/src/event_loop/AnyEventLoop.rs +++ b/src/event_loop/AnyEventLoop.rs @@ -67,8 +67,8 @@ impl Default for AnyEventLoop { } impl AnyEventLoop { - /// Owning thread: the poster other threads use to deliver JS-loop tasks - /// to this loop's VM; `None` for a mini loop. + /// Owning thread: the weak poster other threads use to deliver JS-loop + /// tasks to this loop's VM; `None` for a mini loop. pub fn js_poster(&self) -> Option { match self { AnyEventLoop::Js { owner } => Some(owner.js_poster()), @@ -433,9 +433,9 @@ impl EventLoopHandle { EnteredEventLoop(self) } - /// Owning thread: the poster other threads use to deliver JS-loop tasks to - /// this handle's VM; `None` for a mini loop (post to it directly — it is - /// owned by, and outlives the work of, its thread). + /// Owning thread: the weak poster other threads use to deliver JS-loop + /// tasks to this handle's VM; `None` for a mini loop (post to it directly — + /// it is owned by, and outlives the work of, its thread). pub fn js_poster(&self) -> Option { match self { EventLoopHandle::Js { owner } => Some(owner.js_poster()), @@ -525,15 +525,16 @@ impl EventLoopHandle { // ─────────────────────────── JsPoster ────────────────────────────────────── // -// How code below `bun_jsc` (spawn's waiter thread, the bundler's JS-loop hops, -// shell/fs work that may serve a JS VM) posts a `ConcurrentTask` to a JS VM -// from another thread. It is an erased `bun_jsc::VmHandle` clone: `bun_jsc` -// fills the vtable; holders just call `post`. The VM's teardown closes the -// underlying handle, after which `post` refuses (returns the task) and the -// caller releases it on its own thread. Valid for as long as it is held. - -/// Result of posting a task to a JS loop from another thread: it was queued, or -/// the loop's VM is gone and the caller has the task back to release on this +// How code below `bun_jsc` reaches a JS VM from another thread: an erased, +// *uncounted* `bun_jsc::VmHandle` (`bun_jsc` fills the vtable) — what something +// that merely refers to a VM holds (spawn's process-wide waiter thread, a +// bundle owned by a JS loop). Its `post` is deliver-or-refuse: once the VM has +// closed, the task comes back and the caller releases it — its own payload — +// on its own thread. Work that a VM must *wait* for holds a `bun_jsc::Ticket` +// instead (a `Bun.build`'s completion task carries one for the bundle thread). + +/// Result of a weak post to a JS loop from another thread: it was queued, or +/// the loop's VM is closed and the caller has the task back to release on this /// thread. #[must_use = "a refused task must be released by its producer"] pub enum Posted { @@ -543,9 +544,6 @@ pub enum Posted { pub struct JsPosterVTable { pub post: unsafe fn(data: *const (), task: NonNull) -> Posted, - /// `VmHandle::embedded_work_scheduled` / `_finished` (see there). - pub embedded_work_scheduled: unsafe fn(data: *const ()), - pub embedded_work_finished: unsafe fn(data: *const ()), pub clone: unsafe fn(data: *const ()) -> *const (), pub drop: unsafe fn(data: *const ()), } @@ -564,31 +562,19 @@ unsafe impl Sync for JsPoster {} impl JsPoster { /// # Safety /// `data`/`vtable` come from one of `bun_jsc::vm_handle`'s `to_js_poster` - /// implementations (`VmHandle` / `LoopHandle` / the isolated poster). + /// implementations (`VmHandle` / the isolated poster). #[inline] pub unsafe fn from_raw(data: *const (), vtable: &'static JsPosterVTable) -> Self { Self { data, vtable } } /// Queue `task` on the VM this poster was created for and wake it, or hand - /// it back if the VM has been torn down. + /// it back if the VM has closed. #[inline] pub fn post(&self, task: NonNull) -> Posted { // SAFETY: vtable contract. unsafe { (self.vtable.post)(self.data, task) } } - - #[inline] - /// Count work whose storage the VM (indirectly) owns; it waits for the - /// matching `embedded_work_finished` before closing. See `VmHandle`. - pub fn embedded_work_scheduled(&self) { - // SAFETY: vtable contract. - unsafe { (self.vtable.embedded_work_scheduled)(self.data) } - } - pub fn embedded_work_finished(&self) { - // SAFETY: vtable contract. - unsafe { (self.vtable.embedded_work_finished)(self.data) } - } } impl Clone for JsPoster { diff --git a/src/event_loop/ConcurrentTask.rs b/src/event_loop/ConcurrentTask.rs index 80dcef9d439f..5f642e2f551c 100644 --- a/src/event_loop/ConcurrentTask.rs +++ b/src/event_loop/ConcurrentTask.rs @@ -137,12 +137,12 @@ pub struct Task { /// freed when it will never run. Implement on every type that can be /// enqueued; the impl lives in whatever crate owns the type. /// -/// A queued task ends one of three ways: it runs (`bun_runtime::dispatch:: -/// run_task`); it is refused at post because its VM already closed (the -/// poster frees it — `Postable::release_refused` / the `Posted::Refused` arm); -/// or it was queued in time but its VM stops before running it — +/// A queued task ends one of two ways: it runs (`bun_runtime::dispatch:: +/// run_task`), or its VM stops before running it — /// [`release_unrun`](Self::release_unrun), required here so no type can be -/// queued without having decided it. +/// queued without having decided it. (A *weak* poster — `JsPoster` — can also +/// get its task back unqueued once the VM has closed; that task never entered +/// a queue and is the poster's own to free: [`ConcurrentTask::release_refused`].) /// /// Re-exported from `bun_jsc` for ergonomics, but defined here (lowest tier on /// the hot-dispatch list, see PORTING.md §Dispatch) so that @@ -312,23 +312,36 @@ impl ConcurrentTask { self } - /// A poster got `task` back because the target VM is gone: free it if it - /// is a heap task (`create*`); an intrusive one belongs to its container. + /// Consuming thread: unwrap the payload, freeing the carrier if it was + /// heap-allocated (`create*`); an intrusive carrier stays with its container. /// /// # Safety - /// `task` was just refused and is not queued anywhere. - pub unsafe fn release_refused(task: core::ptr::NonNull) { + /// `this` came off a queue (or was never queued) and is not used afterwards. + pub unsafe fn into_task(this: core::ptr::NonNull) -> Task { // SAFETY: fn contract. unsafe { - // A callback task (`from_callback`, `ManagedTask::new*`) owns a - // heap `ManagedTask` behind `task.ptr` as well. - let inner = task.as_ref().task; - if inner.tag == crate::task_tag::ManagedTask { - crate::ManagedTask::ManagedTask::release(inner.ptr.cast()); - } - if task.as_ref().auto_delete() { - drop(bun_core::heap::take(task.as_ptr())); + let (task, auto_delete) = (this.as_ref().task, this.as_ref().auto_delete()); + if auto_delete { + drop(bun_core::heap::take(this.as_ptr())); } + task + } + } + + /// A weak poster got `task` back because the target VM has closed: free + /// it if it is a heap task (`create*`); an intrusive one belongs to its + /// container. + /// + /// # Safety + /// `task` was just refused and is not queued anywhere. + pub unsafe fn release_refused(task: core::ptr::NonNull) { + // SAFETY: fn contract. + let inner = unsafe { Self::into_task(task) }; + // A callback task (`from_callback`, `ManagedTask::new*`) owns a heap + // `ManagedTask` behind `task.ptr` as well. + if inner.tag == crate::task_tag::ManagedTask { + // SAFETY: as above; refused ⇒ ours. + unsafe { crate::ManagedTask::ManagedTask::release(inner.ptr.cast()) }; } } diff --git a/src/event_loop/lib.rs b/src/event_loop/lib.rs index 08a31e32cfb4..afa323575731 100644 --- a/src/event_loop/lib.rs +++ b/src/event_loop/lib.rs @@ -61,6 +61,7 @@ bun_dispatch::link_interface! { fn enter(); fn exit(); fn enqueue_task(task: Task); + fn enqueue_task_after_yield(task: Task); fn js_poster() -> any_event_loop::JsPoster; fn env() -> *mut bun_dotenv::Loader; fn top_level_dir() -> *const [u8]; diff --git a/src/jsc/AsyncModule.rs b/src/jsc/AsyncModule.rs index ca499307f9a1..59313cde4fd2 100644 --- a/src/jsc/AsyncModule.rs +++ b/src/jsc/AsyncModule.rs @@ -74,11 +74,13 @@ pub struct Queue { /// What the resolver's `WakeHandler` carries as its opaque context: the /// module queue (for the JS-thread dependency-error callback) and the VM's -/// handle (for wake-ups from install / HTTP threads). Allocated once per VM at -/// registration and kept for the VM's lifetime. +/// weak handle (for wake-ups from the process-wide install / HTTP threads, +/// which outlive any one VM). Allocated once per VM at registration and kept +/// for the VM's lifetime. pub struct WakeContext { pub queue: *mut Queue, - pub loop_handle: crate::LoopHandle, + pub handle: crate::VmHandle, + pub kind: crate::LoopKind, } impl Queue { @@ -382,8 +384,8 @@ impl Queue { // SAFETY: `ctx` is the leaked `WakeContext` registered with this handler. let ctx = unsafe { &*ctx.cast::() }; let task = ConcurrentTaskItem::create_from(ctx.queue); - if let crate::vm_handle::Posted::Refused(task) = ctx.loop_handle.post_task(task) { - // VM torn down: nobody is waiting on these modules any more. + if let crate::vm_handle::Posted::Refused(task) = ctx.handle.post(ctx.kind, task) { + // That VM has closed: nobody is waiting on these modules any more. // SAFETY: refused ⇒ we own the task box. unsafe { drop(bun_core::heap::take(task.as_ptr())) }; } diff --git a/src/jsc/CppTask.rs b/src/jsc/CppTask.rs index e113524fd5b4..b4e0a86826db 100644 --- a/src/jsc/CppTask.rs +++ b/src/jsc/CppTask.rs @@ -2,12 +2,8 @@ use crate::{JSGlobalObject, JsResult}; use bun_event_loop::{TaskTag, Taskable, task_tag}; use bun_threading::work_pool::{Task as WorkPoolTask, WorkPool}; -#[allow(improper_ctypes)] // `Shared` is opaque to C++ (`BunVmHandleRef`) unsafe extern "C" { fn Bun__EventLoopTaskNoContext__performTask(task: *mut EventLoopTaskNoContext); - safe fn Bun__EventLoopTaskNoContext__vmHandle( - task: &EventLoopTaskNoContext, - ) -> *const crate::vm_handle::Shared; } bun_opaque::opaque_ffi! { @@ -53,54 +49,46 @@ impl EventLoopTaskNoContext { // SAFETY: caller guarantees `this` is a valid C++ EventLoopTaskNoContext; performTask consumes/frees it. unsafe { Bun__EventLoopTaskNoContext__performTask(this) } } - - /// The handle of the VM this task was created in (a reference the C++ - /// task holds for its lifetime). - pub(crate) fn vm_handle(&self) -> crate::vm_handle::BorrowedRef { - // SAFETY: C++ stores a `BunVmHandleRef` from `Bun__VmHandle__retainRef` - // for the task's whole lifetime. - unsafe { crate::VmHandle::borrow_ref(Bun__EventLoopTaskNoContext__vmHandle(self)) } - } } -/// A task created from C++ code that runs inside the workpool, usually via ScriptExecutionContext. +/// A task created from C++ code that runs inside the workpool (WebCrypto's +/// `PhonyWorkQueue`). Holds the creating VM's ticket: the C++ closure captures +/// context-affine objects and posts its result back by context id. #[repr(C)] pub struct ConcurrentCppTask { pub(crate) cpp_task: *mut EventLoopTaskNoContext, + pub(crate) ticket: crate::Ticket, pub(crate) workpool_task: WorkPoolTask, } bun_threading::owned_task!(ConcurrentCppTask, workpool_task); impl ConcurrentCppTask { + #[allow(clippy::boxed_local)] // `owned_task!`'s required signature fn run_owned(self: Box) { - // Extract all the info we need from `self` and `cpp_task` before we call functions that - // free them. - let cpp_task = self.cpp_task; - // `EventLoopTaskNoContext` is an `opaque_ffi!` ZST handle; `opaque_ref` - // is the centralised non-null deref proof. Valid until `run` consumes it. - // Clone before `run` consumes (and frees) the C++ task that holds the reference. - let handle: crate::VmHandle = EventLoopTaskNoContext::opaque_ref(cpp_task) - .vm_handle() - .clone(); - drop(self); + let ConcurrentCppTask { + cpp_task, ticket, .. + } = *self; // SAFETY: `cpp_task` is the valid C++ handle stored by `ConcurrentCppTask__createAndRun`; - // `opaque_ref` above proved it non-null and it has not yet been freed — `run` consumes it here. + // `run` consumes it here. unsafe { EventLoopTaskNoContext::run(cpp_task) }; - handle.unref_keep_alive(crate::LoopKind::Regular); + ticket.unref_keep_alive(); } } +/// JS thread (`PhonyWorkQueue::dispatch`). #[unsafe(no_mangle)] -extern "C" fn ConcurrentCppTask__createAndRun(cpp_task: *mut EventLoopTaskNoContext) { +extern "C" fn ConcurrentCppTask__createAndRun( + global: &JSGlobalObject, + cpp_task: *mut EventLoopTaskNoContext, +) { crate::mark_binding!(); - // `EventLoopTaskNoContext` is an `opaque_ffi!` ZST handle; `opaque_ref` is - // the centralised non-null deref proof. C++ just handed it over. - EventLoopTaskNoContext::opaque_ref(cpp_task) - .vm_handle() - .ref_keep_alive(crate::LoopKind::Regular); + let vm = global.bun_vm(); + vm.event_loop_shared().ref_keep_alive(); + let ticket = vm.ticket(); WorkPool::schedule_new(ConcurrentCppTask { cpp_task, + ticket, workpool_task: WorkPoolTask::default(), }); } diff --git a/src/jsc/Debugger.rs b/src/jsc/Debugger.rs index da6c7a0aa0a1..c3ea7428c972 100644 --- a/src/jsc/Debugger.rs +++ b/src/jsc/Debugger.rs @@ -174,6 +174,16 @@ unsafe extern "C" { static FUTEX_ATOMIC: AtomicU32 = AtomicU32::new(0); static HAS_CREATED_DEBUGGER: AtomicBool = AtomicBool::new(false); +/// What the debugger thread takes from the debuggee VM's thread. +struct DebuggerThreadInit { + debuggee: crate::VmHandle, + ctx_id: u32, + is_connect: bool, + is_node_inspector: bool, + from_env: &'static [u8], + path_or_port: Option<&'static [u8]>, +} + impl Debugger { /// `Debugger.waitForDebuggerIfNecessary(vm)` — block on the futex until /// `start()` (debugger thread) signals, then run the wait-loop until a @@ -383,26 +393,23 @@ impl Debugger { if !this_ref.has_started_debugger { this_ref.as_mut().has_started_debugger = true; - // `std::thread::spawn` requires `Send`; raw `*mut - // VirtualMachine` is `!Send`. Wrap in a `Send` newtype — the - // pointer is only ever dereferenced on the debugger thread under - // `holdAPILock` (see `start_js_debugger_thread` doc), and the VM - // outlives the process. - struct SendVmPtr(*mut VirtualMachine); - // SAFETY: see comment above — cross-thread access is mediated - // by `holdAPILock` / the futex; the VM allocation is `'static`. - unsafe impl Send for SendVmPtr {} - let send_vm = SendVmPtr(this); + // Everything the debugger thread needs from this VM, copied here; + // it reaches back only through the (uncounted) handle to wake us. + let init = DebuggerThreadInit { + debuggee: this_ref.handle(), + ctx_id: dbg.script_execution_context_id, + is_connect: dbg.mode == Mode::Connect, + is_node_inspector: dbg.protocol == Protocol::NodeInspector, + from_env: dbg.from_environment_variable, + path_or_port: dbg.path_or_port, + }; // Rust's `std::thread` default stack (2 MiB) is too small to run // a full `VirtualMachine::init` + JS module load on this thread, // so use 16 MiB. std::thread::Builder::new() .name("Debugger".to_string()) .stack_size(16 * 1024 * 1024) - .spawn(move || { - let send_vm = send_vm; - Debugger::start_js_debugger_thread(send_vm.0); - }) + .spawn(move || Debugger::start_js_debugger_thread(init)) .map_err(|_| crate::CrateError::ThreadSpawnFailed)?; // The `JoinHandle` is dropped here, detaching the thread. } @@ -418,15 +425,8 @@ impl Debugger { } /// Debugger-thread entry: build a second `VirtualMachine`, hold the API - /// lock, run `start()`. - /// - /// `other_vm` is the *parent thread's* VM. The parent thread - /// continues executing (and mutating that VM) concurrently with this - /// thread. - /// Taking `&mut VirtualMachine` here would assert exclusive access we do - /// not have — UB. We hold a raw `*VirtualMachine` and - /// never materialize a `&`/`&mut VirtualMachine` to the foreign-thread VM. - pub(crate) fn start_js_debugger_thread(other_vm: *mut VirtualMachine) { + /// lock, and run [`Debugger::start`] inside it. + fn start_js_debugger_thread(init: DebuggerThreadInit) { // The global allocator is mimalloc and `InitOptions` does not carry // `allocator`/`env_loader` (those are wired by // `RuntimeHooks::init_runtime_state`). @@ -450,73 +450,34 @@ impl Debugger { vm.event_loop_mut().ensure_waker(); extern "C" fn start_trampoline(ctx: *mut c_void) { - // Forward the raw pointer unchanged — see fn doc above - // for why we never form `&mut VirtualMachine` to the parent VM. - Debugger::start(ctx.cast::()); + // SAFETY: `ctx` is `&mut slot` below; `hold_api_lock` calls this + // synchronously on the same frame. + let init = unsafe { (*ctx.cast::>()).take() }; + Debugger::start(init.expect("init")); } + let mut slot = Some(init); #[allow(deprecated)] vm.global() .vm() - .hold_api_lock(other_vm.cast(), start_trampoline); + .hold_api_lock((&raw mut slot).cast(), start_trampoline); } - /// Runs inside `holdAPILock` on the - /// debugger thread. Publishes the inspector URL(s), wakes the futex the - /// parent VM is blocked on, then spins this thread's event loop forever. - /// - /// Aliasing: every `VirtualMachine` / `EventLoop` access here - /// goes through a raw pointer with a fresh short-lived `&mut *p` formed at - /// the call site, never bound to a long-lived reference. Reasons: - /// - /// 1. `other_vm` is owned by the parent thread (see - /// `start_js_debugger_thread` doc); after the futex wake the parent - /// resumes its tick loop concurrently. Holding `&mut VirtualMachine` - /// across that point is a data race on a `&mut`-covered allocation. - /// 2. `this.event_loop()` returns a self-pointer into the inline - /// `regular_event_loop` field (VirtualMachine.rs:489), so a long-lived - /// `&mut EventLoop` overlaps any later `&mut VirtualMachine` use. - /// 3. `Bun__startJSDebuggerThread` and `tick()` re-enter JS, which calls - /// `VirtualMachine::get()` / `event_loop()` and mints fresh `&mut` to - /// the same allocations — holding our own across those calls is UB. - fn start(other_vm: *mut VirtualMachine) { + /// Runs inside `holdAPILock` on the debugger thread. Publishes the + /// inspector URL(s), wakes the futex the debuggee VM is blocked on, then + /// spins this thread's event loop forever. + fn start(init: DebuggerThreadInit) { jsc::mark_binding(); - // `this` is this thread's own VM (created in `start_js_debugger_thread`) - // — safe to hold as `&'static`. `other_vm` remains a raw pointer (see - // aliasing note above): the parent thread mutates it concurrently after the - // futex wake, so forming `&VirtualMachine` to it would be a data race. let this: &VirtualMachine = VirtualMachine::get(); - // SAFETY: `other_vm` is the parent-thread VM, live for process - // lifetime. We read its `event_loop` self-pointer once *before* the - // futex wake (while the parent is still blocked / not yet past the - // wait-loop) and reuse the raw pointer for the cross-thread `wakeup()` - // calls below. `wakeup()` takes `&self` and is the documented - // thread-safe path (event_loop.rs:779). - let other_loop: *mut crate::event_loop::EventLoop = unsafe { (*other_vm).event_loop() }; let global: &JSGlobalObject = this.global(); - - // Copy the four scalars we need from the parent VM's - // debugger before re-entering JS or waking the parent. We run inside - // an `extern "C"` trampoline where unwinding is UB — if `debugger` is - // missing, wake the parent and bail instead (unreachable in - // practice; `create()` always populates `debugger` before spawning). - // SAFETY: `other_vm` live; short-lived shared borrow of `debugger` - // ends before any other access to `*other_vm`. - let (ctx_id, is_connect, is_node_inspector, from_env, path_or_port) = - match unsafe { (*other_vm).debugger.as_deref() } { - Some(d) => ( - d.script_execution_context_id, - d.mode == Mode::Connect, - d.protocol == Protocol::NodeInspector, - d.from_environment_variable, - d.path_or_port, - ), - None => { - FUTEX_ATOMIC.store(0, Ordering::Relaxed); - bun_threading::Futex::wake(&FUTEX_ATOMIC, 1); - return; - } - }; + let DebuggerThreadInit { + debuggee, + ctx_id, + is_connect, + is_node_inspector, + from_env, + path_or_port, + } = init; if !from_env.is_empty() { let mut url = BunString::clone_utf8(from_env); @@ -546,16 +507,12 @@ impl Debugger { FUTEX_ATOMIC.store(0, Ordering::Relaxed); bun_threading::Futex::wake(&FUTEX_ATOMIC, 1); - // SAFETY: `other_loop` is the parent VM's event loop, live for process - // lifetime; `wakeup()` takes `&self` and is thread-safe. - unsafe { (*other_loop).wakeup() }; - // Re-read `this.event_loop()` here rather than reusing - // the cached `loop` — `vm.event_loop` may have flipped between - // `regular_event_loop` and `macro_event_loop` inside the re-entrant JS - // above. `event_loop_mut()` re-reads the slot on every call. + debuggee.wake(); + // `vm.event_loop` may have flipped between `regular_event_loop` and + // `macro_event_loop` inside the re-entrant JS above; + // `event_loop_mut()` re-reads the slot. this.event_loop_mut().tick(); - // SAFETY: see above. - unsafe { (*other_loop).wakeup() }; + debuggee.wake(); loop { // Each call forms a fresh short-lived `&`/`&mut` (via the safe diff --git a/src/jsc/JSSecrets.rs b/src/jsc/JSSecrets.rs index 4c58fc528526..f7c8d5fd72a0 100644 --- a/src/jsc/JSSecrets.rs +++ b/src/jsc/JSSecrets.rs @@ -9,7 +9,7 @@ bun_opaque::opaque_ffi! { pub struct SecretsJobOptions; } // to the cell. `deinit` consumes/frees the C++ allocation and so stays // `unsafe fn` (double-free precondition). unsafe extern "C" { - safe fn Bun__SecretsJobOptions__runTask(ctx: &mut SecretsJobOptions, global: &JSGlobalObject); + safe fn Bun__SecretsJobOptions__runTask(ctx: &mut SecretsJobOptions); safe fn Bun__SecretsJobOptions__runFromJS( ctx: &mut SecretsJobOptions, global: &JSGlobalObject, @@ -32,21 +32,14 @@ impl Drop for SecretsOptions { /// `Bun.secrets.{get,set,delete}` off the JS thread. pub(crate) struct SecretsJob { options: SecretsOptions, - global: crate::JsPtr, } impl crate::JobContext for SecretsJob { type OffThread = Self; type Js = Strong; - fn run( - this: &mut Self, - vm: &crate::vm_handle::Borrow, - done: crate::Completion, - ) -> Option> { - // SAFETY: the creating global, alive under the borrow; C++ only threads it through. - let global = unsafe { this.global.under_borrow(vm) }; - Bun__SecretsJobOptions__runTask(SecretsJobOptions::opaque_mut(this.options.0), global); + fn run(this: &mut Self, done: crate::Completion) -> Option> { + Bun__SecretsJobOptions__runTask(SecretsJobOptions::opaque_mut(this.options.0)); Some(done) } @@ -79,8 +72,6 @@ extern "C" fn Bun__Secrets__scheduleJob( &cx, SecretsJob { options: SecretsOptions(options), - // SAFETY: the creating global outlives every borrow of its VM. - global: unsafe { crate::JsPtr::new(core::ptr::NonNull::from(global)) }, }, Strong::create(promise, global), ); diff --git a/src/jsc/RuntimeTranspilerStore.rs b/src/jsc/RuntimeTranspilerStore.rs index 609a8c9b87f1..e9b68f2e919c 100644 --- a/src/jsc/RuntimeTranspilerStore.rs +++ b/src/jsc/RuntimeTranspilerStore.rs @@ -232,13 +232,11 @@ impl RuntimeTranspilerStore { Self::default() } - // 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. - /// VM teardown (JS thread, heap alive, script forbidden, embedded work - /// waited for so no job is mid-flight): jobs whose completion will not run — - /// queued after the last tick, or posted after `close()` began — release - /// their source, log and module promise here instead of running. + /// VM teardown (JS thread, heap alive, script forbidden; called on every + /// turn of the wait): jobs already handed back whose completion will not + /// run release their source, log and module promise here instead. Queued ⇒ + /// the pool thread's last touch of the slot was the push (its ticket was + /// moved out first), so the slot is this thread's again. pub fn release_queued_jobs_for_teardown(&mut self) { let batch = self.queue.pop_batch(); let mut iter = batch.iterator(); @@ -247,8 +245,7 @@ impl RuntimeTranspilerStore { if job.is_null() { break; } - // SAFETY: a live job popped from the intrusive queue; this thread - // owns it now (its worker-thread part finished before `close()`). + // SAFETY: a live job popped from the intrusive queue; see fn doc. unsafe { (*job).promise.deinit(); (*job).reset_for_pool(); @@ -257,6 +254,9 @@ impl RuntimeTranspilerStore { } } + // 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. pub fn run_from_js_thread( &mut self, event_loop: NonNull, @@ -346,7 +346,7 @@ impl RuntimeTranspilerStore { global_this: BackRef::new(global_object), non_threadsafe_referrer: OwnedString::new(referrer), vm, - loop_handle: global_object.bun_vm().loop_handle(), + ticket: None, log: bun_ast::Log::init(), loader, promise: StrongOptional::create(JSValue::from_cell(promise), global_object), @@ -403,10 +403,10 @@ pub struct TranspilerJob { // raw pointers/BackRefs are used (BACKREF — VM owns the // store and outlives every job). pub(crate) vm: *mut VirtualMachine, - /// The pool thread runs this job under `loop_handle.borrow()`: the job's - /// own slot, the transpiler it copies and the store queue it pushes to are - /// all VM-owned, and the VM's teardown waits for the borrow to end. - pub(crate) loop_handle: crate::LoopHandle, + /// Held from `schedule` until the pool thread has dispatched the job back: + /// the job's own slot, the transpiler it copies and the store queue it + /// pushes to are all VM-owned, and the VM's teardown waits for the ticket. + pub(crate) ticket: Option, pub global_this: BackRef, pub(crate) fetcher: Fetcher, pub(crate) poll_ref: KeepAlive, @@ -514,23 +514,17 @@ impl TranspilerJob { // replacement a second time). } - fn dispatch_to_main_thread(&mut self) { + /// Pool thread: hand the slot back. `ticket` was moved out of `self` + /// first — the JS thread may reuse the slot the moment it is queued. + fn dispatch_to_main_thread(&mut self, ticket: &crate::Ticket) { let vm = self.vm; - let loop_handle = self.loop_handle.clone(); - // SAFETY: vm outlives the job (BACKREF — VM owns the store). + // SAFETY: the VM outlives the ticket (it owns the store). 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 - // (the handle was cloned out above for exactly this reason). The VM - // waits for embedded work before closing its handle, so this is queued. - let crate::vm_handle::Posted::Queued = - loop_handle.post_task(ConcurrentTask::create_from(transpiler_store)) - else { - unreachable!("VM handle closed with embedded transpile work outstanding"); - }; + ticket.post(ConcurrentTask::create_from(transpiler_store)); } fn run_from_js_thread(&mut self) -> JsResult<()> { @@ -594,9 +588,8 @@ 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)); - // The job is a slot inside this VM: counted, so teardown waits for it - // (see `VmHandle::embedded_work_scheduled`). - self.loop_handle.embedded_work_scheduled(); + // SAFETY: JS thread; the VM owns the store this slot lives in. + self.ticket = Some(unsafe { (*self.vm).ticket() }); WorkPool::schedule(&raw mut self.work_task); } @@ -606,24 +599,23 @@ impl TranspilerJob { // `transpile`; the WorkPool calls back with exactly that field, so // `from_field_ptr!` recovers the live `TranspilerJob` parent. let this = unsafe { bun_core::from_field_ptr!(TranspilerJob, work_task, work_task) }; - // The slot lives inside the VM and the VM waits for us (embedded work), - // so it is alive throughout. Transpile only while the VM is still - // running; either way hand the job back to the JS thread, which - // completes or releases it. - // SAFETY: as above. - let handle = unsafe { (*this).loop_handle.clone() }; - if let Some(_vm) = handle.borrow_if_running() { + // The slot lives inside the VM, which waits for this ticket, so it is + // alive throughout. Transpile only while the VM still runs script; + // either way hand the job back to the JS thread, which completes or + // releases it. + // SAFETY: as above; set in `schedule`. + let ticket = + unsafe { (*this).ticket.take() }.expect("scheduled transpile job holds a ticket"); + if ticket.script_allowed() { // SAFETY: live slot, exclusively ours until dispatched. - unsafe { (*this).run() }; + unsafe { (*this).run(&ticket) }; } else { // SAFETY: as above. - unsafe { (*this).dispatch_to_main_thread() }; + unsafe { (*this).dispatch_to_main_thread(&ticket) }; } - // Last touch of the slot from this thread was the dispatch. - handle.embedded_work_finished(); } - fn run(&mut self) { + fn run(&mut self, ticket: &crate::Ticket) { // Stack-local per call, bulk-freed on return. An earlier version hoisted // this to a per-worker-thread leaked `Box` (and a second // one inside a leaked `ASTMemoryAllocator`) and only `reset()` it at @@ -645,7 +637,7 @@ impl TranspilerJob { scopeguard::defer! { // SAFETY: `self` outlives this guard (guard drops before fn return); // no other &mut alias is live at drop time. - unsafe { (*this_ptr).dispatch_to_main_thread() }; + unsafe { (*this_ptr).dispatch_to_main_thread(ticket) }; } // SAFETY contract: `vm` outlives the job (BACKREF — VM owns the store). diff --git a/src/jsc/VirtualMachine.rs b/src/jsc/VirtualMachine.rs index ac42b0903872..ca925367b68a 100644 --- a/src/jsc/VirtualMachine.rs +++ b/src/jsc/VirtualMachine.rs @@ -209,9 +209,8 @@ pub struct VirtualMachine { pub heap_profiler_config: Option, pub counters: Counters, - // LAYERING: real type is `bun_runtime::cli::Command::HotReload` (forward - // dep); stored as its `u8` repr (see `HOT_RELOAD_*` constants). - pub hot_reload: u8, + /// `--hot` / `--watch` mode this VM runs under. + pub hot_reload: HotReload, pub jsc_vm: *mut VM, /// hide bun:wrap from stack traces @@ -246,8 +245,6 @@ pub struct VirtualMachine { /// does not count). What is still pending on the entry promise after that /// is a top-level await. pub entry_evaluation_started: bool, - /// This VM's live pool jobs (`bun_jsc::job`); JS thread only, zero-valid. - pub(crate) jobs: core::cell::UnsafeCell, pub(crate) had_errors: bool, @@ -331,8 +328,10 @@ pub struct VirtualMachine { /// only: `WebWorker::create` pushes, `release_parent_poll_ref` removes, /// `join_child_workers` drains at exit. pub child_workers: Vec<*mut crate::web_worker::WebWorker>, - /// The one object other threads hold to reach this VM (post completions, - /// wake, keep-alive, borrow VM-owned memory); closed by `teardown`. + /// This VM's live pool jobs (`bun_jsc::job`); JS thread only, zero-valid. + pub(crate) jobs: crate::JsCell, + /// The door out of this thread (`bun_jsc::vm_handle`): tickets for work + /// that leaves it are taken here, and `teardown` waits on it. handle: core::mem::ManuallyDrop, pub pending_ipc: Option, pub hot_reload_counter: u32, @@ -405,8 +404,19 @@ unsafe extern "C" { bun_core::define_scoped_log!(teardown_log, Worker, hidden); -pub const HOT_RELOAD_HOT: u8 = 1; -pub const HOT_RELOAD_WATCH: u8 = 2; +pub use bun_options_types::context::HotReload; + +/// The `bun build --compile` executable's embedded module graph, if this +/// process is one: set when the first VM is created with it, the same for +/// every VM after. +static STANDALONE_MODULE_GRAPH: std::sync::OnceLock< + &'static dyn bun_resolver::StandaloneModuleGraph, +> = std::sync::OnceLock::new(); + +/// See [`STANDALONE_MODULE_GRAPH`]. Any thread. +pub fn standalone_module_graph() -> Option<&'static dyn bun_resolver::StandaloneModuleGraph> { + STANDALONE_MODULE_GRAPH.get().copied() +} // ────────────────────────────────────────────────────────────────────────── // Nested types @@ -687,18 +697,22 @@ impl Drop for MacroModeGuard { } } -// SAFETY: `VirtualMachine` is a per-JS-thread singleton (see `VMHolder`). -// All access is same-thread; the `Sync` impl exists so `&'static -// VirtualMachine` can be returned from [`VirtualMachine::get`] and passed -// through `'static`-bound closures / trait objects without `T: Sync` -// cascading. Cross-thread paths go through `ConcurrentTask` which never -// hands out a `&VirtualMachine`. Fields mutated post-init are wrapped in -// [`JsCell`] for interior mutability. -unsafe impl Sync for VirtualMachine {} -// SAFETY: see the `Sync` impl above — the VM is only ever accessed from its -// owning JS thread; `Send` lets the boxed VM be moved into the worker thread -// that will own it during `Worker` startup. -unsafe impl Send for VirtualMachine {} +// `VirtualMachine` is deliberately `!Send + !Sync` (its raw-pointer fields +// make it so): a `&VirtualMachine`, a pointer to it or to its loops cannot be +// carried to another thread. Other threads hold a `bun_jsc::Ticket` / +// `VmHandle` and reach the VM only by posting to it. Fields mutated post-init +// are wrapped in [`JsCell`] for same-thread interior mutability. +// Fails to compile ("multiple applicable items") if `VirtualMachine` is +// `Send` or `Sync`. +const _: () = { + trait AmbiguousIfImpl { + fn some_item() {} + } + impl AmbiguousIfImpl<()> for T {} + impl AmbiguousIfImpl for T {} + impl AmbiguousIfImpl for T {} + let _ = >::some_item; +}; impl VirtualMachine { /// Safe `&'static` accessor for the current thread's VM. The VM is a @@ -752,7 +766,7 @@ impl VirtualMachine { #[allow(clippy::mut_from_ref)] pub fn as_mut(&self) -> &mut VirtualMachine { debug_assert!(core::ptr::eq(self, Self::get_mut_ptr())); - // SAFETY: single-JS-thread invariant — see `unsafe impl Sync` above. + // SAFETY: JS thread only (`VirtualMachine` is `!Send + !Sync`). // Provenance comes from the thread-local `*mut` set in `init()`. unsafe { &mut *Self::get_mut_ptr() } } @@ -771,7 +785,7 @@ impl VirtualMachine { /// `BackRef` was constructed. #[inline(always)] pub fn get_mut() -> &'static mut VirtualMachine { - // SAFETY: single-JS-thread invariant — see `unsafe impl Sync` above. + // SAFETY: JS thread only (`VirtualMachine` is `!Send + !Sync`). // Provenance comes from the thread-local `*mut` set in `init()`. unsafe { &mut *Self::get_mut_ptr() } } @@ -827,7 +841,7 @@ impl VirtualMachine { #[allow(clippy::mut_from_ref)] pub fn event_loop_mut(&self) -> &mut EventLoop { // SAFETY: `event_loop` points at a sibling field of this VM; non-null - // after `init()`; single-JS-thread invariant per `unsafe impl Sync`. + // after `init()`; JS thread only (`VirtualMachine` is `!Sync`). unsafe { &mut *self.event_loop } } @@ -840,13 +854,19 @@ impl VirtualMachine { unsafe { &*self.event_loop } } - /// A clone of this VM's [`crate::VmHandle`] — what off-thread work captures - /// instead of a pointer to the VM or its event loop. + /// A clone of this VM's uncounted [`crate::VmHandle`] — what something + /// that refers to this VM from elsewhere holds. Work that leaves the thread + /// on this VM's behalf takes a [`ticket`](Self::ticket) instead. #[inline] pub fn handle(&self) -> crate::VmHandle { (*self.handle).clone() } + #[inline] + pub(crate) fn handle_ref(&self) -> &crate::VmHandle { + &self.handle + } + /// May native code enter user JavaScript right now? `true` in normal /// operation and while the exit handlers run; `false` once teardown has /// forbidden execution — the start of [`teardown`](Self::teardown), or @@ -1006,8 +1026,7 @@ impl VirtualMachine { #[allow(clippy::mut_from_ref)] pub fn uws_loop_mut(&self) -> &mut uws::Loop { // SAFETY: `uws_loop()` returns the per-VM loop pointer; non-null on - // the JS thread once `init()` ran. Single-JS-thread invariant per - // `unsafe impl Sync`. + // the JS thread once `init()` ran; JS thread only (`!Sync`). unsafe { &mut *self.uws_loop() } } @@ -1555,7 +1574,7 @@ impl VirtualMachine { } pub fn hot_map(&mut self) -> Option<&mut crate::rare_data::HotMap> { - if self.hot_reload != HOT_RELOAD_HOT { + if self.hot_reload != HotReload::Hot { return None; } Some(self.rare_data().hot_map()) @@ -1713,11 +1732,10 @@ impl VirtualMachine { /// fetch/S3/Bun.build aborted or cancelled), socket groups and the DNS /// channel closed. /// B. release — cron, user timers and GC-controller timers cancelled; child - /// workers joined; this VM's sqlite connections closed; the JS side of - /// boxed pool jobs released; counted off-thread work waited for; main: - /// HTTP thread parked; the VM handle closed; (Windows worker) in-flight - /// uv requests completed; queued tasks and RareData's JS handles - /// released without running. + /// workers joined; this VM's sqlite connections closed; **the wait**: + /// every ticket held off-thread comes back while queued work is + /// released without running; the handle closes; main: HTTP thread + /// parked; RareData's JS handles released. /// C. JSC VM destroyed (finalizers close what only they own; JSC's RunLoop /// timers ride the timer heap until ~VM returns); sockets they closed /// drained; then the timer heap's loop handles closed. @@ -1728,7 +1746,7 @@ impl VirtualMachine { /// /// # Safety /// `this` is this thread's VM and no other thread can reach it any more - /// (worker: unpublished under `vm_lock`); `is_shutting_down` is set and + /// (worker: its handle unpublished); `is_shutting_down` is set and /// the exit handlers have run. pub(crate) unsafe fn teardown(this: *mut Self, kind: Teardown) { // SAFETY: per fn contract — sole owner on the owning thread. Shared @@ -1803,19 +1821,23 @@ impl VirtualMachine { // Children have closed their own; now this VM's sqlite connections // checkpoint and close, before finalizers could. vm.close_sqlite_databases_for_exit(); - // Work still out on other threads (pool jobs, fetches): its JS side - // — promises, callbacks, pins, protected buffers, keep-alives — is - // released here, on this thread with the heap alive. After `close()` - // below the other thread cannot hand it back; it frees only its own part. - vm.jobs().release_all_js(&vm.global().js_thread()); - // Pool work stored inside JS-owned objects (transpile slots, zlib - // streams) must be back before the handle closes: it completes into the - // still-open queue and is released below, on this thread. + // The one place the invariant is enforced: nothing below runs until + // everything that left this thread (pool jobs, fetches, uv work, C++ + // work-queue tasks, child threads) has come back. Whatever arrives + // meanwhile — and whatever was already queued — is released here, on + // this thread with the heap alive, never run. teardown_log!( - "teardown: waiting for {} unit(s) of off-thread work", - vm.handle.embedded_work_outstanding() + "teardown: waiting for {} ticket(s) held off-thread", + vm.handle.tickets_outstanding() ); - vm.handle.wait_for_embedded_work(); + // A released completion can be a native continuation that opens + // something new (a multipart upload's next part); sweep again so it is + // cancelled rather than waited out. + // SAFETY: fn contract (statement-scoped exclusive access). + vm.handle.close_and_wait(|| unsafe { + (*this).release_queued_work(); + let _ = Self::stop_phase_sweep(this, kind); + }); // The exiting main thread now parks the process-wide HTTP thread — // after the children it also served are joined and this VM's own // requests are back — so it cannot touch what process exit frees. If @@ -1824,21 +1846,13 @@ impl VirtualMachine { teardown_log!("teardown: HTTP thread unresponsive; skipping to process exit"); return; } - // From here no other thread reaches this VM: posts are refused (the - // poster releases its task itself), wake/keep-alive are no-ops, and any - // job still using VM-owned memory has finished (close waits for it). - vm.handle.close(); - // Tasks posted by other threads (HTTP, children before they were - // joined) or by the request completions in A: release, do not run — - // their JSC handles must drop against a live heap. // SAFETY: fn contract (statement-scoped exclusive access). unsafe { - (*this).release_queued_work(); if let Some(rare) = (*this).rare_data.as_deref_mut() { rare.release_js_handles(); } } - teardown_log!("teardown: script forbidden, resources cancelled, children joined"); + teardown_log!("teardown: script forbidden, resources cancelled, off-thread work back"); // ---- C. JSC VM ------------------------------------------------------- match kind { @@ -1892,6 +1906,10 @@ impl VirtualMachine { unsafe fn stop_phase_sweep(this: *mut Self, kind: Teardown) -> SweepResult { let hooks = runtime_hooks(); let mut result = SweepResult::Idle; + // Pool jobs parked on something external (a pipe that never becomes + // readable): their completions then arrive through the wait. + // SAFETY: fn contract. + unsafe { (*this).jobs.get() }.cancel_all(); if let Some(hooks) = hooks { // SAFETY: fn contract. result = result.and(unsafe { (hooks.stop_active_handles_for_vm_teardown)(this) }); @@ -2461,6 +2479,9 @@ impl VirtualMachine { addr_of_mut!((*vm).proxy_env_storage).write(Default::default()); addr_of_mut!((*vm).gc_controller).write(Default::default()); addr_of_mut!((*vm).channel_ref).write(Default::default()); + if let Some(graph) = opts.graph { + let _ = STANDALONE_MODULE_GRAPH.set(graph); + } addr_of_mut!((*vm).standalone_module_graph).write(opts.graph); addr_of_mut!((*vm).initial_script_execution_context_identifier).write(context_id); // Mutex fields: zeroed atomics ARE valid-unlocked, but write the @@ -3772,7 +3793,7 @@ impl VirtualMachine { /// Performs a hot reload: re-evaluates the entry point once any pending entry-point load settles. pub(crate) fn reload(&mut self, _: Option<&mut crate::hot_reloader::HotReloadTask>) { - if self.hot_reload == HOT_RELOAD_WATCH { + if self.hot_reload == HotReload::Watch { // Watch reload replaces the process: never defer on a pending // entry promise (node restarts regardless of child state), and // emit the --watch-kill-signal JS handlers first, like node. @@ -3947,22 +3968,15 @@ impl VirtualMachine { // SAFETY: `vm` is the unique live VM on this thread. let vm_ref = unsafe { &mut *vm }; vm_ref.worker = Some(std::ptr::from_ref::(worker).cast()); - #[cfg(debug_assertions)] - if bun_core::env_var::feature_flag::BUN_DEBUG_TEST_WORKER_REFUSAL_GATE::get() - .unwrap_or(false) - { - vm_ref.handle.park_posts_until_closed(); + if worker.arm_test_gate() { + vm_ref.handle.arm_test_gate(); } - // `parent_vm()` is a `BackRef`; the parent outlives this worker while - // `parent_poll_ref` is held (see web_worker.rs file header). - let parent = worker.parent_vm(); - vm_ref.standalone_module_graph = parent.standalone_module_graph; // The worker's resolver also // needs the standalone graph, otherwise embedded `/$bunfs/...` specifiers // (e.g. a `new Worker("./worker.ts")` entry point inside a compiled // executable) resolve against the real filesystem and fail. vm_ref.transpiler.resolver.standalone_module_graph = opts.graph; - vm_ref.hot_reload = parent.hot_reload; + vm_ref.hot_reload = worker.hot_reload(); vm_ref.initial_script_execution_context_identifier = worker.execution_context_id() as i32; vm_ref.transpiler.resolver.store_fd = opts.store_fd; if opts.graph.is_none() { @@ -4898,7 +4912,7 @@ impl VirtualMachine { /// Tracks a listening socket so watch-mode reloads can close it. pub fn add_listening_socket_for_watch_mode(&mut self, socket: bun_sys::Fd) { - if self.hot_reload != HOT_RELOAD_WATCH && !self.test_isolation_enabled { + if self.hot_reload != HotReload::Watch && !self.test_isolation_enabled { return; } self.rare_data().add_listening_socket_for_watch_mode(socket); @@ -4906,7 +4920,7 @@ impl VirtualMachine { /// Stops tracking a watch-mode listening socket. pub fn remove_listening_socket_for_watch_mode(&mut self, socket: bun_sys::Fd) { - if self.hot_reload != HOT_RELOAD_WATCH && !self.test_isolation_enabled { + if self.hot_reload != HotReload::Watch && !self.test_isolation_enabled { return; } self.rare_data() diff --git a/src/jsc/VmHandle.rs b/src/jsc/VmHandle.rs index 1a536254a518..0e4d5cce3321 100644 --- a/src/jsc/VmHandle.rs +++ b/src/jsc/VmHandle.rs @@ -1,62 +1,85 @@ -//! [`VmHandle`] — the only way another thread reaches a [`VirtualMachine`]. +//! The door out of a VM's thread: [`Ticket`] and [`VmHandle`]. //! -//! Off-thread code (thread-pool jobs, the HTTP thread, watcher/waiter threads, -//! addon threads) legitimately needs three things from a VM: post a completion -//! and wake its loop, ref/unref its keep-alive, and — while running — sometimes -//! use memory the VM owns. It never needs the `VirtualMachine`, its global or -//! its heap directly; whatever touches those runs later, on the JS thread, from -//! the posted task. A `VmHandle` provides exactly those three, safely and for -//! as long as anyone holds it (it outlives the VM), and the VM's teardown -//! *closes* it: after `close()` returns no thread can reach the VM's queues, -//! waker or memory through any handle, and posts are refused (the poster gets -//! its task back and releases it on its own thread — deliver-or-discard, as -//! WebKit's WorkerRunLoop does). The same object carries the script-forbidden -//! bit that native→JS entry points consult (Node's `can_call_into_js`). +//! Invariant: *a VM is destroyed only after everything that left its thread has +//! come back.* Anything that runs on, or is referenced from, another thread on +//! behalf of a VM holds a [`Ticket`] for it. Creating a ticket counts it; +//! dropping it uncounts it; the VM's teardown ([`VmHandle::close_and_wait`]) +//! forbids script, cancels what it can, and then *waits* — servicing its queue +//! so returning work is released on this thread with the heap alive — until no +//! ticket is outstanding, and only then destroys the JSC VM, the loops and the +//! `VirtualMachine`. So no thread can hold anything of a VM's while it is being +//! destroyed, whatever the work captured (VM state, JS buffers, atom strings, +//! arena memory) and whether or not its author thought about teardown. //! -//! Gate: posters/borrowers hold `active` for the duration of their access and -//! then check `state`; `close()` publishes `Closed` and waits for `active == 0` -//! (SeqCst on both sides — the Dekker pair). So an access either finished -//! before `close()` returned or observed `Closed` and touched nothing. +//! Counting *is* holding the ticket: there is no separate register/finished +//! call to forget. A ticket also carries which of the VM's loops its completion +//! belongs on, and posting through it cannot fail — there is no "VM already +//! gone" case for work that holds one. +//! +//! [`VmHandle`] is the uncounted form: what something that merely *refers* to a +//! VM from elsewhere holds (another context's message queue, a JSC helper +//! thread, the process-wide child waiter, a file-watcher thread). It cannot +//! reach the VM; it can post to it — deliver-or-refuse, WebKit's +//! `postTaskTo(identifier)`. Long-lived holders (a JS-owned object, a struct +//! the VM frees) hold this and never a ticket: a ticket freed only *after* the +//! wait would deadlock it, and the debug build's wait names any such holder. +//! +//! The raw thread-crossing primitives (the work pool, the HTTP thread, thread +//! spawning) are not called from VM code except through a type that embeds a +//! ticket (`bun_jsc::Job`, or a struct holding a `Ticket` for its in-flight +//! duration); `test/internal/source-lints/vm-thread-door.test.ts` freezes the +//! set of call sites and of `unsafe impl Send/Sync` in the VM crates so a new +//! path around the door needs a justification rather than a reviewer's luck. +#[cfg(debug_assertions)] +use core::panic::Location; use core::ptr::NonNull; use core::sync::atomic::{AtomicU8, AtomicU32, Ordering}; use std::sync::Arc; -use bun_threading::{Condvar, Mutex}; +use bun_threading::{Condvar, Guarded}; use crate::event_loop::EventLoop; use crate::virtual_machine::VirtualMachine; use bun_event_loop::ConcurrentTask::ConcurrentTask as ConcurrentTaskItem; +pub use bun_event_loop::Posted; + #[repr(u8)] #[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Debug)] enum State { /// Normal operation. Open = 0, /// The VM is going away — a parent's `terminate()` (from its thread) or - /// this thread's own exit/teardown: native code enters no more script and - /// starts no new off-thread work; posts are still accepted so completions - /// of already-running work are delivered (and released by the teardown). + /// this thread's own exit: native code enters no more script. Tickets are + /// still issued and posts still accepted, so work already running comes + /// back and follow-on work it starts is counted like any other. Stopping = 1, - /// `close()` ran: nothing off-thread reaches the VM any more. - Closed = 2, -} - -/// Which of the VM's two embedded loops a task belongs to, fixed when the task -/// is created on the JS thread (a task started while a macro runs completes -/// into the macro loop). `Bun.spawnSync`'s isolated loop is not one of these: -/// its producers post through that loop's own [`JsPoster`]. + /// Teardown is waiting for outstanding tickets. Ticket holders post as + /// before (their completions are released on the JS thread as they + /// arrive); no new ticket is issued to another thread; a [`VmHandle`]'s + /// posts are still delivered (and released). + Draining = 2, + /// No ticket is outstanding and none can be created: nothing off-thread + /// reaches the VM any more. Weak posts are refused. + Closed = 3, +} + +/// Which of the VM's two embedded loops a completion belongs to, fixed when +/// the ticket is taken on the JS thread (work started while a macro runs +/// completes into the macro loop). `Bun.spawnSync`'s isolated loop is not one +/// of these: its producers post through that loop's own [`JsPoster`]. +/// +/// [`JsPoster`]: bun_event_loop::JsPoster #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub enum LoopKind { Regular, Macro, } -/// The part of [`Shared`] every native→JS entry on the JS thread reads -/// (`state`, and `vm` on each post) but that changes twice per VM lifetime. -/// Kept on its own cache line: the counters below are RMW'd by pool / HTTP -/// threads on every completion, and sharing a line with them made each of the -/// JS thread's reads a miss whenever another thread had just posted. +/// `state` (read by every native→JS entry on the JS thread) and `vm`, on +/// their own cache line: the counters after it are RMW'd by pool / HTTP +/// threads on every completion. #[cfg_attr( any( target_arch = "x86_64", @@ -68,90 +91,231 @@ pub enum LoopKind { #[cfg_attr(target_arch = "s390x", repr(align(128)))] struct ReadMostly { state: AtomicU8, - /// Dereferenced only while an `Access` guard is held and `state != Closed`, - /// or on the JS thread. Nulled by `close()`. - vm: core::cell::UnsafeCell<*mut VirtualMachine>, + /// The VM. Dereferenced by a ticket holder (the VM outlives every ticket), + /// by a weak accessor inside the `active` gate before `Closed`, or on the + /// JS thread. + vm: *mut VirtualMachine, } -#[cfg_attr( - any( - target_arch = "x86_64", - target_arch = "aarch64", - target_arch = "powerpc64" - ), - repr(align(64)) -)] -#[cfg_attr(target_arch = "s390x", repr(align(128)))] pub struct Shared { hot: ReadMostly, - /// Threads currently inside `post`/`wake`/`ref`/`unref` or holding a - /// [`Borrow`]. `close()` waits for zero after publishing `Closed`. + /// Outstanding [`Ticket`]s. Teardown waits for zero. + tickets: AtomicU32, + /// Threads currently inside a weak `post`/keep-alive/wake. `Closed` is + /// published and then this is waited to zero (the Dekker pair), so a weak + /// access either finished before close returned or saw `Closed`. active: AtomicU32, - /// For `close()` to sleep on while `active` drains (borrows may be long), - /// and `wait_for_embedded_work()` while `embedded` drains. - drained: (Mutex, Condvar), - /// Pool work scheduled with storage inside a JS-owned object (see - /// [`VmHandle::embedded_work_scheduled`]); teardown waits for zero. - embedded: AtomicU32, + /// The tearing-down JS thread sleeps here; ticket drops and posts notify + /// it once draining has begun. + drained: (Guarded<()>, Condvar), #[cfg(debug_assertions)] + debug: DebugState, +} + +/// Debug builds: the JS thread's id, where every live ticket was taken (so a +/// wait that does not end can say who it is waiting for), and the test gate. +#[cfg(debug_assertions)] +struct DebugState { js_thread: std::thread::ThreadId, - /// Test suite only — see [`refusal_gate`]. - #[cfg(debug_assertions)] - park_posts: core::sync::atomic::AtomicBool, + live: Guarded, + /// Test suite only — see [`test_gate`]. + gate: core::sync::atomic::AtomicBool, +} + +#[cfg(debug_assertions)] +#[derive(Default)] +struct LiveTickets { + next_id: u64, + at: bun_collections::HashMap>, } -// SAFETY: `vm` is only dereferenced under the gate described in the module doc; -// everything else is atomics / std sync primitives. +// SAFETY: `vm` is dereferenced only under the discipline in the module doc +// (ticket held ⇒ VM alive; weak ⇒ inside the `active` gate before `Closed`); +// everything else is atomics / sync primitives. unsafe impl Send for Shared {} // SAFETY: as above. unsafe impl Sync for Shared {} +// Orderings: two store→load (Dekker) pairs need `SeqCst` on all four +// operations — `Ticket::drop`'s `tickets` decrement / `state` load against the +// wait's `Draining` store / `tickets` load, and `enter`'s `active` increment / +// `state` load against the wait's `Closed` store / `active` load. Everything +// on `state`, `tickets` and `active` is `SeqCst` so no site has to argue which +// pair it is in; the cost is in the RMWs, not the ordering. +impl Shared { + #[inline] + fn state(&self) -> State { + match self.hot.state.load(Ordering::SeqCst) { + 0 => State::Open, + 1 => State::Stopping, + 2 => State::Draining, + _ => State::Closed, + } + } + + #[inline] + fn loop_of(&self, kind: LoopKind) -> &EventLoop { + // SAFETY: caller holds a ticket or is inside the weak gate before + // `Closed`; the VM and both embedded loops are alive. + unsafe { + match kind { + LoopKind::Regular => &(*self.hot.vm).regular_event_loop, + LoopKind::Macro => &(*self.hot.vm).macro_event_loop, + } + } + } + + fn notify(&self) { + let _g = self.drained.0.lock(); + self.drained.1.notify_all(); + } + + /// Push + wake; while draining, also wake the waiting teardown (it sleeps + /// on the condvar, not on the loop). + fn deliver(&self, kind: LoopKind, task: NonNull) { + let el = self.loop_of(kind); + el.concurrent_tasks.push(task); + if self.state() >= State::Draining { + self.notify(); + } else { + el.wakeup(); + } + } + + fn add_keep_alive(&self, kind: LoopKind, delta: i32) { + let el = self.loop_of(kind); + let _ = el.concurrent_ref.fetch_add(delta, Ordering::SeqCst); + el.wakeup(); + } +} + +// ── Ticket ──────────────────────────────────────────────────────────────── + +/// One unit of "something of this VM's is on another thread". See the module +/// doc. `Send + Sync`; obtain on the JS thread with [`VirtualMachine::ticket`] +/// (or by cloning one you hold, on any thread), keep it in the in-flight +/// operation — never in a JS-owned or VM-owned object — and drop it when the +/// operation's last touch of the VM's memory from another thread is done +/// (normally: right after posting the completion). +pub struct Ticket { + shared: Arc, + kind: LoopKind, + #[cfg(debug_assertions)] + id: u64, +} + +impl Ticket { + #[track_caller] + fn issue(shared: &Arc, kind: LoopKind) -> Ticket { + shared.tickets.fetch_add(1, Ordering::SeqCst); + #[cfg(debug_assertions)] + let id = { + let mut live = shared.debug.live.lock(); + let id = live.next_id; + live.next_id += 1; + live.at.insert(id, Location::caller()); + id + }; + Ticket { + shared: Arc::clone(shared), + kind, + #[cfg(debug_assertions)] + id, + } + } + + /// Queue `task` on the loop this ticket was taken for and wake it. Any + /// thread. Cannot fail: the VM waits for this ticket before it goes. + /// + /// The JS thread may consume `task` — and free whatever it points into — + /// before this returns, so `self` must not live inside that memory: move + /// the ticket out of the work's struct first, post, then drop it. + pub fn post(&self, task: NonNull) { + test_gate::before_ticket_post(self); + debug_assert!( + self.shared.state() != State::Closed, + "ticket post after its VM closed (a ticket was created after the wait)" + ); + self.shared.deliver(self.kind, task); + } + + /// Release a keep-alive taken on the VM's loop (any thread). + pub fn unref_keep_alive(&self) { + self.shared.add_keep_alive(self.kind, -1); + } + + /// Whether the VM is still running script (not stopping). What an + /// off-thread body checks before doing work whose only consumer is + /// script; either way it posts its completion back. + #[inline] + pub fn script_allowed(&self) -> bool { + self.shared.state() == State::Open + } + + /// Whether the VM has begun its final wait: work it has not started yet is + /// no longer wanted (Node `uv_cancel`s queued work at the same point) — + /// hand it straight back. + #[inline] + pub fn cancelled(&self) -> bool { + self.shared.state() >= State::Draining + } +} + +impl Clone for Ticket { + /// One more ticket for the same VM and loop (any thread). + #[track_caller] + fn clone(&self) -> Ticket { + Ticket::issue(&self.shared, self.kind) + } +} + +impl Drop for Ticket { + fn drop(&mut self) { + #[cfg(debug_assertions)] + self.shared.debug.live.lock().at.remove(&self.id); + if self.shared.tickets.fetch_sub(1, Ordering::SeqCst) == 1 + && self.shared.state() >= State::Draining + { + self.shared.notify(); + } + } +} + +// ── VmHandle (uncounted) ────────────────────────────────────────────────── + /// See the module documentation. `repr(transparent)` over the `Arc` so a -/// `*const VmHandle` can cross FFI (C++ / napi hold boxed clones). +/// `*const Shared` can cross FFI (C++ / napi hold references). #[derive(Clone)] #[repr(transparent)] pub struct VmHandle(Arc); -pub use bun_event_loop::Posted; - -/// RAII: one unit of `active`. While held, `close()` cannot complete. +/// RAII: one unit of `active`. While held, `close_and_wait` cannot return. struct Access<'a>(&'a Shared); impl Drop for Access<'_> { fn drop(&mut self) { - if self.0.active.fetch_sub(1, Ordering::SeqCst) == 1 - && self.0.hot.state.load(Ordering::SeqCst) == State::Closed as u8 - { - self.0.drained.0.lock(); - self.0.drained.1.notify_all(); - self.0.drained.0.unlock(); + if self.0.active.fetch_sub(1, Ordering::SeqCst) == 1 && self.0.state() == State::Closed { + self.0.notify(); } } } -/// An off-thread job is using memory the VM owns (request buffers, a JS -/// buffer's backing store) for as long as this is held; the VM's teardown -/// waits for it before freeing anything. Obtain with [`VmHandle::borrow`]. -pub struct Borrow { - _access: Access<'static>, - /// Keeps the `Shared` that `_access` borrows alive. - _handle: VmHandle, -} - impl VmHandle { /// JS thread, at VM creation. pub(crate) fn new(vm: *mut VirtualMachine) -> Self { VmHandle(Arc::new(Shared { hot: ReadMostly { state: AtomicU8::new(State::Open as u8), - vm: core::cell::UnsafeCell::new(vm), + vm, }, + tickets: AtomicU32::new(0), active: AtomicU32::new(0), - drained: (Mutex::new(), Condvar::new()), - embedded: AtomicU32::new(0), - #[cfg(debug_assertions)] - js_thread: std::thread::current().id(), + drained: (Guarded::new(()), Condvar::new()), #[cfg(debug_assertions)] - park_posts: core::sync::atomic::AtomicBool::new(false), + debug: DebugState { + js_thread: std::thread::current().id(), + live: Default::default(), + gate: core::sync::atomic::AtomicBool::new(false), + }, })) } @@ -159,55 +323,26 @@ impl VmHandle { fn enter(&self) -> Option> { self.0.active.fetch_add(1, Ordering::SeqCst); let a = Access(&self.0); - if self.0.hot.state.load(Ordering::SeqCst) == State::Closed as u8 { - drop(a); - return None; - } - Some(a) - } - - /// # Safety - /// Caller holds an `Access` obtained from `enter()` (so `state != Closed` - /// was observed after `active` was raised, and `close()` cannot have - /// returned), or is the JS thread before `close()`. - #[inline] - unsafe fn vm(&self) -> *mut VirtualMachine { - // SAFETY: per fn contract. - unsafe { *self.0.hot.vm.get() } + (self.0.state() != State::Closed).then_some(a) } - #[inline] - fn loop_of<'a>(vm: *mut VirtualMachine, kind: LoopKind) -> &'a EventLoop { - // SAFETY: caller is inside the gate; the VM and both embedded loops are alive. - unsafe { - match kind { - LoopKind::Regular => &(*vm).regular_event_loop, - LoopKind::Macro => &(*vm).macro_event_loop, - } - } - } - - // ── off-thread API ──────────────────────────────────────────────────── + // ── any-thread API ───────────────────────────────────────────────────── - /// Queue `task` on the VM's `kind` loop and wake it, or hand it back. + /// Queue `task` on the VM's `kind` loop and wake it, or hand it back if + /// the VM is closed. For posters that hold no ticket (their payload is + /// their own to free on refusal). pub fn post(&self, kind: LoopKind, task: NonNull) -> Posted { - refusal_gate::before_post(self); - let Some(_a) = self.enter() else { - // SAFETY: handed to us by the caller and not yet queued anywhere. - let tag = unsafe { task.as_ref() }.task.tag; - refusal_gate::refused(self, format_args!("post: {}", tag.name())); - return Posted::Refused(task); - }; - // SAFETY: inside the gate. - let el = Self::loop_of(unsafe { self.vm() }, kind); - el.concurrent_tasks.push(task); - el.wakeup(); - Posted::Queued + test_gate::weak_post(&self.0, task, |task| { + let Some(_a) = self.enter() else { + return Posted::Refused(task); + }; + self.0.deliver(kind, task); + Posted::Queued + }) } /// Queue a C++ `EventLoopTask` from another thread (WebCore's - /// `postTaskConcurrently`), or delete it unrun if the VM is gone — the - /// same release teardown applies to queued C++ tasks. + /// `postTaskConcurrently`), or delete it unrun if the VM is closed. /// /// # Safety /// `task` is a live heap `WebCore::EventLoopTask` the caller hands over. @@ -225,235 +360,279 @@ impl VmHandle { } } - /// Keep the VM's loop alive from another thread (no-op once closed; the - /// teardown ignores keep-alives anyway). - pub fn ref_keep_alive(&self, kind: LoopKind) { + /// Adjust the VM's keep-alive from another thread (no-op once closed). + pub fn add_keep_alive(&self, kind: LoopKind, delta: i32) { if let Some(_a) = self.enter() { - // SAFETY: inside the gate. - let el = Self::loop_of(unsafe { self.vm() }, kind); - let _ = el.concurrent_ref.fetch_add(1, Ordering::SeqCst); - el.wakeup(); - } - } - - pub fn unref_keep_alive(&self, kind: LoopKind) { - if let Some(_a) = self.enter() { - // SAFETY: inside the gate. - let el = Self::loop_of(unsafe { self.vm() }, kind); - let _ = el.concurrent_ref.fetch_sub(1, Ordering::SeqCst); - el.wakeup(); + self.0.add_keep_alive(kind, delta); } } - /// This job is about to use VM-owned memory off-thread; `None` if the VM - /// is closed (touch nothing). Hold the result until done. Jobs that could - /// block indefinitely on an external party must own their memory instead. - pub fn borrow(&self) -> Option { - let a = self.enter()?; - // SAFETY: lifetime extension is sound because `Borrow` also holds a - // clone of the Arc that `a` borrows from. - let a: Access<'static> = unsafe { core::mem::transmute(a) }; - Some(Borrow { - _access: a, - _handle: self.clone(), - }) - } - - /// As [`borrow`](Self::borrow), but only while the VM is still running - /// (not yet stopping): what a pool body checks before doing work whose - /// only consumer is script. - pub fn borrow_if_running(&self) -> Option { - let b = self.borrow()?; - (self.0.hot.state.load(Ordering::SeqCst) == State::Open as u8).then_some(b) - } - - // ── embedded work ───────────────────────────────────────────────────── - // - // Pool work whose storage is a field of a JS-owned object (a transpile - // slot inside the VM, a zlib stream's native part) cannot be boxed into a - // `Job` and cannot outlive the VM. It is counted instead: teardown waits - // for the count before the handle closes, so such work always posts its - // completion into a live queue and is released on the JS thread — the - // pool side never sees a dead VM. Bodies check `borrow_if_running` so a - // stopping VM only waits for the pool to *reach* the work, not to do it. - - /// JS thread, before handing embedded work to the pool. Script starts - /// such work only while the VM is open; a native continuation that runs - /// during teardown (a release arm retrying a request) checks - /// [`accepting_work`](Self::accepting_work) first and fails instead. - pub fn embedded_work_scheduled(&self) { - debug_assert!( - self.0.hot.state.load(Ordering::SeqCst) != State::Closed as u8, - "embedded work started on a closed VM handle" + /// The VM is going away: `Open → Stopping` (idempotent; never reopens). + /// Any thread — a parent's `terminate()` calls it at request time, as + /// Node's `Environment::ExitEnv` sets `is_stopping` from the requesting + /// thread; this thread's own exit path calls it via + /// `VirtualMachine::forbid_script`. + pub fn stop(&self) { + let _ = self.0.hot.state.compare_exchange( + State::Open as u8, + State::Stopping as u8, + Ordering::SeqCst, + Ordering::SeqCst, ); - self.0.embedded.fetch_add(1, Ordering::SeqCst); } - /// Whether new off-thread work may still be started for this VM (it has - /// not begun stopping). JS thread. - pub fn accepting_work(&self) -> bool { - self.0.hot.state.load(Ordering::SeqCst) == State::Open as u8 + /// [`stop`](Self::stop), raise a JSC `TerminationException` in the VM at + /// its next safepoint, and wake its loop. Any thread (a parent's + /// `worker.terminate()`); no-op once the VM is closed. + pub fn request_termination(&self) { + self.stop(); + if let Some(_a) = self.enter() { + // SAFETY: inside the gate before `Closed` ⇒ the VM is alive; + // `notify_need_termination` is thread-safe (VMTraps). Raw field + // read, no `&VirtualMachine` formed off-thread. + unsafe { (*(*self.0.hot.vm).jsc_vm.cast_const()).notify_need_termination() }; + self.0.loop_of(LoopKind::Regular).wakeup(); + } } - /// Pool thread, after its last touch of the embedded storage (i.e. after - /// posting the completion). - pub fn embedded_work_finished(&self) { - if self.0.embedded.fetch_sub(1, Ordering::SeqCst) == 1 - && self.0.hot.state.load(Ordering::SeqCst) != State::Open as u8 - { - self.0.drained.0.lock(); - self.0.drained.1.notify_all(); - self.0.drained.0.unlock(); + /// Wake the VM's loop (no-op once closed). Any thread. + pub fn wake(&self) { + if let Some(_a) = self.enter() { + self.0.loop_of(LoopKind::Regular).wakeup(); } } - pub(crate) fn embedded_work_outstanding(&self) -> u32 { - self.0.embedded.load(Ordering::SeqCst) + /// May native code call into user JS / settle its promises right now? + /// (Node's `can_call_into_js()`.) Any thread; meaningful on the JS thread. + #[inline] + pub fn script_allowed(&self) -> bool { + self.0.state() == State::Open } - /// Teardown (JS thread, stopping, before `close()`): wait until the pool - /// holds no embedded work of this VM. - pub(crate) fn wait_for_embedded_work(&self) { - self.assert_js_thread(); - debug_assert!(self.0.hot.state.load(Ordering::SeqCst) != State::Open as u8); - if self.0.embedded.load(Ordering::SeqCst) != 0 { - self.0.drained.0.lock(); - while self.0.embedded.load(Ordering::SeqCst) != 0 { - self.0.drained.1.wait(&self.0.drained.0); - } - self.0.drained.0.unlock(); - } + pub(crate) fn tickets_outstanding(&self) -> u32 { + self.0.tickets.load(Ordering::SeqCst) } // ── JS-thread API ───────────────────────────────────────────────────── #[cfg(debug_assertions)] pub(crate) fn assert_js_thread(&self) { - debug_assert_eq!(std::thread::current().id(), self.0.js_thread); + debug_assert_eq!(std::thread::current().id(), self.0.debug.js_thread); } #[cfg(not(debug_assertions))] #[inline(always)] pub(crate) fn assert_js_thread(&self) {} - /// The VM is going away: `Open → Stopping` (idempotent; never reopens or - /// un-closes). Any thread — a parent's `terminate()` calls it at request - /// time, as Node's `Environment::ExitEnv` sets `is_stopping` from the - /// requesting thread; this thread's own exit path calls it via - /// `VirtualMachine::forbid_script`. - pub fn stop(&self) { - let _ = self.0.hot.state.compare_exchange( - State::Open as u8, - State::Stopping as u8, - Ordering::SeqCst, - Ordering::SeqCst, - ); + /// Teardown, phase B of `VirtualMachine::teardown` (JS thread, script + /// forbidden, everything cancellable cancelled): wait until no ticket is outstanding, calling `service` + /// (release everything queued, on this thread, heap alive) whenever + /// something may have arrived; then refuse weak accessors and wait out any + /// mid-call. After this returns nothing off-thread can reach the VM. + /// + /// Unbounded by design: a job that cannot be cancelled makes this take as + /// long as the job (as Node's environment cleanup does). Every wake source + /// (ticket drop, post) notifies the condvar; the 1 s timeout is a backstop + /// and the debug build's cadence for naming the outstanding tickets. + pub(crate) fn close_and_wait(&self, mut service: impl FnMut()) { + self.assert_js_thread(); + let s = &*self.0; + s.hot.state.store(State::Draining as u8, Ordering::SeqCst); + test_gate::draining(self); + #[cfg(debug_assertions)] + let started = std::time::Instant::now(); + #[cfg(debug_assertions)] + let mut next_report = test_gate::first_report_secs(self); + loop { + service(); + let mut g = s.drained.0.lock(); + if !s.loop_of(LoopKind::Regular).concurrent_tasks.is_empty() + || !s.loop_of(LoopKind::Macro).concurrent_tasks.is_empty() + { + continue; + } + if s.tickets.load(Ordering::SeqCst) == 0 { + s.hot.state.store(State::Closed as u8, Ordering::SeqCst); + break; + } + let _ = s.drained.1.timed_wait_guarded(&mut g, 1_000_000_000); + drop(g); + #[cfg(debug_assertions)] + { + let secs = started.elapsed().as_secs(); + if secs >= next_report { + next_report = secs + 10; + self.dump_outstanding(secs); + } + } + } + if s.active.load(Ordering::SeqCst) != 0 { + let mut g = s.drained.0.lock(); + while s.active.load(Ordering::SeqCst) != 0 { + s.drained.1.wait_guarded(&mut g); + } + } + // A weak post that entered before `Closed` was published. + service(); } - /// May native code call into user JS / settle its promises right now? - /// (Node's `can_call_into_js()`.) Any thread; meaningful on the JS thread. - pub fn script_allowed(&self) -> bool { - self.0.hot.state.load(Ordering::Acquire) == State::Open as u8 + #[cfg(debug_assertions)] + fn dump_outstanding(&self, secs: u64) { + let live = self.0.debug.live.lock(); + let mut sites: Vec<(&'static str, u32)> = + live.at.values().map(|l| (l.file(), l.line())).collect(); + sites.sort_unstable(); + let w = bun_core::output::error_writer(); + let _ = writeln!( + w, + "[vm] teardown has waited {secs}s for {} ticket(s) still held off-thread:", + sites.len() + ); + for run in sites.chunk_by(|a, b| a == b) { + let _ = writeln!( + w, + "[vm] {}× taken at {}:{}", + run.len(), + run[0].0, + run[0].1 + ); + } + let _ = w.flush(); } +} - /// Teardown, JS thread, after children are joined and before queued work - /// is released: refuse every future post/wake/ref/borrow and wait until no - /// thread is inside one. After this returns nothing off-thread can reach - /// the VM; whatever was posted before is in the queues for the teardown to - /// release. - pub(crate) fn close(&self) { - self.assert_js_thread(); - self.0 - .hot - .state - .store(State::Closed as u8, Ordering::SeqCst); - refusal_gate::closed(self); - if self.0.active.load(Ordering::SeqCst) != 0 { - self.0.drained.0.lock(); - while self.0.active.load(Ordering::SeqCst) != 0 { - self.0.drained.1.wait(&self.0.drained.0); - } - self.0.drained.0.unlock(); - } - // SAFETY: JS thread; no accessor can be inside any more. - unsafe { *self.0.hot.vm.get() = core::ptr::null_mut() }; +impl VirtualMachine { + /// JS thread: a ticket for work about to leave this thread — this VM, and + /// the loop it is currently ticking. Hold it in the in-flight operation + /// and drop it after the completion is posted. Infallible until the wait + /// has finished (after which nothing on this thread starts off-thread work). + #[track_caller] + #[inline] + pub fn ticket(&self) -> Ticket { + let h = self.handle_ref(); + h.assert_js_thread(); + debug_assert!( + h.0.state() != State::Closed, + "off-thread work started after the VM finished draining" + ); + Ticket::issue(&h.0, self.current_loop_kind()) } } -// ── Test suite only: deterministic refusals ─────────────────────────────── +// ── Test suite only: deterministic late completions ─────────────────────── // -// `BUN_DEBUG_TEST_WORKER_REFUSAL_GATE` (worker VMs; builds with debug -// assertions): a post from another thread — unless counted work is -// outstanding, whose producer must post before its count can return — waits -// until this handle is closed and only then proceeds, so it is refused with the -// real preconditions (the JS side already released, the handle really closed) -// and the producer's own release path runs every time rather than only when it -// happens to lose the race with teardown. Each refusal is named on stderr. +// `BUN_DEBUG_TEST_WORKER_TEARDOWN_GATE` (first-level worker VMs; builds with +// debug assertions): a post from another thread is held until the worker's +// teardown has begun waiting, so it always arrives *during* the wait — the +// ticketed path (queued, released on the JS thread, then the wait ends) and +// the weak path (queued-and-released while draining, or refused once closed) +// run with their real preconditions every time instead of only when they lose +// the race. Each is named on stderr. The parked thread keeps whatever locks it +// holds (the fetch tasklet's mutex, a streaming body's buffer lock), so a row +// whose worker then blocks on that same lock never reaches teardown: a hang +// under the gate, not in production. #[cfg(debug_assertions)] -mod refusal_gate { - use super::{Ordering, State, VmHandle}; +mod test_gate { + use super::{Ordering, Posted, Shared, State, Ticket, VmHandle}; + type Task = core::ptr::NonNull; impl VmHandle { - pub(crate) fn park_posts_until_closed(&self) { - self.0.park_posts.store(true, Ordering::Relaxed); - } - fn posts_parked(&self) -> bool { - self.0.park_posts.load(Ordering::Relaxed) + pub(crate) fn arm_test_gate(&self) { + self.0.debug.gate.store(true, Ordering::Relaxed); } } - - pub(super) fn before_post(h: &VmHandle) { - if !h.posts_parked() - || std::thread::current().id() == h.0.js_thread - || h.0.embedded.load(Ordering::SeqCst) != 0 - { - return; + fn on(s: &Shared) -> bool { + s.debug.gate.load(Ordering::Relaxed) + } + fn armed(s: &Shared) -> bool { + on(s) && std::thread::current().id() != s.debug.js_thread + } + fn park_until_draining(s: &Shared) { + let mut g = s.drained.0.lock(); + while s.state() < State::Draining { + s.drained.1.wait_guarded(&mut g); } - // Not holding `active` here: close() waits for that to drain. - h.0.drained.0.lock(); - while h.0.hot.state.load(Ordering::SeqCst) != State::Closed as u8 { - h.0.drained.1.wait(&h.0.drained.0); + } + /// One line at a time: pool threads report concurrently. + static SAY: bun_threading::Mutex = bun_threading::Mutex::new(); + fn say(what: core::fmt::Arguments<'_>) { + let _g = SAY.lock_guard(); + let w = bun_core::output::error_writer(); + let _ = writeln!(w, "[vm] {what}"); + let _ = w.flush(); + } + + pub(super) fn before_ticket_post(t: &Ticket) { + if armed(&t.shared) { + park_until_draining(&t.shared); + let l = *t + .shared + .debug + .live + .lock() + .at + .get(&t.id) + .expect("live ticket"); + say(format_args!( + "late completion from {}:{}", + l.file(), + l.line() + )); } - h.0.drained.0.unlock(); } - - /// close(), after publishing Closed: parked posts go now (and are refused). - pub(super) fn closed(h: &VmHandle) { - if h.posts_parked() { - h.0.drained.0.lock(); - h.0.drained.1.notify_all(); - h.0.drained.0.unlock(); + pub(super) fn weak_post(s: &Shared, task: Task, post: impl FnOnce(Task) -> Posted) -> Posted { + if !armed(s) { + return post(task); } + park_until_draining(s); + // SAFETY: handed over by the caller and not yet queued anywhere. + let tag = unsafe { task.as_ref() }.task.tag; + let r = post(task); + let outcome = match r { + Posted::Queued => "released by the wait", + Posted::Refused(_) => "refused", + }; + say(format_args!("late post: {} ({outcome})", tag.name())); + r } - - pub(super) fn refused(h: &VmHandle, what: core::fmt::Arguments<'_>) { - if h.posts_parked() { - let w = bun_core::output::error_writer(); - let _ = writeln!(w, "[vm_handle] refused {what}"); - let _ = w.flush(); + /// The wait began: parked posts go now. + pub(super) fn draining(h: &VmHandle) { + if on(&h.0) { + h.0.notify(); } } + /// The gate's tests read the outstanding-ticket dump; everyone else only + /// after a wait long enough to be worth explaining even on a slow build. + pub(super) fn first_report_secs(h: &VmHandle) -> u64 { + if on(&h.0) { 2 } else { 10 } + } } #[cfg(not(debug_assertions))] -mod refusal_gate { - use super::VmHandle; +mod test_gate { + use super::{Posted, Shared, Ticket, VmHandle}; + type Task = core::ptr::NonNull; + impl VmHandle { + #[inline(always)] + pub(crate) fn arm_test_gate(&self) {} + } #[inline(always)] - pub(super) fn before_post(_: &VmHandle) {} + pub(super) fn before_ticket_post(_: &Ticket) {} #[inline(always)] - pub(super) fn closed(_: &VmHandle) {} + pub(super) fn weak_post(_: &Shared, task: Task, post: impl FnOnce(Task) -> Posted) -> Posted { + post(task) + } #[inline(always)] - pub(super) fn refused(_: &VmHandle, _: core::fmt::Arguments<'_>) {} + pub(super) fn draining(_: &VmHandle) {} } -// ── C++ holds counted references to a handle ───────────────────────────── +// ── C++ holds references ────────────────────────────────────────────────── // -// One representation crosses the FFI: `*const Shared`, a strong count on the -// Arc every `VmHandle` clone points at (`BunVmHandleRef` in C++). Long-lived -// holders (JSVMClientData, EventLoopTaskNoContext, NapiEnv) `retain` one and -// `release` it; a call that merely uses a reference someone else holds borrows -// it for the duration ([`VmHandle::borrow_ref`]). Nothing is boxed. - -/// A `VmHandle` view over a reference C++ holds, for the duration of one call: -/// the count stays C++'s. +// `*const Shared` (`BunVmHandleRef`) is one strong count on the Arc behind a +// [`VmHandle`] — what a long-lived C++ holder (JSVMClientData, NapiEnv) keeps +// and posts through. C++ work bound for another thread (WebCrypto's +// `EventLoopTaskNoContext`) is carried by a Rust task that holds the ticket +// (`ConcurrentCppTask`), so no ticket crosses the FFI. + +/// A `VmHandle` view over a reference C++ holds, for the duration of one call. pub struct BorrowedRef(core::mem::ManuallyDrop); impl core::ops::Deref for BorrowedRef { type Target = VmHandle; @@ -469,8 +648,8 @@ impl VmHandle { } /// # Safety - /// `r` is a live reference obtained from [`VmHandle::into_ref`] (directly or - /// via `Bun__VmHandle__retain*`) that its holder keeps for the duration. + /// `r` is a live reference obtained from [`VmHandle::into_ref`] that its + /// holder keeps for the duration. pub unsafe fn borrow_ref(r: *const Shared) -> BorrowedRef { // SAFETY: fn contract; ManuallyDrop leaves the holder's count untouched. BorrowedRef(core::mem::ManuallyDrop::new(VmHandle(unsafe { @@ -492,8 +671,7 @@ pub extern "C" fn Bun__VmHandle__retain(vm: &VirtualMachine) -> *const Shared { vm.handle().into_ref() } -/// Any thread: one more reference on the same handle (for something that may -/// outlive whoever it got the reference from). +/// Any thread: one more reference on the same handle. /// /// # Safety /// `r` is a live reference its holder keeps for the duration of the call. @@ -514,8 +692,7 @@ pub unsafe extern "C" fn Bun__VmHandle__release(r: *const Shared) { } /// Any thread: post a C++ task through a reference and give the reference up -/// (queued, or deleted unrun if the VM is gone). For a caller that took the -/// reference only to keep the VM reachable past a lock it was about to drop. +/// (queued, or deleted unrun if the VM is closed). /// /// # Safety /// `r` came from `Bun__VmHandle__retain*` and is not used afterwards; `task` is @@ -531,9 +708,7 @@ pub unsafe extern "C" fn Bun__VmHandle__postAndRelease( unsafe { handle.post_cpp_task(task) }; } -/// JS thread: adjust this VM's keep-alive directly (balanced pairs from -/// MessagePort / BroadcastChannel / ScriptExecutionContext stay balanced through -/// teardown; the cross-thread route below stops applying once the VM closes). +/// JS thread: adjust this VM's keep-alive directly. #[unsafe(no_mangle)] pub extern "C" fn Bun__eventLoop__refKeepAlive(vm: &VirtualMachine, delta: core::ffi::c_int) { if delta > 0 { @@ -550,16 +725,10 @@ pub extern "C" fn Bun__eventLoop__refKeepAlive(vm: &VirtualMachine, delta: core: #[unsafe(no_mangle)] pub unsafe extern "C" fn Bun__VmHandle__refKeepAlive(r: *const Shared, delta: core::ffi::c_int) { // SAFETY: fn contract. - let handle = unsafe { VmHandle::borrow_ref(r) }; - if delta > 0 { - handle.ref_keep_alive(LoopKind::Regular); - } else { - handle.unref_keep_alive(LoopKind::Regular); - } + unsafe { VmHandle::borrow_ref(r) }.add_keep_alive(LoopKind::Regular, delta.signum()); } -/// Any thread: Node's `can_call_into_js()` — false once the VM's stop was -/// requested (a parent's terminate(), the worker's own exit, teardown). +/// Any thread: Node's `can_call_into_js()`. /// /// # Safety /// `r` is a live reference its holder keeps for the duration of the call. @@ -570,8 +739,7 @@ pub unsafe extern "C" fn Bun__VmHandle__scriptAllowed(r: *const Shared) -> bool } /// The address of this handle's state byte, for C++ to test -/// `*addr == BUN_VM_HANDLE_STATE_OPEN` inline on its native→JS entries instead -/// of calling out per callback. Valid as long as the reference is held. +/// `*addr == BUN_VM_HANDLE_STATE_OPEN` inline on its native→JS entries. /// /// # Safety /// `r` is a live reference. @@ -586,29 +754,41 @@ const _: () = assert!(State::Open as u8 == 0); // ── Producers that serve either a JS VM or a MiniEventLoop ──────────────── // -// fs.cp (also used by the shell), shell builtins, password hashing, zlib run -// on the work pool for whichever loop created them. For the JS case the -// completion goes through the VM's handle; a MiniEventLoop (bundler / shell / -// install threads) is owned by its thread and outlives the work it schedules, -// so its concurrent queue is posted to directly, as before. - -/// Where an off-thread completion goes: a JS VM (through its handle) or a -/// mini event loop. Captured on the owning thread when the work is created. -#[derive(Clone)] +// fs.cp (also used by the shell), shell builtins, zlib run on the work pool +// for whichever loop created them. For the JS case the work holds a ticket; a +// MiniEventLoop (bundler / shell / install threads) is owned by its thread and +// outlives the work it schedules, so its concurrent queue is posted to directly. + +/// Where an off-thread completion goes: a JS VM (through a ticket the work +/// holds) or a mini event loop. Captured on the owning thread when the work is +/// created; dropped when the work is done with the loop. Cloning the JS arm +/// takes one more ticket. pub enum ConcurrentPoster { - /// Erased handle of the JS loop's VM (obtained from the `EventLoopHandle` - /// itself, so it is correct whichever thread constructs the poster). - Js(bun_event_loop::JsPoster), + Js(Ticket), Mini(bun_ptr::BackRef), } +impl Clone for ConcurrentPoster { + #[track_caller] + fn clone(&self) -> Self { + match self { + ConcurrentPoster::Js(t) => ConcurrentPoster::Js(t.clone()), + ConcurrentPoster::Mini(m) => ConcurrentPoster::Mini(*m), + } + } +} + impl ConcurrentPoster { - /// From an `EventLoopHandle`: the JS arm asks the loop for its VM's poster - /// (a JS-thread-owned handle knows its VM); the mini arm posts directly. + /// Owning thread: for a JS loop, take a ticket on its VM; for a mini loop, + /// post directly. + #[track_caller] pub fn from_event_loop_handle(h: &bun_event_loop::EventLoopHandle) -> Self { match h { bun_event_loop::EventLoopHandle::Js { owner } => { - ConcurrentPoster::Js(owner.js_poster()) + // SAFETY: a `Js` handle is only formed on its VM's thread from + // the live VM (`EventLoopHandle::init` contract). + let vm = unsafe { &*owner.bun_vm().cast::() }; + ConcurrentPoster::Js(vm.ticket()) } bun_event_loop::EventLoopHandle::Mini(mini) => ConcurrentPoster::Mini(*mini), } @@ -618,32 +798,15 @@ impl ConcurrentPoster { matches!(self, ConcurrentPoster::Js(..)) } - /// JS arm: count embedded work on the VM (see `VmHandle`). A mini loop is - /// owned by its thread and outlives its work, so there is nothing to count. - pub fn embedded_work_scheduled(&self) { - if let ConcurrentPoster::Js(p) = self { - p.embedded_work_scheduled(); - } - } - pub fn embedded_work_finished(&self) { - if let ConcurrentPoster::Js(p) = self { - p.embedded_work_finished(); - } - } - - /// Post a JS-loop `ConcurrentTask`. `Refused` ⇒ VM torn down, caller - /// releases. Panics (debug) if this poster is `Mini`. - pub fn post_js(&self, task: NonNull) -> Posted { + /// Post a JS-loop `ConcurrentTask`. Panics (debug) if this poster is `Mini`. + pub fn post_js(&self, task: NonNull) { match self { - ConcurrentPoster::Js(p) => p.post(task), - ConcurrentPoster::Mini(_) => { - debug_assert!(false, "post_js on a Mini poster"); - Posted::Refused(task) - } + ConcurrentPoster::Js(t) => t.post(task), + ConcurrentPoster::Mini(_) => debug_assert!(false, "post_js on a Mini poster"), } } - /// Post a mini-loop task (always accepted; the mini loop outlives its work). + /// Post a mini-loop task (the mini loop outlives its work). pub fn post_mini( &self, task: NonNull, @@ -682,28 +845,14 @@ unsafe fn poster_drop(data: *const ()) { // SAFETY: as above; balances `into_raw`/`increment_strong_count`. unsafe { drop(Arc::from_raw(data.cast::())) }; } -unsafe fn poster_embedded_scheduled(data: *const ()) { - // SAFETY: as `poster_post`. - unsafe { &*data.cast::() } - .handle - .embedded_work_scheduled(); -} -unsafe fn poster_embedded_finished(data: *const ()) { - // SAFETY: as `poster_post`. - unsafe { &*data.cast::() } - .handle - .embedded_work_finished(); -} static POSTER_VTABLE: bun_event_loop::JsPosterVTable = bun_event_loop::JsPosterVTable { post: poster_post, - embedded_work_scheduled: poster_embedded_scheduled, - embedded_work_finished: poster_embedded_finished, clone: poster_clone, drop: poster_drop, }; impl VmHandle { - /// An erased poster for `kind`, for code that cannot name `VmHandle`. + /// An erased weak poster for `kind`, for code that cannot name `VmHandle`. pub fn to_js_poster(&self, kind: LoopKind) -> bun_event_loop::JsPoster { let data = Arc::into_raw(Arc::new(PosterData { handle: self.clone(), @@ -716,129 +865,9 @@ impl VmHandle { } impl VirtualMachine { - /// JS thread: an erased poster for the current loop of this VM. + /// JS thread: an erased weak poster for the current loop of this VM. pub fn js_poster(&self) -> bun_event_loop::JsPoster { - self.loop_handle().to_js_poster() - } -} - -// ── LoopHandle: "where this job's completion goes" ──────────────────────── - -/// A [`VmHandle`] plus the loop of that VM the completion belongs on — what a -/// job created on the JS thread captures (`vm.loop_handle()`) and posts back -/// through from whatever thread finishes it. -#[derive(Clone)] -pub struct LoopHandle { - vm: VmHandle, - kind: LoopKind, -} - -/// A job that finishes off the JS thread and is posted back to its VM as a -/// `ConcurrentTask` — see [`post_job`]. It says where its [`LoopHandle`] lives -/// and how to release itself when the VM is already gone. There is -/// deliberately no default for the release: a job that cannot release itself -/// does not compile. -pub trait Postable: bun_event_loop::Taskable + Sized { - /// The handle captured at creation (`vm.loop_handle()`), stored in the job. - /// - /// # Safety - /// `this` is live. - unsafe fn loop_handle(this: *mut Self) -> *const LoopHandle; - - /// The `ConcurrentTask` that carries `this`: a fresh heap one by default; - /// jobs with an embedded task return that instead. - /// - /// # Safety - /// `this` is live. - unsafe fn concurrent_task(this: *mut Self) -> NonNull { - ConcurrentTaskItem::create_from(this) - } - - /// The VM refused the completion (torn down). Runs on the posting thread, - /// usually *not* the JS thread: free what the job owns, do not touch JSC - /// handles (they die with the VM), and free the allocation itself. - /// - /// # Safety - /// `this` is the live job; nothing uses it afterwards. - unsafe fn release_refused(this: *mut Self); -} - -/// Post a finished job's completion back to the VM it came from. If that VM -/// has been torn down, the job releases itself here; callers have nothing to -/// check either way. -/// -/// # Safety -/// `job` is a live heap job whose off-thread part is finished; the caller does -/// not touch it afterwards (it now belongs to the VM's queue, or was released). -pub unsafe fn post_job(job: *mut T) { - // Clone the handle out first: a refusal frees `job`, handle field included. - // SAFETY: fn contract. - let handle = unsafe { (*T::loop_handle(job)).clone() }; - // SAFETY: fn contract. - let task = unsafe { T::concurrent_task(job) }; - if let Posted::Refused(task) = handle.post_task(task) { - refusal_gate::refused( - &handle.vm, - format_args!("job: {}", core::any::type_name::()), - ); - // SAFETY: handed back unqueued; `job` per fn contract. - unsafe { - ConcurrentTaskItem::release_refused(task); - T::release_refused(job); - } - } -} - -impl LoopHandle { - /// Post an already-built task. Prefer [`post_job`], which leaves the caller - /// nothing to check; this hands a refusal back. - pub fn post_task(&self, task: NonNull) -> Posted { - self.vm.post(self.kind, task) - } - pub fn borrow(&self) -> Option { - self.vm.borrow() - } - pub fn borrow_if_running(&self) -> Option { - self.vm.borrow_if_running() - } - pub fn accepting_work(&self) -> bool { - self.vm.accepting_work() - } - pub fn embedded_work_scheduled(&self) { - self.vm.embedded_work_scheduled() - } - pub fn embedded_work_finished(&self) { - self.vm.embedded_work_finished() - } - pub fn ref_keep_alive(&self) { - self.vm.ref_keep_alive(self.kind) - } - pub fn unref_keep_alive(&self) { - self.vm.unref_keep_alive(self.kind) - } - /// An erased poster for this loop, for code that cannot name `bun_jsc`. - pub fn to_js_poster(&self) -> bun_event_loop::JsPoster { - self.vm.to_js_poster(self.kind) - } -} - -impl VirtualMachine { - /// This VM's live pool jobs. JS thread only. - #[allow(clippy::mut_from_ref)] - #[inline] - pub fn jobs(&self) -> &mut crate::job::JobList { - // SAFETY: JS-thread-only intrusive list; callers never hold two at once - // (each call is a single push/unlink/release statement). - unsafe { &mut *self.jobs.get() } - } - - /// JS thread: the handle a new job captures — this VM, and the loop it is - /// currently ticking (regular, macro, or a spawnSync isolated loop). - pub fn loop_handle(&self) -> LoopHandle { - LoopHandle { - vm: self.handle(), - kind: self.current_loop_kind(), - } + self.handle_ref().to_js_poster(self.current_loop_kind()) } } @@ -847,7 +876,8 @@ impl VirtualMachine { // spawnSync runs a third, heap-allocated `EventLoop` on the JS thread while it // blocks; process exits (waiter thread) and pool completions for that call // must land on *its* concurrent queue, not the VM's. It gets its own small -// poster with the same gate discipline, closed before the loop is freed. +// weak poster with the same gate discipline, closed before the loop is freed. +// The VM cannot tear down under it: its creator drives it synchronously. /// Opaque outside this crate: the poster of a spawnSync isolated loop. pub struct IsolatedPosterInner { @@ -915,13 +945,8 @@ unsafe fn isolated_drop(data: *const ()) { // SAFETY: as above. unsafe { drop(Arc::from_raw(data.cast::())) }; } -// An isolated loop is driven to completion synchronously by its creator on -// the JS thread (spawnSync), so its VM cannot tear down under its work. -unsafe fn isolated_embedded_noop(_data: *const ()) {} static ISOLATED_POSTER_VTABLE: bun_event_loop::JsPosterVTable = bun_event_loop::JsPosterVTable { post: isolated_post, - embedded_work_scheduled: isolated_embedded_noop, - embedded_work_finished: isolated_embedded_noop, clone: isolated_clone, drop: isolated_drop, }; diff --git a/src/jsc/bindings/EventLoopTaskNoContext.cpp b/src/jsc/bindings/EventLoopTaskNoContext.cpp index d2590245b5c7..b85c8fd98f01 100644 --- a/src/jsc/bindings/EventLoopTaskNoContext.cpp +++ b/src/jsc/bindings/EventLoopTaskNoContext.cpp @@ -7,9 +7,4 @@ extern "C" void Bun__EventLoopTaskNoContext__performTask(EventLoopTaskNoContext* task->performTask(); } -extern "C" const ::BunVmHandleRef* Bun__EventLoopTaskNoContext__vmHandle(const EventLoopTaskNoContext* task) -{ - return task->vmHandle(); -} - } // namespace Bun diff --git a/src/jsc/bindings/EventLoopTaskNoContext.h b/src/jsc/bindings/EventLoopTaskNoContext.h index 9052ce618014..89f3ac287b7d 100644 --- a/src/jsc/bindings/EventLoopTaskNoContext.h +++ b/src/jsc/bindings/EventLoopTaskNoContext.h @@ -2,41 +2,31 @@ #include "ZigGlobalObject.h" #include "root.h" -#include "BunClientData.h" namespace Bun { -// Just like WebCore::EventLoopTask but does not take a ScriptExecutionContext +// Just like WebCore::EventLoopTask but does not take a ScriptExecutionContext. +// The Rust `ConcurrentCppTask` that carries one to the work pool holds the +// creating VM's ticket, so that VM outlives the task. class EventLoopTaskNoContext { WTF_MAKE_TZONE_ALLOCATED(EventLoopTaskNoContext); public: - EventLoopTaskNoContext(JSC::JSGlobalObject* globalObject, Function&& task) - : m_vmHandle(Bun__VmHandle__retainRef(WebCore::clientData(JSC::getVM(globalObject))->vmHandle)) - , m_task(WTF::move(task)) + EventLoopTaskNoContext(Function&& task) + : m_task(WTF::move(task)) { } - ~EventLoopTaskNoContext() - { - Bun__VmHandle__release(m_vmHandle); - } - void performTask() { m_task(); delete this; } - // A reference to the creating VM's handle, since a pool task can outlive that VM. - const ::BunVmHandleRef* vmHandle() const { return m_vmHandle; } - private: - const ::BunVmHandleRef* m_vmHandle; Function m_task; }; extern "C" void Bun__EventLoopTaskNoContext__performTask(EventLoopTaskNoContext* task); -extern "C" const ::BunVmHandleRef* Bun__EventLoopTaskNoContext__vmHandle(const EventLoopTaskNoContext* task); } // namespace Bun diff --git a/src/jsc/bindings/JSSecrets.cpp b/src/jsc/bindings/JSSecrets.cpp index d28dcde973f9..ae61c1ea6a86 100644 --- a/src/jsc/bindings/JSSecrets.cpp +++ b/src/jsc/bindings/JSSecrets.cpp @@ -243,7 +243,7 @@ struct SecretsJobOptions { extern "C" { // Runs on the threadpool - does the actual platform API work -void Bun__SecretsJobOptions__runTask(SecretsJobOptions* opts, JSGlobalObject* global) +void Bun__SecretsJobOptions__runTask(SecretsJobOptions* opts) { // Already have CString fields, pass them directly to platform APIs switch (opts->op) { diff --git a/src/jsc/bindings/webcrypto/PhonyWorkQueue.cpp b/src/jsc/bindings/webcrypto/PhonyWorkQueue.cpp index f465658ecded..a23dbbce5b1c 100644 --- a/src/jsc/bindings/webcrypto/PhonyWorkQueue.cpp +++ b/src/jsc/bindings/webcrypto/PhonyWorkQueue.cpp @@ -11,11 +11,11 @@ Ref PhonyWorkQueue::create(WTF::ASCIILiteral name) return adoptRef(*new PhonyWorkQueue); } -extern "C" void ConcurrentCppTask__createAndRun(EventLoopTaskNoContext* task); +extern "C" void ConcurrentCppTask__createAndRun(JSC::JSGlobalObject*, EventLoopTaskNoContext* task); void PhonyWorkQueue::dispatch(JSC::JSGlobalObject* globalObject, WTF::Function&& function) { - ConcurrentCppTask__createAndRun(new EventLoopTaskNoContext(globalObject, WTF::move(function))); + ConcurrentCppTask__createAndRun(globalObject, new EventLoopTaskNoContext(WTF::move(function))); } } // namespace Bun diff --git a/src/jsc/event_loop.rs b/src/jsc/event_loop.rs index 5d981820d9fa..6437e63f9456 100644 --- a/src/jsc/event_loop.rs +++ b/src/jsc/event_loop.rs @@ -744,23 +744,19 @@ impl EventLoop { break; } // SAFETY: `node` is non-null and owned by the popped batch; the - // iterator advanced past it before returning, so reading then - // freeing here is sound. - let (task, auto_delete) = unsafe { ((*node).task, (*node).auto_delete()) }; + // iterator advanced past it before returning. + let task = + unsafe { ConcurrentTask::ConcurrentTask::into_task(NonNull::new_unchecked(node)) }; let _ = self.tasks.write_item(task); - if auto_delete { - // SAFETY: heap-owned (see `ConcurrentTask::create`); not yet - // freed, and the iterator no longer references it. - drop(unsafe { bun_core::heap::take(node) }); - } } } /// Release, without running, every task still queued — what other /// threads posted and what this thread enqueued — through each type's /// `Taskable::release_unrun`, and refuse (release on arrival) anything - /// enqueued from here on. Teardown phase B: JS thread, script forbidden, - /// JSC heap alive, HTTP thread parked / children joined. + /// enqueued from here on. Teardown phase B (JS thread, script forbidden, + /// JSC heap alive, children joined): called on every turn of the wait, and + /// once more after `Closed`. pub fn release_queued_tasks(&mut self) { self.closed_for_tasks = true; self.take_concurrent_tasks(); @@ -1012,7 +1008,7 @@ impl EventLoop { } } - /// JS thread: the poster other threads use to reach the loop this + /// JS thread: the weak poster other threads use to reach the loop this /// `EventLoop` is — the VM's handle for its embedded loops, or the isolated /// loop's own poster for a spawnSync loop. pub fn js_poster(&self) -> bun_event_loop::JsPoster { @@ -1343,6 +1339,7 @@ bun_event_loop::link_impl_JsEventLoop! { enter() => (*this).enter(), exit() => (*this).exit(), enqueue_task(task) => (*this).enqueue_task(task), + enqueue_task_after_yield(task) => (*this).enqueue_task_after_yield(task), js_poster() => (*this).js_poster(), env() => (*this).vm_ref().transpiler.env, top_level_dir() => core::ptr::from_ref::<[u8]>((*this).vm_ref().top_level_dir()), diff --git a/src/jsc/job.rs b/src/jsc/job.rs index d71a8b480ee0..6428045e1ab6 100644 --- a/src/jsc/job.rs +++ b/src/jsc/job.rs @@ -1,25 +1,21 @@ //! Work that leaves the JS thread and comes back. //! //! A [`Job`] is created on the JS thread, does its heavy part on the -//! [`WorkPool`], and completes on the JS thread again — unless its VM went away -//! meanwhile. Which thread may touch which part of it is in the types: +//! [`WorkPool`], and completes on the JS thread again. It holds a +//! [`Ticket`](crate::Ticket) for the whole trip, so its VM is guaranteed to be +//! alive throughout and its completion is always delivered: `then` runs while +//! the VM may still run script, and a completion that lands after the VM began +//! stopping is dropped instead — on the JS thread, heap alive — like every +//! other queued task the teardown releases. //! -//! * [`JobContext::OffThread`] is what the pool body sees. It is `Send`, and it -//! runs under a VM [`Borrow`] the carrier takes for it, so the VM's teardown -//! waits for a body that is mid-flight and a body never starts against a VM -//! that is already closed. JS-backed memory it needs is reachable only through -//! [`JsPtr`], i.e. only while that borrow (or a [`JsThread`]) is in hand. -//! * [`JobContext::Js`] is the completion's JS-thread state (promise, callback, -//! wrapper refs, pins, protected buffers). It is [`JsAffine`] and lives in a -//! [`JsSide`], which opens only with a [`JsThread`] token and is never dropped -//! implicitly. Every VM keeps the list of its live jobs ([`JobList`]) and -//! releases their JS sides itself at teardown, on its own thread with the -//! heap alive; a job the pool finishes after that frees only its off-thread -//! part. So there is no per-job "the VM is gone" code, and a JS side is -//! never touched off its thread. +//! The two halves are a convenience, not a safety mechanism: +//! [`JobContext::OffThread`] is what the pool body gets (`Send`); +//! [`JobContext::Js`] is the completion's JS-thread state (promise, callback, +//! wrapper refs, pins, protected buffers) and is only ever touched on the JS +//! thread. JS-backed memory the body reads in place is reachable through +//! [`JsPtr`], dereferenceable only with the job's ticket or a [`JsThread`]. //! -//! Node's equivalent is `ThreadPoolWork` + `req_wrap` (with the environment -//! cancelling/settling its reqs at cleanup); WebCore's is +//! Node's equivalent is `ThreadPoolWork` + `req_wrap`; WebCore's is //! `WorkerRunLoop::postTask` with `ActiveDOMObject`-owned completions. use core::marker::PhantomData; @@ -31,15 +27,15 @@ use bun_threading::work_pool::{Task as WorkPoolTask, WorkPool}; use crate::debugger::AsyncTaskTracker; use crate::virtual_machine::VirtualMachine; -use crate::vm_handle::{Borrow, LoopHandle}; +use crate::vm_handle::Ticket; use crate::{JSGlobalObject, JsResult}; // ── tokens ──────────────────────────────────────────────────────────────── /// Proof that the holder is on `global`'s JS thread with its heap alive: what -/// it takes to open a [`JsSide`] or dereference a [`JsPtr`] outside a pool -/// borrow. Host functions and event-loop dispatch have one by construction -/// ([`JSGlobalObject::js_thread`]). Not `Send`. +/// it takes to dereference a [`JsPtr`] outside a pool body. Host functions and +/// event-loop dispatch have one by construction ([`JSGlobalObject::js_thread`]). +/// Not `Send`. pub struct JsThread<'a> { global: &'a JSGlobalObject, _not_send: PhantomData<*mut ()>, @@ -62,7 +58,7 @@ impl JSGlobalObject { #[inline] pub fn js_thread(&self) -> JsThread<'_> { #[cfg(debug_assertions)] - self.bun_vm().handle().assert_js_thread(); + self.bun_vm().handle_ref().assert_js_thread(); JsThread { global: self, _not_send: PhantomData, @@ -70,14 +66,14 @@ impl JSGlobalObject { } } -// ── JsAffine / JsSide ───────────────────────────────────────────────────── +// ── JsAffine ────────────────────────────────────────────────────────────── /// A value that may only be used and dropped on its VM's JS thread: GC /// handles, keep-alives, wrapper back-pointers, pins, GC protection, and /// anything built from them. In a job it lives on the [`Js`](JobContext::Js) -/// side, which the carrier guarantees is opened and dropped only there. -/// Derive it (`#[derive(bun_jsc::JsAffine)]`) for aggregates; the derive -/// checks every field. +/// side, which the carrier only touches there. Derive it +/// (`#[derive(bun_jsc::JsAffine)]`) for aggregates; the derive checks every +/// field. /// /// # Safety /// Implement only for types whose every use and whose `Drop` are sound on the @@ -85,7 +81,7 @@ impl JSGlobalObject { pub unsafe trait JsAffine {} // SAFETY (each below): a GC/loop handle or plain data — used and dropped only -// on the owning JS thread by construction of `JsSide`. +// on the owning JS thread by construction of `Job`. // SAFETY: see the group note above. unsafe impl JsAffine for crate::Strong {} // SAFETY: see the group note above. @@ -142,53 +138,13 @@ impl Drop for Protected { } } -/// The JS-thread partition of a job. Opens only with a [`JsThread`] and has -/// no implicit `Drop`: its contents are released by [`take`](Self::take) on the -/// JS thread (completion, or the VM's teardown), which is what makes it sound -/// to carry across threads inside a job. -#[repr(transparent)] -pub struct JsSide(ManuallyDrop); - -// SAFETY: the contents are unreachable without a `JsThread`, which exists only -// on the owning JS thread; off that thread a `JsSide` is inert bytes. -unsafe impl Send for JsSide {} - -impl JsSide { - #[inline] - pub fn new(js: J, _: &JsThread<'_>) -> Self { - Self(ManuallyDrop::new(js)) - } - #[inline] - pub fn get(&self, _: &JsThread<'_>) -> &J { - &self.0 - } - #[inline] - pub fn get_mut(&mut self, _: &JsThread<'_>) -> &mut J { - &mut self.0 - } - /// Move the contents out to use and drop them normally (JS thread). - #[inline] - pub fn take(self, _: &JsThread<'_>) -> J { - ManuallyDrop::into_inner(self.0) - } - /// Drop the contents in place (JS thread), leaving `self` logically empty. - /// - /// # Safety - /// `self` is not opened or taken afterwards. - #[inline] - unsafe fn release_in_place(&mut self, _: &JsThread<'_>) { - // SAFETY: fn contract. - unsafe { ManuallyDrop::drop(&mut self.0) } - } -} - /// A pointer into JS-owned memory (an ArrayBuffer's bytes, a pinned cell, the /// creating global) that a job carries off-thread. It can be *passed around* -/// anywhere but dereferenced only with proof the VM is alive: a pool -/// [`Borrow`] or a [`JsThread`]. +/// anywhere but dereferenced only with proof the VM is alive: the job's +/// [`Ticket`] or a [`JsThread`]. #[repr(transparent)] pub struct JsPtr(NonNull); -// SAFETY: dereferenceable only under a Borrow/JsThread (see type doc). +// SAFETY: dereferenceable only under a Ticket/JsThread (see type doc). unsafe impl Send for JsPtr {} impl Clone for JsPtr { fn clone(&self) -> Self { @@ -212,9 +168,9 @@ impl JsPtr { /// # Safety /// No other live reference aliases the pointee for `'b`. #[inline] - #[allow(clippy::mut_from_ref)] // the `&Borrow` is a liveness witness, not the pointee - pub unsafe fn under_borrow<'b>(self, _: &'b Borrow) -> &'b mut T { - // SAFETY: borrow held ⇒ VM alive ⇒ pointee alive (type contract); aliasing per fn contract. + #[allow(clippy::mut_from_ref)] // the `&Ticket` is a liveness witness, not the pointee + pub unsafe fn under_ticket<'b>(self, _: &'b Ticket) -> &'b mut T { + // SAFETY: ticket held ⇒ VM alive ⇒ pointee alive (type contract); aliasing per fn contract. unsafe { &mut *self.0.as_ptr() } } /// # Safety @@ -229,43 +185,59 @@ impl JsPtr { // ── Job ─────────────────────────────────────────────────────────────────── -/// What a particular kind of job does. See the module doc for the partition. +/// What a particular kind of job does. pub trait JobContext: Sized + 'static { type OffThread: Send; type Js: JsAffine; - /// Pool thread, under a VM borrow the carrier holds for the whole call. - /// Return `done` to complete now; keep it (e.g. across async I/O that - /// finishes on another thread) and call [`Completion::finish`] later to - /// complete then. Work that outlives this call runs under no borrow and - /// must touch only `off`. - fn run( - off: &mut Self::OffThread, - vm: &Borrow, - done: Completion, - ) -> Option>; - - /// JS thread: the completion. Both partitions are handed over to use and - /// drop normally. + /// Whether [`cancel`](Self::cancel) does anything, i.e. whether the job + /// can wait on something external. Only such jobs are tracked by the VM. + const CANCELLABLE: bool = false; + + /// Pool thread, VM not yet in its final wait when the pool reached the job + /// (a job reached later is handed back unrun, as Node's environment + /// cleanup `uv_cancel`s queued work). Return `done` to complete now; keep + /// it (e.g. across async I/O that finishes on another thread) and call + /// [`Completion::finish`] later to complete then. `done.ticket()` is the + /// job's ticket: proof the VM is alive (for [`JsPtr::under_ticket`]), and + /// `script_allowed()` on it says whether the result still has a consumer. + /// `off` borrows the job, which the JS thread may free the moment + /// [`Completion::finish`] queues it: do not touch it after finishing (work + /// that continues past `run` reaches its state through + /// [`Completion::off_thread`]). + fn run(off: &mut Self::OffThread, done: Completion) -> Option>; + + /// JS thread, VM still running script: the completion. Both halves are + /// handed over to use and drop normally. fn then(off: Self::OffThread, js: Self::Js, cx: &JsThread<'_>) -> JsResult<()>; + + /// JS thread, the VM's stop phase (possibly more than once): make a job + /// that is waiting on something *external* — not computing — finish soon, + /// so the VM's wait for it is short. Runs concurrently with wherever the + /// job is (queued, in `run`, parked on another thread's loop): touch only + /// what that tolerates (atomics, thread-safe queues). The default, for + /// jobs that only compute, does nothing. + /// + /// # Safety + /// `off` points at the live job's off-thread half. + unsafe fn cancel(off: *mut Self::OffThread) { + let _ = off; + } } -/// The type-erased head of every [`Job`]: dispatch entries (one task tag -/// serves every `C`) and the VM's live-job links. +/// The type-erased head of every [`Job`] (one task tag serves every `C`), +/// linked into its VM's [`JobList`] while the job is live. #[repr(C)] pub struct JobHeader { complete: unsafe fn(*mut JobHeader, &JsThread<'_>) -> JsResult<()>, - release_unrun: unsafe fn(*mut JobHeader, &JsThread<'_>), - release_js: unsafe fn(*mut JobHeader, &JsThread<'_>), + release_unrun: unsafe fn(*mut JobHeader), + cancel: unsafe fn(*mut JobHeader), prev: *mut JobHeader, next: *mut JobHeader, - /// The VM already released this job's JS side (teardown); JS thread only. - js_released: bool, } -/// A VM's live jobs (intrusive through [`JobHeader`]); JS thread only, and -/// zero-valid (empty). The pool never touches the links: a job joins at -/// `schedule` and leaves at its completion / release, all on the JS thread. +/// A VM's live [cancellable](JobContext::CANCELLABLE) jobs (JS thread only; +/// zero-valid), so its stop phase can [`cancel`](JobContext::cancel) them. pub struct JobList { head: *mut JobHeader, } @@ -295,199 +267,164 @@ impl JobList { if !next.is_null() { (*next).prev = prev; } - (*job).prev = core::ptr::null_mut(); - (*job).next = core::ptr::null_mut(); } } - /// VM teardown (JS thread, heap alive, script forbidden, before the handle - /// closes): release the JS side of every live job. Whatever the pool still - /// holds afterwards frees only its off-thread part. - pub fn release_all_js(&mut self, cx: &JsThread<'_>) { - let mut job = core::mem::replace(&mut self.head, core::ptr::null_mut()); + /// The VM's stop phase (JS thread): ask every live job to finish soon. + pub fn cancel_all(&self) { + let mut job = self.head; while !job.is_null() { - // SAFETY: linked ⇒ live (jobs unlink before they are freed on this - // thread; the pool frees only after the handle closed, i.e. later). + // SAFETY: linked ⇒ live (jobs unlink, on this thread, before they + // are freed); `cancel` neither frees nor unlinks. unsafe { - let next = (*job).next; - ((*job).release_js)(job, cx); - (*job).prev = core::ptr::null_mut(); - (*job).next = core::ptr::null_mut(); - job = next; + ((*job).cancel)(job); + job = (*job).next; } } } } -/// One pool-then-complete job. Heap-allocated by [`Job::schedule`]; freed by -/// exactly one of: its completion on the JS thread, the queue's release at VM -/// teardown (JS thread, heap alive), or — once the VM is gone — the pool -/// thread, which by then has only the off-thread part left to drop. +/// One pool-then-complete job. Heap-allocated by [`Job::schedule`]; freed on +/// the JS thread by its completion or by the teardown's release. #[repr(C)] pub struct Job { + /// First (asserted at the bottom of the file): erased dispatch casts + /// `*mut Job` to `*mut JobHeader`. header: JobHeader, - loop_handle: LoopHandle, + /// Moved into the [`Completion`] when the pool picks the job up; `None` + /// from then on (never touched on the JS side). + ticket: Option, task: WorkPoolTask, - keep_alive: JsSide, + keep_alive: KeepAlive, off: C::OffThread, - js: JsSide, + js: C::Js, } impl bun_event_loop::Taskable for Job { const TAG: bun_event_loop::TaskTag = bun_event_loop::task_tag::AnyTaskJob; - /// A completion the pool posted whose `then` will not run. (Dispatch goes - /// through the erased header — [`release_unrun_erased`] — since the queue - /// only knows the shared tag; this is the same thing for a known `C`.) + /// Reached through the header (`release_unrun_erased`): the tag is shared. unsafe fn release_unrun(this: *mut Self) { - let vm = VirtualMachine::get(); // SAFETY: fn contract; JS thread with the heap alive. - unsafe { Self::release_unrun_on(this, &vm.global().js_thread()) } + drop(unsafe { Self::take(this, VirtualMachine::get()) }) } } impl Job { /// JS thread: build the job, keep the loop alive for it, hand it to the pool. + #[track_caller] pub fn schedule(cx: &JsThread<'_>, off: C::OffThread, js: C::Js) { let mut keep_alive = KeepAlive::default(); keep_alive.ref_(bun_io::js_vm_ctx()); let job = bun_core::heap::into_raw(Box::new(Self { header: JobHeader { - // SAFETY: (this and the two entries below) the erased dispatchers - // are only reached through this header, so `p` is this `Job`. + // SAFETY: (this and the entry below) the erased dispatchers are + // only reached through this header, so `p` is this `Job`. complete: |p, cx| unsafe { Self::complete(p.cast::(), cx) }, // SAFETY: as above. - release_unrun: |p, cx| unsafe { Self::release_unrun_on(p.cast::(), cx) }, - // SAFETY: as above. - release_js: |p, cx| unsafe { Self::release_js(p.cast::(), cx) }, + release_unrun: |p| unsafe { + ::release_unrun(p.cast::()) + }, + // SAFETY: linked ⇒ live; see `JobContext::cancel`. + cancel: |p| unsafe { C::cancel(&raw mut (*p.cast::()).off) }, prev: core::ptr::null_mut(), next: core::ptr::null_mut(), - js_released: false, }, - loop_handle: cx.vm().loop_handle(), + ticket: Some(cx.vm().ticket()), task: WorkPoolTask { node: Default::default(), callback: Self::run_on_pool, }, - keep_alive: JsSide::new(keep_alive, cx), + keep_alive, off, - js: JsSide::new(js, cx), + js, })); - cx.vm().jobs().push(job.cast()); - // SAFETY: live until one of the three releases; the pool owns it now. - WorkPool::schedule(unsafe { &raw mut (*job).task }); + // SAFETY: live until completed/released on this thread; the pool owns it now. + unsafe { + if C::CANCELLABLE { + cx.vm().jobs.with_mut(|j| j.push(&raw mut (*job).header)); + } + WorkPool::schedule(&raw mut (*job).task); + } } fn run_on_pool(task: *mut WorkPoolTask) { // SAFETY: only reachable through the `task.callback` slot wired in // `schedule`; the pool calls back with exactly that field of a live job. let this: *mut Self = unsafe { bun_core::from_field_ptr!(Self, task, task) }; - // SAFETY: live job, exclusively the pool's for this callback. - let handle = unsafe { (*this).loop_handle.clone() }; - let done = Completion(NonNull::new(this).expect("job")); - let Some(vm) = handle.borrow() else { - // VM already gone: nothing ran; `finish` releases. - return done.finish(); + // SAFETY: live job, exclusively the pool's for this callback; the + // ticket leaves the job here, for good. + let (off, ticket) = unsafe { (&mut (*this).off, (*this).ticket.take().expect("job")) }; + let done = Completion { + job: NonNull::new(this).expect("job"), + ticket, }; - // SAFETY: as above; the borrow keeps the VM (and any JsPtr target) alive. - if let Some(done) = C::run(unsafe { &mut (*this).off }, &vm, done) { - drop(vm); + if done.ticket().cancelled() { + return done.finish(); + } + if let Some(done) = C::run(off, done) { done.finish(); } } - /// JS thread dispatch: run the completion and free the job. + /// JS thread: reclaim a posted job and drop its keep-alive; the two halves + /// are the caller's to complete or drop. /// /// # Safety - /// `this` is the job its `Completion` posted; called once. - unsafe fn complete(this: *mut Self, cx: &JsThread<'_>) -> JsResult<()> { - // SAFETY: fn contract. - unsafe { - debug_assert!( - !(*this).header.js_released, - "job dispatched after its VM released it" - ); - cx.vm().jobs().unlink(this.cast()); - let Job { - keep_alive, - off, - js, - .. - } = *Box::from_raw(this); - keep_alive.take(cx).unref(bun_io::js_vm_ctx()); - C::then(off, js.take(cx), cx) + /// `this` is the job its `Completion` posted; called once, on `vm`'s thread. + unsafe fn take(this: *mut Self, vm: &VirtualMachine) -> (C::OffThread, C::Js) { + if C::CANCELLABLE { + // SAFETY: fn contract. + vm.jobs + .with_mut(|j| j.unlink(unsafe { &raw mut (*this).header })); } - } - - /// JS thread, VM tearing down with the heap alive: a completion that was - /// queued but will never dispatch. Everything left is dropped normally. - /// - /// # Safety - /// As [`complete`](Self::complete). - unsafe fn release_unrun_on(this: *mut Self, cx: &JsThread<'_>) { // SAFETY: fn contract. - unsafe { - if !(*this).header.js_released { - cx.vm().jobs().unlink(this.cast()); - Self::release_js(this, cx); - } - core::ptr::drop_in_place(&raw mut (*this).off); - core::ptr::drop_in_place(&raw mut (*this).loop_handle); - drop(Box::from_raw(this.cast::>())); - } + let Job { + mut keep_alive, + off, + js, + .. + } = unsafe { *Box::from_raw(this) }; + keep_alive.unref(bun_io::js_vm_ctx()); + (off, js) } - /// JS thread: drop the JS side (and keep-alive) in place; the job stays - /// allocated for whoever frees the rest. + /// JS thread dispatch: run the completion and free the job. /// /// # Safety - /// `this` is live and already unlinked; called at most once. - unsafe fn release_js(this: *mut Self, cx: &JsThread<'_>) { - // SAFETY: fn contract. - unsafe { - debug_assert!(!(*this).header.js_released); - (*this).header.js_released = true; - (*this).js.release_in_place(cx); - let mut keep_alive = core::ptr::read(&raw const (*this).keep_alive).take(cx); - keep_alive.unref(bun_io::js_vm_ctx()); - } - } -} - -impl crate::Postable for Job { - unsafe fn loop_handle(this: *mut Self) -> *const LoopHandle { + /// As [`take`](Self::take). + unsafe fn complete(this: *mut Self, cx: &JsThread<'_>) -> JsResult<()> { // SAFETY: fn contract. - unsafe { &raw const (*this).loop_handle } - } - /// VM gone, pool thread: its teardown already released the JS side and - /// keep-alive on its own thread ([`JobList::release_all_js`]); drop the - /// off-thread part and the handle and free the storage. - unsafe fn release_refused(this: *mut Self) { - // SAFETY: fn contract; refused ⇒ handle closed ⇒ teardown's release ran. - unsafe { - debug_assert!( - (*this).header.js_released, - "VM closed without releasing its jobs" - ); - core::ptr::drop_in_place(&raw mut (*this).off); - core::ptr::drop_in_place(&raw mut (*this).loop_handle); - drop(Box::from_raw(this.cast::>())); - } + let (off, js) = unsafe { Self::take(this, cx.vm()) }; + C::then(off, js, cx) } } -/// The obligation to complete a running job exactly once: returned from +/// The obligation to complete a running job exactly once — and, being what +/// the other thread holds, the holder of the job's [`Ticket`]. Returned from /// [`JobContext::run`] to complete immediately, or kept and -/// [`finish`](Self::finish)ed later from any thread. Completing delivers the -/// job to its VM (or, if that is gone, releases it). +/// [`finish`](Self::finish)ed later from any thread. #[must_use = "a job must be finished exactly once"] -pub struct Completion(NonNull>); -// SAFETY: `finish` only posts the job through its (thread-safe) LoopHandle. +pub struct Completion { + job: NonNull>, + ticket: Ticket, +} +// SAFETY: `finish` only posts the job through its (thread-safe) ticket. unsafe impl Send for Completion {} impl Completion { + /// Post the job back to its VM. The ticket outlives the post (the JS + /// thread may free the job the moment it is queued) and is dropped here. pub fn finish(self) { // Consumed: the obligation is met here, so its Drop check must not run. - let job = core::mem::ManuallyDrop::new(self).0.as_ptr(); - // SAFETY: the live heap job this token was created for; consumed once. - unsafe { crate::post_job(job) }; + let me = ManuallyDrop::new(self); + // SAFETY: moving the field out of a value that is never dropped. + let ticket = unsafe { core::ptr::read(&raw const me.ticket) }; + ticket.post(bun_event_loop::ConcurrentTask::ConcurrentTask::create_from( + me.job.as_ptr(), + )); + } + /// The job's ticket: its VM is alive while this is held. + #[inline] + pub fn ticket(&self) -> &Ticket { + &self.ticket } /// The job's off-thread part, for work that continues after `run` returned. /// @@ -495,7 +432,7 @@ impl Completion { /// No other reference to it is live (the pool callback has returned). pub unsafe fn off_thread(&self) -> *mut C::OffThread { // SAFETY: live job. - unsafe { &raw mut (*self.0.as_ptr()).off } + unsafe { &raw mut (*self.job.as_ptr()).off } } } impl Drop for Completion { @@ -517,23 +454,25 @@ pub unsafe fn complete_erased(ptr: *mut (), cx: &JsThread<'_>) -> JsResult<()> { // on `!can_call_into_js()`. if !cx.vm().script_allowed() { // SAFETY: as below; released exactly once, here. - unsafe { ((*header).release_unrun)(header, cx) }; + unsafe { ((*header).release_unrun)(header) }; return Ok(()); } // SAFETY: `Job` is `#[repr(C)]` with the header first. unsafe { ((*header).complete)(header, cx) } } -/// Teardown's release for a queued, never-dispatched `Job` completion. +/// Teardown's release for a queued, never-dispatched `Job` completion +/// (JS thread, heap alive). /// /// # Safety /// As [`complete_erased`]. -pub unsafe fn release_unrun_erased(ptr: *mut (), cx: &JsThread<'_>) { +pub unsafe fn release_unrun_erased(ptr: *mut ()) { let header = ptr.cast::(); // SAFETY: as above. - unsafe { ((*header).release_unrun)(header, cx) } + unsafe { ((*header).release_unrun)(header) } } +// The erased dispatchers above cast `*mut Job` to `*mut JobHeader`. const _: () = assert!(core::mem::offset_of!(Job, header) == 0); #[doc(hidden)] @@ -541,7 +480,7 @@ pub enum Never {} impl JobContext for Never { type OffThread = (); type Js = (); - fn run(_: &mut (), _: &Borrow, done: Completion) -> Option> { + fn run(_: &mut (), done: Completion) -> Option> { Some(done) } fn then(_: (), _: (), _: &JsThread<'_>) -> JsResult<()> { diff --git a/src/jsc/lib.rs b/src/jsc/lib.rs index 377b657f5ab7..2861d4bda9fb 100644 --- a/src/jsc/lib.rs +++ b/src/jsc/lib.rs @@ -1342,9 +1342,7 @@ pub mod virtual_machine; pub mod vm_handle; pub use self::virtual_machine as VirtualMachine; pub use self::virtual_machine::InitOptions as VirtualMachineInitOptions; -pub use self::vm_handle::{ - ConcurrentPoster, LoopHandle, LoopKind, Postable, Posted, VmHandle, post_job, -}; +pub use self::vm_handle::{ConcurrentPoster, LoopKind, Posted, Ticket, VmHandle}; #[path = "ModuleLoader.rs"] pub mod module_loader; @@ -1388,7 +1386,7 @@ pub use self::event_loop::{ JsTerminatedResult, ManagedTask, MiniEventLoop, PosixSignalHandle, PosixSignalTask, Task, WorkPool, WorkPoolTask, }; -pub use self::job::{Completion, Job, JobContext, JsPtr, JsSide, JsThread, Protected}; +pub use self::job::{Completion, Job, JobContext, JsPtr, JsThread, Protected}; #[cfg(unix)] pub type PlatformEventLoop = bun_uws::Loop; #[cfg(not(unix))] diff --git a/src/jsc/node_path.rs b/src/jsc/node_path.rs index 8603ae36b8bb..cae9c21ddb74 100644 --- a/src/jsc/node_path.rs +++ b/src/jsc/node_path.rs @@ -54,9 +54,9 @@ impl ThreadSafe { } // SAFETY: this is what the type asserts — the JS-backed views inside `T` are -// GC-protected for as long as it is held, so they may be read from another -// thread (under that thread's VM borrow); the protection itself is released -// only on a JS thread (see `Drop`). +// GC-protected for as long as it is held, so a pool job may read them (under +// its `Ticket`, which keeps the VM alive); the job comes back to the JS +// thread, where this is dropped and the protection released. unsafe impl Send for ThreadSafe {} impl core::ops::Deref for ThreadSafe { @@ -77,8 +77,10 @@ impl core::ops::DerefMut for ThreadSafe { impl Drop for ThreadSafe { #[inline] fn drop(&mut self) { - // The protection is engine state: released here on a JS thread, gone - // with the heap anywhere else (a pool thread releasing a dead VM's job). + // The same argument types serve mini-loop threads (the shell's `cp` via + // `ShellAsyncCpTask`, `bun exec`), which have no VM and never protected + // anything; only a JS thread has a protection to release. A JS VM's job + // always comes back to its own thread to drop this (its VM waits for it). if crate::virtual_machine::VirtualMachine::get_or_null().is_some() { self.0.unprotect(); } diff --git a/src/jsc/web_worker.rs b/src/jsc/web_worker.rs index ce6fca61493a..cf2c637b0e53 100644 --- a/src/jsc/web_worker.rs +++ b/src/jsc/web_worker.rs @@ -14,7 +14,7 @@ //! touches the parent's `Worker` object or a thread-affine ref. //! //! Thread lifecycle (`thread_main`): -//! 1. `start_vm()` — arena, cloned env, `VirtualMachine`, publish `vm` under `vm_lock`. +//! 1. `start_vm()` — arena, env snapshot, `VirtualMachine`, publish its handle (`vm_handle`). //! 2. `spin()` — load the entry point, `workerGlobalScopeStarted`, run the //! event loop until it drains or termination is requested, //! `beforeExit` on a natural drain. @@ -29,9 +29,13 @@ //! performs the parent-side release itself (`parentContextWillDestroy`). This //! is Node's `stop_sub_worker_contexts()`; there is no process-global list. //! -//! `vm_lock` closes the TOCTOU between another thread reading a non-null `vm` -//! (to raise a TerminationException / wake the loop) and this thread freeing -//! it: held while publishing, while unpublishing, and around that read. +//! Threads: everything the worker thread needs from its parent VM (transform +//! options, an env snapshot, the standalone graph) is copied on the parent +//! thread in `create()`, and the thread holds a `Ticket` on the parent for its +//! whole life, so the parent cannot be destroyed under it. The parent (or an +//! exiting ancestor) reaches the worker's VM only through `vm_handle` — the +//! worker VM's uncounted handle, published once the VM exists — never through +//! a pointer to it. use crate::JsCell; use core::cell::Cell; @@ -42,7 +46,6 @@ use std::thread::JoinHandle; use bun_core::{String as BunString, WTFStringImpl}; use bun_io::KeepAlive; -use bun_threading::Mutex; use crate::virtual_machine::{self, VirtualMachine, runtime_hooks}; use crate::{self as jsc, JSGlobalObject, JSValue, JsError, LogJsc}; @@ -55,11 +58,17 @@ pub struct WebWorker { /// The C++ `WorkerMessagingProxy`; the thread holds a ref on it, so it is /// valid for as long as this thread runs. Opaque here. messaging_proxy: *mut c_void, - /// The `VirtualMachine` of the thread that created this worker. Read on the - /// worker thread by `start_vm()` (transform options, env, standalone graph) - /// and on the parent thread for `parent_poll_ref` / `child_workers`. Valid - /// because a parent joins its children before its own VM is destroyed. - parent: bun_ptr::BackRef, + /// The `VirtualMachine` of the thread that created this worker. + /// **Parent thread only** (`child_workers`, `parent_poll_ref`); the worker + /// thread never dereferences it — what it needs was copied below. + parent: *mut VirtualMachine, + /// The parent's `--hot` / `--watch` mode, inherited by the worker VM. + hot_reload: crate::virtual_machine::HotReload, + /// Whether the worker VM arms `bun_jsc::vm_handle`'s test gate (debug + /// builds, `BUN_DEBUG_TEST_WORKER_TEARDOWN_GATE`, first-level workers only: + /// a nested worker parked on a post to its worker parent would keep that + /// parent from ever reaching its own wait). + arm_test_gate: bool, execution_context_id: u32, mini: bool, eval_mode: bool, @@ -80,10 +89,10 @@ pub struct WebWorker { /// the worker itself (`process.exit()`); polled by the worker loop between /// ticks and turned into a JSC TerminationException for running script. requested_terminate: AtomicBool, - /// The worker's `VirtualMachine`, null before `start_vm()` publishes it and - /// after `shutdown()` unpublishes it. Cross-thread readers hold `vm_lock`. - vm: Cell<*mut VirtualMachine>, - vm_lock: Mutex, + /// The worker VM's uncounted handle: how the parent (or an exiting + /// ancestor) asks it to terminate. `None` before `start_vm()` publishes it + /// and after `shutdown()` unpublishes it. + vm_handle: bun_threading::Guarded>, // ---- Parent-thread only --------------------------------------------------- /// Keep-alive on the parent's event loop: taken in `create()`, toggled by @@ -95,6 +104,9 @@ pub struct WebWorker { // ---- Worker-thread only ----------------------------------------------------- // Mutated only on the worker thread, but through `&self` because other // threads hold `&WebWorker` concurrently; hence the cells. + /// The worker's `VirtualMachine`; null before `start_vm()` and after + /// `shutdown()`. + vm: Cell<*mut VirtualMachine>, status: Cell, // The VM's allocator IS this arena. arena: JsCell>, @@ -105,10 +117,18 @@ pub struct WebWorker { exit_called: AtomicBool, /// The parent asked this thread to stop (`worker.terminate()` or an exiting /// parent) while its VM was live — as opposed to the thread stopping itself, - /// or being stopped before it started. Written under `vm_lock`. + /// or being stopped before it started. Written under the `vm_handle` lock. terminated_by_parent: AtomicBool, } +/// Copied from the parent VM on its thread at `new Worker()`; consumed by +/// `start_vm()` on the worker thread. +struct WorkerVmInit { + transform_options: bun_options_types::schema::api::TransformOptions, + env_map: bun_dotenv::Map, + proxy_env_slots: jsc::rare_data::ProxyEnvSlots, +} + enum EntryOutcome { Continue, /// The entry module rejected and no handler took it: the worker exits. @@ -195,12 +215,7 @@ impl WebWorker { self.requested_terminate.load(Ordering::Acquire) } - /// Raw read of the `vm` cell. Worker-thread-only callers (which are also - /// the writers) may call this without `vm_lock`; cross-thread callers - /// (`request_termination`, an exiting ancestor) must hold - /// `vm_lock`. The cell itself is `Cell<*mut _>` so the read is a safe - /// `Copy` load; synchronization (where required) is the caller's - /// responsibility per the doc above. + /// Worker thread only. #[inline] fn vm_ptr(&self) -> *mut VirtualMachine { self.vm.get() @@ -325,13 +340,59 @@ impl WebWorker { } } - // SAFETY: `parent` is live (see above); borrow ends at `;`. - let store_fd = unsafe { (*parent).transpiler.resolver.store_fd }; + // Everything the worker thread needs from this VM is copied here, on + // its own thread; the worker never dereferences `parent`. + // SAFETY: `parent` is the calling thread's live VM. + let parent_ref = unsafe { &*parent }; + let store_fd = parent_ref.transpiler.resolver.store_fd; + let mut transform_options = (*parent_ref.transpiler.options.transform_options).clone(); + if !inherit_exec_argv { + let hooks = runtime_hooks().expect("RuntimeHooks not installed"); + // SAFETY: caller passed valid (ptr,len) borrowed from C++ WorkerOptions; + // the hook only reads the slice. Only honours `--no-addons` today; + // `None` on parse failure keeps the parent's setting. + let parsed = unsafe { + (hooks.parse_worker_exec_argv_allow_addons)(bun_core::ffi::slice( + exec_argv_ptr, + exec_argv_len, + )) + }; + if let Some(allow) = parsed { + let parent_allows = transform_options.allow_addons.unwrap_or(true); + transform_options.allow_addons = Some(parent_allows && allow); + } + } + // The worker's `process.env` starts as a copy of the parent's now (as in + // Node). Proxy-env values may be RefCountedEnvValue bytes owned by the + // parent's proxy_env_storage: snapshot slots + map under its lock so + // every slice copied is backed by a ref the snapshot holds. + let mut proxy_env_slots = jsc::rare_data::ProxyEnvSlots::default(); + let mut env_map = { + let parent_slots = parent_ref.proxy_env_storage.lock(); + proxy_env_slots.clone_from(&parent_slots); + match parent_ref.env_loader().map.clone_with_allocator() { + Ok(m) => m, + Err(_) => { + *error_message = BunString::static_(b"Out of memory"); + return core::ptr::null_mut(); + } + } + }; + proxy_env_slots.sync_into(&mut env_map); + let init = WorkerVmInit { + transform_options, + env_map, + proxy_env_slots, + }; let worker = bun_core::heap::into_raw(Box::new(WebWorker { messaging_proxy: proxy, - // `parent` is the calling thread's live VM; non-null by FFI contract. - parent: bun_ptr::BackRef::from(NonNull::new(parent).expect("parent VM")), + parent, + hot_reload: parent_ref.hot_reload, + arm_test_gate: cfg!(debug_assertions) + && parent_ref.is_main_thread() + && bun_core::env_var::feature_flag::BUN_DEBUG_TEST_WORKER_TEARDOWN_GATE::get() + .unwrap_or(false), execution_context_id: this_context_id, mini, eval_mode, @@ -350,8 +411,8 @@ impl WebWorker { }, ref_count: bun_ptr::ThreadSafeRefCount::init(), requested_terminate: AtomicBool::new(false), + vm_handle: bun_threading::Guarded::new(None), vm: Cell::new(core::ptr::null_mut()), - vm_lock: Mutex::new(), parent_poll_ref: JsCell::new(KeepAlive::init()), join_handle: JsCell::new(None), status: Cell::new(Status::Start), @@ -375,18 +436,37 @@ impl WebWorker { // The thread's own ref, taken before it exists so it can never observe zero. worker_ref.ref_(); - struct SendPtr(*mut WebWorker); - // SAFETY: heap-allocated, refcounted; the new thread holds the ref taken above. - unsafe impl Send for SendPtr {} - let send = SendPtr(worker); + // The thread is something of this VM's on another thread for as long as + // it runs: the parent joins it before its own teardown's wait, which + // this ticket would otherwise hold. + let parent_ticket = parent_ref.ticket(); + /// What the worker thread is handed: its refcounted `WebWorker` (the ref + /// taken above is the thread's), the parent's snapshot, and a ticket on + /// the parent VM. + struct ThreadStart { + worker: *mut WebWorker, + init: WorkerVmInit, + _parent_ticket: crate::Ticket, + } + // SAFETY: `WebWorker` is shared across threads by design (atomics, + // `Guarded`, thread-confined cells — see the struct doc) and holds no + // parent-VM state; `init` is an owned copy — byte buffers, scalars and + // `Arc`s, no JSC or atom strings; the parent VM + // itself is kept by `_parent_ticket`. + unsafe impl Send for ThreadStart {} + let start = ThreadStart { + worker, + init, + _parent_ticket: parent_ticket, + }; let spawn = std::thread::Builder::new() .stack_size(bun_threading::thread_pool::DEFAULT_THREAD_STACK_SIZE as usize) .spawn(move || { - let send = send; - // SAFETY: `send.0` is live (the thread's ref); `&WebWorker`, never `&mut`. - unsafe { (*send.0).thread_main() }; - // SAFETY: dropping the thread's ref; nothing below touches `send.0`. - unsafe { WebWorker::deref(send.0) }; + let start = start; + // SAFETY: `worker` is live (the thread's ref); `&WebWorker`, never `&mut`. + unsafe { (*start.worker).thread_main(start.init) }; + // SAFETY: dropping the thread's ref; nothing below touches `worker`. + unsafe { WebWorker::deref(start.worker) }; }); match spawn { Ok(handle) => { @@ -463,38 +543,25 @@ impl WebWorker { #[unsafe(export_name = "WebWorker__requestTermination")] pub(crate) extern "C" fn request_termination(this: *mut WebWorker) { let this = bun_ptr::ParentRef::from(NonNull::new(this).expect("WebWorker FFI ptr")); - // vm_lock serialises against shutdown() nulling `vm` and freeing the - // arena it lives in — and is taken *before* the flag is published: a - // worker that breaks out of its loop because it saw the flag then blocks - // in shutdown() until `terminated_by_parent` and the gate are set here, - // instead of racing past with neither. - this.vm_lock.lock(); + // The handle's lock is taken *before* the flag is published: a worker + // that breaks out of its loop because it saw the flag then blocks in + // shutdown() (unpublish) until `terminated_by_parent` and the stop are + // set here, instead of racing past with neither. + let handle = this.vm_handle.lock(); if this.set_requested_terminate() { - this.vm_lock.unlock(); return; } log!("[{}] requestTermination", this.execution_context_id); - // vm_lock held; `vm` is published/unpublished under vm_lock. - let vm_ptr = this.vm_ptr(); - if !vm_ptr.is_null() { + if let Some(handle) = &*handle { // Node: being stopped only counts (exit code 1) once the environment // exists and before the thread starts tearing it down on its own. this.terminated_by_parent.store(true, Ordering::Relaxed); // From now on the worker's native code enters no script and settles // no promises (Node's `ExitEnv` → `is_stopping`), even before its - // thread notices: whatever completes on its loop meanwhile bails. - // SAFETY: vm_ptr published under vm_lock; the handle is any-thread. - unsafe { (*vm_ptr).handle().stop() }; - // SAFETY: vm_ptr published under vm_lock and non-null here. - // jsc_vm is a valid JSC::VM*; notify_need_termination is - // documented thread-safe (VMTraps). Cast through the real opaque - // `crate::VM` (the `crate::VM` stub is layout-only). No - // `&VirtualMachine` binding (raw field reads only, off-thread). - unsafe { (*(*vm_ptr).jsc_vm.cast_const()).notify_need_termination() }; - // SAFETY: event_loop() returns the live `*mut EventLoop` self-ptr. - unsafe { (*(*vm_ptr).event_loop()).wakeup() }; + // thread notices; a TerminationException is raised at its next + // safepoint and its loop woken. + handle.request_termination(); } - this.vm_lock.unlock(); } /// The parent is releasing this thread: drop the keep-alive on the parent's @@ -504,15 +571,20 @@ impl WebWorker { let this_ref = bun_ptr::ParentRef::from(NonNull::new(this).expect("WebWorker FFI ptr")); this_ref.with_parent_poll_ref(|p| p.unref(bun_io::js_vm_ctx())); // SAFETY: parent thread; `parent` outlives its children (it joins them). - let children = unsafe { &mut (*NonNull::from(this_ref.parent).as_ptr()).child_workers }; + let children = unsafe { &mut (*this_ref.parent).child_workers }; if let Some(i) = children.iter().position(|&c| core::ptr::eq(c, this)) { children.swap_remove(i); } } #[inline] - pub(crate) fn parent_vm(&self) -> bun_ptr::BackRef { - self.parent + pub(crate) fn hot_reload(&self) -> crate::virtual_machine::HotReload { + self.hot_reload + } + + #[inline] + pub(crate) fn arm_test_gate(&self) -> bool { + self.arm_test_gate } #[inline] @@ -541,7 +613,7 @@ impl WebWorker { // an exiting ancestor), so materialising `&mut WebWorker` here would // be aliased-&mut UB. Worker-thread-only mutable fields are wrapped in // `Cell` / `UnsafeCell` instead. - fn thread_main(&self) { + fn thread_main(&self, init: WorkerVmInit) { bun_analytics::features::workers_spawned.fetch_add(1, Ordering::Relaxed); if !self.name.is_empty() { @@ -559,7 +631,7 @@ impl WebWorker { return; } - let vm_ptr = match self.start_vm() { + let vm_ptr = match self.start_vm(init) { Ok(vm) => vm, Err(err) => { bun_core::output::panic(format_args!( @@ -574,9 +646,9 @@ impl WebWorker { return; } - // `start_vm()` published `vm_ptr` under `vm_lock` AND installed it as - // this thread's per-thread VM (`VirtualMachine::init` → `VMHolder`), so - // the safe thread-local accessor returns the same allocation. + // `start_vm()` installed `vm_ptr` as this thread's per-thread VM + // (`VirtualMachine::init` → `VMHolder`), so the safe thread-local + // accessor returns the same allocation. debug_assert!(core::ptr::eq(vm_ptr, VirtualMachine::get_mut_ptr())); let global = VirtualMachine::get().global(); // Take the API lock for the thread's whole life and abandon it with the @@ -590,73 +662,25 @@ impl WebWorker { /// /// Returns the published VM pointer; `Ok(null)` means the early-terminate /// checkpoint already ran `shutdown()`. - fn start_vm(&self) -> Result<*mut VirtualMachine, crate::CrateError> { + fn start_vm(&self, init: WorkerVmInit) -> Result<*mut VirtualMachine, crate::CrateError> { debug_assert!(self.status.get() == Status::Start); debug_assert!(self.vm_ptr().is_null()); let hooks = runtime_hooks().expect("RuntimeHooks not installed"); - - // `parent` is a `BackRef` and outlives this worker while - // `parent_poll_ref` is held (see file header). The parent VM runs - // concurrently on its own thread, so we must NOT materialise a - // `&mut VirtualMachine` here — a - // `&mut` would assert uniqueness we don't have. All uses - // below are read-only (clone of transform_options, locked read of - // proxy_env_storage / env.map, copy of standalone_module_graph), - // so a shared reference is sufficient. - let parent = self.parent.get(); - // Deref-clone out of the `Arc` — worker mutates `allow_addons` below - // and passes the owned struct as `args` to the new VM. - let mut transform_options = (*parent.transpiler.options.transform_options).clone(); - - if let Some(exec_argv) = self.exec_argv() { - // Parse `execArgv` with the - // RunCommand param table. The param table lives in - // `bun_runtime::cli` (forward-dep), so dispatch through - // `RuntimeHooks::parse_worker_exec_argv_allow_addons`. Currently - // only honours `--no-addons`; the hook owns the temporary UTF-8 - // alloc + clap parse + `args.deinit()`. `None` on parse failure - // (the parent's setting is kept). - - // SAFETY: `exec_argv` borrows C++ `WorkerOptions` kept alive by the - // owning `WebCore::Worker` for `self`'s lifetime; the hook only - // reads the slice and owns its own temporary allocations. - let parsed = unsafe { (hooks.parse_worker_exec_argv_allow_addons)(exec_argv) }; - if let Some(allow_addons) = parsed { - let parent_allows = transform_options.allow_addons.unwrap_or(true); - transform_options.allow_addons = Some(parent_allows && allow_addons); - } - } + let WorkerVmInit { + transform_options, + env_map, + proxy_env_slots, + } = init; // worker-thread only field; no other thread reads `arena`. self.arena.set(Some(bun_alloc::Arena::new())); - // Proxy-env values may be RefCountedEnvValue bytes owned by the - // parent's proxy_env_storage. We need a consistent snapshot of - // (storage slots + env.map entries) so every slice we copy is backed - // by a ref we hold. The parent's storage.lock serialises against - // Bun__setEnvValue on the main thread — it covers both the slot swap - // and the map.put, so cloneFrom and cloneWithAllocator see the same - // state. - let mut temp_proxy_slots = jsc::rare_data::ProxyEnvSlots::default(); - - // Box the Loader on the global heap and hand ownership to the VM via - // `transpiler.env`; reclaimed in `vm.destroy()` in `shutdown()`. - let mut map = { - let parent_slots = parent.proxy_env_storage.lock(); - temp_proxy_slots.clone_from(&parent_slots); - // SAFETY: `parent.transpiler.env` is the parent-owned `DotEnv::Loader` - // set in `Transpiler::init`; valid while `parent` lives. Read-only. - parent.env_loader().map.clone_with_allocator()? - }; - // Ensure map entries point at the exact bytes we hold refs on. - temp_proxy_slots.sync_into(&mut map); - // `heap::alloc`'d and stashed on `self` so `shutdown()` step 5 reclaims // it on every path — including the early-terminate checkpoint below, // which calls `shutdown()` before the VM exists. let loader_ptr: *mut bun_dotenv::Loader = - bun_core::heap::into_raw(Box::new(bun_dotenv::Loader::init_with_map(map))); + bun_core::heap::into_raw(Box::new(bun_dotenv::Loader::init_with_map(env_map))); self.worker_env_loader.set(loader_ptr); // Checkpoint before the expensive part: initWorker builds a full JSC @@ -664,7 +688,6 @@ impl WebWorker { // above, bail now rather than spending ~50–100ms (release) creating a // VM that will immediately tear down. if self.has_requested_terminate() { - drop(temp_proxy_slots); self.shutdown(); return Ok(core::ptr::null_mut()); } @@ -675,16 +698,12 @@ impl WebWorker { args: transform_options, env_loader: NonNull::new(loader_ptr), store_fd: self.store_fd, - graph: parent.standalone_module_graph, + graph: crate::virtual_machine::standalone_module_graph(), ..Default::default() }, )?; - // Pre-publish init: the VM is not yet visible to the parent thread, - // so a scoped `&mut VirtualMachine` is safe here. The borrow MUST - // end before the publish below — once `self.vm` is published under - // `vm_lock`, `request_termination` - // may concurrently dereference the same pointer on another thread, - // and a still-live `&mut VirtualMachine` would be aliased-&mut UB. + // Scoped `&mut VirtualMachine` for the worker-specific fields; ends + // before anything else on this thread re-derives access to the VM. { // SAFETY: init_worker returns a valid heap-allocated VM ptr; // not yet published, so this `&mut` is exclusive. @@ -696,15 +715,14 @@ impl WebWorker { .arena .with_mut(|a| NonNull::new(std::ptr::from_mut(a.as_mut().unwrap()))); - // Move the pre-cloned proxy storage into the worker VM. - *vm_ref.proxy_env_storage.lock() = core::mem::take(&mut temp_proxy_slots); + *vm_ref.proxy_env_storage.lock() = proxy_env_slots; vm_ref.is_main_thread = false; VirtualMachine::set_is_main_thread_vm(false); vm_ref.on_unhandled_rejection = on_unhandled_rejection; } - // Publish `vm` now (rather than at the end of startVM) so that: + // Publish now (rather than at the end of startVM) so that: // - a concurrent request_termination() (parent, or an exiting ancestor) can // wake us once JS starts running, and // - early returns below reach spin()/shutdown() with this.vm set, @@ -714,15 +732,10 @@ impl WebWorker { // non-null vm runs vm.onExit() (JS), which requires holdAPILock. // Instead we return; threadMain enters holdAPILock(spin) and spin()'s // first check observes requested_terminate. - self.vm_lock.lock(); - // vm_lock held; this is the publish point. self.vm.set(vm); - self.vm_lock.unlock(); + // SAFETY: `vm` is the live VM just built on this thread. + *self.vm_handle.lock() = Some(unsafe { (*vm).handle() }); - // Post-publish: do NOT re-form `&mut VirtualMachine`. Field/method - // access goes through the raw `*mut` so any autoref is scoped to the - // single expression. The parent-thread readers likewise never bind - // `&VirtualMachine` (see `request_termination`). // SAFETY: `vm` is a valid heap-allocated VM ptr (checked above). unsafe { let b = &mut (*vm).transpiler; @@ -730,7 +743,7 @@ impl WebWorker { b.options.env.behavior = bun_options_types::schema::api::DotEnvBehavior::LoadAllWithoutInlining; - if let Some(graph) = parent.standalone_module_graph { + if let Some(graph) = crate::virtual_machine::standalone_module_graph() { (hooks.apply_standalone_runtime_flags)(b, graph); } } @@ -743,7 +756,7 @@ impl WebWorker { return Ok(vm); } - // SAFETY: see post-publish note above. + // SAFETY: this thread's live VM; per-expression derefs, no long-lived `&mut`. unsafe { if (*vm).transpiler.configure_defines().is_err() { // Fall through to spin() → shutdown() for full teardown under @@ -767,20 +780,13 @@ impl WebWorker { fn spin(&self) { log!("[{}] spin start", self.execution_context_id); - // vm published in start_vm; non-null past this point. Do NOT bind a - // long-lived `&mut VirtualMachine`: while the event loop runs, the - // parent / main thread may dereference the same pointer under - // `vm_lock` (`request_termination`, an exiting ancestor). - // Those cross-thread paths only form raw-ptr field reads (never - // `&mut VirtualMachine`), so holding `&VirtualMachine` here is sound; - // mutation goes through `vm.as_mut()` which forms a fresh short-lived - // `&mut` per call (the `JsCell` escape hatch — provenance from the - // thread-local `*mut`). + // vm set in start_vm; non-null past this point. Mutation goes through + // `vm.as_mut()` which forms a fresh short-lived `&mut` per call (the + // `JsCell` escape hatch — provenance from the thread-local `*mut`). let vm_ptr: *mut VirtualMachine = self.vm_ptr(); - // vm published in `start_vm` under `vm_lock`; non-null and live for the - // worker thread's duration. This IS the worker thread's per-thread VM - // (set by `VirtualMachine::init` → `VMHolder`), so the safe - // thread-local accessor returns the same allocation. + // This IS the worker thread's per-thread VM (set by + // `VirtualMachine::init` → `VMHolder`), so the safe thread-local + // accessor returns the same allocation. debug_assert!(core::ptr::eq(vm_ptr, VirtualMachine::get_mut_ptr())); let vm: &VirtualMachine = VirtualMachine::get(); debug_assert!(self.status.get() == Status::Start); @@ -804,9 +810,7 @@ impl WebWorker { // here would never run anyway. let mut resolve_error = BunString::empty(); let vm_log = vm.log_mut().unwrap(); - // SAFETY: `vm_ptr` is the live worker-thread VM; the fn takes a raw ptr - // (no `&mut`) because `vm` is already published under `vm_lock` — see - // `resolve_entry_point_specifier` Safety contract. + // SAFETY: `vm_ptr` is the live worker-thread VM. let path = match unsafe { resolve_entry_point_specifier( vm_ptr, @@ -975,8 +979,8 @@ impl WebWorker { self.shutdown(); } - /// Phase 3: unpublish `vm` under `vm_lock` (a racing `requestTermination` - /// now sees null), run the user 'exit' handlers, then the shared + /// Phase 3: unpublish the VM's handle (a racing `requestTermination` now + /// finds none), run the user 'exit' handlers, then the shared /// [`VirtualMachine::teardown`] (stop → forbid script → ~VM → loops → /// destroy), free the thread's remaining state, and last of all report /// `workerGlobalScopeDestroyed` — the parent joins this thread from that @@ -992,16 +996,14 @@ impl WebWorker { let env_loader = self.worker_env_loader.replace(core::ptr::null_mut()); // ---- 1. Unpublish vm ------------------------------------------------ - self.vm_lock.lock(); - // vm_lock held; this is the unpublish point. + drop(self.vm_handle.lock().take()); let vm_ptr = self.vm.replace(core::ptr::null_mut()); - self.vm_lock.unlock(); // ---- 2. User exit handlers ----------------------------------------- let mut exit_code: i32 = 0; 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; no other thread holds a pointer to it (they + // only ever held its handle) — `&mut` is exclusive. let vm = unsafe { &mut *vm_ptr }; vm.is_shutting_down = true; vm.on_exit(); @@ -1011,8 +1013,8 @@ impl WebWorker { self.execution_context_id ); - // ---- 3–5. Stop, forbid script, ~VM, loops, destroy --------------- - // SAFETY: unpublished under `vm_lock`; this thread is the sole owner. + // ---- 3–5. Stop, forbid script, wait, ~VM, loops, destroy ---------- + // SAFETY: this thread's VM; sole owner. unsafe { VirtualMachine::teardown(vm_ptr, crate::virtual_machine::Teardown::Worker) }; // `destroy()` deinits the fields; reclaim the storage `init` put on @@ -1089,18 +1091,14 @@ impl WebWorker { // Stop subsequent JS at the next safepoint. `this.vm` is null during // `vm.onExit()` (shutdown nulls it first), so a re-entrant // process.exit() from an exit handler does not re-arm the trap. - // worker-thread only; `vm` is read here on the same thread - // that publishes/unpublishes it, so no lock is needed for the load. let vm_ptr = self.vm_ptr(); if !vm_ptr.is_null() { - // SAFETY: vm_ptr non-null; jsc_vm is a valid JSC::VM*; - // notify_need_termination is documented thread-safe (VMTraps). - // Cast through the real opaque `crate::VM`. + // SAFETY: this thread's live VM. unsafe { // As for a parent's terminate(): nothing more may enter script // (Node's `Stop(env)` on the worker's own exit). - (*vm_ptr).handle().stop(); - (*(*vm_ptr).jsc_vm.cast_const()).notify_need_termination(); + (*vm_ptr).handle_ref().stop(); + (*vm_ptr).jsc_vm().notify_need_termination(); } } } @@ -1280,7 +1278,7 @@ fn on_unhandled_rejection( // (VMTraps). The gate closes with it, as for exit()/terminate(): the // native→JS entries still reached this tick refuse rather than run into // the pending termination. - vm.handle().stop(); + vm.handle_ref().stop(); vm.jsc_vm().notify_need_termination(); } @@ -1290,14 +1288,10 @@ fn on_unhandled_rejection( /// free it. /// /// # Safety -/// `parent` must point at a live `VirtualMachine`. Passed as a raw pointer -/// (not `&mut`) because when called from `spin()` the WORKER's VM has already -/// been published under `vm_lock`; the parent / main thread may concurrently -/// dereference the same allocation in `request_termination` -/// (`(*vm_ptr).jsc_vm`, `(*vm_ptr).event_loop()`). -/// A live `&mut VirtualMachine` here would be aliased-&mut UB. Per-use -/// `(*parent)` derefs keep any autoref scoped to the single expression — the -/// same pattern `spin()` uses post-publish. +/// `parent` must point at this thread's live `VirtualMachine`. Passed as a raw +/// pointer (not `&mut`) because callers hold other borrows into the VM (its +/// log) across the call; per-use `(*parent)` derefs keep any autoref scoped to +/// the single expression. unsafe fn resolve_entry_point_specifier<'s>( parent: *mut VirtualMachine, str: &'s [u8], @@ -1400,8 +1394,7 @@ unsafe fn resolve_entry_point_specifier<'s>( // SAFETY: per fn contract; `global` is a read-only field, and the resolver // (`transpiler`) is mutated only on `parent`'s owning thread — both call // sites (`create()` on the parent thread, `spin()` on the worker thread) - // satisfy that. The cross-thread readers under `vm_lock` never touch - // `transpiler`. + // satisfy that. let global = unsafe { (*parent).global }; // SAFETY: same as above — `parent`'s `transpiler` is mutated only on its // owning thread (the caller's thread per fn contract). diff --git a/src/jsc_macros/lib.rs b/src/jsc_macros/lib.rs index 291df899082a..18a20ef9c62f 100644 --- a/src/jsc_macros/lib.rs +++ b/src/jsc_macros/lib.rs @@ -987,8 +987,8 @@ fn classify_uws_arg(ty: &syn::Type) -> UwsArg { UwsArg::PassThrough(ty.clone()) } -/// `#[derive(JsAffine)]` — the struct/enum may live in a job's JS-side -/// partition (`bun_jsc::job::JsSide`): every field must itself be +/// `#[derive(JsAffine)]` — the struct/enum may be (part of) a job's `Js` +/// half (`bun_jsc::JobContext::Js`): every field must itself be /// `JsAffine`, which the expansion checks with one bound per field type, so /// a field that owns process memory (a `Vec`, a `Box`, a C library handle) is /// a compile error here rather than a leak-or-UAF decision at teardown. diff --git a/src/runtime/api/Archive.rs b/src/runtime/api/Archive.rs index 33922cf458e1..e54e329c6914 100644 --- a/src/runtime/api/Archive.rs +++ b/src/runtime/api/Archive.rs @@ -687,11 +687,7 @@ pub struct AsyncTask(core::marker::PhantomData); impl bun_jsc::JobContext for AsyncTask { type OffThread = C; type Js = JSPromiseStrong; - fn run( - ctx: &mut C, - _vm: &bun_jsc::vm_handle::Borrow, - done: bun_jsc::Completion, - ) -> Option> { + fn run(ctx: &mut C, done: bun_jsc::Completion) -> Option> { ctx.run(); Some(done) } diff --git a/src/runtime/api/BunObject.rs b/src/runtime/api/BunObject.rs index fdedb2be588f..31ef65b461bb 100644 --- a/src/runtime/api/BunObject.rs +++ b/src/runtime/api/BunObject.rs @@ -2858,7 +2858,6 @@ pub mod JSZstd { fn run( this: &mut Self, - _vm: &jsc::vm_handle::Borrow, done: bun_jsc::Completion, ) -> Option> { let input = this.buffer.slice(); diff --git a/src/runtime/api/JSTranspiler.rs b/src/runtime/api/JSTranspiler.rs index 27b16aa07e26..d99b774bd830 100644 --- a/src/runtime/api/JSTranspiler.rs +++ b/src/runtime/api/JSTranspiler.rs @@ -677,12 +677,8 @@ pub(crate) struct TransformJs { impl jsc::JobContext for TransformTask { type OffThread = Self; type Js = TransformJs; - fn run( - this: &mut Self, - vm: &jsc::vm_handle::Borrow, - done: bun_jsc::Completion, - ) -> Option> { - TransformTask::run(this, vm); + fn run(this: &mut Self, done: bun_jsc::Completion) -> Option> { + TransformTask::run(this, done.ticket()); Some(done) } fn then(mut this: Self, mut js: TransformJs, cx: &jsc::JsThread<'_>) -> JsResult<()> { @@ -748,13 +744,13 @@ impl TransformTask { value } - fn run(&mut self, vm: &jsc::vm_handle::Borrow) { + fn run(&mut self, vm: &jsc::Ticket) { let name = self.loader.stdin_name(); let resolver_ptr: *mut _ = &raw mut self.transpiler.resolver; self.transpiler.linker.resolver = resolver_ptr; - // SAFETY: the wrapper's config, alive under the borrow (see `schedule`). + // SAFETY: the wrapper's config, alive under the job's ticket (see `schedule`). let tsconfig: Option<&TSConfigJSON> = - self.tsconfig.map(|p| &*unsafe { p.under_borrow(vm) }); + self.tsconfig.map(|p| &*unsafe { p.under_ticket(vm) }); let arena = Arena::new(); diff --git a/src/runtime/api/cron.rs b/src/runtime/api/cron.rs index b492f929e32a..ea2e7e3630ef 100644 --- a/src/runtime/api/cron.rs +++ b/src/runtime/api/cron.rs @@ -21,7 +21,7 @@ use std::cell::Cell; use bun_core::env_var; use bun_io::BufferedReader as OutputReader; use bun_io::{KeepAlive, Loop as AsyncLoop}; -use bun_jsc::virtual_machine::{HOT_RELOAD_HOT, VirtualMachine}; +use bun_jsc::virtual_machine::{HotReload, VirtualMachine}; use bun_jsc::{ self as jsc, CallFrame, EventLoopHandle, GlobalRef, JSFunction, JSGlobalObject, JSObject, JSValue, JsCell, JsRef, JsResult, @@ -1987,7 +1987,7 @@ impl CronJob { // The cron_jobs list exists so --hot reload and worker teardown can // stop/release jobs. Main-thread VMs without --hot never enumerate it, // so skip the list ref + append entirely. - if vm.hot_reload == HOT_RELOAD_HOT || vm.worker.is_some() { + if vm.hot_reload == HotReload::Hot || vm.worker.is_some() { job_ref.ref_(); // owned by cron_jobs entry // Note: `RareData::cron_jobs` stores the opaque high-tier // placeholder type; cast through `*mut ()` and let inference pick diff --git a/src/runtime/api/glob.rs b/src/runtime/api/glob.rs index a44ab7a77e96..f457921c1d65 100644 --- a/src/runtime/api/glob.rs +++ b/src/runtime/api/glob.rs @@ -247,11 +247,7 @@ impl JobContext for WalkTask { type OffThread = Self; type Js = WalkJs; - fn run( - this: &mut Self, - _vm: &bun_jsc::vm_handle::Borrow, - done: bun_jsc::Completion, - ) -> Option> { + fn run(this: &mut Self, done: bun_jsc::Completion) -> Option> { let result = match this.walker.walk() { Ok(r) => r, Err(err) => { diff --git a/src/runtime/api/js_bundle_completion_task.rs b/src/runtime/api/js_bundle_completion_task.rs index 448346417932..d22d1a8fb29e 100644 --- a/src/runtime/api/js_bundle_completion_task.rs +++ b/src/runtime/api/js_bundle_completion_task.rs @@ -54,8 +54,10 @@ pub struct JSBundleCompletionTask { // `unsafe impl Send` below for the thread-affinity constraint this imposes. pub(crate) ref_count: RefCount, pub(crate) config: JSBundlerConfig, - /// How the bundle thread (and plugin hops) reach the VM that called Bun.build. - pub(crate) loop_handle: jsc::LoopHandle, + /// Held from creation until the bundle thread posts the completion (or the + /// JS thread releases it unstarted): how the bundle thread and its plugin + /// hops reach the VM that called Bun.build, and what makes it wait. + pub(crate) bundle_ticket: Option, pub global_this: BackRef, pub(crate) promise: jsc::JSPromiseStrong, pub poll_ref: KeepAlive, @@ -145,7 +147,7 @@ pub(crate) fn create_and_schedule_completion_task( let completion = bun_core::heap::into_raw(Box::new(JSBundleCompletionTask { ref_count: RefCount::init(), config, - loop_handle: global_this.bun_vm().loop_handle(), + bundle_ticket: Some(global_this.bun_vm().ticket()), global_this: BackRef::new(global_this), promise: jsc::JSPromiseStrong::default(), poll_ref: KeepAlive::init(), @@ -174,9 +176,7 @@ pub(crate) fn create_and_schedule_completion_task( // Out on the bundle thread from here until it posts the completion: it // reads this VM's env loader and the plugin cell, so the VM cancels it at - // teardown (registry) and waits for it (embedded work). - // SAFETY: `completion` is live (refcount==1), JS thread. - unsafe { (*completion).loop_handle.embedded_work_scheduled() }; + // teardown (registry) and waits for it (`bundle_ticket`). crate::jsc_hooks::ActiveHandle::Bundle(NonNull::new(completion).expect("completion")) .register(); bun_bundler::bundle_v2::singleton::enqueue::(completion); @@ -605,12 +605,11 @@ impl JSBundleCompletionTask { Plugin::destroy(plugin.as_ptr()); } (*this).promise = jsc::JSPromiseStrong::default(); - let handle = (*this).loop_handle.clone(); + (*this).bundle_ticket = None; // Publish only now: from here the bundle thread may free `this`. (*this) .stage .store(Stage::ReleasedUnstarted as u8, Ordering::Release); - handle.embedded_work_finished(); return; } if let Some(plugins) = (*this).plugins { @@ -863,14 +862,14 @@ static COMPLETION_VTABLE: dispatch::CompletionDispatch = dispatch::CompletionDis }, enqueue_task_concurrent: |c, task| { // SAFETY: `task` is a fresh non-null `ConcurrentTask` passed through - // from the bundler vtable; the queue takes ownership. The VM waits for - // this build (embedded work) before closing its handle: always queued. + // from the bundler vtable; the queue takes ownership. unsafe { let task = core::ptr::NonNull::new_unchecked(task); - let c = from_completion_handle(c); - let jsc::vm_handle::Posted::Queued = c.loop_handle.post_task(task) else { - unreachable!("VM handle closed with a Bun.build outstanding"); - }; + from_completion_handle(c) + .bundle_ticket + .as_ref() + .expect("a running Bun.build holds a ticket") + .post(task); } }, }; @@ -1116,16 +1115,16 @@ impl CompletionStruct for JSBundleCompletionTask { fn complete_on_bundle_thread(&mut self) { // The bundle thread's last touch of this task and of the VM's memory: - // hand it back (always queued — the VM waits for it) and stop counting. + // move the ticket out (the JS thread may free `self` once queued), + // hand it back, drop the ticket. self.bundle_loop .store(ptr::null_mut(), core::sync::atomic::Ordering::Release); - let handle = self.loop_handle.clone(); + let ticket = self + .bundle_ticket + .take() + .expect("a running Bun.build holds a ticket"); let this = std::ptr::from_mut::(self); - let ct = jsc::ConcurrentTask::create(jsc::Task::init(this)); - let jsc::vm_handle::Posted::Queued = handle.post_task(ct) else { - unreachable!("VM handle closed with a Bun.build outstanding"); - }; - handle.embedded_work_finished(); + ticket.post(jsc::ConcurrentTask::create(jsc::Task::init(this))); } fn set_result(&mut self, result: BundleV2Result) { self.result = result; diff --git a/src/runtime/cli/run_command.rs b/src/runtime/cli/run_command.rs index eb970946ba9d..0d7aa51d98de 100644 --- a/src/runtime/cli/run_command.rs +++ b/src/runtime/cli/run_command.rs @@ -1091,7 +1091,7 @@ Full documentation is available at https://bun.com/docs/cli/run // `ctx.debug.hot_reload` → `vm.hot_reload` (a `u8` until the // b2-cycle widens it to `cli::HotReload`); `Run::start` re-reads it // from `self.ctx` to drive the hot-reloader enable. - vm.hot_reload = ctx.debug.hot_reload as u8; + vm.hot_reload = ctx.debug.hot_reload; Run { ctx, @@ -1280,7 +1280,7 @@ impl Run<'_> { } = self; let _api_lock = vm.global().vm().get_api_lock(); - vm.hot_reload = ctx.debug.hot_reload as u8; + vm.hot_reload = ctx.debug.hot_reload; vm.on_unhandled_rejection = Run::on_unhandled_rejection_before_close; // ── CPU profiler ──────────────────────────────────────────────────── @@ -1448,7 +1448,7 @@ impl Run<'_> { // `uncaughtException` handler swallowed the error), keep the // process alive instead of hard-exiting on a rejected entry. // The core run-loop below does the actual waiting. - if vm.hot_reload != 0 || handled { + if vm.hot_reload != cli::command::HotReload::None || handled { vm.add_main_to_watcher_if_needed(); // SAFETY: `event_loop` is a self-pointer into this VM; // uniquely accessed here. diff --git a/src/runtime/cli/test_command.rs b/src/runtime/cli/test_command.rs index 7ea42e2be5e9..48f22492e5f6 100644 --- a/src/runtime/cli/test_command.rs +++ b/src/runtime/cli/test_command.rs @@ -2615,7 +2615,7 @@ impl TestCommand { if !test_files.is_empty() || (ctx.test_options.changed.is_some() && all_test_files_count != 0) { - vm.hot_reload = ctx.debug.hot_reload as u8; + vm.hot_reload = ctx.debug.hot_reload; // Install the --changed trigger collector BEFORE the watcher // thread starts so a file edit during runAllTests is still @@ -2623,13 +2623,13 @@ impl TestCommand { // runAllTests (separate concern; see O_EVTONLY comment // below). if ctx.test_options.changed.is_some() - && vm.hot_reload == jsc::virtual_machine::HOT_RELOAD_WATCH + && vm.hot_reload == jsc::virtual_machine::HotReload::Watch { ChangedFilesFilter::init_watch_trigger(); } match vm.hot_reload { - jsc::virtual_machine::HOT_RELOAD_HOT => { + jsc::virtual_machine::HotReload::Hot => { // SAFETY: `vm` is the process-lifetime main-thread VM; it // outlives the leaked reloader. unsafe { @@ -2639,7 +2639,7 @@ impl TestCommand { ); } } - jsc::virtual_machine::HOT_RELOAD_WATCH => { + jsc::virtual_machine::HotReload::Watch => { // SAFETY: `vm` is the process-lifetime main-thread VM; it // outlives the leaked reloader. unsafe { @@ -3009,7 +3009,7 @@ impl TestCommand { reporter.write_timings_if_needed(); } - if vm.hot_reload == jsc::virtual_machine::HOT_RELOAD_WATCH { + if vm.hot_reload == jsc::virtual_machine::HotReload::Watch { let vm_ptr: *mut VirtualMachine = vm; // SAFETY: `vm_ptr` reborrows the live `&mut VirtualMachine`; // `run_with_api_lock` takes `&self` only, so the closure holds the diff --git a/src/runtime/crypto/PBKDF2.rs b/src/runtime/crypto/PBKDF2.rs index e70ffa4f1a70..db1936f61e55 100644 --- a/src/runtime/crypto/PBKDF2.rs +++ b/src/runtime/crypto/PBKDF2.rs @@ -270,11 +270,7 @@ impl JobContext for Pbkdf2Job { type OffThread = Self; type Js = JSPromiseStrong; - fn run( - this: &mut Self, - _vm: &bun_jsc::vm_handle::Borrow, - done: bun_jsc::Completion, - ) -> Option> { + fn run(this: &mut Self, done: bun_jsc::Completion) -> Option> { let len = usize::try_from(this.pbkdf2.length).expect("int cast"); // `Vec` allocation aborts on OOM; use try_reserve to surface an error instead. let mut buf = Vec::new(); diff --git a/src/runtime/crypto/PasswordObject.rs b/src/runtime/crypto/PasswordObject.rs index 2b2bb24c255b..f225555fc764 100644 --- a/src/runtime/crypto/PasswordObject.rs +++ b/src/runtime/crypto/PasswordObject.rs @@ -567,11 +567,7 @@ impl Drop for PasswordJob { impl bun_jsc::JobContext for PasswordJob { type OffThread = Self; type Js = JSPromiseStrong; - fn run( - this: &mut Self, - _vm: &bun_jsc::vm_handle::Borrow, - done: bun_jsc::Completion, - ) -> Option> { + fn run(this: &mut Self, done: bun_jsc::Completion) -> Option> { this.value = Some(this.op.compute(&this.password)); Some(done) } diff --git a/src/runtime/dispatch.rs b/src/runtime/dispatch.rs index df0996b0438a..5e2a61b949c8 100644 --- a/src/runtime/dispatch.rs +++ b/src/runtime/dispatch.rs @@ -1184,9 +1184,8 @@ fn __bun_release_task_unrun(task: bun_event_loop::Task) { match task.tag { task_tag::AnyTaskJob => { // The one erased tag: every payload is a `Job` reached through its header. - let js = VirtualMachine::get().global().js_thread(); // SAFETY: as `release!`. - unsafe { bun_jsc::job::release_unrun_erased(task.ptr, &js) } + unsafe { bun_jsc::job::release_unrun_erased(task.ptr) } } task_tag::AsyncModule => release!(bun_jsc::async_module::AsyncModule), task_tag::BakeHotReloadEvent => release!(BakeHotReloadEvent), diff --git a/src/runtime/dns_jsc/dns.rs b/src/runtime/dns_jsc/dns.rs index cb77f288f2af..220d9817bfd6 100644 --- a/src/runtime/dns_jsc/dns.rs +++ b/src/runtime/dns_jsc/dns.rs @@ -1012,7 +1012,6 @@ pub mod get_addr_info_request { type Js = LibcRequest; fn run( this: &mut Self, - _vm: &bun_jsc::vm_handle::Borrow, done: bun_jsc::Completion, ) -> Option> { this.backend.run(); diff --git a/src/runtime/image/Image.rs b/src/runtime/image/Image.rs index 7a02e6a4d7ff..6a90ee95521e 100644 --- a/src/runtime/image/Image.rs +++ b/src/runtime/image/Image.rs @@ -1465,11 +1465,7 @@ impl Drop for PendingTask { impl jsc::JobContext for PipelineTask { type OffThread = Self; type Js = PipelineJs; - fn run( - this: &mut Self, - _vm: &jsc::vm_handle::Borrow, - done: bun_jsc::Completion, - ) -> Option> { + fn run(this: &mut Self, done: bun_jsc::Completion) -> Option> { this.run(); Some(done) } diff --git a/src/runtime/jsc_hooks.rs b/src/runtime/jsc_hooks.rs index cad7564a5787..baa49d74122a 100644 --- a/src/runtime/jsc_hooks.rs +++ b/src/runtime/jsc_hooks.rs @@ -474,7 +474,8 @@ unsafe fn init_runtime_state( .wake_ctx .insert(Box::new(bun_jsc::async_module::WakeContext { queue: &raw mut (*vm).modules, - loop_handle: (*vm).loop_handle(), + handle: (*vm).handle(), + kind: (*vm).current_loop_kind(), })); t.resolver.on_wake_package_manager = bun_resolver::install_types::WakeHandler { context: core::ptr::NonNull::new(wake_ctx.cast()), @@ -3504,13 +3505,8 @@ fn transpile_source_code_inner( // .sqlite / .sqlite_embedded // ──────────────────────────────────────────────────────────────────── L::Sqlite | L::SqliteEmbedded => { - // The low-tier - // `VirtualMachine.hot_reload` slot is a raw `u8`; compare against - // the real `HotReload` enum discriminant (`!= 0` would also match - // `.watch`, which is wrong). // SAFETY: per fn contract — `jsc_vm` is the live per-thread VM. - let hot = - unsafe { &*jsc_vm }.hot_reload == bun_options_types::context::HotReload::Hot as u8; + let hot = unsafe { &*jsc_vm }.hot_reload == bun_options_types::context::HotReload::Hot; let sqlite_module_source_code_string: &'static [u8] = if hot { SQLITE_MODULE_SOURCE_HOT } else { diff --git a/src/runtime/napi/napi_body.rs b/src/runtime/napi/napi_body.rs index 4696c1dab0bd..f25df298ddf8 100644 --- a/src/runtime/napi/napi_body.rs +++ b/src/runtime/napi/napi_body.rs @@ -1797,9 +1797,10 @@ pub(super) enum AsyncWorkStatus { pub(crate) struct napi_async_work { pub task: WorkPoolTask, pub(crate) concurrent_task: ConcurrentTask, - // Note: BackRef — `enqueue_task` needs `&mut EventLoop`; reborrowed at use sites. - /// How the pool thread delivers completion / cancellation to the VM. - pub(crate) loop_handle: bun_jsc::LoopHandle, + /// Held while the work is out on the pool (`schedule` until it is posted + /// back): how the pool thread delivers completion / cancellation, and what + /// makes the VM wait for it. + pub(crate) ticket: Option, /// JS thread only. pub global: GlobalRef, pub(crate) env: NapiEnvRef, @@ -1832,7 +1833,7 @@ impl napi_async_work { // SAFETY: env outlives the async work; clone bumps the C++ refcount. env: unsafe { NapiEnvRef::clone_from_raw(env.as_mut_ptr()) }, execute, - loop_handle: global.bun_vm().loop_handle(), + ticket: None, complete, data, status: AtomicU32::new(AsyncWorkStatus::Pending as u32), @@ -1857,9 +1858,9 @@ impl napi_async_work { self.scheduled = true; self.poll_ref.ref_(bun_io::js_vm_ctx()); // The work object belongs to the addon and `execute` receives this - // env: counted, so the VM waits for it (Node likewise settles its - // threadpool requests before an environment is freed). - self.loop_handle.embedded_work_scheduled(); + // env, so the VM waits for it (Node likewise settles its threadpool + // requests before an environment is freed). + self.ticket = Some(self.global.bun_vm().ticket()); WorkPool::schedule(&raw mut self.task); } @@ -1871,12 +1872,14 @@ impl napi_async_work { fn run(&mut self) { let self_ptr: *mut Self = self; - let handle = self.loop_handle.clone(); + // Moved out: the JS thread may free `self` the moment it is posted back. + let ticket = self + .ticket + .take() + .expect("scheduled napi async work holds a ticket"); // A VM that is already stopping cancels work it has not started, as - // Node's environment cleanup does (uv_cancel); otherwise `execute` runs - // with the VM held open. - let vm = handle.borrow_if_running(); - let started = vm.is_some() + // Node's environment cleanup does (uv_cancel). + let started = ticket.script_allowed() && match self.status.compare_exchange( AsyncWorkStatus::Pending as u32, AsyncWorkStatus::Started as u32, @@ -1893,25 +1896,20 @@ impl napi_async_work { } else { let _ = self.cancel(); } - drop(vm); - self.post_to_js_thread(self_ptr); - // `self` may already be freed by the JS thread; the handle is ours. - handle.embedded_work_finished(); + self.post_to_js_thread(self_ptr, &ticket); } /// Pool thread → JS thread: run `complete` there. `concurrent_task` is the /// live inline field of this heap work; the queue takes ownership of its - /// `next` link. Counted work, so the VM has not closed its handle; a VM - /// tearing down runs `complete` from its queue release (status cancelled - /// if `execute` never ran), as Node does at environment cleanup. - fn post_to_js_thread(&mut self, self_ptr: *mut Self) { + /// `next` link. A VM tearing down runs `complete` from its queue release + /// (status cancelled if `execute` never ran), as Node does at environment + /// cleanup. + fn post_to_js_thread(&mut self, self_ptr: *mut Self, ticket: &bun_jsc::Ticket) { let ct = core::ptr::NonNull::from( self.concurrent_task .from(self_ptr, AutoDeinit::ManualDeinit), ); - let bun_jsc::vm_handle::Posted::Queued = self.loop_handle.post_task(ct) else { - unreachable!("VM handle closed with napi async work outstanding"); - }; + ticket.post(ct); } pub(crate) fn cancel(&mut self) -> bool { @@ -2467,8 +2465,11 @@ pub(crate) struct ThreadSafeFunction { /// JS-thread uses; `None` once the env has been torn down. pub(crate) event_loop: Option>, /// How addon threads (`napi_call_threadsafe_function`) schedule a - /// dispatch on the VM. - pub(crate) loop_handle: bun_jsc::LoopHandle, + /// dispatch on the VM. Weak: addon threads hold this function for as long + /// as they like (Node: calls after env cleanup get `napi_closing`), so it + /// cannot be something the VM waits for. + pub(crate) handle: bun_jsc::VmHandle, + pub(crate) loop_kind: bun_jsc::LoopKind, pub(crate) tracker: Debugger::AsyncTaskTracker, /// Dropped on the JS thread by `env_teardown`; `None` afterwards. @@ -2860,8 +2861,10 @@ impl ThreadSafeFunction { return; } let ct = ConcurrentTask::create_from(self_ptr); - if let bun_jsc::vm_handle::Posted::Refused(ct) = self.loop_handle.post_task(ct) { - // VM torn down before the env cleanup hook ran here: no + if let bun_jsc::vm_handle::Posted::Refused(ct) = + self.handle.post(self.loop_kind, ct) + { + // VM closed before the env cleanup hook ran here: no // dispatch will happen; the queued calls are released by the // teardown path. Free the task and fall back to Idle. // SAFETY: refused ⇒ we own the task box. @@ -3150,7 +3153,8 @@ extern "C" fn napi_create_threadsafe_function( // SAFETY: the loop is live now; `NapiEnv::cleanup()` clears this field // (via `env_teardown`) before the VirtualMachine holding it is freed. event_loop: Some(unsafe { bun_ptr::BackRef::from_raw_mut(vm.event_loop()) }), - loop_handle: vm.loop_handle(), + handle: vm.handle(), + loop_kind: vm.current_loop_kind(), // SAFETY: env is a live C++-owned napi_env. env: Some(unsafe { NapiEnvRef::clone_from_raw(env.as_mut_ptr()) }), callback, diff --git a/src/runtime/node/node_crypto_binding.rs b/src/runtime/node/node_crypto_binding.rs index ac3c88150877..d4f633282a94 100644 --- a/src/runtime/node/node_crypto_binding.rs +++ b/src/runtime/node/node_crypto_binding.rs @@ -5,7 +5,6 @@ use core::ffi::{c_char, c_void}; use bun_boringssl as boringssl; use bun_collections::CaseInsensitiveAsciiStringArrayHashMap; -use bun_jsc::vm_handle::Borrow; use bun_jsc::{ self as jsc, ArrayBuffer, CallFrame, JSGlobalObject, JSValue, Job, JobContext, JsPtr, JsResult, JsThread, Protected, Strong, @@ -127,13 +126,12 @@ macro_rules! extern_crypto_job { fn run( this: &mut Self, - vm: &Borrow, done: bun_jsc::Completion, ) -> Option> { - // SAFETY: the creating global, alive under the borrow; C++ + // SAFETY: the creating global, alive under the job's ticket; C++ // only threads it through to error reporting state. ctx_run_task(Ctx::opaque_ref(this.ctx.0), unsafe { - this.global.under_borrow(vm) + this.global.under_ticket(done.ticket()) }); Some(done) } @@ -210,8 +208,8 @@ pub mod random { /// `crypto.randomFill` / `randomBytes` off the JS thread. enum RandomFillJob { /// `randomBytes`: the ArrayBuffer was allocated by us and nothing else - /// can observe it yet, so fill its bytes directly (under the VM borrow - /// that keeps it alive). + /// can observe it yet, so fill its bytes directly (under the job's ticket, + /// which keeps its VM alive). InPlace { bytes: JsPtr, length: usize }, /// `randomFill`: the caller's buffer stays untouched until completion; /// fill `scratch` off-thread and copy it in at `offset` on the JS thread. @@ -237,18 +235,17 @@ pub mod random { fn run( this: &mut Self, - vm: &Borrow, done: bun_jsc::Completion, ) -> Option> { match this { RandomFillJob::Scratch { scratch, .. } => boringssl::rand_bytes(scratch), RandomFillJob::InPlace { bytes, length } => { // SAFETY: `bytes` points into the ArrayBuffer `value` keeps alive; - // the borrow keeps the VM (and so that buffer) alive; `length` is + // the ticket keeps the VM (and so that buffer) alive; `length` is // the buffer's own allocation size. let slice = unsafe { core::slice::from_raw_parts_mut( - core::ptr::from_mut(bytes.under_borrow(vm)), + core::ptr::from_mut(bytes.under_ticket(done.ticket())), *length, ) }; @@ -1045,7 +1042,7 @@ mod _impl { } /// `crypto.scrypt` off the JS thread: derives straight into the result - /// ArrayBuffer's bytes under the VM borrow that keeps them alive. + /// ArrayBuffer's bytes under the job's ticket, which keeps their VM alive. pub(crate) struct ScryptJob { params: bun_jsc::ThreadSafe, result: JsPtr<[u8]>, @@ -1064,11 +1061,10 @@ mod _impl { fn run( this: &mut Self, - vm: &Borrow, done: bun_jsc::Completion, ) -> Option> { - // SAFETY: `result` is `buf`'s backing store (kept by the Js side); VM alive under the borrow. - let key = unsafe { this.result.under_borrow(vm) }; + // SAFETY: `result` is `buf`'s backing store (kept by the Js side); VM alive under the ticket. + let key = unsafe { this.result.under_ticket(done.ticket()) }; this.err = this.params.run_task_impl(key); Some(done) } diff --git a/src/runtime/node/node_fs.rs b/src/runtime/node/node_fs.rs index 6046c4ad2fd5..593ff32f8ce5 100644 --- a/src/runtime/node/node_fs.rs +++ b/src/runtime/node/node_fs.rs @@ -569,9 +569,12 @@ mod _async_tasks { /// Used internally. Not from JavaScript. pub struct AsyncMkdirp { pub(crate) completion_ctx: *mut (), - pub(crate) completion: fn(*mut (), Maybe<()>), + /// Pool thread; `ticket` is this task's, for the callee to post its + /// hop back through. + pub(crate) completion: fn(*mut (), Maybe<()>, &bun_jsc::Ticket), /// Memory is not owned by this struct pub path: *const [u8], // BORROW: not owned + pub(crate) ticket: bun_jsc::Ticket, pub task: WorkPoolTask, } @@ -606,26 +609,15 @@ mod _async_tasks { // `with_path` already clones into a fresh `Box<[u8]>`; pass the // existing path slice. Err(err.with_path(&err.path)), + &self.ticket, ); } Ok(_) => { - (self.completion)(self.completion_ctx, Ok(())); + (self.completion)(self.completion_ctx, Ok(()), &self.ticket); } } } } - - #[cfg(windows)] - impl Default for AsyncMkdirp { - fn default() -> Self { - Self { - completion_ctx: core::ptr::null_mut(), - completion: |_, _| {}, - path: core::ptr::slice_from_raw_parts(core::ptr::null(), 0), - task: WorkPoolTask::default(), - } - } - } } // ────────────────────────────────────────────────────────────────────────── @@ -1228,7 +1220,7 @@ mod _async_tasks { } /// One `fs.promises.*` operation on the work pool. The arguments' JS-backed - /// buffers are protected (`ThreadSafe`) and read under the pool's VM borrow. + /// buffers are protected (`ThreadSafe`) and read under the job's ticket. pub struct AsyncFSTask { pub args: ThreadSafe, pub(crate) result: Maybe, @@ -1254,7 +1246,6 @@ mod _async_tasks { fn run( this: &mut Self, - _vm: &bun_jsc::vm_handle::Borrow, done: bun_jsc::Completion, ) -> Option> { let mut node_fs = NodeFS::default(); @@ -1362,8 +1353,11 @@ mod _async_tasks { pub args: ThreadSafe, /// Owning-thread uses (global object, keep-alive context). pub(crate) evtloop: EventLoopHandle, - /// How the last subtask's thread delivers the completion. - pub(crate) poster: bun_jsc::ConcurrentPoster, + /// How the last subtask's thread delivers the completion (moved out + /// by it: the loop may free `self` once the completion is queued). For + /// a JS loop this is the ticket its VM waits for — the arguments may + /// point into JS buffers and the promise lives on the JS heap. + pub(crate) poster: core::cell::Cell>, pub task: WorkPoolTask, /// Written from any workpool thread (first `finish_concurrently` caller wins via /// `has_result` CAS); read on the JS thread in `run_from_js_thread`. Wrapped in @@ -1546,7 +1540,7 @@ mod _async_tasks { JSPromiseStrong::init(global_object), cp_args, EventLoopHandle::init(vm.event_loop.cast()), - bun_jsc::ConcurrentPoster::Js(vm.js_poster()), + bun_jsc::ConcurrentPoster::Js(vm.ticket()), tracker, core::ptr::null_mut(), ); @@ -1589,7 +1583,7 @@ mod _async_tasks { // `has_result` CAS) before any read on the JS thread. result: core::cell::Cell::new(Ok(())), evtloop, - poster, + poster: core::cell::Cell::new(Some(poster)), task: work_pool_task(Self::work_pool_callback), r#ref: KeepAlive::default(), tracker, @@ -1601,9 +1595,6 @@ mod _async_tasks { if !IS_SHELL { task.r#ref.ref_(event_loop_handle_to_ctx(task.evtloop)); } - // Its arguments may point into JS buffers and its promise lives on - // the JS heap: counted, so the VM waits for it (embedded work). - task.poster.embedded_work_scheduled(); let raw = bun_core::heap::release(task); WorkPool::schedule(&raw mut raw.task); @@ -1669,13 +1660,14 @@ mod _async_tasks { // Count reached zero ⇒ exclusive access. `this` carries mutable // provenance from `Box::leak`, so the enqueued callback may safely // form `&mut *this` on the JS thread. - let poster = this_ref.poster.clone(); + let poster = this_ref + .poster + .take() + .expect("fs.cp in flight holds its poster"); if poster.is_js() { - let ct = ConcurrentTask::ConcurrentTask::create(bun_jsc::Task::init(this)); - // Counted work: the VM has not closed its handle. - let bun_jsc::vm_handle::Posted::Queued = poster.post_js(ct) else { - unreachable!("VM handle closed with an fs.cp outstanding"); - }; + poster.post_js(ConcurrentTask::ConcurrentTask::create(bun_jsc::Task::init( + this, + ))); } else { let at = AnyTaskWithExtraContext::from_callback_auto_deinit( this, @@ -1688,7 +1680,7 @@ mod _async_tasks { poster.post_mini(core::ptr::NonNull::new(at).expect("heap task")); } // The pool side is done (`this` may already be freed by its loop). - poster.embedded_work_finished(); + drop(poster); } pub(crate) fn run_from_js_thread_mini(&mut self, _: *mut c_void) { @@ -2131,7 +2123,7 @@ mod _async_tasks { /// `readdir(.., { recursive: true })`: a scan fanned out over pool subtasks /// that share this state (it is the job's off-thread part, so its address /// is stable while any subtask runs). Subtasks touch only owned data here — - /// never the JS-backed `args` — since they run outside the VM borrow. + /// never the JS-backed `args` — since they run outside `run`. pub struct AsyncReaddirRecursiveTask { /// Protected arguments; their JS-backed path is not read off-thread /// (`root_path` is the owned copy). @@ -2185,7 +2177,6 @@ mod _async_tasks { fn run( this: &mut Self, - _vm: &bun_jsc::vm_handle::Borrow, done: bun_jsc::Completion, ) -> Option> { this.done = Some(done); @@ -2372,8 +2363,8 @@ mod _async_tasks { ret::ReaddirTag::WithFileTypes => ResultListEntryValue::WithFileTypes(Vec::new()), ret::ReaddirTag::Buffers => ResultListEntryValue::Buffers(Vec::new()), }; - // Subtasks read the root path outside the VM borrow, so it must be an - // owned copy rather than the (possibly JS-backed) argument. NUL-terminated. + // Subtasks read the root path after `run` has returned its borrow of the + // arguments, so it must be an owned copy. NUL-terminated. let root_path = { let src = args.path.slice(); let mut owned = Vec::with_capacity(src.len() + 1); diff --git a/src/runtime/node/node_fs_stat_watcher.rs b/src/runtime/node/node_fs_stat_watcher.rs index 741db2b5260a..58d0903b88f5 100644 --- a/src/runtime/node/node_fs_stat_watcher.rs +++ b/src/runtime/node/node_fs_stat_watcher.rs @@ -61,8 +61,9 @@ pub struct StatWatcherScheduler { main_thread: ThreadId, /// JS-thread uses only (`timer_callback`). vm: BackRef, - /// How the pool thread asks the JS thread to (re)arm the timer. - loop_handle: bun_jsc::LoopHandle, + /// Held while the periodic stat pass is out on the pool (set in + /// `timer_callback`, moved out by `work_pool_callback`). + ticket: Cell>, watchers: WatcherQueue, pub(crate) event_loop_timer: EventLoopTimer, @@ -187,8 +188,7 @@ impl StatWatcherScheduler { main_thread: thread::current().id(), // JSC_BORROW: `vm` is the live per-thread VM (never null). vm: BackRef::from(core::ptr::NonNull::new(vm).expect("vm")), - // SAFETY: `vm` is the live per-thread VM; this runs on its thread. - loop_handle: unsafe { (*vm).loop_handle() }, + ticket: Cell::new(None), watchers: WatcherQueue::default(), event_loop_timer: EventLoopTimer::init_paused(EventLoopTimerTag::StatWatcherScheduler), ref_count: ThreadSafeRefCount::init(), @@ -228,12 +228,16 @@ impl StatWatcherScheduler { StatWatcher::ref_(watcher.as_ptr()); // BACKREF — `this` is live (caller holds a ref). let this_ref = ParentRef::from(NonNull::new(this).expect("append: scheduler")); + debug_assert_eq!(this_ref.main_thread, thread::current().id()); this_ref.watchers.push(watcher); log!("push watcher {:x}", watcher.as_ptr() as usize); let current = this_ref.get_interval(); if current == 0 || current > w.interval { // we are not running or the new watcher has a smaller interval - Self::set_interval(this, w.interval); + this_ref + .current_interval + .store(w.interval, Ordering::Relaxed); + Self::set_timer(this, w.interval); } } @@ -241,22 +245,6 @@ impl StatWatcherScheduler { self.current_interval.load(Ordering::Relaxed) } - /// Update the current interval and set the timer (this function is thread safe) - fn set_interval(this: *mut Self, interval: i32) { - // BACKREF — `this` is live (caller holds a ref); `ParentRef` Deref - // gives safe `&Self` for the atomic store / thread-id check below. - let this_ref = ParentRef::from(NonNull::new(this).expect("set_interval: scheduler")); - this_ref.current_interval.store(interval, Ordering::Relaxed); - - if this_ref.main_thread == thread::current().id() { - // we are in the main thread we can set the timer - Self::set_timer(this, interval); - return; - } - // we are not in the main thread we need to schedule a task to set the timer - Self::schedule_timer_update(this); - } - /// Set the timer (this function is not thread safe, should be called only from the main thread) fn set_timer(this: *mut Self, interval: i32) { // jsc/runtime crate cycle: `vm.timer: api.Timer.All` lives in `RuntimeState` (this crate), @@ -291,24 +279,14 @@ impl StatWatcherScheduler { } /// Schedule a task to set the timer in the main thread - fn schedule_timer_update(this: *mut Self) { + fn schedule_timer_update(this: *mut Self, ticket: &bun_jsc::Ticket) { let holder = Box::new(StatWatcherTimerUpdate { // SAFETY: `this` is the live ref'd scheduler (write provenance for // `set_timer`), kept alive across the hop by the watcher's RefPtr. scheduler: unsafe { ParentRef::from_raw_mut(this) }, }); - // SAFETY: `this` is live (kept by the watcher's RefPtr across the hop). - unsafe { - let holder = bun_core::heap::into_raw(holder); - let ct = ConcurrentTask::create(Task::new( - ::TAG, - holder.cast::<()>(), - )); - // Posted from the counted pool task: the VM has not closed its handle. - let bun_jsc::vm_handle::Posted::Queued = (*this).loop_handle.post_task(ct) else { - unreachable!("VM handle closed with the stat scheduler's pool task outstanding"); - }; - } + let holder = bun_core::heap::into_raw(holder); + ticket.post(ConcurrentTask::create_from(holder)); } pub(crate) fn timer_callback(&mut self) { @@ -353,9 +331,7 @@ impl StatWatcherScheduler { // of accumulating one leak per `set_interval(0)` / re-arm. // SAFETY: `self` is live (`&mut self`). Self::ref_(core::ptr::from_mut(self)); - // The task is a field of this per-VM scheduler: counted, so the VM - // waits for it (see `VmHandle::embedded_work_scheduled`). - self.loop_handle.embedded_work_scheduled(); + self.ticket.set(Some(self.vm().ticket())); WorkPool::schedule(&raw mut self.task); } @@ -374,6 +350,12 @@ impl StatWatcherScheduler { // BACKREF — `this` is alive (ref'd when the timer was scheduled); // `ParentRef` Deref gives safe `&Self` for the queue/interval reads. let this_ref = ParentRef::from(NonNull::new(this).expect("work_pool_callback: scheduler")); + // Moved out before anything is posted: the JS thread may re-arm (and + // store the next pass's ticket) as soon as it hears from this pass. + let ticket = this_ref + .ticket + .take() + .expect("stat scheduler pass holds a ticket"); // Instant.now will not fail on our target platforms. let now = Instant::now(); @@ -408,7 +390,7 @@ impl StatWatcherScheduler { if time_since >= interval.saturating_sub(500) { w.last_check.set(now); - w.restat(); + w.restat(&ticket); } else { closest_next_check = (interval - time_since).min(closest_next_check); } @@ -423,10 +405,9 @@ impl StatWatcherScheduler { this_ref.current_interval.store(0, Ordering::Relaxed); } else if contain_watchers { // choose the smallest interval or the closest time to the next check - Self::set_interval( - this, - min_interval.min(i32::try_from(closest_next_check).expect("int cast")), - ); + let interval = min_interval.min(i32::try_from(closest_next_check).expect("int cast")); + this_ref.current_interval.store(interval, Ordering::Relaxed); + Self::schedule_timer_update(this, &ticket); } else { // we do not have watchers, we can stop the timer this_ref.current_interval.store(0, Ordering::Relaxed); @@ -434,9 +415,8 @@ impl StatWatcherScheduler { // Publish the queue writes above before declaring the work-pool hop // finished; `shutdown_for_exit` Acquire-loads this and then drains. this_ref.work_pool_in_flight.store(false, Ordering::Release); - let handle = this_ref.loop_handle.clone(); drop(_ref_guard); - handle.embedded_work_finished(); + drop(ticket); } /// Drain every queued [`StatWatcher`] and release the per-VM scheduler ref @@ -519,10 +499,8 @@ pub struct StatWatcher { /// JS-thread uses only. ctx: BackRef, - /// How the pool thread delivers stat results to the VM. /// The pending pool→JS hop, if any (one at a time: the initial stat, then restats). pending_hop: Cell, - loop_handle: bun_jsc::LoopHandle, ref_count: ThreadSafeRefCount, @@ -677,14 +655,10 @@ impl StatWatcher { } /// Pool thread → JS thread: `hop` runs there and consumes one ref on - /// `self`. Posted from counted (embedded) pool work, so always queued; a - /// VM tearing down releases the ref from its queue instead of running it. - fn post_to_js_thread(&self, hop: StatWatcherHop) { + /// `self`; a VM tearing down releases the ref from its queue instead. + fn post_to_js_thread(&self, hop: StatWatcherHop, ticket: &bun_jsc::Ticket) { self.pending_hop.set(hop as u8); - let task = ConcurrentTask::create(Task::init(self.as_ctx_ptr())); - let bun_jsc::vm_handle::Posted::Queued = self.loop_handle.post_task(task) else { - unreachable!("VM handle closed with stat-watcher pool work outstanding"); - }; + ticket.post(ConcurrentTask::create(Task::init(self.as_ctx_ptr()))); } /// JS thread dispatch of a [`post_to_js_thread`](Self::post_to_js_thread) hop. @@ -925,8 +899,8 @@ impl StatWatcher { result.map(drop).map_err(Into::into) } - /// Called from any thread - fn restat(&self) { + /// Pool thread (the scheduler's pass). + fn restat(&self, ticket: &bun_jsc::Ticket) { log!("recalling stat"); let stat = restat_impl(&self.path); let res = match stat { @@ -964,7 +938,7 @@ impl StatWatcher { // shared (`&*const`), so no write provenance is required. let this_ptr: *mut StatWatcher = self.as_ctx_ptr(); Self::ref_(this_ptr); - self.post_to_js_thread(StatWatcherHop::Changed); + self.post_to_js_thread(StatWatcherHop::Changed, ticket); } /// After a restat found the file changed, this calls the listener function. @@ -1038,8 +1012,6 @@ impl StatWatcher { // SAFETY: `bun_vm_ptr()` is the live per-thread VM, non-null, outlives the watcher. ctx: unsafe { BackRef::from_raw_mut(vm) }, pending_hop: Cell::new(0), - // SAFETY: `vm` is the live per-thread VM; this runs on its thread. - loop_handle: unsafe { (*vm).loop_handle() }, ref_count: ThreadSafeRefCount::init(), closed: AtomicBool::new(false), path: alloc_file_path, @@ -1210,6 +1182,7 @@ pub(crate) struct InitialStatTask { // payload). We hold the strong ref via `ref_()`/`deref()` and keep the // raw `*mut`. watcher: *mut StatWatcher, + ticket: bun_jsc::Ticket, task: WorkPoolTask, } @@ -1223,12 +1196,11 @@ impl InitialStatTask { // the task lifetime (balanced by `deref()` in run_owned's closed path or // by the main-thread `initial_stat_*_on_main_thread` callbacks). StatWatcher::ref_(watcher); - // The watcher is a JS-owned m_ctx: counted, so its VM waits for this - // (see `VmHandle::embedded_work_scheduled`). - // SAFETY: per fn contract. - unsafe { (*watcher).loop_handle.embedded_work_scheduled() }; WorkPool::schedule_new(InitialStatTask { watcher, + // The watcher is a JS-owned m_ctx; its VM waits for this ticket. + // SAFETY: per fn contract; JS thread. + ticket: unsafe { (*watcher).ctx.get() }.ticket(), task: WorkPoolTask::default(), }); } @@ -1249,8 +1221,7 @@ impl InitialStatTask { // both also deref as shared (R-2), so aliased `&` is sound. // `ParentRef` Deref gives that shared `&`. let this_ref = ParentRef::from(NonNull::new(this).expect("run_owned: watcher")); - let handle = this_ref.loop_handle.clone(); - let _finished = scopeguard::guard((), |()| handle.embedded_work_finished()); + let ticket = self.ticket; if this_ref.closed.load(Ordering::Relaxed) { // Balance the ref() from createAndSchedule(). @@ -1264,14 +1235,14 @@ impl InitialStatTask { Ok(ref res) => { // we store the stat, but do not call the callback this_ref.set_last_stat(res); - this_ref.post_to_js_thread(StatWatcherHop::InitialStatSuccess); + this_ref.post_to_js_thread(StatWatcherHop::InitialStatSuccess, &ticket); } Err(_) => { // on enoent, eperm, we call cb with two zeroed stat objects // and store previous stat as a zeroed stat object, and then call the callback. // SAFETY: all-zero is a valid PosixStat (POD #[repr(C)]) this_ref.set_last_stat(&bun_core::ffi::zeroed::()); - this_ref.post_to_js_thread(StatWatcherHop::InitialStatError); + this_ref.post_to_js_thread(StatWatcherHop::InitialStatError, &ticket); } } // ref ownership transferred to main-thread callback diff --git a/src/runtime/node/node_fs_watcher.rs b/src/runtime/node/node_fs_watcher.rs index 2130b424d2c6..86ffeb8943ce 100644 --- a/src/runtime/node/node_fs_watcher.rs +++ b/src/runtime/node/node_fs_watcher.rs @@ -46,10 +46,14 @@ pub struct FSWatcher { // codegen: jsc.Codegen.JSFSWatcher provides toJS/fromJS/fromJSDirect /// JS-thread uses only. ctx: *mut VirtualMachine, - /// How the watcher thread delivers event batches to the VM (POSIX; on - /// Windows libuv delivers fs events on the JS thread). + /// How the (process-wide) watcher thread delivers event batches to the + /// VM while this watcher is attached (POSIX; on Windows libuv delivers fs + /// events on the JS thread). Weak: `detach()` — close, the VM's stop + /// phase, or finalize — is what ends the thread's access to `self`. #[cfg(not(windows))] - loop_handle: bun_jsc::LoopHandle, + handle: bun_jsc::VmHandle, + #[cfg(not(windows))] + loop_kind: bun_jsc::LoopKind, verbose: bool, mutex: Mutex, @@ -103,7 +107,7 @@ impl FSWatcher { &self, task: core::ptr::NonNull, ) -> bun_jsc::vm_handle::Posted { - self.loop_handle.post_task(task) + self.handle.post(self.loop_kind, task) } /// `self`'s address as `*mut Self` for path-watcher / abort-signal / @@ -1131,7 +1135,9 @@ impl FSWatcher { let ctx = bun_core::heap::into_raw(Box::new(FSWatcher { ctx: vm, #[cfg(not(windows))] - loop_handle: vm_ref.loop_handle(), + handle: vm_ref.handle(), + #[cfg(not(windows))] + loop_kind: vm_ref.current_loop_kind(), current_task: JsCell::new(FSWatchTask { ctx: None, ..Default::default() diff --git a/src/runtime/node/node_zlib_binding.rs b/src/runtime/node/node_zlib_binding.rs index 787f89b44f55..e03aabe80f99 100644 --- a/src/runtime/node/node_zlib_binding.rs +++ b/src/runtime/node/node_zlib_binding.rs @@ -203,8 +203,9 @@ pub(crate) trait CompressionStreamImpl: Sized + Taskable + 'static { /// Implementations store a `BackRef`; the single unsafe /// deref lives in `BackRef::get`, so callers and impls are safe. fn global_this(&self) -> &JSGlobalObject; - /// How the pool thread reaches the VM (captured at construction). - fn loop_handle(&self) -> &bun_jsc::LoopHandle; + /// The in-flight write's ticket: set in `write` before the task leaves + /// the thread, moved out by the pool thread before it posts back. + fn ticket(&self) -> &Cell>; fn stream(&self) -> &JsCell; /// Write `(avail_out, avail_in)` into the JS-owned 2-element `Uint32Array` @@ -466,9 +467,7 @@ impl CompressionStream { callback: Self::async_job_run_task, }); this.poll_ref().with_mut(|p| p.ref_(vm)); - // The task is a field of this JS-owned stream: counted, so the VM waits - // for it (see `VmHandle::embedded_work_scheduled`). - this.loop_handle().embedded_work_scheduled(); + this.ticket().set(Some(vm.ticket())); WorkPool::schedule(this.task().as_ptr()); Ok(JSValue::UNDEFINED) @@ -493,20 +492,19 @@ impl CompressionStream { // (R-2). `ParentRef` Deref collapses the per-site raw deref. let this_ref = ParentRef::from(NonNull::new(this).expect("async_job_run: this")); - // The stream reads and writes JS ArrayBuffer backing stores: only while - // the VM is running, under a borrow. Either way the completion goes - // back to the JS thread, which finishes or releases the write there — - // the VM waits for this (embedded work) before its handle closes. - let loop_handle = this_ref.loop_handle().clone(); - if let Some(_vm) = loop_handle.borrow_if_running() { + // The stream reads and writes JS ArrayBuffer backing stores, alive + // while the ticket is held; skip the work once the VM is stopping. + // Either way the completion goes back to the JS thread, which finishes + // or releases the write there. + let ticket = this_ref + .ticket() + .take() + .expect("scheduled zlib write holds a ticket"); + if ticket.script_allowed() { this_ref.stream().with_mut(|s| s.do_work()); } - let ct = ConcurrentTask::create(Task::init(this)); - let bun_jsc::vm_handle::Posted::Queued = loop_handle.post_task(ct) else { - unreachable!("VM handle closed with an embedded zlib write outstanding"); - }; - // `this` may already be freed by the JS thread; the handle is ours. - loop_handle.embedded_work_finished(); + // `this` may be freed by the JS thread the moment this is queued. + ticket.post(ConcurrentTask::create(Task::init(this))); } /// VM teardown, JS thread, heap alive: a completion that was queued but @@ -1050,7 +1048,7 @@ macro_rules! __impl_compression_stream { type Stream = $ctx; #[inline] fn global_this(&self) -> &::bun_jsc::JSGlobalObject { self.global_this.get() } - #[inline] fn loop_handle(&self) -> &::bun_jsc::LoopHandle { &self.loop_handle } + #[inline] fn ticket(&self) -> &::core::cell::Cell> { &self.ticket } #[inline] fn stream(&self) -> &::bun_jsc::JsCell { &self.stream } #[inline] fn poll_ref(&self) -> &::bun_jsc::JsCell<$crate::node::node_zlib_binding::CountedKeepAlive> { &self.poll_ref } #[inline] fn this_value(&self) -> &::bun_jsc::JsCell<::bun_jsc::StrongOptional> { &self.this_value } diff --git a/src/runtime/node/zlib/NativeBrotli.rs b/src/runtime/node/zlib/NativeBrotli.rs index d43461514804..8b0a9642bba4 100644 --- a/src/runtime/node/zlib/NativeBrotli.rs +++ b/src/runtime/node/zlib/NativeBrotli.rs @@ -86,7 +86,7 @@ mod _impl { // centralises the single unsafe deref so the trait impl is safe. pub global_this: bun_ptr::BackRef, /// How the pool thread delivers a finished write to the VM. - pub loop_handle: bun_jsc::LoopHandle, + pub ticket: Cell>, pub stream: JsCell, pub poll_ref: JsCell, // TODO: Strong self-ref on the wrapper → JsRef per PORTING.md §JSC (Strong back-ref to own wrapper leaks) @@ -149,7 +149,7 @@ mod _impl { Ok(Box::new(Self { ref_count: Cell::new(1), global_this: bun_ptr::BackRef::new(global_this), - loop_handle: global_this.bun_vm().loop_handle(), + ticket: Cell::new(None), stream: JsCell::new(stream), poll_ref: JsCell::new(CountedKeepAlive::default()), this_value: JsCell::new(StrongOptional::empty()), diff --git a/src/runtime/node/zlib/NativeZlib.rs b/src/runtime/node/zlib/NativeZlib.rs index 12ebe68be172..e491c192cbcb 100644 --- a/src/runtime/node/zlib/NativeZlib.rs +++ b/src/runtime/node/zlib/NativeZlib.rs @@ -43,7 +43,7 @@ mod _impl { // centralises the single unsafe deref so the trait impl is safe. pub global_this: bun_ptr::BackRef, /// How the pool thread delivers a finished write to the VM. - pub loop_handle: bun_jsc::LoopHandle, + pub ticket: Cell>, pub stream: JsCell, pub poll_ref: JsCell, pub this_value: JsCell, // jsc.Strong.Optional @@ -97,7 +97,7 @@ mod _impl { Ok(Box::new(Self { ref_count: Cell::new(1), global_this: bun_ptr::BackRef::new(global), - loop_handle: global.bun_vm().loop_handle(), + ticket: Cell::new(None), stream: JsCell::new(stream), poll_ref: JsCell::new(CountedKeepAlive::default()), this_value: JsCell::new(StrongOptional::empty()), diff --git a/src/runtime/node/zlib/NativeZstd.rs b/src/runtime/node/zlib/NativeZstd.rs index b773775b847b..1ba8df873f61 100644 --- a/src/runtime/node/zlib/NativeZstd.rs +++ b/src/runtime/node/zlib/NativeZstd.rs @@ -42,7 +42,7 @@ mod _impl { // `BackRef` centralises the single unsafe deref so the trait impl is safe. pub global_this: bun_ptr::BackRef, /// How the pool thread delivers a finished write to the VM. - pub loop_handle: bun_jsc::LoopHandle, + pub ticket: Cell>, pub stream: JsCell, pub poll_ref: JsCell, pub this_value: JsCell, // jsc.Strong.Optional @@ -110,7 +110,7 @@ mod _impl { // JSC_BORROW — the JSGlobalObject outlives this payload (the C++ // wrapper is owned by that global's heap). global_this: bun_ptr::BackRef::new(global), - loop_handle: global.bun_vm().loop_handle(), + ticket: Cell::new(None), stream: JsCell::new(stream), poll_ref: JsCell::new(CountedKeepAlive::default()), this_value: JsCell::new(StrongOptional::empty()), diff --git a/src/runtime/shell/builtin/cp.rs b/src/runtime/shell/builtin/cp.rs index bb84b01e0054..8a7a39d19a51 100644 --- a/src/runtime/shell/builtin/cp.rs +++ b/src/runtime/shell/builtin/cp.rs @@ -485,7 +485,7 @@ impl ShellCpTask { pub(crate) unsafe fn cp_on_finish(this: *mut ShellCpTask, result: bun_sys::Maybe<()>) { // SAFETY: caller contract — JS thread, from the `ShellAsyncCpTask`'s // completion; `this` is live and ours. The pool side finished (and - // stopped counting) when it handed the copy to that task, so continue + // dropped its poster) when it handed the copy to that task, so continue // in place rather than bouncing through the concurrent queue again. unsafe { if let Err(e) = result { @@ -514,8 +514,7 @@ impl ShellCpTask { let st = &raw mut (*this).task; (*st).task.callback = Self::work_pool_callback; (*st).keep_alive.ref_((*st).event_loop.as_event_loop_ctx()); - // Counted until `ShellTask::on_finish` (see `ShellTask::schedule_no_ref`). - (*st).poster.embedded_work_scheduled(); + (*st).arm(); WorkPool::schedule(&raw mut (*st).task); } } @@ -533,15 +532,22 @@ impl ShellCpTask { task, ::TASK_OFFSET, ); - let poster = (*this).task.poster.clone(); - if let Some(e) = (*this).run_from_thread_pool_impl() { + // Moved out first: on success the copy is handed to a + // `ShellAsyncCpTask` whose completion may free `*this` at once. + let poster = (*this) + .task + .poster + .take() + .expect("shell cp task on the pool is armed"); + if let Some(e) = (*this).run_from_thread_pool_impl(&poster) { (*this).err = Some(e); + (*this).task.poster = Some(poster); Self::enqueue_to_event_loop(this); } else { - // The copy now belongs to a `ShellAsyncCpTask` (counted on its - // own, completes on the JS thread via `cp_on_finish`); this - // task's pool part is over. - poster.embedded_work_finished(); + // The copy now belongs to a `ShellAsyncCpTask` (holding its + // own poster, completing on the JS thread via `cp_on_finish`); + // this task's pool part is over. + drop(poster); } } } @@ -585,7 +591,10 @@ impl ShellCpTask { /// POSIX `cp` synopses /// (), then hands off to /// the node:fs async cp implementation. - fn run_from_thread_pool_impl(&mut self) -> Option { + fn run_from_thread_pool_impl( + &mut self, + poster: &bun_jsc::ConcurrentPoster, + ) -> Option { use resolve_path::{Platform, platform}; let mut buf2 = bun_paths::PathBuffer::uninit(); @@ -721,7 +730,7 @@ impl ShellCpTask { let _ = crate::node::fs::ShellAsyncCpTask::create_for_shell( args, self.task.event_loop, - self.task.poster.clone(), + poster.clone(), std::ptr::from_mut::(self), ); diff --git a/src/runtime/shell/builtin/rm.rs b/src/runtime/shell/builtin/rm.rs index ca6fc8be361e..8c4cde84e18d 100644 --- a/src/runtime/shell/builtin/rm.rs +++ b/src/runtime/shell/builtin/rm.rs @@ -710,8 +710,7 @@ impl ShellRmTask { let st = &raw mut (*this).task; (*st).task.callback = Self::work_pool_callback; (*st).keep_alive.ref_((*st).event_loop.as_event_loop_ctx()); - // Counted until `ShellTask::on_finish` (see `ShellTask::schedule_no_ref`). - (*st).poster.embedded_work_scheduled(); + (*st).arm(); WorkPool::schedule(&raw mut (*st).task); } } @@ -1491,18 +1490,19 @@ impl DirTask { ShellRmTask::decr_pending_and_maybe_deinit(tm); return; } - let poster = (*me.task_manager).task.poster.clone(); + // The root rm task is still out (pending > 0), so its poster is set. + let poster = (*me.task_manager) + .task + .poster + .as_ref() + .expect("rm root task on the pool is armed") + .clone(); (me, poster) }; match &mut me.concurrent_task { EventLoopTask::Js(ct) => { ct.from(this, AutoDeinit::ManualDeinit); - // Posted while the rm task is counted work: the VM has not closed. - let bun_jsc::vm_handle::Posted::Queued = - poster.post_js(core::ptr::NonNull::from(ct)) - else { - unreachable!("VM handle closed with shell rm work outstanding"); - }; + poster.post_js(core::ptr::NonNull::from(ct)); } EventLoopTask::Mini(at) => { let at = at.from(this, dir_task_run_from_main_thread_mini); diff --git a/src/runtime/shell/builtin/yes.rs b/src/runtime/shell/builtin/yes.rs index 5b97cb46616b..b22ec9934a64 100644 --- a/src/runtime/shell/builtin/yes.rs +++ b/src/runtime/shell/builtin/yes.rs @@ -5,7 +5,6 @@ use crate::shell::io_writer::{ChildPtr, WriterTag}; use crate::shell::states::cmd::Exec; use crate::shell::yield_::Yield; -use bun_event_loop::ConcurrentTask::AutoDeinit; use bun_event_loop::{EventLoopTask, TaskTag, Taskable, task_tag}; #[derive(Clone, Copy, PartialEq, Eq, Default)] @@ -217,14 +216,9 @@ impl YesTask { // backrefs (single-threaded shell). unsafe { match (*this).evtloop { + // Next loop iteration, after I/O has had a turn. EventLoopHandle::Js { owner } => { - owner.tick(); - let ct = core::ptr::NonNull::from(match &mut (*this).concurrent_task { - EventLoopTask::Js(ct) => ct.from(this, AutoDeinit::ManualDeinit), - EventLoopTask::Mini(_) => unreachable!(), - }); - // Same-thread bounce on the loop's own thread: always accepted. - let _ = owner.js_poster().post(ct); + owner.enqueue_task_after_yield(bun_jsc::Task::init(this)); } EventLoopHandle::Mini(mut mini) => { (*mini.loop_).tick(); diff --git a/src/runtime/shell/dispatch_tasks.rs b/src/runtime/shell/dispatch_tasks.rs index 696202ec1274..7a8d1fc7b65a 100644 --- a/src/runtime/shell/dispatch_tasks.rs +++ b/src/runtime/shell/dispatch_tasks.rs @@ -9,8 +9,6 @@ //! resume the parent state via NodeId). use crate::shell::interpreter::{Interpreter, NodeId, ShellTask}; -use bun_jsc::ConcurrentTask::ConcurrentTask; - /// Task payload for [`ShellAsync`](crate::shell::states::r#async::Async)'s /// bounce back to the main thread. The state lives in `interp.nodes`, so /// the enqueued payload is `(interp, node)`. @@ -18,7 +16,6 @@ use bun_jsc::ConcurrentTask::ConcurrentTask; pub(crate) struct ShellAsyncTask { pub interp: *mut Interpreter, pub node: NodeId, - pub concurrent_task: ConcurrentTask, } /// Stat task backing shell conditional expressions (`[ -f x ]` etc.). Wraps an diff --git a/src/runtime/shell/interpreter.rs b/src/runtime/shell/interpreter.rs index 842e5cfdf689..226abbb97188 100644 --- a/src/runtime/shell/interpreter.rs +++ b/src/runtime/shell/interpreter.rs @@ -2590,8 +2590,11 @@ pub struct ShellTask { /// no-op`). pub task: WorkPoolTask, pub(crate) event_loop: EventLoopHandle, - /// How the pool thread bounces the task back to its owning loop. - pub(crate) poster: bun_jsc::ConcurrentPoster, + /// How the pool thread bounces the task back to its owning loop; held only + /// while the task is out on the pool (`arm` until `on_finish`). For a JS + /// loop this is the ticket its VM waits for: the context lives in + /// interpreter state a JS wrapper may own. + pub(crate) poster: Option, pub(crate) keep_alive: bun_io::KeepAlive, /// Back-ref to the owning [`Interpreter`]. The high-tier dispatch /// (`runtime::dispatch::run_task`) recovers `&mut Interpreter` from this @@ -2614,13 +2617,19 @@ impl ShellTask { /// A subtask created on a pool thread (`ls -R` discovering a directory): /// it reports to the same loop as `parent`, whose poster was captured on /// the JS thread — nothing here derives one from the VM. + #[track_caller] pub(crate) fn new_child(parent: &ShellTask) -> Self { ShellTask { task: WorkPoolTask { node: Default::default(), callback: shell_task_unset_callback, }, - poster: parent.poster.clone(), + // Spelled out so `#[track_caller]` names this site, not `Option::clone`. + #[allow(clippy::manual_map)] + poster: match &parent.poster { + Some(p) => Some(p.clone()), + None => None, + }, event_loop: parent.event_loop, keep_alive: Default::default(), interp: core::ptr::null_mut(), @@ -2628,7 +2637,6 @@ impl ShellTask { } } - /// JS thread (the interpreter's): derives the poster for `event_loop`. pub(crate) fn new(event_loop: EventLoopHandle) -> Self { ShellTask { task: WorkPoolTask { @@ -2637,7 +2645,7 @@ impl ShellTask { // fires if a caller forgets the `` (debug-asserted there). callback: shell_task_unset_callback, }, - poster: bun_jsc::ConcurrentPoster::from_event_loop_handle(&event_loop), + poster: None, event_loop, keep_alive: Default::default(), interp: core::ptr::null_mut(), @@ -2677,14 +2685,23 @@ impl ShellTask { unsafe { let this = ctx.byte_add(C::TASK_OFFSET).cast::(); (*this).task.callback = shell_task_trampoline::; - // The context lives in interpreter state a JS wrapper may own: - // counted until `on_finish`, so that VM waits for it (see - // `VmHandle::embedded_work_scheduled`). - (*this).poster.embedded_work_scheduled(); + (*this).arm(); WorkPool::schedule(&raw mut (*this).task); } } + /// About to leave the owning thread: take the poster (for a JS loop, a + /// ticket on its VM) unless a pool-thread parent already handed one down + /// (`new_child`). Owning thread otherwise. + #[track_caller] + pub(crate) fn arm(&mut self) { + if self.poster.is_none() { + self.poster = Some(bun_jsc::ConcurrentPoster::from_event_loop_handle( + &self.event_loop, + )); + } + } + /// Called from the worker /// thread once `C::run_from_thread_pool` returns; enqueues the embedded /// concurrent task so the main thread re-enters via @@ -2705,17 +2722,16 @@ impl ShellTask { // this thread until the enqueue below. unsafe { let this = ctx.byte_add(C::TASK_OFFSET).cast::(); - let poster = (*this).poster.clone(); + // Moved out: the owning thread may free `*this` once it is queued. + let poster = (*this) + .poster + .take() + .expect("shell task on the pool is armed"); match &mut (*this).concurrent_task { EventLoopTask::Js(ct) => { // Tag resolved via `C: Taskable`. ct.from(ctx, AutoDeinit::ManualDeinit); - // Counted work: the VM has not closed its handle. - let bun_jsc::vm_handle::Posted::Queued = - poster.post_js(core::ptr::NonNull::from(ct)) - else { - unreachable!("VM handle closed with shell pool work outstanding"); - }; + poster.post_js(core::ptr::NonNull::from(ct)); } EventLoopTask::Mini(at) => { // Pass the monomorphised callback explicitly. @@ -2723,9 +2739,7 @@ impl ShellTask { poster.post_mini(core::ptr::NonNull::new(at).expect("intrusive task")); } } - // The pool side is done with this context (`this` may already be - // freed by the main thread; the poster is ours). - poster.embedded_work_finished(); + drop(poster); } } diff --git a/src/runtime/shell/states/Async.rs b/src/runtime/shell/states/Async.rs index 91774346209e..17ae8edde377 100644 --- a/src/runtime/shell/states/Async.rs +++ b/src/runtime/shell/states/Async.rs @@ -57,7 +57,6 @@ impl Async { bun_core::heap::alloc(crate::shell::dispatch_tasks::ShellAsyncTask { interp: interp.as_ctx_ptr(), node: id, - concurrent_task: Default::default(), }); id } @@ -150,34 +149,25 @@ impl Async { /// Bounce `run_from_main_thread` through the event loop so the async body runs on subsequent ticks while the /// parent proceeds. fn enqueue_self(interp: &Interpreter, this: NodeId) { - use bun_event_loop::ConcurrentTask::AutoDeinit; let me = interp.as_async_mut(this); let task = me.task; debug_assert!(!task.is_null()); - // Same-thread "next tick" bounce through the owning loop's concurrent queue. - let poster = bun_jsc::ConcurrentPoster::from_event_loop_handle(&me.event_loop); match me.event_loop { - EventLoopHandle::Js { .. } => { - // SAFETY: `task` is the live heap payload allocated in `init` - // and freed only in `actually_deinit`. The embedded - // `ConcurrentTask` is reused for each bounce and is never - // in-flight twice: every enqueue is dispatched (dequeued) - // before the state machine can enqueue again. - unsafe { - let ct = (*task).concurrent_task.from(task, AutoDeinit::ManualDeinit); - // This runs on the VM's own thread while it executes, so the - // post is always accepted. - let _ = poster.post_js(core::ptr::NonNull::from(ct)); - } + // Next loop iteration, after I/O has had a turn. `task` is the live + // heap payload allocated in `init` and freed only in `actually_deinit`. + EventLoopHandle::Js { owner } => { + owner.enqueue_task_after_yield(bun_jsc::Task::init(task)) } - EventLoopHandle::Mini(_) => { + EventLoopHandle::Mini(mut mini) => { // The payload embeds only the JS-arm `ConcurrentTask`, so the // mini arm heap-allocates an auto-deinit wrapper per bounce. let any = bun_jsc::AnyTaskWithExtraContext::AnyTaskWithExtraContext::from_callback_auto_deinit( task, run_from_main_thread_mini, ); - poster.post_mini(core::ptr::NonNull::new(any).expect("heap task")); + // SAFETY: the shell's own mini loop, on its thread. + unsafe { mini.get_mut() } + .enqueue_task_concurrent(core::ptr::NonNull::new(any).expect("heap task")); } } } diff --git a/src/runtime/webcore/Blob.rs b/src/runtime/webcore/Blob.rs index eb149d320b7a..760b68c48a8e 100644 --- a/src/runtime/webcore/Blob.rs +++ b/src/runtime/webcore/Blob.rs @@ -54,6 +54,9 @@ pub use store::{Store, StoreRef}; #[path = "blob/copy_file.rs"] pub mod copy_file; +#[cfg(not(windows))] +#[path = "blob/io_parking.rs"] +pub(crate) mod io_parking; #[path = "blob/read_file.rs"] pub mod read_file; #[path = "blob/write_file.rs"] @@ -7032,6 +7035,9 @@ pub trait FileOpener: Sized { } } +#[cfg(not(windows))] +pub(crate) use io_parking::IoParking; + // TODO: move to bun_sys? pub trait FileCloser: Sized { const IO_TAG: bun_io::Tag; diff --git a/src/runtime/webcore/CompressionStreamCoder.rs b/src/runtime/webcore/CompressionStreamCoder.rs index 4f09b3658456..2115bcc30545 100644 --- a/src/runtime/webcore/CompressionStreamCoder.rs +++ b/src/runtime/webcore/CompressionStreamCoder.rs @@ -811,7 +811,7 @@ impl Drop for CompressionAsyncCtx { fn drop(&mut self) { // SAFETY: `coder` was ref'd in `CompressionStreamCoder__transformAsync`; this ctx owns that // reference and drops it exactly once (in `then`, or when the job is - // released unrun / its off-thread part finishes after the VM is gone). + // released unrun). unsafe { bun_ptr::ThreadSafeRefCount::::deref(self.coder) }; } } @@ -834,11 +834,7 @@ impl bun_jsc::JobContext for CompressionAsyncCtx { type OffThread = Self; type Js = CompressionAsyncJs; - fn run( - this: &mut Self, - _vm: &bun_jsc::vm_handle::Borrow, - done: bun_jsc::Completion, - ) -> Option> { + fn run(this: &mut Self, done: bun_jsc::Completion) -> Option> { // SAFETY: `coder` is kept alive by the reference this ctx holds (the // cell's finalizer only releases its own); see the field doc. this.error = unsafe { (*this.coder).transform(this.input.slice(), this.finish) }.err(); diff --git a/src/runtime/webcore/blob/copy_file.rs b/src/runtime/webcore/blob/copy_file.rs index a4b016417992..427d94140e94 100644 --- a/src/runtime/webcore/blob/copy_file.rs +++ b/src/runtime/webcore/blob/copy_file.rs @@ -75,11 +75,7 @@ unsafe impl Send for CopyFile {} impl jsc::JobContext for CopyFile { type OffThread = Self; type Js = jsc::JSPromiseStrong; - fn run( - this: &mut Self, - _vm: &jsc::vm_handle::Borrow, - done: bun_jsc::Completion, - ) -> Option> { + fn run(this: &mut Self, done: bun_jsc::Completion) -> Option> { this.run_async(); Some(done) } @@ -1071,8 +1067,6 @@ pub struct CopyFileWindows<'a> { // TODO(refactor): lifetime — heap-allocated and re-entered from libuv callbacks; // likely should be *const jsc::EventLoop. pub(crate) event_loop: &'a jsc::event_loop::EventLoop, - /// How the mkdirp pool completion gets back to the VM. - pub(crate) loop_handle: jsc::LoopHandle, pub(crate) size: SizeType, @@ -1378,7 +1372,6 @@ impl<'a> CopyFileWindows<'a> { promise: jsc::JSPromiseStrong::init(global), // SAFETY: all-zero is a valid libuv::fs_t io_request: bun_core::ffi::zeroed::(), - loop_handle: jsc::VirtualMachine::VirtualMachine::get().loop_handle(), event_loop, mkdirp_if_not_exists, destination_mode, @@ -1799,7 +1792,8 @@ impl<'a> CopyFileWindows<'a> { completion: on_mkdirp_complete_concurrent, completion_ctx: core::ptr::from_mut(self).cast::<()>(), path, - ..Default::default() + ticket: jsc::VirtualMachine::VirtualMachine::get().ticket(), + task: Default::default(), }); } @@ -1909,7 +1903,7 @@ extern "C" fn on_chmod(req: *mut libuv::fs_t) { } #[cfg(windows)] -fn on_mkdirp_complete_concurrent(ctx: *mut (), err_: bun_sys::Maybe<()>) { +fn on_mkdirp_complete_concurrent(ctx: *mut (), err_: bun_sys::Maybe<()>, ticket: &jsc::Ticket) { bun_sys::syslog!("mkdirp complete"); // SAFETY: `ctx` is the `*mut CopyFileWindows` stored in `AsyncMkdirp.completion_ctx` // by `mkdirp` above; sole owner on this concurrent path. @@ -1927,15 +1921,9 @@ fn on_mkdirp_complete_concurrent(ctx: *mut (), err_: bun_sys::Maybe<()>) { unsafe { (*this).on_mkdirp_complete() }; Ok(()) } - let ct = jsc::ConcurrentTask::create(jsc::ManagedTask::ManagedTask::new::( - this, - call_erased, + ticket.post(jsc::ConcurrentTask::create( + jsc::ManagedTask::ManagedTask::new::(this, call_erased), )); - if let jsc::vm_handle::Posted::Refused(ct) = this.loop_handle.post_task(ct) { - // VM torn down: nobody will settle the promise; free the hop. - // SAFETY: refused ⇒ we own the task box. - unsafe { bun_event_loop::ConcurrentTask::ConcurrentTask::release_refused(ct) }; - } } // ─────────────────────────────────────────────────────────────────────────── diff --git a/src/runtime/webcore/blob/io_parking.rs b/src/runtime/webcore/blob/io_parking.rs new file mode 100644 index 000000000000..e2f9ec9f49fa --- /dev/null +++ b/src/runtime/webcore/blob/io_parking.rs @@ -0,0 +1,88 @@ +//! Ownership handshake for a `ReadFile`/`WriteFile` parked on the io loop — +//! see [`IoParking`]. + +use core::sync::atomic::{AtomicU8, Ordering}; + +/// Who currently owns a `ReadFile`/`WriteFile` that may park on the io +/// loop waiting for a pipe/tty/socket to become ready — the one wait a +/// `Bun.file` read or write can be stuck on indefinitely, and so the one +/// its VM's stop phase must be able to end +/// ([`bun_jsc::JobContext::cancel`]). All transitions are +/// compare-exchanges on one byte, so the JS thread's cancel, the io +/// thread's arm/fire, and the pool thread's park agree on who completes +/// the job, exactly once. Cancellation is sticky: once cancelled the job +/// never parks again. +pub(crate) struct IoParking(AtomicU8); + +/// A pool thread has it (running, or about to). +const IDLE: u8 = 0; +/// Its wait request is queued for the io thread, not yet processed. +const PARKED: u8 = 1; +/// The io thread registered its poll; readiness sends it back to the pool. +const ARMED: u8 = 2; +/// Cancelled while parked/armed: the io thread closes it out. +const CANCELLED: u8 = 3; +/// Cancelled while a pool thread had it: that thread fails it at its next +/// attempt to park. +const DOOMED: u8 = 4; + +impl IoParking { + pub(crate) const fn new() -> Self { + Self(AtomicU8::new(IDLE)) + } + + /// Pool thread, before queuing the wait request: `false` ⇒ cancelled + /// already — fail the operation instead of parking. + pub(crate) fn park(&self) -> bool { + self.0 + .compare_exchange(IDLE, PARKED, Ordering::SeqCst, Ordering::SeqCst) + .is_ok() + } + + /// io thread, processing the wait request: `true` ⇒ register the poll; + /// `false` ⇒ cancelled meanwhile — close it out instead. + pub(crate) fn arm(&self) -> bool { + let r = self + .0 + .compare_exchange(PARKED, ARMED, Ordering::SeqCst, Ordering::SeqCst); + debug_assert!( + matches!(r, Ok(_) | Err(CANCELLED)), + "io wait request processed while not parked ({r:?})" + ); + r.is_ok() + } + + /// io thread, the poll fired or errored: `true` ⇒ hand back to the + /// pool; `false` ⇒ cancelled meanwhile — do nothing, the re-queued + /// wait request closes it out. + pub(crate) fn fire(&self) -> bool { + self.0 + .compare_exchange(ARMED, IDLE, Ordering::SeqCst, Ordering::SeqCst) + .is_ok() + } + + /// JS thread (the VM's stop phase): cancel, whichever thread has it. + /// `true` ⇒ the poll was registered: re-queue the wait request so the + /// io thread sees the cancellation (it is not queued, and no other + /// thread queues it once cancelled). `false` ⇒ nothing more to do + /// here: the still-queued wait request will see the flag, or the pool + /// thread that has the job fails it when it next tries to park (or + /// just finishes). + pub(crate) fn cancel(&self) -> bool { + loop { + let cur = self.0.load(Ordering::SeqCst); + let next = match cur { + IDLE => DOOMED, + PARKED | ARMED => CANCELLED, + _ => return false, + }; + if self + .0 + .compare_exchange(cur, next, Ordering::SeqCst, Ordering::SeqCst) + .is_ok() + { + return cur == ARMED; + } + } + } +} diff --git a/src/runtime/webcore/blob/read_file.rs b/src/runtime/webcore/blob/read_file.rs index 384e91115a61..001f62b09f26 100644 --- a/src/runtime/webcore/blob/read_file.rs +++ b/src/runtime/webcore/blob/read_file.rs @@ -220,15 +220,12 @@ pub type ReadFileTask = bun_jsc::Completion; unsafe impl Send for ReadFile {} impl bun_jsc::JobContext for ReadFile { + const CANCELLABLE: bool = cfg!(not(windows)); type OffThread = Self; - /// Where the bytes go: completed by `then`, or cancelled when the VM releases the JS sides of - /// its live jobs at teardown (a refused or unrun read then frees only this off-thread part). + /// Where the bytes go: completed by `then`, or cancelled (its `Drop`) when the job comes + /// back to a VM that is no longer running script and is released unrun. type Js = ReadFileCompletionFns; - fn run( - this: &mut Self, - _vm: &bun_jsc::vm_handle::Borrow, - done: bun_jsc::Completion, - ) -> Option> { + fn run(this: &mut Self, done: bun_jsc::Completion) -> Option> { // Starts the read; finishes from the io loop via the token. this.run(done); None @@ -240,6 +237,19 @@ impl bun_jsc::JobContext for ReadFile { ) -> jsc::JsResult<()> { Ok(ReadFile::then(this, completion, cx.global())?) } + /// A read parked on a pipe/tty that never becomes readable is the one + /// state this job can be stuck in; end that wait (the read fails with + /// ECANCELED through the usual close path). + #[cfg(not(windows))] + unsafe fn cancel(this: *mut Self) { + // SAFETY: fn contract; `io_parking` is atomic, and a `true` means no + // other thread touches `io_request` until it is queued again here. + unsafe { + if (*this).io_parking.cancel() { + io::IoRequestLoop::schedule(&mut (*this).io_request); + } + } + } } #[cfg(not(windows))] @@ -285,6 +295,8 @@ pub struct ReadFile { pub(crate) io_poll: io::Poll, pub(crate) io_request: io::Request, #[cfg(not(windows))] + pub(crate) io_parking: super::IoParking, + #[cfg(not(windows))] pub(crate) could_block: bool, pub(crate) close_after_io: bool, pub(crate) state: AtomicU8, // ClosingState @@ -380,6 +392,7 @@ impl ReadFile { callback: Self::on_request_readable, scheduled: false, }, + io_parking: super::IoParking::new(), could_block: false, close_after_io: false, state: AtomicU8::new(ClosingState::Running as u8), @@ -392,6 +405,10 @@ impl ReadFile { pub fn on_ready(&mut self) { bloblog!("ReadFile.onReady"); + #[cfg(not(windows))] + if !self.io_parking.fire() { + return; + } self.task = WorkPoolTask { node: Default::default(), callback: Self::do_read_loop_task, @@ -409,6 +426,10 @@ impl ReadFile { pub(crate) fn on_io_error(&mut self, err: &bun_sys::Error) { bloblog!("ReadFile.onIOError"); + #[cfg(not(windows))] + if !self.io_parking.fire() { + return; + } self.errno = Some(bun_errno::from_errno(err.errno as i32).into()); self.system_error = Some(err.to_system_error().into()); self.task = WorkPoolTask { @@ -444,6 +465,10 @@ impl ReadFile { std::ptr::from_mut::(request) )) }; + if !this.io_parking.arm() { + this.fail_cancelled(); + return ::schedule_close(request); + } io::Action::Readable(FileAction { on_error: Self::on_io_error_thunk, ctx: std::ptr::from_mut::(this).cast::<()>(), @@ -453,15 +478,32 @@ impl ReadFile { }) } + /// The wait was cancelled (io thread: while parked — the close path that + /// follows finishes the job; pool thread: before it could park — the + /// caller finishes it): fail with ECANCELED. + #[cfg(not(windows))] + fn fail_cancelled(&mut self) { + let err = bun_sys::Error::from_code(bun_sys::E::ECANCELED, bun_sys::Tag::read); + self.errno = Some(bun_errno::from_errno(err.errno as i32).into()); + self.system_error = Some(err.to_system_error().into()); + self.state + .store(ClosingState::Closing as u8, Ordering::SeqCst); + } + + /// Pool thread: park on the io loop until the fd is readable (or finish + /// now if the VM cancelled the job meanwhile). The caller returns without + /// touching `self` again: from here the io thread has it. #[cfg(not(windows))] pub(crate) fn wait_for_readable(&mut self) { bloblog!("ReadFile.waitForReadable"); + if !self.io_parking.park() { + self.fail_cancelled(); + return self.on_finish(); + } self.close_after_io = true; self.io_request .store_callback_seq_cst(Self::on_request_readable); - if !self.io_request.scheduled { - io::IoRequestLoop::schedule(&mut self.io_request); - } + io::IoRequestLoop::schedule(&mut self.io_request); } /// Pick the read target: `buffer`'s spare capacity if it is at least as diff --git a/src/runtime/webcore/blob/write_file.rs b/src/runtime/webcore/blob/write_file.rs index 42f384894d8a..ad89ea4fd244 100644 --- a/src/runtime/webcore/blob/write_file.rs +++ b/src/runtime/webcore/blob/write_file.rs @@ -28,6 +28,16 @@ bun_output::declare_scope!(WriteFile, hidden); // as a plain Rust enum: it only ever travels through the Rust fn-pointer // callbacks below (`WriteFileOnWriteFileCallback`), never across FFI, so the // layout is unconstrained. +/// One `write()` attempt on the pool thread. +#[cfg(not(windows))] +pub(crate) enum WriteStep { + Wrote(usize), + /// A pipe/socket is full: park on the io loop. + WouldBlock, + /// `errno`/`system_error` are set. + Failed, +} + pub enum WriteFileResultType { Result(SizeType), Err(Box), @@ -45,14 +55,11 @@ pub type WriteFileTask = bun_jsc::Completion; unsafe impl Send for WriteFile {} impl bun_jsc::JobContext for WriteFile { + const CANCELLABLE: bool = cfg!(not(windows)); type OffThread = Self; /// The completion is delivered through `on_complete_callback(ctx, ..)`. type Js = (); - fn run( - this: &mut Self, - _vm: &bun_jsc::vm_handle::Borrow, - done: bun_jsc::Completion, - ) -> Option> { + fn run(this: &mut Self, done: bun_jsc::Completion) -> Option> { // Starts the write; finishes from the io loop via the token. this.run(done); None @@ -60,6 +67,17 @@ impl bun_jsc::JobContext for WriteFile { fn then(this: Self, _: (), cx: &bun_jsc::JsThread<'_>) -> jsc::JsResult<()> { Ok(WriteFile::then(this, cx.global())?) } + /// As `ReadFile`: a write parked on a full pipe nobody drains is the one + /// state this job can be stuck in. + #[cfg(not(windows))] + unsafe fn cancel(this: *mut Self) { + // SAFETY: fn contract; see `ReadFile::cancel`. + unsafe { + if (*this).io_parking.cancel() { + io::IoRequestLoop::schedule(&mut (*this).io_request); + } + } + } } impl WriteFile { @@ -83,6 +101,8 @@ pub struct WriteFile { pub(crate) io_task: Option, pub(crate) io_poll: io::Poll, pub(crate) io_request: io::Request, + #[cfg(not(windows))] + pub(crate) io_parking: super::IoParking, pub(crate) state: AtomicU8, // ClosingState pub(crate) on_complete_ctx: *mut c_void, @@ -181,6 +201,10 @@ impl WriteFile { pub fn on_ready(&mut self) { bun_output::scoped_log!(WriteFile, "WriteFile.onReady()"); + #[cfg(not(windows))] + if !self.io_parking.fire() { + return; + } self.task = WorkPoolTask { node: Default::default(), callback: Self::do_write_loop_task, @@ -192,6 +216,10 @@ impl WriteFile { bun_output::scoped_log!(WriteFile, "WriteFile.onIOError()"); // SAFETY: ctx was set to `self as *mut WriteFile` in `on_request_writable`. let this = unsafe { bun_ptr::callback_ctx::(this.cast()) }; + #[cfg(not(windows))] + if !this.io_parking.fire() { + return; + } this.errno = Some(bun_errno::from_errno(err.errno as i32).into()); this.system_error = Some(err.to_system_error().into()); this.task = WorkPoolTask { @@ -207,6 +235,12 @@ impl WriteFile { request.scheduled = false; // SAFETY: `request` points to WriteFile.io_request (intrusive); recover parent. let this = unsafe { WriteFile::from_io_request(std::ptr::from_mut(request)) }; + // SAFETY: `this` is the live parent (see above); io thread owns it while parked. + if !unsafe { (*this).io_parking.arm() } { + // SAFETY: as above. + unsafe { (*this).fail_cancelled() }; + return ::schedule_close(request); + } // SAFETY: `request` points to WriteFile.io_request (intrusive), so `this` is the // live parent; `fd` copy and the `io_poll` field borrow are the only borrows formed. let (fd, poll) = unsafe { ((*this).opened_fd, &mut (*this).io_poll) }; @@ -219,14 +253,28 @@ impl WriteFile { }) } + /// See `ReadFile::fail_cancelled`. + #[cfg(not(windows))] + fn fail_cancelled(&mut self) { + let err = sys::Error::from_code(sys::E::ECANCELED, sys::Tag::write); + self.errno = Some(bun_errno::from_errno(err.errno as i32).into()); + self.system_error = Some(err.to_system_error().into()); + self.state + .store(ClosingState::Closing as u8, Ordering::SeqCst); + } + + /// See `ReadFile::wait_for_readable`: the caller returns without touching + /// `self` again. #[cfg(not(windows))] pub(crate) fn wait_for_writable(&mut self) { + if !self.io_parking.park() { + self.fail_cancelled(); + return self.on_finish(); + } self.close_after_io = true; self.io_request .store_callback_seq_cst(Self::on_request_writable); - if !self.io_request.scheduled { - io::IoRequestLoop::schedule(&mut self.io_request); - } + io::IoRequestLoop::schedule(&mut self.io_request); } #[cfg(not(windows))] @@ -250,6 +298,8 @@ impl WriteFile { io_task: None, io_poll: io::Poll::default(), io_request: io::Request::new(Self::on_request_writable), + #[cfg(not(windows))] + io_parking: super::IoParking::new(), state: AtomicU8::new(ClosingState::Running as u8), on_complete_ctx: on_write_file_context, on_complete_callback, @@ -287,7 +337,7 @@ impl WriteFile { // reshaped for borrowck — take (off, len) here and re-derive the slice // internally so callers don't hold a borrow of self across the &mut self call. #[cfg(not(windows))] - pub(crate) fn do_write(&mut self, off: usize, len: usize, wrote: &mut usize) -> bool { + pub(crate) fn do_write(&mut self, off: usize, len: usize) -> WriteStep { let fd = self.opened_fd; debug_assert!(fd != Fd::INVALID); @@ -296,35 +346,23 @@ impl WriteFile { // // On macOS, it is an error to use pwrite() on a // non-seekable file. - let result: bun_sys::Result = - sys::write(fd, &self.bytes_blob.shared_view()[off..off + len]); - loop { - match &result { - bun_sys::Result::Ok(res) => { - *wrote = *res; - self.total_written += *res; + match sys::write(fd, &self.bytes_blob.shared_view()[off..off + len]) { + Ok(wrote) => { + self.total_written += wrote; + return WriteStep::Wrote(wrote); } - bun_sys::Result::Err(err) => { - if err.get_errno() == io::RETRY { - if !self.could_block { - // regular files cannot use epoll. - // this is fine on kqueue, but not on epoll. - continue; - } - self.wait_for_writable(); - return false; - } else { - self.errno = Some(bun_errno::from_errno(err.errno as i32).into()); - self.system_error = Some(err.to_system_error().into()); - return false; - } + // regular files cannot use epoll. + // this is fine on kqueue, but not on epoll. + Err(err) if err.get_errno() == io::RETRY && !self.could_block => continue, + Err(err) if err.get_errno() == io::RETRY => return WriteStep::WouldBlock, + Err(err) => { + self.errno = Some(bun_errno::from_errno(err.errno as i32).into()); + self.system_error = Some(err.to_system_error().into()); + return WriteStep::Failed; } } - break; } - - true } pub(crate) fn then(mut this: WriteFile, _global: &JSGlobalObject) -> Result<(), JsTerminated> { @@ -507,18 +545,11 @@ impl WriteFile { let remain_len = remain_full.len() - off; if remain_len > 0 && self.errno.is_none() { - let mut wrote: usize = 0; - let continue_writing = self.do_write(off, remain_len, &mut wrote); - if !continue_writing { - // Stop writing, we errored - if self.errno.is_some() { - self.on_finish(); - return; - } - - // Stop writing, we need to wait for it to become writable. - return; - } + let wrote = match self.do_write(off, remain_len) { + WriteStep::Wrote(n) => n, + WriteStep::WouldBlock => return self.wait_for_writable(), + WriteStep::Failed => return self.on_finish(), + }; // Do not immediately attempt to write again if it's not a regular file. if self.could_block @@ -580,8 +611,6 @@ mod windows_impl { pub(crate) err: Option, pub(crate) total_written: usize, pub(crate) event_loop: *mut EventLoop, - /// How the mkdirp pool completion gets back to the VM. - pub(crate) loop_handle: bun_jsc::LoopHandle, pub poll_ref: KeepAlive, pub(crate) owned_fd: bool, @@ -639,7 +668,6 @@ mod windows_impl { base: null_mut(), len: 0, }], - loop_handle: bun_jsc::virtual_machine::VirtualMachine::get().loop_handle(), event_loop, fd: -1, err: None, @@ -923,7 +951,8 @@ mod windows_impl { path: bun_core::dirname(path) // this shouldn't happen .unwrap_or(path) as *const [u8], - ..Default::default() + ticket: bun_jsc::virtual_machine::VirtualMachine::get().ticket(), + task: Default::default(), }); } @@ -965,7 +994,11 @@ mod windows_impl { Ok(()) } - fn on_mkdirp_complete_concurrent(ctx: *mut (), err_: bun_sys::Result<()>) { + fn on_mkdirp_complete_concurrent( + ctx: *mut (), + err_: bun_sys::Result<()>, + ticket: &bun_jsc::Ticket, + ) { // SAFETY: `ctx` is the `*mut Self` stored in `AsyncMkdirp.completion_ctx` // by `mkdirp` above; sole owner on this concurrent path. let this = unsafe { bun_ptr::callback_ctx::(ctx.cast()) }; @@ -975,17 +1008,9 @@ mod windows_impl { bun_sys::Result::Err(e) => Some(e), bun_sys::Result::Ok(()) => None, }; - let ct = ConcurrentTask::create(ManagedTask::new::( - this, - Self::on_mkdirp_complete_task, + ticket.post(ConcurrentTask::create( + ManagedTask::new::(this, Self::on_mkdirp_complete_task), )); - if let bun_jsc::vm_handle::Posted::Refused(ct) = this.loop_handle.post_task(ct) { - // VM torn down: nobody will settle the promise. Free the hop (the - // ConcurrentTask owns the boxed ManagedTask); the operation's - // buffers/fd go with the process's teardown of its owner. - // SAFETY: refused ⇒ we own the task box. - unsafe { bun_event_loop::ConcurrentTask::ConcurrentTask::release_refused(ct) }; - } } extern "C" fn on_write_complete(req: *mut uv::fs_t) { diff --git a/src/runtime/webcore/fetch/FetchTasklet.rs b/src/runtime/webcore/fetch/FetchTasklet.rs index 1e8ea8278f0c..4c26be67f477 100644 --- a/src/runtime/webcore/fetch/FetchTasklet.rs +++ b/src/runtime/webcore/fetch/FetchTasklet.rs @@ -17,7 +17,6 @@ use bun_http::{ }; use bun_io::KeepAlive; use bun_jsc::debugger::AsyncTaskTracker; -use bun_jsc::virtual_machine::VirtualMachine; use bun_jsc::{ self as jsc, GlobalRef, JSGlobalObject, JSValue, JsResult, StringJsc, StrongOptional, }; @@ -69,8 +68,9 @@ impl FetchTaskletDeinitHop { impl Taskable for FetchTasklet { const TAG: bun_event_loop::TaskTag = bun_event_loop::task_tag::FetchTasklet; /// A progress hop the HTTP thread posted: it carries the +1 that - /// `on_progress_update` would have dropped. The HTTP thread is parked / - /// this VM's requests are back, so a 1→0 here deinits against a live heap. + /// `on_progress_update` would have dropped. The HTTP thread's own +1 is + /// released only after its last touch of the tasklet, so a 1→0 here means + /// it is done with it, and this runs on the JS thread with the heap alive. unsafe fn release_unrun(this: *mut Self) { FetchTasklet::deref(this); } @@ -95,9 +95,11 @@ pub struct FetchTasklet { pub(crate) http: Option>>, pub(crate) result: HTTPClientResult<'static>, pub(crate) metadata: Option, - /// How the HTTP thread reaches the VM (post progress/deinit tasks). JS-thread - /// code uses the VM through `global_this` instead. - pub(crate) loop_handle: jsc::LoopHandle, + /// Held while the request is out on the HTTP thread (`queue` until its + /// final callback / `release_at_shutdown`): how that thread posts progress + /// and deinit tasks, and what makes the VM wait for it. JS-thread code uses + /// the VM through `global_this` instead and never touches this. + pub(crate) http_ticket: Option, pub global_this: GlobalRef, pub(crate) request_body: HTTPRequestBody, // ThreadSafeStreamBuffer is intrusively refcounted (`ref_count: AtomicU32`, @@ -287,6 +289,8 @@ impl HTTPRequestBody { } impl FetchTasklet { + const HOLDS_TICKET: &str = "fetch on the HTTP thread holds a ticket"; + // ───── raw-ptr field accessors (centralised unsafe) ─────────────────── // // `signal` / `sink` / `native_response` are intrusive-refcounted heap @@ -329,13 +333,6 @@ impl FetchTasklet { unsafe { &*this } } - /// HTTP thread → JS thread: queue `task` on the tasklet's VM. Returns the - /// task back if the VM has been torn down (caller releases what it holds). - #[inline] - fn post(&self, task: core::ptr::NonNull) -> jsc::vm_handle::Posted { - self.loop_handle.post_task(task) - } - /// Wrap a borrowed body chunk in a `StreamResult::Temporary*` for /// synchronous delivery to `ByteStream::on_data`. /// @@ -415,23 +412,27 @@ impl FetchTasklet { // Forwards `this` to ThreadSafeRefCount/dealloc without dereferencing; signature must // stay `*mut` because the call may drop the last ref and free the allocation. #[allow(clippy::not_unsafe_ptr_arg_deref)] - fn deref_from_thread(this: *mut FetchTasklet) { + fn deref_from_thread(this: *mut FetchTasklet, ticket: &jsc::Ticket) { // SAFETY: caller contract. if !unsafe { bun_ptr::ThreadSafeRefCount::::release(this) } { return; } - let self_ = Self::from_raw_ref(this); // Last ref dropped on the HTTP thread: deinit must run on the JS thread // (it drops JSC Strong/Weak handles), so hop there — as a task with its - // own tag, so a VM that is tearing down releases it from its queue. The - // VM waits for its fetches (embedded work) before closing its handle, - // so this is always queued. - let task = ConcurrentTask::create(bun_event_loop::Task::init( + // own tag, so a VM that is tearing down releases it from its queue. + ticket.post(ConcurrentTask::create(bun_event_loop::Task::init( this.cast::(), - )); - let jsc::vm_handle::Posted::Queued = self_.post(task) else { - unreachable!("VM handle closed with a fetch outstanding on the HTTP thread"); - }; + ))); + } + + /// HTTP thread, final callback: the fetch is back. Move the ticket out + /// (nothing here touches the tasklet after the ref drop) and drop this + /// thread's ref through it. + #[allow(clippy::not_unsafe_ptr_arg_deref)] + fn hand_back(this: *mut FetchTasklet) { + // SAFETY: caller contract; the field is HTTP-thread-only. + let ticket = unsafe { (*this).http_ticket.take() }.expect(Self::HOLDS_TICKET); + Self::deref_from_thread(this, &ticket); } fn clear_sink(&mut self) { @@ -546,14 +547,16 @@ impl FetchTasklet { /// `deinit` to the JS thread, which teardown runs from its queue release. /// * `true` — a non-final `on_progress_update` is queued (this entry is /// still in `in_flight`, so the *final* `callback` hasn't run). That - /// queued node owns the JS-side ref. The JS thread releases it from - /// `release_queued_tasks` *after* the HTTP daemon parks; - /// dropping it here too would leave the queued node pointing at a - /// freed `FetchTasklet`. Drop only the HTTP-side ref. + /// queued node owns the JS-side ref and its VM releases it from its + /// queue; dropping it here too would leave the queued node pointing at + /// a freed `FetchTasklet`. Drop only the HTTP-side ref. /// - /// `has_schedule_callback` is written exclusively by the HTTP-thread - /// `callback` and the JS-thread `on_progress_update`; the JS thread is - /// parked in `wait_timeout_while` here, so the load is race-free. + /// Only reachable for a request whose VM has *not* torn down (a worker + /// still running when the main thread exits): a VM's teardown waits for its + /// fetches' tickets — i.e. for their final callback — before the exiting + /// main thread parks the HTTP thread. `has_schedule_callback` is written by + /// the HTTP-thread `callback` and the JS-thread `on_progress_update` under + /// its own compare-exchange discipline, which this load relies on. /// /// SAFETY: `this` is the live `*mut FetchTasklet` registered as /// `result_callback.ctx` in `get()`; HTTP-thread-only at this point. @@ -565,18 +568,17 @@ impl FetchTasklet { let queued_progress_update = unsafe { (*this).has_schedule_callback.load(Ordering::Acquire) }; // SAFETY: caller contract — `this` is live and HTTP-thread-exclusive. - let handle = unsafe { + let ticket = unsafe { (*this).scheduled_response_buffer = MutableString::default(); - (*this).loop_handle.clone() - }; - // SAFETY: caller contract — `this` is live and HTTP-thread-exclusive. - FetchTasklet::deref_from_thread(this); + (*this).http_ticket.take() + } + .expect(Self::HOLDS_TICKET); + FetchTasklet::deref_from_thread(this, &ticket); if !queued_progress_update { - // SAFETY: caller contract — `this` is live and HTTP-thread-exclusive. - FetchTasklet::deref_from_thread(this); + FetchTasklet::deref_from_thread(this, &ticket); } // The HTTP thread is done with this fetch. - handle.embedded_work_finished(); + drop(ticket); } fn get_current_response(&self) -> Option<*mut Response> { @@ -1800,12 +1802,11 @@ impl FetchTasklet { // reshaped for borrowck — capture metadata fields before to_body_value() takes &mut self let headers = FetchHeaders::create_from_pico_headers(http_response.headers.list); let status_code = http_response.status_code as u16; - // status_text and url must NOT be atomized: the Response can be - // destroyed from the HTTP thread via deref_from_thread() -> deinit() - // when the VM is shutting down (see is_shutting_down() branch), and - // atom strings live in a per-thread table — deref'ing them off-thread - // trips the `wasRemoved` RELEASE_ASSERT in AtomStringImpl::remove(). - // Plain WTFStringImpl refcounts are atomic, so clone_utf8 is safe. + // status_text and url must NOT be atomized: this runs on the HTTP + // thread, and atom strings live in a per-thread table — creating or + // deref'ing them off the JS thread trips the `wasRemoved` + // RELEASE_ASSERT in AtomStringImpl::remove(). Plain WTFStringImpl + // refcounts are atomic, so clone_utf8 is safe. // Fast path: when the wire reason phrase matches the canonical text for // this status code, store a StaticZigString (deref is a no-op, so still // safe to drop off-thread) and skip the WTF allocation entirely. @@ -1911,9 +1912,6 @@ impl FetchTasklet { fetch_options: FetchOptions, promise: jsc::JSPromiseStrong, ) -> crate::Result<*mut FetchTasklet> { - // SAFETY: bun_vm() returns the FFI `*mut VirtualMachine`; the VM outlives - // this tasklet (process-lifetime singleton on the JS thread). - let jsc_vm: &'static VirtualMachine = global_this.bun_vm(); let mut fetch_tasklet = Box::new(FetchTasklet { sink: None, // `AsyncHTTP` has no `Default`/zero-init; defer the Box until @@ -1921,7 +1919,7 @@ impl FetchTasklet { http: None, result: HTTPClientResult::default(), metadata: None, - loop_handle: jsc_vm.loop_handle(), + http_ticket: None, global_this: GlobalRef::from(global_this), request_body: fetch_options.body, request_body_streaming_buffer: None, @@ -2170,10 +2168,11 @@ impl FetchTasklet { this_ref.ref_(); // `from_callback` heap-allocates a fresh `ConcurrentTaskItem`. let task = ConcurrentTask::from_callback(this, FetchTasklet::resume_request_data_stream); - // In flight ⇒ still counted work of its VM: the handle is open. - let jsc::vm_handle::Posted::Queued = this_ref.post(task) else { - unreachable!("VM handle closed with a fetch outstanding on the HTTP thread"); - }; + this_ref + .http_ticket + .as_ref() + .expect(Self::HOLDS_TICKET) + .post(task); } /// This is ALWAYS called from the main thread @@ -2390,8 +2389,8 @@ impl FetchTasklet { // increment ref so we can keep it alive until the http client is done node_ref.ref_(); // Out on the HTTP thread from here until its final callback: the VM - // aborts it at teardown (registry) and waits for it (embedded work). - node_ref.loop_handle.embedded_work_scheduled(); + // aborts it at teardown (registry) and waits for it (the ticket). + node_ref.http_ticket = Some(global.bun_vm().ticket()); crate::jsc_hooks::ActiveHandle::Fetch(NonNull::new(node).expect("tasklet")).register(); http::HTTPThread::schedule(batch); @@ -2415,9 +2414,6 @@ impl FetchTasklet { // at this point only this thread is accessing result to is no race condition let is_done = !result.has_more; let task_ref = Self::from_raw_mut(task); - // The final callback is where the HTTP thread hands the fetch back - // (`embedded_work_finished` below, after our deref may have freed it). - let done_handle = is_done.then(|| task_ref.loop_handle.clone()); task_ref.mutex.lock(); // we need to unlock before task.deref(); @@ -2488,11 +2484,6 @@ impl FetchTasklet { if success && task_ref.result.has_more { // we are ignoring the body so we should not receive more data, so will only signal when result.has_more = true task_ref.mutex.unlock(); - if let Some(handle) = done_handle { - // SAFETY: `task` is the live heap tasklet; HTTP-thread ref held. - FetchTasklet::deref_from_thread(task); - handle.embedded_work_finished(); - } return; } } else if success { @@ -2541,10 +2532,8 @@ impl FetchTasklet { ) { if has_schedule_callback { task_ref.mutex.unlock(); - if let Some(handle) = done_handle { - // SAFETY: `task` is the live heap tasklet; HTTP-thread ref held. - FetchTasklet::deref_from_thread(task); - handle.embedded_work_finished(); + if is_done { + FetchTasklet::hand_back(task); } return; } @@ -2556,19 +2545,19 @@ impl FetchTasklet { .from(task, AutoDeinit::ManualDeinit), ); // `ct` is the inline `concurrent_task` field of the heap tasklet; the - // queue takes ownership of its `next` link. The VM waits for its - // fetches (embedded work) before closing its handle: always queued. - let jsc::vm_handle::Posted::Queued = task_ref.post(ct) else { - unreachable!("VM handle closed with a fetch outstanding on the HTTP thread"); - }; + // queue takes ownership of its `next` link. This thread's ref keeps the + // tasklet (and the ticket in it) alive across the post. + task_ref + .http_ticket + .as_ref() + .expect(Self::HOLDS_TICKET) + .post(ct); task_ref.mutex.unlock(); // we are done with the http client so we can deref our side // this is a atomic operation and will enqueue a task to deinit on the main thread - if let Some(handle) = done_handle { - // SAFETY: `task` is the live heap tasklet; HTTP-thread ref held. - FetchTasklet::deref_from_thread(task); - handle.embedded_work_finished(); + if is_done { + FetchTasklet::hand_back(task); } } } diff --git a/src/runtime/webcore/s3/client.rs b/src/runtime/webcore/s3/client.rs index 329617c105d6..18f1d34e8c9c 100644 --- a/src/runtime/webcore/s3/client.rs +++ b/src/runtime/webcore/s3/client.rs @@ -301,7 +301,7 @@ pub(crate) fn list_objects( callback_context, callback: s3_simple_request::Callback::ListObjects(callback), headers, - loop_handle: VirtualMachine::get().loop_handle(), + http_ticket: None, response_buffer: MutableString::default(), result: bun_http::HTTPClientResult::default(), concurrent_task: Default::default(), @@ -372,8 +372,8 @@ pub(crate) fn list_objects( // SAFETY: `http` was initialised by `task.http.write(...)` immediately above. unsafe { task.http.assume_init_mut() }.schedule(&mut batch); // Out on the HTTP thread until its final callback: the VM aborts it at - // teardown (registry) and waits for it (embedded work). - task.loop_handle.embedded_work_scheduled(); + // teardown (registry) and waits for it (the ticket). + task.http_ticket = Some(VirtualMachine::get().ticket()); crate::jsc_hooks::ActiveHandle::S3Request(core::ptr::NonNull::new(task_ptr).expect("task")) .register(); bun_http::HTTPThread::schedule(batch); @@ -1235,8 +1235,7 @@ fn download_stream( .expect("callers always pass a non-null Box-allocated context"), callback, headers, - // `VirtualMachine::get()` returns the live per-thread VM singleton. - loop_handle: VirtualMachine::get().loop_handle(), + http_ticket: None, has_schedule_callback: core::sync::atomic::AtomicBool::new(false), signal_store: Default::default(), signals: Default::default(), @@ -1315,8 +1314,8 @@ fn download_stream( let mut batch = bun_threading::thread_pool::Batch::default(); http.schedule(&mut batch); // Out on the HTTP thread until its final callback: the VM aborts it at - // teardown (registry) and waits for it (embedded work). - task.loop_handle.embedded_work_scheduled(); + // teardown (registry) and waits for it (the ticket). + task.http_ticket = Some(VirtualMachine::get().ticket()); crate::jsc_hooks::ActiveHandle::S3Download(core::ptr::NonNull::new(task_ptr).expect("task")) .register(); bun_http::HTTPThread::schedule(batch); diff --git a/src/runtime/webcore/s3/download_stream.rs b/src/runtime/webcore/s3/download_stream.rs index a07a56e11f1b..9e4cc0c09469 100644 --- a/src/runtime/webcore/s3/download_stream.rs +++ b/src/runtime/webcore/s3/download_stream.rs @@ -19,8 +19,9 @@ pub struct S3HttpDownloadStreamingTask { // `MaybeUninit` because `AsyncHTTP` contains non-null references, so // `mem::zeroed()` can't be used here (mirrors `S3HttpSimpleTask`). pub(crate) http: core::mem::MaybeUninit>, - /// How the HTTP thread reaches the VM to deliver chunks. - pub(crate) loop_handle: bun_jsc::LoopHandle, + /// Held while the download is out on the HTTP thread: how it delivers + /// chunks, and what makes the VM wait for it. + pub(crate) http_ticket: Option, pub(crate) sign_result: SignResult, pub(crate) headers: Headers, pub(crate) callback_context: NonNull<()>, @@ -55,6 +56,8 @@ impl Taskable for S3HttpDownloadStreamingTask { } impl S3HttpDownloadStreamingTask { + const HOLDS_TICKET: &str = "S3 download on the HTTP thread holds a ticket"; + pub(crate) fn new(init: Self) -> Box { Box::new(init) } @@ -278,30 +281,29 @@ impl S3HttpDownloadStreamingTask { // concurrent reference and `mutex` serializes against `on_response`. `async_http` is the // live HTTP-thread copy, non-null for the callback's duration. Borrows scoped to the call. let is_done = !result.has_more; - // The final callback is where the HTTP thread hands the request back - // (`embedded_work_finished` below, after `this` may have been freed). - // SAFETY: `this` is live for the duration of the request. - let done_handle = is_done.then(|| unsafe { (*this).loop_handle.clone() }); + // No refcount here: on the final callback `on_response` may free `this` + // as soon as the task is queued, so the ticket has to be out first. + let done_ticket = is_done.then(|| { + // SAFETY: as above; HTTP-thread field. + unsafe { (*this).http_ticket.take() }.expect(Self::HOLDS_TICKET) + }); // SAFETY: as above; the HTTP thread is the only one touching it here. if unsafe { (*this).process_http_callback(&mut *async_http, result) } { // we are always unlocked here and its safe to enqueue // SAFETY: same exclusivity as above; `task` is the inline `concurrent_task` field of - // this heap request and the queue takes ownership of its `next` link. The VM waits - // for its S3 requests (embedded work) before closing its handle: always queued. + // this heap request and the queue takes ownership of its `next` link. Not done ⇒ + // `this` (and the ticket in it) outlives the post. unsafe { let task = core::ptr::NonNull::from( (*this).concurrent_task.from(this, AutoDeinit::ManualDeinit), ); - let bun_jsc::vm_handle::Posted::Queued = (*this).loop_handle.post_task(task) else { - unreachable!( - "VM handle closed with an S3 download outstanding on the HTTP thread" - ); - }; + done_ticket + .as_ref() + .unwrap_or_else(|| (*this).http_ticket.as_ref().expect(Self::HOLDS_TICKET)) + .post(task); } } - if let Some(handle) = done_handle { - handle.embedded_work_finished(); - } + drop(done_ticket); } /// `HTTPClientResultCallback::release_at_shutdown`: the exiting main @@ -316,7 +318,7 @@ impl S3HttpDownloadStreamingTask { // SAFETY: fn contract — nothing else touches the task now (the JS // thread is waiting in the HTTP shutdown). unsafe { - let handle = (*this).loop_handle.clone(); + let ticket = (*this).http_ticket.take().expect(Self::HOLDS_TICKET); let should_enqueue = { let _guard = (*this).mutex.lock_guard(); let mut state = (*this).get_state(); @@ -333,13 +335,9 @@ impl S3HttpDownloadStreamingTask { let task = core::ptr::NonNull::from( (*this).concurrent_task.from(this, AutoDeinit::ManualDeinit), ); - let bun_jsc::vm_handle::Posted::Queued = handle.post_task(task) else { - unreachable!( - "VM handle closed with an S3 download outstanding on the HTTP thread" - ); - }; + ticket.post(task); } - handle.embedded_work_finished(); + drop(ticket); } } diff --git a/src/runtime/webcore/s3/simple_request.rs b/src/runtime/webcore/s3/simple_request.rs index 30eea745852d..3e49035cfca9 100644 --- a/src/runtime/webcore/s3/simple_request.rs +++ b/src/runtime/webcore/s3/simple_request.rs @@ -115,8 +115,9 @@ pub struct S3HttpSimpleTask { // `execute_simple_s3_request` before the task pointer escapes, so every later access (in // `http_callback` / `Drop`) may `assume_init`. pub(crate) http: core::mem::MaybeUninit>, - /// How the HTTP thread reaches the VM to deliver the response. - pub(crate) loop_handle: bun_jsc::LoopHandle, + /// Held while the request is out on the HTTP thread: how it delivers the + /// response, and what makes the VM wait for it. + pub(crate) http_ticket: Option, pub(crate) sign_result: SignResult, pub(crate) headers: Headers, pub(crate) callback_context: *mut c_void, @@ -207,6 +208,8 @@ enum ErrorType { } impl S3HttpSimpleTask { + const HOLDS_TICKET: &str = "S3 request on the HTTP thread holds a ticket"; + // bun.TrivialNew(@This()) — heap-allocate; pointer crosses thread boundary via http callback pub(crate) fn new(init: Self) -> *mut Self { bun_core::heap::into_raw(Box::new(init)) @@ -430,20 +433,14 @@ impl S3HttpSimpleTask { unsafe { (*this).stage_http_result(async_http, result) }; if is_done { // SAFETY: same exclusivity as above; the queue takes ownership of the inline - // `concurrent_task` field's `next` link. The VM waits for its S3 requests - // (embedded work) before closing its handle: always queued. + // `concurrent_task` field's `next` link. The ticket is moved out first: the + // JS thread may free `this` the moment it is queued. unsafe { - let handle = (*this).loop_handle.clone(); + let ticket = (*this).http_ticket.take().expect(Self::HOLDS_TICKET); let queued = core::ptr::NonNull::from( (*this).concurrent_task.from(this, AutoDeinit::ManualDeinit), ); - let bun_jsc::vm_handle::Posted::Queued = handle.post_task(queued) else { - unreachable!( - "VM handle closed with an S3 request outstanding on the HTTP thread" - ); - }; - // The HTTP thread is done with this request (`this` may already be freed). - handle.embedded_work_finished(); + ticket.post(queued); } } } @@ -460,14 +457,11 @@ impl S3HttpSimpleTask { unsafe { (*this).result.fail = Some(bun_http::Error::Aborted); (*this).result.has_more = false; - let handle = (*this).loop_handle.clone(); + let ticket = (*this).http_ticket.take().expect(Self::HOLDS_TICKET); let queued = core::ptr::NonNull::from( (*this).concurrent_task.from(this, AutoDeinit::ManualDeinit), ); - let bun_jsc::vm_handle::Posted::Queued = handle.post_task(queued) else { - unreachable!("VM handle closed with an S3 request outstanding on the HTTP thread"); - }; - handle.embedded_work_finished(); + ticket.post(queued); } } @@ -565,8 +559,8 @@ pub(crate) fn execute_simple_s3_request( callback_context: *mut c_void, ) -> JsTerminatedResult<()> { // A multipart/retry continuation can reach here from teardown's queue - // release; nothing new leaves a VM that is shutting down. - if !VirtualMachine::get().handle().accepting_work() { + // release; nothing new leaves a VM that is stopping. + if !VirtualMachine::get().script_allowed() { drop(options.range); callback.fail( b"ERR_S3_VM_SHUTDOWN", @@ -637,7 +631,7 @@ pub(crate) fn execute_simple_s3_request( callback_context, callback, headers, - loop_handle: VirtualMachine::get().loop_handle(), + http_ticket: None, response_buffer: MutableString::default(), result: HTTPClientResult::default(), concurrent_task: ConcurrentTask::default(), @@ -711,9 +705,9 @@ pub(crate) fn execute_simple_s3_request( // SAFETY: `http` was initialised immediately above; scoped exclusive access. unsafe { (*task_ptr).http.assume_init_mut() }.schedule(&mut batch); // Out on the HTTP thread until its final callback: the VM aborts it at - // teardown (registry) and waits for it (embedded work). + // teardown (registry) and waits for it (the ticket). // SAFETY: as above. - unsafe { (*task_ptr).loop_handle.embedded_work_scheduled() }; + unsafe { (*task_ptr).http_ticket = Some(VirtualMachine::get().ticket()) }; crate::jsc_hooks::ActiveHandle::S3Request(core::ptr::NonNull::new(task_ptr).expect("task")) .register(); bun_http::HTTPThread::schedule(batch); diff --git a/test/internal/source-lints/vm-thread-door.inventory.json b/test/internal/source-lints/vm-thread-door.inventory.json new file mode 100644 index 000000000000..942c25664315 --- /dev/null +++ b/test/internal/source-lints/vm-thread-door.inventory.json @@ -0,0 +1,270 @@ +{ + "src/event_loop/AnyEventLoop.rs": { + "unsafe impl Send": [ + "JsPoster" + ], + "unsafe impl Sync": [ + "JsPoster" + ] + }, + "src/jsc/CppTask.rs": { + "WorkPool::schedule*": 1, + "owned_task!": [ + "ConcurrentCppTask" + ] + }, + "src/jsc/Debugger.rs": { + "thread spawn": 1 + }, + "src/jsc/JSCell.rs": { + "unsafe impl Send": [ + "JsCell" + ], + "unsafe impl Sync": [ + "JsCell" + ] + }, + "src/jsc/JSSecrets.rs": { + "unsafe impl Send": [ + "SecretsOptions" + ] + }, + "src/jsc/NodeCompileCache.rs": { + "thread spawn": 1, + "unsafe impl Send": [ + "AlignedBlob", + "CacheState" + ] + }, + "src/jsc/RuntimeTranspilerStore.rs": { + "WorkPool::schedule*": 1 + }, + "src/jsc/TopExceptionScope.rs": { + "unsafe impl Send": [ + "SourceLocation" + ], + "unsafe impl Sync": [ + "SourceLocation" + ] + }, + "src/jsc/hot_reloader.rs": { + "thread spawn": 1, + "unsafe impl Send": [ + "WatchChangedPaths" + ], + "unsafe impl Sync": [ + "WatchChangedPaths" + ] + }, + "src/jsc/node_path.rs": { + "unsafe impl Send": [ + "ThreadSafe" + ] + }, + "src/jsc/web_worker.rs": { + "thread spawn": 1, + "unsafe impl Send": [ + "ThreadStart" + ] + }, + "src/jsc/webcore_types.rs": { + "unsafe impl Send": [ + "Blob", + "Bytes", + "StoreRef" + ], + "unsafe impl Sync": [ + "Blob", + "Bytes", + "StoreRef" + ] + }, + "src/runtime/api/JSBundler.rs": { + "worker_pool.schedule(Batch)": 1 + }, + "src/runtime/api/JSTranspiler.rs": { + "unsafe impl Send": [ + "TransformTask" + ] + }, + "src/runtime/api/bun/Terminal.rs": { + "thread spawn": 1 + }, + "src/runtime/api/glob.rs": { + "unsafe impl Send": [ + "WalkTask" + ] + }, + "src/runtime/api/js_bundle_completion_task.rs": { + "unsafe impl Send": [ + "JSBundleCompletionTask" + ] + }, + "src/runtime/bake/production.rs": { + "unsafe impl Sync": [ + "DotenvSingleton" + ] + }, + "src/runtime/cli/create_command.rs": { + "thread spawn": 1 + }, + "src/runtime/cli/open.rs": { + "thread spawn": 1 + }, + "src/runtime/cli/publish_command.rs": { + "thread spawn": 1 + }, + "src/runtime/cli/run_command.rs": { + "HTTPThread::schedule": 1 + }, + "src/runtime/dns_jsc/dns.rs": { + "WorkPool::schedule*": 1, + "unsafe impl Send": [ + "GlobalCache", + "SendPtr" + ] + }, + "src/runtime/image/Image.rs": { + "unsafe impl Send": [ + "PipelineTask" + ] + }, + "src/runtime/napi/napi_body.rs": { + "WorkPool::schedule*": 1, + "intrusive_work_task!": [ + "napi_async_work" + ], + "unsafe impl Sync": [ + "napi_node_version" + ] + }, + "src/runtime/node/fs_events.rs": { + "thread spawn": 1, + "unsafe impl Send": [ + "CoreFoundation", + "FSEventsLoop" + ], + "unsafe impl Sync": [ + "CoreFoundation", + "FSEventsLoop" + ] + }, + "src/runtime/node/memory_pressure.rs": { + "thread spawn": 1 + }, + "src/runtime/node/node_crypto_binding.rs": { + "unsafe impl Send": [ + "OwnedCtx" + ] + }, + "src/runtime/node/node_fs.rs": { + "WorkPool::schedule*": 4, + "owned_task!": [ + "AsyncMkdirp", + "ReaddirSubtask" + ], + "unsafe impl Send": [ + "AsyncFSTask", + "AsyncReaddirRecursiveTask" + ] + }, + "src/runtime/node/node_fs_stat_watcher.rs": { + "WorkPool::schedule*": 2, + "owned_task!": [ + "InitialStatTask" + ] + }, + "src/runtime/node/node_process.rs": { + "unsafe impl Sync": [ + "CStrPtr" + ] + }, + "src/runtime/node/node_zlib_binding.rs": { + "WorkPool::schedule*": 1 + }, + "src/runtime/node/path_watcher.rs": { + "thread spawn": 2, + "unsafe impl Send": [ + "PathWatcherManager" + ], + "unsafe impl Sync": [ + "PathWatcherManager" + ] + }, + "src/runtime/shell/IOReader.rs": { + "unsafe impl Send": [ + "IOReader" + ], + "unsafe impl Sync": [ + "IOReader" + ] + }, + "src/runtime/shell/IOWriter.rs": { + "unsafe impl Send": [ + "IOWriter" + ], + "unsafe impl Sync": [ + "IOWriter" + ] + }, + "src/runtime/shell/builtin/cp.rs": { + "WorkPool::schedule*": 1 + }, + "src/runtime/shell/builtin/rm.rs": { + "WorkPool::schedule*": 2, + "unsafe impl Send": [ + "DirTask", + "ShellRmTask" + ] + }, + "src/runtime/shell/interpreter.rs": { + "WorkPool::schedule*": 1 + }, + "src/runtime/webcore/Blob.rs": { + "WorkPool::schedule*": 1 + }, + "src/runtime/webcore/CompressionStreamCoder.rs": { + "unsafe impl Send": [ + "AsyncInput", + "CompressionAsyncCtx", + "CompressionStreamCoder" + ] + }, + "src/runtime/webcore/blob/copy_file.rs": { + "unsafe impl Send": [ + "CopyFile" + ] + }, + "src/runtime/webcore/blob/read_file.rs": { + "WorkPool::schedule*": 2, + "intrusive_work_task!": [ + "ReadFile" + ], + "unsafe impl Send": [ + "ReadFile" + ] + }, + "src/runtime/webcore/blob/write_file.rs": { + "WorkPool::schedule*": 2, + "intrusive_work_task!": [ + "WriteFile" + ], + "unsafe impl Send": [ + "WriteFile" + ] + }, + "src/runtime/webcore/fetch/FetchTasklet.rs": { + "HTTPThread::schedule": 1 + }, + "src/runtime/webcore/s3/client.rs": { + "HTTPThread::schedule": 2 + }, + "src/runtime/webcore/s3/simple_request.rs": { + "HTTPThread::schedule": 1 + }, + "src/sql_jsc/jsc.rs": { + "unsafe impl Send": [ + "SSLConfig" + ] + } +} diff --git a/test/internal/source-lints/vm-thread-door.test.ts b/test/internal/source-lints/vm-thread-door.test.ts new file mode 100644 index 000000000000..6bd0ac3428c8 --- /dev/null +++ b/test/internal/source-lints/vm-thread-door.test.ts @@ -0,0 +1,120 @@ +// The door out of a VM's thread (src/jsc/VmHandle.rs). +// +// A `VirtualMachine` is destroyed only after everything that left its thread +// has come back: work that runs on, or is referenced from, another thread on a +// VM's behalf holds a `bun_jsc::Ticket`, and the VM's teardown waits for every +// ticket. That is only a guarantee if there is no way *around* the door, so +// this lint freezes, for the VM crates (`jsc`, `runtime`, `event_loop`, +// `sql_jsc`, `http_jsc`), the two things that could open one: +// +// 1. `unsafe impl Send` / `unsafe impl Sync`, the `owned_task!` macro +// (which emits one) and `intrusive_work_task!` (which is what makes a +// type schedulable on the pool). `VirtualMachine`, +// `EventLoop` and `JSGlobalObject` are `!Send + !Sync`, so VM state can +// only reach another thread inside a type someone declared `Send` by hand. +// Every such type must either carry a `Ticket` for the VM whose state it +// holds, or hold no VM/JS state at all. +// 2. Direct calls to the raw thread-crossing primitives — the work pool, the +// HTTP thread, thread spawning, `uv_queue_work`. VM code reaches these +// through `bun_jsc::Job` or through a struct that holds a `Ticket` for +// its in-flight duration; a new direct call site is a new path that has +// to be shown to do the same. +// +// If this fails because you ADDED one: make the payload carry a `Ticket` +// (taken on the JS thread with `vm.ticket()`, dropped after the completion is +// posted) or show it is VM-free, say so in a `// SAFETY:` comment, then +// regenerate the inventory: +// bun ./test/internal/source-lints/vm-thread-door.test.ts --update +// If it fails because you REMOVED one: regenerate the same way. + +import { file } from "bun"; +import { describe, expect, test } from "bun:test"; +import { realpathSync } from "fs"; +import path from "path"; +import { globAllSources } from "../../../scripts/glob-sources.ts"; + +const root = path.resolve(import.meta.dir, "..", "..", ".."); +const INVENTORY = import.meta.dir + "/vm-thread-door.inventory.json"; + +const SCOPED = ["src/jsc/", "src/runtime/", "src/event_loop/", "src/sql_jsc/", "src/http_jsc/"]; +// The door itself. +const DOOR = new Set(["src/jsc/VmHandle.rs", "src/jsc/job.rs"]); + +const PATTERNS: [name: string, re: RegExp][] = [ + ["unsafe impl Send", /\bunsafe\s+impl(?:\s*<[^>]*>)?\s+Send\s+for\s+([A-Za-z_][\w:<>, ']*)/g], + ["unsafe impl Sync", /\bunsafe\s+impl(?:\s*<[^>]*>)?\s+Sync\s+for\s+([A-Za-z_][\w:<>, ']*)/g], + ["owned_task!", /\b(?:bun_threading::)?owned_task!\s*\(\s*([A-Za-z_]\w*)/g], + ["intrusive_work_task!", /\b(?:bun_threading::)?intrusive_work_task!\s*\(\s*([A-Za-z_]\w*)/g], + ["WorkPool::schedule*", /\bWorkPool::(?:schedule|schedule_new|schedule_owned|go)\b/g], + ["worker_pool.schedule(Batch)", /\)\s*\.schedule\(\s*(?:bun_threading::thread_pool::)?Batch::from/g], + ["HTTPThread::schedule", /\bHTTPThread::schedule\s*\(/g], + ["thread spawn", /\bthread::(?:Builder::new\s*\(|spawn\s*\()/g], + ["uv_queue_work", /(? | null = (() => { + const r = Bun.spawnSync({ + cmd: ["git", "-C", root, "ls-tree", "-r", "--name-only", "-z", "HEAD"], + stdout: "pipe", + stderr: "ignore", + }); + if (!r.success) return null; + return new Set(r.stdout.toString().split("\0").filter(Boolean)); +})(); + +type Inventory = Record>; +const found: Inventory = {}; + +for (const abs of globAllSources().rust.filter(p => p.endsWith(".rs"))) { + const source = path.relative(root, abs).replaceAll(path.sep, "/"); + if (path.relative(root, realpathSync(abs)).replaceAll(path.sep, "/") !== source) continue; + if (!SCOPED.some(p => source.startsWith(p)) || DOOR.has(source)) continue; + if (tracked !== null && !tracked.has(source)) continue; + const stripped = (await file(abs).text()).replace(/^\s*\/\/.*$/gm, ""); + for (const [name, re] of PATTERNS) { + const matches = [...stripped.matchAll(re)]; + if (matches.length === 0) continue; + const entry = (found[source] ??= {}); + if (matches[0].length > 1) { + entry[name] = matches.map(m => m[1].trim().replace(/\s+/g, " ")).sort(); + } else { + entry[name] = matches.length; + } + } +} + +const sortKeys = (o: Record): Record => + Object.fromEntries(Object.entries(o).sort(([a], [b]) => (a < b ? -1 : a > b ? 1 : 0))); +const normalized: Inventory = sortKeys(Object.fromEntries(Object.entries(found).map(([k, v]) => [k, sortKeys(v)]))); + +if (process.argv.includes("--update")) { + await Bun.write(INVENTORY, JSON.stringify(normalized, null, 2) + "\n"); + console.log(`Wrote ${Object.keys(normalized).length} files to ${path.basename(INVENTORY)}`); + process.exit(0); +} + +const inventory: Inventory = await Bun.file(INVENTORY).json(); + +describe("VM thread door", () => { + const files = [...new Set([...Object.keys(inventory), ...Object.keys(normalized)])].sort(); + test.each(files)("%s", source => { + const expected = inventory[source] ?? {}; + const actual = normalized[source] ?? {}; + if (!Bun.deepEquals(actual, expected)) { + throw new Error( + `${source}: thread-crossing inventory changed.\n` + + ` expected: ${JSON.stringify(expected)}\n` + + ` actual: ${JSON.stringify(actual)}\n` + + `A new \`unsafe impl Send/Sync\` or a new direct call to a thread-crossing primitive in a VM crate ` + + `must carry a \`bun_jsc::Ticket\` for the VM whose state it holds (or hold none) — see src/jsc/VmHandle.rs. ` + + `Then regenerate: bun ./test/internal/source-lints/vm-thread-door.test.ts --update`, + ); + } + }); + + test("VirtualMachine stays !Send + !Sync", async () => { + const vm = await file(path.join(root, "src/jsc/VirtualMachine.rs")).text(); + expect(vm).not.toMatch(/unsafe\s+impl\s+(?:Send|Sync)\s+for\s+VirtualMachine\b/); + expect(vm).toContain(">::some_item"); + }); +}); diff --git a/test/js/web/workers/worker-late-completion.test.ts b/test/js/web/workers/worker-late-completion.test.ts new file mode 100644 index 000000000000..7d27ea293c9a --- /dev/null +++ b/test/js/web/workers/worker-late-completion.test.ts @@ -0,0 +1,417 @@ +// A worker VM is destroyed only after everything it sent to another thread has +// come back (src/jsc/VmHandle.rs): work out on the thread pool / HTTP thread / +// bundle thread holds a *ticket* on the VM, and the worker's teardown waits +// for every ticket — releasing whatever arrives meanwhile on its own thread, +// heap alive — before the JSC VM goes. Something that merely refers to the +// worker from elsewhere (another thread's MessagePort, the child-process +// waiter thread) holds no ticket; its post is delivered-and-released while the +// worker drains, or refused once it has closed, and it frees its own payload. +// +// Here each of those paths runs deterministically for one producer at a time: +// with BUN_DEBUG_TEST_WORKER_TEARDOWN_GATE the other thread's post is held +// until the worker's teardown is already waiting, so it always lands *during* +// the wait, and the runtime names it on stderr. A row passes only if the named +// line appeared (the work really was on another thread, really came back +// during teardown, and — for ticketed work — was taken through the door at +// the expected site) and the process exited cleanly; on the ASAN build the +// release paths are also checked for use-after-free and leaks. Builds with +// debug assertions only (debug, ASAN): the gate does not exist in release. +import { describe, expect, test } from "bun:test"; +import { bunEnv, bunExe, isAndroid, isASAN, isDebug, isLinux, isWindows, tempDir } from "harness"; +import fs from "node:fs"; +import path from "node:path"; + +type Row = { + name: string; + // Runs in the worker before it exits; starts exactly one piece of off-thread work. + worker: string; + // Runs in the parent before the worker is created; may add to `data` (workerData). + prelude?: string; + // Runs in the parent once the worker says it is set up (for producers on the parent's side). + parent?: string; + // Runs in the parent when the worker has exited (release what the prelude holds). + onExit?: string; + env?: Record; + files?: Record; + skip?: boolean; +} & (Ticketed | Weak); +// Ticketed work: substring of the site (file) the ticket was taken at, as +// logged by "[vm] late completion from :". +type Ticketed = { ticket: string; weak?: never }; +// Weak posters: the task tag logged by "[vm] late post: (...)". +type Weak = { weak: string; ticket?: never }; + +const ROWS: Row[] = [ + // ── thread pool: bun_jsc::Job ──────────────────────────────────────────── + { name: "fs.readFile", worker: `require("node:fs").readFile(process.execPath, () => {});`, ticket: "node_fs.rs" }, + { name: "fs.promises.stat", worker: `require("node:fs").promises.stat(process.execPath);`, ticket: "node_fs.rs" }, + { name: "fs.realpath", worker: `require("node:fs").realpath(process.execPath, () => {});`, ticket: "node_fs.rs" }, + { + // The pool thread reads the JS buffer's bytes in place. + name: "fs.writeFile from a Buffer", + worker: `require("node:fs").writeFile(require("node:path").join(workerData.dir, "out.bin"), Buffer.alloc(1 << 20, 7), () => {});`, + ticket: "node_fs.rs", + files: {}, + }, + { + // The pool thread writes into the JS buffer's bytes in place. + name: "fs.read into a Buffer", + worker: `const fs = require("node:fs"); fs.read(fs.openSync(process.execPath, "r"), Buffer.alloc(1 << 20), 0, 1 << 20, 0, () => {});`, + ticket: "node_fs.rs", + }, + { name: "Bun.file().text()", worker: `Bun.file(process.execPath).slice(0, 65536).text();`, ticket: "read_file.rs" }, + { + // Same read job, different completion: the image's read chain is handed + // ECANCELED at teardown and has to free itself. + name: "Bun.Image(Bun.file()).metadata()", + worker: `new Bun.Image(Bun.file(process.execPath).slice(0, 65536)).metadata().catch(() => {});`, + ticket: "read_file.rs", + }, + { + name: "crypto.pbkdf2", + worker: `require("node:crypto").pbkdf2("p", "s", 1000, 32, "sha256", () => {});`, + ticket: "PBKDF2.rs", + }, + { + name: "crypto.scrypt", + worker: `require("node:crypto").scrypt("p", "s", 32, () => {});`, + ticket: "node_crypto_binding.rs", + }, + { + name: "crypto.randomFill", + worker: `require("node:crypto").randomFill(Buffer.alloc(65536), () => {});`, + ticket: "node_crypto_binding.rs", + }, + { + name: "crypto.generateKeyPair", + worker: `require("node:crypto").generateKeyPair("ec", { namedCurve: "P-256" }, () => {});`, + ticket: "node_crypto_binding.rs", + }, + { + // WebCrypto's work queue: a C++ closure on the pool, carried (with a + // ticket) by ConcurrentCppTask; its *result* comes back by context id — + // WebCore's postTaskTo(), a weak post — and because the ticket kept the + // worker draining rather than closed, that post is delivered and its + // promise/callback refs are released on the worker's thread. + name: "crypto.subtle.digest", + worker: `crypto.subtle.digest("SHA-256", Buffer.alloc(65536));`, + weak: "CppTask", + }, + { + name: "Bun.password.hash", + worker: `Bun.password.hash("x", { algorithm: "bcrypt", cost: 4 });`, + ticket: "PasswordObject.rs", + }, + { + name: "Bun.Glob scan", + worker: `new Bun.Glob("**/*").scan({ cwd: workerData.dir })[Symbol.asyncIterator]().next();`, + ticket: "glob.rs", + files: { "a/b/c/d.txt": "x", "a/e.txt": "y", "f/g/h.txt": "z" }, + }, + { + name: "dns lookup on the thread pool", + worker: `Bun.dns.lookup("localhost", { backend: "libc" });`, + ticket: "dns.rs", + }, + { + name: "Bun.Transpiler.transform", + worker: `new Bun.Transpiler().transform("export const a: number = 1");`, + ticket: "JSTranspiler.rs", + }, + { + name: "CompressionStream", + worker: `new Response(new Blob([Buffer.alloc(1 << 20, 9)]).stream().pipeThrough(new CompressionStream("gzip"))).arrayBuffer();`, + ticket: "CompressionStreamCoder.rs", + }, + { + // A Job whose subtasks fan out across the pool and finish it from whichever ends last. + name: "fs.promises.readdir recursive", + worker: `require("node:fs").promises.readdir(workerData.dir, { recursive: true });`, + ticket: "node_fs.rs", + files: { "a/b/c/d.txt": "x", "a/e.txt": "y", "f/g/h.txt": "z" }, + }, + { + // The completion is posted by the last of the copy's pool subtasks. + name: "fs.cp recursive", + worker: `require("node:fs").cp(require("node:path").join(workerData.dir, "src"), require("node:path").join(workerData.dir, "dst"), { recursive: true }, () => {});`, + ticket: "node_fs.rs", + files: { "src/a/b/c.txt": "x", "src/d.txt": "y", "src/e/f.txt": "z" }, + }, + // ── thread pool: Bun.$ builtins (interpreter state a JS wrapper owns) ──── + { + // Subtasks created on the pool inherit the parent's ticket (new_child). + name: "$ ls -R", + worker: `Bun.$\`ls -R \${workerData.dir}\`.quiet().catch(() => {});`, + ticket: "interpreter.rs", + files: { "a/b/c/d.txt": "x", "a/e.txt": "y", "f/g/h.txt": "z" }, + }, + { + // The builtin's pool task hands the copy to an fs.cp task carrying a + // clone of its poster; that task's last subtask posts the completion. + name: "$ cp -R", + worker: `Bun.$\`cp -R \${workerData.dir}/src \${workerData.dir}/dst\`.quiet().catch(() => {});`, + ticket: "cp.rs", + files: { "src/a/b/c.txt": "x", "src/d.txt": "y", "src/e/f.txt": "z" }, + // The cp builtin is Windows-only unless opted into (POSIX spawns cp(1)). + env: { BUN_ENABLE_EXPERIMENTAL_SHELL_BUILTINS: "1" }, + }, + { + name: "$ rm -rf", + worker: `Bun.$\`rm -rfv \${workerData.dir}/gone\`.quiet().catch(() => {});`, + ticket: "rm.rs", + files: { "gone/a/b/c.txt": "x", "gone/d.txt": "y", "gone/e/f/g.txt": "z" }, + }, + // ── thread pool: storage inside a JS-owned object ──────────────────────── + { + // The zlib stream's native part is the pool task; input/output are JS buffers. + name: "zlib.gzip", + worker: `require("node:zlib").gzip(Buffer.alloc(1 << 20, 3), () => {});`, + ticket: "node_zlib_binding.rs", + }, + { + // The transpile job is a slot inside the VM itself. + name: "import() of a TypeScript module", + worker: `import(require("node:path").join(workerData.dir, "mod.ts"));`, + ticket: "RuntimeTranspilerStore.rs", + files: { "mod.ts": `export const x: number = ${Date.now()};\n` }, + }, + // ── HTTP thread ────────────────────────────────────────────────────────── + { + // Parked on a peer that never answers: the stop phase aborts it and the + // HTTP thread hands it back during the wait. + name: "fetch in flight", + prelude: `const server = Bun.serve({ port: 0, fetch: () => new Promise(() => {}) }); data.url = server.url.href;`, + worker: `fetch(workerData.url).catch(() => {});`, + onExit: `server.stop(true);`, + ticket: "FetchTasklet.rs", + }, + // ── bundle thread ──────────────────────────────────────────────────────── + { + name: "Bun.build", + worker: `Bun.build({ entrypoints: [require("node:path").join(workerData.dir, "entry.ts")] });`, + ticket: "js_bundle_completion_task.rs", + files: { "entry.ts": `export default 1;\n` }, + }, + // ── weak posters (no ticket): delivered while draining, or refused ─────── + { + name: "child exit reported by the waiter thread", + worker: `require("node:child_process").execFile(process.execPath, ["-e", "0"], () => {});`, + weak: "ProcessWaiterThreadTask", + // The waiter thread is a POSIX fallback path, opted into here the way the runtime's own tests do. + env: { BUN_GARBAGE_COLLECTOR_LEVEL: "0", BUN_FEATURE_FLAG_FORCE_WAITER_THREAD: "1" }, + // The flag is honoured on Linux/Android only (kqueue platforms always have EVFILT_PROC). + skip: !isLinux && !isAndroid, + }, + { + name: "BroadcastChannel message from another thread", + worker: `const bc = new BroadcastChannel("wlc"); bc.onmessage = () => {};`, + // An open channel keeps the parent's loop alive (as in Node); close it once posted. + parent: `const pc = new BroadcastChannel("wlc"); pc.postMessage("late"); pc.close();`, + weak: "CppTask", + }, + { + name: "MessagePort message from another thread", + worker: `workerData.port.on("message", () => {});`, + parent: `port1.postMessage("late");`, + weak: "CppTask", + }, +]; + +// The host: one worker, armed gate. The worker starts its work, reports +// "armed" and exits by itself two turns later; the parent never posts anything +// the worker waits for (with the gate armed, a parent→worker post is itself a +// cross-thread post that waits for the worker's teardown). Rows with a parent +// side post in response to "armed". +function host(row: Row, dir: string) { + const worker = ` + const { parentPort, workerData } = require("node:worker_threads"); + parentPort.on("message", () => {}); + ${row.worker} + parentPort.postMessage("armed"); + setImmediate(() => setImmediate(() => process.exit(0))); + `; + return ` + const { Worker, MessageChannel } = require("node:worker_threads"); + const { port1, port2 } = new MessageChannel(); + const data = { port: port2, dir: ${JSON.stringify(dir)} }; + ${row.prelude ?? ""} + const w = new Worker(${JSON.stringify(worker)}, { eval: true, workerData: data, transferList: [port2] }); + w.on("error", e => { console.error("worker error:", e && e.message); process.exitCode = 1; }); + w.once("message", () => { ${row.parent ?? ""} }); + w.on("exit", code => { port1.close(); ${row.onExit ?? ""} if (code !== 0) { console.error("worker exit code", code); process.exitCode = 1; } }); + `; +} + +describe.skipIf(!isDebug && !isASAN)("work that comes back after its worker began tearing down", () => { + for (const row of ROWS) { + test.concurrent.skipIf(!!row.skip)(row.name, async () => { + using dir = tempDir("worker-late-completion", row.files ?? {}); + await using proc = Bun.spawn({ + cmd: [bunExe(), "-e", host(row, String(dir))], + env: { ...bunEnv, ...row.env, BUN_DEBUG_TEST_WORKER_TEARDOWN_GATE: "1" }, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + const lines = stderr.split("\n").filter(l => l.startsWith("[vm] ")); + const seen = + "ticket" in row && row.ticket + ? lines.some(l => l.startsWith("[vm] late completion from ") && l.includes(row.ticket)) + : lines.some(l => l.startsWith(`[vm] late post: ${row.weak} (`)); + expect({ + exitCode, + seen, + // On a failure, everything the host printed. + detail: exitCode === 0 && seen ? "" : stdout + stderr, + }).toEqual({ exitCode: 0, seen: true, detail: "" }); + }); + } +}); + +// A `Bun.file()` / `Bun.stdin` read of a pipe or tty that has no data yet +// parks on the io loop instead of blocking a pool thread, so terminate() can +// and does cancel it: the worker goes away promptly and the read never settles. +describe.skipIf(isWindows)("terminate() cancels a read parked on the io loop", () => { + test.concurrent.each([ + ["Bun.stdin.text()", [`Bun.stdin.text()`]], + ["Bun.file(fifo).text()", [`Bun.file(workerData).text()`]], + ["Bun.file(fifo).bytes() twice", [`Bun.file(workerData).bytes()`, `Bun.file(workerData).bytes()`]], + ])("%s", async (_, reads) => { + const worker = ` + const { parentPort, workerData } = require("node:worker_threads"); + const settled = w => v => parentPort.postMessage(w); + for (const p of [${reads.join(",")}]) p.then(settled("resolved"), settled("rejected")); + parentPort.postMessage("reading"); + `; + using dir = tempDir("worker-terminate-cancels", {}); + const fifo = path.join(String(dir), "fifo"); + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + ` + const { Worker } = require("node:worker_threads"); + const { execFileSync } = require("node:child_process"); + const fifo = ${JSON.stringify(fifo)}; + execFileSync("mkfifo", [fifo]); + // Hold the FIFO open read-write (does not block, unlike a write-only + // open) so the worker's open() succeeds and its read parks waiting for + // data that never comes. + const writer = require("node:fs").openSync(fifo, "r+"); + const w = new Worker(${JSON.stringify(worker)}, { eval: true, workerData: fifo }); + const seen = []; + w.on("message", async m => { + seen.push(m); + if (m !== "reading") return; + const code = await w.terminate(); + console.log(JSON.stringify({ code, seen })); + require("node:fs").closeSync(writer); + }); + `, + ], + env: bunEnv, + // The parent's stdin is the worker's Bun.stdin: a pipe nobody writes to. + stdin: "pipe", + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect({ stdout: stdout.trim(), stderr: stderr.trim() }).toEqual({ + stdout: JSON.stringify({ code: 1, seen: ["reading"] }), + stderr: "", + }); + expect(exitCode).toBe(0); + }); +}); + +// The wait is unbounded by design: a job that cannot be cancelled makes +// terminate() take as long as the job (as Node's environment cleanup does), +// and then complete cleanly. Here the job is a read() blocking a pool thread on +// a FIFO nobody has written to yet (node:fs reads that way), so its duration is +// entirely the test's to decide — no timing thresholds. Debug builds also name +// what the wait is waiting for. +describe.skipIf(isWindows)("terminate() waits for work that cannot be cancelled", () => { + test("a pool thread parked in read() holds the worker's teardown until it returns", async () => { + using dir = tempDir("worker-terminate-waits", {}); + const fifo = path.join(String(dir), "fifo"); + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + ` + const { Worker } = require("node:worker_threads"); + const { execFileSync } = require("node:child_process"); + const fifo = ${JSON.stringify(fifo)}; + execFileSync("mkfifo", [fifo]); + const w = new Worker( + 'require("node:fs").readFile(require("node:worker_threads").workerData, () => {});' + + 'require("node:worker_threads").parentPort.postMessage("reading");', + { eval: true, workerData: fifo }, + ); + w.on("error", e => { console.error("worker error:", e); process.exitCode = 1; }); + w.once("message", () => { + w.terminate().then(code => console.log("exit code:", code)); + console.log("terminating"); + }); + `, + ], + // The gate env also brings the debug build's outstanding-ticket report + // forward (2s instead of 10s). + env: { ...bunEnv, BUN_DEBUG_TEST_WORKER_TEARDOWN_GATE: "1" }, + stdout: "pipe", + stderr: "pipe", + }); + let stdout = ""; + let stderr = ""; + const out = proc.stdout.getReader(); + const err = proc.stderr.getReader(); + const pump = async ( + r: ReadableStreamDefaultReader, + sink: (s: string) => void, + until: () => boolean, + ) => { + const dec = new TextDecoder(); + while (!until()) { + const { value, done } = await r.read(); + if (done) break; + sink(dec.decode(value, { stream: true })); + } + }; + // terminate() has been called... + await pump( + out, + s => (stdout += s), + () => stdout.includes("terminating\n"), + ); + if (isDebug || isASAN) { + // ...and the wait names what it is waiting for (deterministic: no window)... + await pump( + err, + s => (stderr += s), + () => /taken at .*node_fs\.rs:\d+/.test(stderr), + ); + expect(stderr).toContain("ticket(s) still held off-thread"); + } else { + // ...and long after an idle worker would have gone (milliseconds)... + await Bun.sleep(500); + } + // ...it has not resolved, because the read has not returned. + expect(stdout).toBe("terminating\n"); + // Release the read: open the FIFO for writing and close it (EOF). + fs.closeSync(fs.openSync(fifo, "w")); + await Promise.all([ + pump( + out, + s => (stdout += s), + () => false, + ), + pump( + err, + s => (stderr += s), + () => false, + ), + ]); + expect(stdout).toBe("terminating\nexit code: 1\n"); + expect(await proc.exited).toBe(0); + }); +}); diff --git a/test/js/web/workers/worker-refused-completion.test.ts b/test/js/web/workers/worker-refused-completion.test.ts deleted file mode 100644 index 08d246187e35..000000000000 --- a/test/js/web/workers/worker-refused-completion.test.ts +++ /dev/null @@ -1,155 +0,0 @@ -// When a worker is gone by the time another thread finishes work the worker -// started, that thread's completion is refused at the worker's VM handle and -// the producer releases what it owns itself. Here that path runs -// deterministically for one producer at a time: with -// BUN_DEBUG_TEST_WORKER_REFUSAL_GATE the completion waits for the worker's -// handle to close and is then refused, and the runtime names each refusal on -// stderr. A row passes only if the named refusal happened (the work really was -// on another thread and its release path ran) and the process exited cleanly; -// on the ASAN build the release path is also checked for use-after-free and -// leaks. Builds with debug assertions only (debug, ASAN): the gate does not -// exist in release builds. -import { describe, expect, test } from "bun:test"; -import { bunEnv, bunExe, isAndroid, isASAN, isDebug, isLinux } from "harness"; - -type Row = { - name: string; - // Runs in the worker before it exits; starts exactly one piece of off-thread work. - worker: string; - // Substring of the refusal the runtime must log for it. - refused: string; - // Runs in the parent once the worker says it is set up (for producers on the parent's side). - parent?: string; - env?: Record; - skip?: boolean; -}; - -const ROWS: Row[] = [ - { - name: "fs.readFile", - worker: `require("node:fs").readFile(process.execPath, () => {});`, - refused: "args::ReadFile", - }, - { name: "fs.promises.stat", worker: `require("node:fs").promises.stat(process.execPath);`, refused: "args::Stat" }, - { - name: "fs.realpath", - worker: `require("node:fs").realpath(process.execPath, () => {});`, - refused: "args::Realpath", - }, - { - name: "Bun.file().text()", - worker: `Bun.file(process.execPath).slice(0, 65536).text();`, - refused: "blob::read_file::ReadFile", - }, - { - // Same read job, different completion: the image's read chain is handed - // ECANCELED at teardown and has to free itself. - name: "Bun.Image(Bun.file()).metadata()", - worker: `new Bun.Image(Bun.file(process.execPath).slice(0, 65536)).metadata().catch(() => {});`, - refused: "blob::read_file::ReadFile", - }, - { - name: "crypto.pbkdf2", - worker: `require("node:crypto").pbkdf2("p", "s", 1000, 32, "sha256", () => {});`, - refused: "Pbkdf2Job", - }, - { name: "crypto.scrypt", worker: `require("node:crypto").scrypt("p", "s", 32, () => {});`, refused: "ScryptJob" }, - { - name: "crypto.randomFill", - worker: `require("node:crypto").randomFill(Buffer.alloc(65536), () => {});`, - refused: "RandomFillJob", - }, - { - name: "crypto.generateKeyPair", - worker: `require("node:crypto").generateKeyPair("ec", { namedCurve: "P-256" }, () => {});`, - refused: "EcKeyPairJob", - }, - { - name: "crypto.subtle.digest", - worker: `crypto.subtle.digest("SHA-256", Buffer.alloc(65536));`, - refused: "refused post: CppTask", - }, - { - name: "Bun.password.hash", - worker: `Bun.password.hash("x", { algorithm: "bcrypt", cost: 4 });`, - refused: "PasswordJob", - }, - { - name: "Bun.Glob scan", - worker: `new Bun.Glob("*").scan({ cwd: require("node:os").tmpdir() })[Symbol.asyncIterator]().next();`, - refused: "glob::WalkTask", - }, - { - name: "dns lookup on the thread pool", - worker: `Bun.dns.lookup("localhost", { backend: "libc" });`, - refused: "get_addr_info_request::LibcLookup", - }, - { - name: "child exit reported by the waiter thread", - worker: `require("node:child_process").execFile(process.execPath, ["-e", "0"], () => {});`, - refused: "ProcessWaiterThreadTask", - // The waiter thread is a POSIX fallback path, opted into here the way the runtime's own tests do. - env: { BUN_GARBAGE_COLLECTOR_LEVEL: "0", BUN_FEATURE_FLAG_FORCE_WAITER_THREAD: "1" }, - // The flag is honoured on Linux/Android only (kqueue platforms always have EVFILT_PROC). - skip: !isLinux && !isAndroid, - }, - { - name: "BroadcastChannel message from another thread", - worker: `const bc = new BroadcastChannel("wrc"); bc.onmessage = () => {};`, - // An open channel keeps the parent's loop alive (as in Node); close it once posted. - parent: `const pc = new BroadcastChannel("wrc"); pc.postMessage("late"); pc.close();`, - refused: "refused post: CppTask", - }, - { - name: "MessagePort message from another thread", - worker: `workerData.port.on("message", () => {});`, - parent: `port1.postMessage("late");`, - refused: "refused post: CppTask", - }, -]; - -// The host: one worker, armed gate. The worker starts its work, reports -// "armed" and exits by itself two turns later; the parent never posts anything -// the worker waits for (with the gate armed, a parent→worker post is itself a -// cross-thread completion that waits for the worker's close). Rows with a -// parent side post in response to "armed": that post is what gets refused. -function host(row: Row) { - return ` - const { Worker, MessageChannel } = require("node:worker_threads"); - const { port1, port2 } = new MessageChannel(); - const w = new Worker(\` - const { parentPort, workerData } = require("node:worker_threads"); - parentPort.on("message", () => {}); - ${row.worker.replace(/`/g, "\\`").replace(/\$\{/g, "\\${")} - parentPort.postMessage("armed"); - setImmediate(() => setImmediate(() => process.exit(0))); - \`, { eval: true, workerData: { port: port2 }, transferList: [port2] }); - w.on("error", e => { console.error("worker error:", e && e.message); process.exitCode = 1; }); - w.once("message", () => { ${row.parent ?? ""} }); - w.on("exit", code => { port1.close(); if (code !== 0) { console.error("worker exit code", code); process.exitCode = 1; } }); - `; -} - -describe.skipIf(!isDebug && !isASAN)( - "a completion for a worker that is gone is refused and released by its producer", - () => { - for (const row of ROWS) { - test.concurrent.skipIf(!!row.skip)(row.name, async () => { - await using proc = Bun.spawn({ - cmd: [bunExe(), "-e", host(row)], - env: { ...bunEnv, ...row.env, BUN_DEBUG_TEST_WORKER_REFUSAL_GATE: "1" }, - stdout: "pipe", - stderr: "pipe", - }); - const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); - const refusals = stderr.split("\n").filter(l => l.startsWith("[vm_handle] refused ")); - expect({ - exitCode, - refused: refusals.some(l => l.includes(row.refused)), - // On a failure, everything the host printed. - detail: exitCode === 0 && refusals.some(l => l.includes(row.refused)) ? "" : stdout + stderr, - }).toEqual({ exitCode: 0, refused: true, detail: "" }); - }); - } - }, -);