From b35c45036e0fde9567a3f6c6f0fa1f44c4e63e20 Mon Sep 17 00:00:00 2001 From: robobun Date: Sat, 25 Jul 2026 18:00:25 +0000 Subject: [PATCH 1/7] worker: post cross-thread completions by ScriptExecutionContext id worker.terminate() frees the VirtualMachine box while work already handed to a process-global thread (WorkPool, HTTP thread, bundle thread) is still in flight; the later completion posted back through a captured BackRef / &VirtualMachine, which is a pointer into the freed box. Every still-reproducing cross-thread UAF in this class converges on EventLoop::enqueue_task_concurrent, and several callers 'guard' with an off-thread vm.is_shutting_down() read that is itself a read of freed memory. Off-thread jobs now carry the originating ScriptExecutionContextIdentifier (a u32, cannot dangle) and post through ScriptExecutionContextIdentifier::post_concurrent_task, which looks the context up and enqueues under allScriptExecutionContextsMapLock, the same lock markTerminating() (already called in WebWorker::shutdown before the VM dealloc) takes to set the terminating flag. Either the poster's critical section ran first (task enqueued; shutdown's drain reclaims it) or markTerminating ran first (poster gets false and runs its abandon path without touching the VM). Converted: the three generic helpers (WorkTask, ConcurrentPromiseTask, AnyTaskJob, covering Bun.file/Bun.write, Transpiler.transform, Glob.scan, pbkdf2/scrypt/generateKeyPair/zstd/Secrets), plus the direct callers FetchTasklet, PasswordJob, NativeZlib/Brotli/Zstd, AsyncFSTask/NewAsyncCpTask/AsyncReaddirRecursiveTask, S3HttpSimpleTask/S3HttpDownloadStreamingTask, Archive AsyncTask, JSBundleCompletionTask, TranspilerJob, and ConcurrentCppTask's unref_concurrently (WebCrypto). Design doc: docs/ROOT-B-SHUTDOWN-FENCE.md. Verify harness: repro/rootB-verify/verify.mjs (100x arm-every-source + terminate; stock canary SIGSEGVs on teardown 1, debug+ASAN heap-UAF on teardown 1-3; 10/10 clean after). --- docs/ROOT-B-SHUTDOWN-FENCE.md | 140 ++++++++++++++++++ repro/rootB-verify/verify.mjs | 39 +++++ repro/rootB-verify/worker-body.mjs | 104 +++++++++++++ src/jsc/ConcurrentPromiseTask.rs | 24 ++- src/jsc/CppTask.rs | 20 ++- src/jsc/JSGlobalObject.rs | 61 ++++++++ src/jsc/RuntimeTranspilerStore.rs | 37 ++++- src/jsc/WorkTask.rs | 25 +++- src/jsc/any_task_job.rs | 41 ++++- src/jsc/bindings/EventLoopTaskNoContext.cpp | 5 + src/jsc/bindings/EventLoopTaskNoContext.h | 5 + src/jsc/bindings/ScriptExecutionContext.cpp | 44 ++++++ src/jsc/virtual_machine_exports.rs | 20 +++ src/runtime/api/Archive.rs | 20 +-- src/runtime/api/js_bundle_completion_task.rs | 17 ++- src/runtime/crypto/PasswordObject.rs | 23 +-- src/runtime/node/node_fs.rs | 73 +++++---- src/runtime/node/node_zlib_binding.rs | 33 +++-- src/runtime/node/zlib/NativeBrotli.rs | 2 + src/runtime/node/zlib/NativeZlib.rs | 2 + src/runtime/node/zlib/NativeZstd.rs | 2 + src/runtime/webcore/fetch/FetchTasklet.rs | 76 +++++----- src/runtime/webcore/s3/client.rs | 6 + src/runtime/webcore/s3/download_stream.rs | 22 +-- src/runtime/webcore/s3/simple_request.rs | 24 ++- .../workers/worker-terminate-lifetime.test.ts | 85 ++++++++++- 26 files changed, 805 insertions(+), 145 deletions(-) create mode 100644 docs/ROOT-B-SHUTDOWN-FENCE.md create mode 100644 repro/rootB-verify/verify.mjs create mode 100644 repro/rootB-verify/worker-body.mjs diff --git a/docs/ROOT-B-SHUTDOWN-FENCE.md b/docs/ROOT-B-SHUTDOWN-FENCE.md new file mode 100644 index 000000000000..57c446ce0c2a --- /dev/null +++ b/docs/ROOT-B-SHUTDOWN-FENCE.md @@ -0,0 +1,140 @@ +# Root B: worker teardown vs. cross-thread completions + +## The failure class + +`WebWorker::shutdown` (`src/jsc/web_worker.rs:1216`) tears down a worker's +`VirtualMachine` in five ordered steps. Step 2 sets `is_shutting_down`, drains +timers, force-closes sockets and the c-ares channel, calls +`ScriptExecutionContext::markTerminating()` (so C++ `postTaskTo` posters are +fenced), and drains the concurrent queue via +`release_queued_tasks_for_shutdown()`. Step 5 `std::alloc::dealloc`'s the raw +`VirtualMachine` box (≈`web_worker.rs:1383`) and frees the uws loop. + +Nothing in that sequence cancels or awaits work already handed to a +process-global thread (WorkPool, the HTTP thread, the bundle thread). Those +jobs complete later and post back through a pointer captured at schedule time: +a `BackRef`, a `&'static VirtualMachine`, or a `*const +JSGlobalObject`. Every one of those pointers is into the freed VM box or the +freed JSC heap. + +All reproduced cross-thread UAFs in this class converge on +**`EventLoop::enqueue_task_concurrent`** (`src/jsc/event_loop.rs:997`). Putting +a check *inside* the funnel does not help: `&self` there is already a pointer +into the freed box. Several callers also "guard" with an off-thread +`vm.is_shutting_down()` read; that read is itself a UAF face (the flag lives in +the freed box). + +## Shutdown map (reference) + +| step | action | fences | +| ---- | -------------------------------------------------------------- | ----------------------------------- | +| 1 | `self.vm = null` under `vm_lock` | parent-thread readers | +| 2a | `is_shutting_down = true`, `on_exit`, drain timers/sockets/DNS | on-thread re-entry | +| 2b | `ScriptExecutionContext::markTerminating()` | C++ `postTaskTo` posters | +| 2c | `Bun__JSCTaskScheduler__markShuttingDown()` | `Atomics.notify` posters | +| 2d | `release_queued_tasks_for_shutdown()` | tasks already queued | +| 3 | `WebWorker__teardownJSCVM` | GC finalizers, JSC heap freed | +| 4 | `WebWorker__dispatchExit` | parent releases its ref | +| 5 | `vm.destroy()`; `dealloc(vm_ptr)`; free uws loop | VM box freed | + +Rust-side cross-thread posters were not serialized with any of 2b/2c/2d. + +## The fence: enqueue by identifier + +Off-thread jobs now carry the worker's `ScriptExecutionContextIdentifier` (a +`u32`) instead of a `BackRef` / `&VirtualMachine`, and post through + +```rust +ScriptExecutionContextIdentifier::post_concurrent_task(id, task) -> bool +``` + +backed by the same locked-registry + `isTerminating()` gate that +`ScriptExecutionContext::postTaskTo` already uses: + +```cpp +extern "C" bool ScriptExecutionContext__postConcurrentTask(Identifier id, void* task) { + Locker locker { allScriptExecutionContextsMapLock }; + auto* ctx = allScriptExecutionContextsMap().get(id); + if (!ctx || ctx->isTerminating()) return false; + Bun__EventLoop__enqueueConcurrentTask(ctx->globalObject(), task); + return true; +} +``` + +`markTerminating()` (shutdown step 2b) takes the same lock to set the flag, so +every poster serializes into exactly one of two cases: + +1. The poster's whole critical section ran before `markTerminating()`: the task + is in the concurrent queue, and step 2d's drain observes and reclaims it. +2. `markTerminating()` ran first: the poster sees `isTerminating()` and returns + `false` without touching the VM. The caller owns the task and runs its + abandon path. + +A `u32` identifier cannot dangle. The funnel's `bool` return replaces every +stale off-thread `is_shutting_down()` read. + +Companion helpers on the same lock: + +- `ScriptExecutionContextIdentifier::is_alive()` — "should I even start?" check + for work bodies that write into JSC-heap buffers (e.g. `Scrypt`'s output + `ArrayBuffer`). A best-effort fast drop; the authoritative gate is + `post_concurrent_task`. +- `ScriptExecutionContextIdentifier::unref_event_loop_concurrently()` — the + `concurrent_ref` decrement that `ConcurrentCppTask` (WebCrypto) needs after + its body ran on the pool thread, without dereferencing the VM. + +## Abandon path + +On `post_concurrent_task` → `false` the target VM and its JSC heap are gone (or +about to be). The abandon path: + +- **must not** touch `Strong`/`Weak`/`JSPromiseStrong`/`JSGlobalObject`/ + `VirtualMachine`/`KeepAlive::unref` — the HandleSet is freed, the loop is + freed; +- **may** free any pure-Rust heap it owns (body buffers, `Vec`s, `Box<[u8]>`); +- **must** free a freshly heap-allocated `ConcurrentTask` node (ownership was + not transferred); +- **may** leak the job box when it holds JSC handles. Bounded: one per + terminated worker per in-flight op. + +## Coverage + +Three generic helpers carry most of the surface: + +| helper | users | +| ------------------------------ | ------------------------------------------------------------- | +| `WorkTask` | `ReadFile`, `WriteFile`, `GetAddrInfoRequest` | +| `ConcurrentPromiseTask` | `CopyFile`, `TransformTask`, `WalkTask`, `PipelineTask` | +| `AnyTaskJob` | `Pbkdf2Ctx`, `CryptoJob`, `ZstdCtx`, `SecretsCtx` | +| `ConcurrentCppTask` | WebCrypto (`PhonyWorkQueue::dispatch`) | + +Direct callers converted alongside: `FetchTasklet`, `PasswordJob`, +`CompressionStream` (zlib/brotli/zstd), `AsyncFSTask` / `NewAsyncCpTask` / +`AsyncReaddirRecursiveTask`, `S3HttpSimpleTask` / `S3HttpDownloadStreamingTask`, +`Archive::AsyncTask`, `JSBundleCompletionTask`, `TranspilerJob`. + +Explicitly **not** enqueue-shaped and left for their own fixes: nested-worker +child-init reading a freed parent VM, `node:quic` finalizer ordering, +`RedisClient::finalize` free-then-read, `Bun.SQL` handle crashes during +terminating-VM JS execution, the `serve.listen`/JS-re-entry assert zone. + +## Reserve alternative (not taken) + +Refcount-deferred VM dealloc + a closed-flag at the funnel: each off-thread job +takes an `Arc` clone of a per-VM gate and brackets its enqueue with a read +lock; `shutdown` takes the write lock before dealloc. Same sweep, but: +- `shutdown` then *waits* on in-flight pool jobs (a slow argon2/RSA can stall + terminate for seconds); +- more atomics on the hot enqueue path; +- the gate must be `Arc`'d so it outlives the VM box anyway. + +The identifier route reuses an existing lock, adds no wait, and matches what +the C++ side already does. + +## Post-fix gate + +`repro/rootB-verify/verify.mjs`: one worker per iteration arms one in-flight op +of every cross-thread source above (self-contained, loopback-only, public API), +the parent `terminate()`s mid-flight, ×100. PASS = rc 0, `ROOT-B VERIFY: PASS`, +zero ASan/assert/panic. Baseline on current canary: SIGSEGV on teardown 1 +(release), heap-use-after-free on teardown 1-3 (debug+ASAN). diff --git a/repro/rootB-verify/verify.mjs b/repro/rootB-verify/verify.mjs new file mode 100644 index 000000000000..bca2cbe13112 --- /dev/null +++ b/repro/rootB-verify/verify.mjs @@ -0,0 +1,39 @@ +// Root-B post-fix gate: one worker per iteration arms one in-flight op of +// every cross-thread completion source (public API, loopback-only), parent +// terminates mid-flight, x ITERATIONS. PASS = rc 0 + the PASS line + zero +// ASan/assert/panic. On stock canary this SIGSEGVs on the first teardown. +// +// Run under the debug+ASAN build so the fail-before is deterministic: +// bun bd repro/rootB-verify/verify.mjs + +import { Worker } from "node:worker_threads"; + +const ITERATIONS = Number(process.env.ROOTB_ITER ?? 100); +const body = new URL("./worker-body.mjs", import.meta.url); + +for (let i = 0; i < ITERATIONS; i++) { + const w = new Worker(body); + // Wait for the worker to finish arming (or give up after a bound). + const armed = await new Promise((resolve) => { + const t = setTimeout(() => resolve("timeout"), 5000); + w.once("message", (m) => { + clearTimeout(t); + resolve(m); + }); + w.once("error", (e) => { + clearTimeout(t); + resolve(e); + }); + }); + if (armed instanceof Error) { + console.error(`iter ${i}: worker error before terminate:`, armed); + process.exit(1); + } + // Small jitter so terminate lands at varying points in the in-flight work. + const jitter = (i * 2654435761 >>> 0) % 5; + if (jitter) await new Promise((r) => setTimeout(r, jitter)); + await w.terminate(); + if (i % 10 === 9) console.log(`… ${i + 1}/${ITERATIONS} teardowns clean`); +} + +console.log(`ROOT-B VERIFY: PASS (${ITERATIONS} teardowns)`); diff --git a/repro/rootB-verify/worker-body.mjs b/repro/rootB-verify/worker-body.mjs new file mode 100644 index 000000000000..ca3b56c84cb6 --- /dev/null +++ b/repro/rootB-verify/worker-body.mjs @@ -0,0 +1,104 @@ +// One worker instance: arm one in-flight op of every cross-thread completion +// source, then sit. Parent terminates us mid-flight. Every op is self-contained +// and loopback-only (no public internet). Catch-and-ignore everywhere: the +// point is to have work airborne on a process-global thread when the VM dies, +// not to observe the result. + +import { parentPort } from "node:worker_threads"; +import fs from "node:fs"; +import fsp from "node:fs/promises"; +import zlib from "node:zlib"; +import crypto from "node:crypto"; +import dns from "node:dns/promises"; +import os from "node:os"; +import path from "node:path"; + +const sink = () => {}; +const swallow = (p) => Promise.resolve(p).then(sink, sink); + +const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "rootB-")); +const tmpFile = path.join(tmp, "a.txt"); +fs.writeFileSync(tmpFile, "x".repeat(1 << 16)); + +// fetch / HTMLRewriter / TLS (HTTP thread -> FetchTasklet) +{ + const srv = Bun.serve({ + port: 0, + fetch: () => new Response("x".repeat(1 << 14)), + }); + swallow(fetch(`http://127.0.0.1:${srv.port}/`).then((r) => r.text())); + swallow( + fetch(`http://127.0.0.1:${srv.port}/`).then((r) => + new HTMLRewriter().on("*", { text() {} }).transform(r).text(), + ), + ); +} + +// Bun.file / Bun.write (WorkPool -> WorkTask/) +swallow(Bun.write(path.join(tmp, "b.txt"), "y".repeat(1 << 16))); +swallow(Bun.file(tmpFile).text()); + +// node:fs promises (WorkPool -> AsyncFSTask) +swallow(fsp.readFile(tmpFile)); +swallow(fsp.stat(tmpFile)); +swallow(fsp.readdir(tmp, { recursive: true })); + +// pbkdf2 / scrypt / generateKeyPair (WorkPool -> AnyTaskJob) +crypto.pbkdf2("p", "s", 100000, 64, "sha512", sink); +crypto.scrypt("p", "saltsalt", 64, sink); +crypto.generateKeyPair("rsa", { modulusLength: 2048 }, sink); + +// Bun.password (WorkPool -> PasswordJob) +swallow(Bun.password.hash("hunter2", { algorithm: "bcrypt", cost: 8 })); + +// zlib (WorkPool -> CompressionStream async_job_run) +zlib.deflate(Buffer.alloc(1 << 18), sink); +zlib.gzip(Buffer.alloc(1 << 18), sink); + +// S3 (HTTP thread -> S3HttpSimpleTask); loopback endpoint that never answers. +{ + const srv = Bun.serve({ port: 0, fetch: () => new Promise(sink) }); + const s3 = new Bun.S3Client({ + accessKeyId: "x", + secretAccessKey: "y", + endpoint: `http://127.0.0.1:${srv.port}`, + bucket: "b", + }); + swallow(s3.file("k").text()); +} + +// Bun.build (BundleThread -> JSBundleCompletionTask) +{ + const entry = path.join(tmp, "entry.ts"); + fs.writeFileSync(entry, `export const x: number = 1;\n`); + swallow(Bun.build({ entrypoints: [entry], target: "bun" })); +} + +// Transpiler.transform (WorkPool -> ConcurrentPromiseTask) +swallow(new Bun.Transpiler({ loader: "tsx" }).transform("const x: number = 1;")); + +// dns.lookup (c-ares; clean-by-construction via close_dns_for_terminate) +swallow(dns.lookup("localhost")); + +// WebCrypto (WorkPool -> ConcurrentCppTask) +swallow( + crypto.subtle.digest("SHA-256", new Uint8Array(1 << 16)), +); +swallow( + crypto.subtle.generateKey({ name: "AES-GCM", length: 256 }, true, [ + "encrypt", + "decrypt", + ]), +); + +// Glob (WorkPool -> ConcurrentPromiseTask) +swallow(Array.fromAsync(new Bun.Glob("**/*").scan(tmp))); + +// zstd (WorkPool -> AnyTaskJob) +swallow(Bun.zstdCompress(Buffer.alloc(1 << 16))); + +// Signal parent that everything is airborne. +parentPort.postMessage("armed"); + +// Keep the loop alive so terminate() lands mid-flight. +setInterval(sink, 1 << 30); diff --git a/src/jsc/ConcurrentPromiseTask.rs b/src/jsc/ConcurrentPromiseTask.rs index 62aba68d025b..69bd61694751 100644 --- a/src/jsc/ConcurrentPromiseTask.rs +++ b/src/jsc/ConcurrentPromiseTask.rs @@ -3,6 +3,7 @@ use bun_io::{self as Async, KeepAlive}; use bun_threading::{IntrusiveWorkTask as _, WorkPoolTask, work_pool::WorkPool}; use crate::event_loop::EventLoop; +use crate::js_global_object::ScriptExecutionContextIdentifier; use crate::js_promise::{JSPromise, Strong as JSPromiseStrong}; use crate::virtual_machine::VirtualMachine; use crate::{JSGlobalObject, JsTerminated}; @@ -31,9 +32,15 @@ pub struct ConcurrentPromiseTask<'a, Context: ConcurrentPromiseTaskContext> { // Owned here so dropping the task frees the context. pub ctx: Box, pub task: WorkPoolTask, - /// BACKREF — captured from the JS-thread VM at create time; the VM (and its - /// `EventLoop`) outlives every task scheduled on it. + /// BACKREF — captured from the JS-thread VM at create time. Only + /// dereferenced on the JS thread; the pool-thread completion goes through + /// [`Self::context_id`] so a worker VM freed by `terminate()` is never + /// touched. pub event_loop: BackRef, + /// Stable `u32` id for the originating `ScriptExecutionContext`. The + /// pool-thread `on_finish` posts the completion via this id under the C++ + /// contexts-map lock (serializing with worker `markTerminating()`). + pub context_id: ScriptExecutionContextIdentifier, pub promise: JSPromiseStrong, pub global_this: &'a JSGlobalObject, pub concurrent_task: ConcurrentTask, @@ -62,6 +69,7 @@ impl<'a, Context: ConcurrentPromiseTaskContext> ConcurrentPromiseTask<'a, Contex let event_loop = BackRef::new(VirtualMachine::get().as_mut().event_loop_shared()); let mut this = Box::new(Self { event_loop, + context_id: global_this.script_execution_context_identifier(), ctx: value, task: WorkPoolTask { node: Default::default(), @@ -108,15 +116,19 @@ impl<'a, Context: ConcurrentPromiseTaskContext> ConcurrentPromiseTask<'a, Contex // `this` while holding `&mut *this` is sound because `from` only stores // the pointer (does not dereference it). let this_ref = unsafe { &mut *this }; - let event_loop = this_ref.event_loop; + let context_id = this_ref.context_id; let task = core::ptr::NonNull::from( this_ref .concurrent_task .from(this, AutoDeinit::ManualDeinit), ); - // `task` is the live `concurrent_task` field of the heap-allocated - // job; the queue takes ownership of its intrusive `next` link. - event_loop.enqueue_task_concurrent(task); + // Post by stable context id: the enqueue dereferences the target VM + // only under the contexts-map lock (serializes with worker + // `markTerminating()`). When the context is gone the box is leaked + // (its `JSPromiseStrong` and `global_this` point into the dead JSC + // heap and cannot be released off-thread); `task` is a field of + // `*this`, so no separate free. + let _ = context_id.post_concurrent_task(task); } /// Frees the heap allocation backing this task. diff --git a/src/jsc/CppTask.rs b/src/jsc/CppTask.rs index 130402469c25..a288aa12cc71 100644 --- a/src/jsc/CppTask.rs +++ b/src/jsc/CppTask.rs @@ -1,5 +1,6 @@ use core::ptr::NonNull; +use crate::js_global_object::ScriptExecutionContextIdentifier; use crate::{JSGlobalObject, JsResult, VirtualMachineRef as VirtualMachine}; use bun_event_loop::{TaskTag, Taskable, task_tag}; use bun_threading::work_pool::{Task as WorkPoolTask, WorkPool}; @@ -10,6 +11,7 @@ unsafe extern "C" { safe fn Bun__EventLoopTaskNoContext__createdInBunVm( task: &EventLoopTaskNoContext, ) -> *mut VirtualMachine; + safe fn Bun__EventLoopTaskNoContext__contextId(task: &EventLoopTaskNoContext) -> u32; } bun_opaque::opaque_ffi! { @@ -55,6 +57,14 @@ impl EventLoopTaskNoContext { pub fn get_vm(&self) -> Option> { NonNull::new(Bun__EventLoopTaskNoContext__createdInBunVm(self)).map(bun_ptr::BackRef::from) } + + /// Stable `u32` id of the `ScriptExecutionContext` that created this task, + /// captured on the JS thread at construction. Safe to use off-thread via + /// [`ScriptExecutionContextIdentifier::unref_event_loop_concurrently`] so + /// the VM is dereferenced only under the contexts-map lock. + pub fn context_id(&self) -> ScriptExecutionContextIdentifier { + ScriptExecutionContextIdentifier(Bun__EventLoopTaskNoContext__contextId(self)) + } } /// A task created from C++ code that runs inside the workpool, usually via ScriptExecutionContext. @@ -73,14 +83,16 @@ impl ConcurrentCppTask { 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. - let maybe_vm = EventLoopTaskNoContext::opaque_ref(cpp_task).get_vm(); + let context_id = EventLoopTaskNoContext::opaque_ref(cpp_task).context_id(); drop(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. unsafe { EventLoopTaskNoContext::run(cpp_task) }; - if let Some(vm) = maybe_vm { - vm.event_loop_shared().unref_concurrently(); - } + // The task body (e.g. a WebCrypto crypto-thread lambda) posts its own + // result back via `ScriptExecutionContext::postTaskTo`, which already + // serializes with `markTerminating()`. The trailing `concurrent_ref` + // decrement dereferences the VM, so route it through the same lock. + context_id.unref_event_loop_concurrently(); } } diff --git a/src/jsc/JSGlobalObject.rs b/src/jsc/JSGlobalObject.rs index 896ca52e75e3..bde5994ef155 100644 --- a/src/jsc/JSGlobalObject.rs +++ b/src/jsc/JSGlobalObject.rs @@ -1702,9 +1702,70 @@ impl ScriptExecutionContextIdentifier { pub fn valid(self) -> bool { self.global_object().is_some() } + + /// `true` while the context exists and has not been marked terminating. + /// Serializes with `markTerminating()` on the C++ contexts-map lock, so + /// unlike [`Self::valid`] this is safe to consult off-thread for the + /// "skip a queued unit of work whose worker has begun shutdown" check. + #[inline] + pub fn is_alive(self) -> bool { + ScriptExecutionContext__isAlive(self.0) + } + + /// Enqueue a heap-allocated [`ConcurrentTaskItem`] onto this context's JS + /// event loop from any thread, without holding any pointer into the target + /// VM. The enqueue is performed under the C++ contexts-map lock (the same + /// lock worker `markTerminating()` takes before its VM is freed), so the + /// target `EventLoop` is dereferenced only while known live. + /// + /// Returns `true` if the task was enqueued (ownership transferred to the + /// queue). Returns `false` if the context is gone or terminating; the + /// caller retains ownership of `task` and must free it (without touching + /// the target VM or JSC heap). + /// + /// [`ConcurrentTaskItem`]: crate::event_loop::ConcurrentTaskItem + #[inline] + pub fn post_concurrent_task( + self, + task: core::ptr::NonNull, + ) -> bool { + ScriptExecutionContext__postConcurrentTask(self.0, task.as_ptr().cast::()) + } + + /// Decrement this context's event-loop `concurrent_ref` from off-thread + /// under the contexts-map lock; no-op when the context is gone or + /// terminating. The paired increment happened on the JS thread before the + /// job was scheduled, so a lost decrement on a terminating worker is the + /// intended behavior (its loop will never tick again). + #[inline] + pub fn unref_event_loop_concurrently(self) { + ScriptExecutionContext__unrefEventLoopConcurrently(self.0); + } } unsafe extern "C" { // safe: by-value `u32` in, raw nullable pointer out (caller checks before deref). safe fn ScriptExecutionContextIdentifier__getGlobalObject(id: u32) -> *mut JSGlobalObject; + // safe: by-value `u32` in, opaque pointer C++ only forwards back to Rust + // under the contexts-map lock; bool out. + safe fn ScriptExecutionContext__postConcurrentTask(id: u32, task: *mut c_void) -> bool; + // safe: by-value `u32` in; bool out. + safe fn ScriptExecutionContext__isAlive(id: u32) -> bool; + // safe: by-value `u32` in; void out. + safe fn ScriptExecutionContext__unrefEventLoopConcurrently(id: u32); + // safe: `&JSGlobalObject` is a live opaque handle; returns the context's id. + safe fn ScriptExecutionContextIdentifier__forGlobalObject(global: &JSGlobalObject) -> u32; +} + +impl JSGlobalObject { + /// The stable `u32` identifier for this global's `ScriptExecutionContext`. + /// Capture on the JS thread when scheduling off-thread work; the pool + /// thread posts its completion back via + /// [`ScriptExecutionContextIdentifier::post_concurrent_task`] instead of a + /// raw `&EventLoop`/`&VirtualMachine`, so a worker VM freed by + /// `terminate()` is never dereferenced. + #[inline] + pub fn script_execution_context_identifier(&self) -> ScriptExecutionContextIdentifier { + ScriptExecutionContextIdentifier(ScriptExecutionContextIdentifier__forGlobalObject(self)) + } } diff --git a/src/jsc/RuntimeTranspilerStore.rs b/src/jsc/RuntimeTranspilerStore.rs index acd5bb4d9438..7e47b1c15086 100644 --- a/src/jsc/RuntimeTranspilerStore.rs +++ b/src/jsc/RuntimeTranspilerStore.rs @@ -34,6 +34,7 @@ use bun_watcher::Watcher; use crate::async_module::AsyncModule; use crate::event_loop::{ConcurrentTask, EventLoop}; use crate::hot_reloader::ImportWatcher; +use crate::js_global_object::ScriptExecutionContextIdentifier; use crate::resolved_source::OwnedResolvedSource; use crate::resolved_source_tag::ResolvedSourceTag; use crate::runtime_transpiler_cache::{ @@ -319,6 +320,7 @@ impl RuntimeTranspilerStore { non_threadsafe_input_specifier: OwnedString::new(input_specifier), path: owned_path, global_this: BackRef::new(global_object), + context_id: global_object.script_execution_context_identifier(), non_threadsafe_referrer: OwnedString::new(referrer), vm, log: bun_ast::Log::init(), @@ -378,6 +380,7 @@ pub struct TranspilerJob { // store and outlives every job). pub vm: *mut VirtualMachine, pub global_this: BackRef, + pub context_id: ScriptExecutionContextIdentifier, pub fetcher: Fetcher, pub poll_ref: KeepAlive, pub generation_number: u32, @@ -485,17 +488,43 @@ impl TranspilerJob { } pub(crate) fn dispatch_to_main_thread(&mut self) { + let context_id = self.context_id; + // The VM owns both `transpiler_store.queue` and the event loop; touch + // neither once the target context has begun teardown. `is_alive()` + // serializes with `markTerminating()` on the contexts-map lock. + if !context_id.is_alive() { + // Reclaim pure-Rust heap owned by this job; leak the JSC-heap + // handles (`promise`, `fetcher`, specifiers, `resolved_source`). + let old_path = core::mem::take(&mut self.path); + if !old_path.text.is_empty() { + // SAFETY: `text` is the `Box<[u8]>` produced by + // `heap::into_raw` in `transpile()`; this is the unique owner. + drop(unsafe { + bun_core::heap::take(ptr::from_ref::<[u8]>(old_path.text).cast_mut()) + }); + } + self.log = bun_ast::Log::init(); + self.parse_error = None; + return; + } let vm = self.vm; - // SAFETY: vm outlives the job (BACKREF — VM owns the store). + // SAFETY: `is_alive()` held the contexts-map lock; the VM is live for + // this queue push (BACKREF — VM 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. - // SAFETY: vm outlives the job; event_loop() returns the live self-pointer. - unsafe { &*(*vm).event_loop() } - .enqueue_task_concurrent(ConcurrentTask::create_from(transpiler_store)); + let task = ConcurrentTask::create_from(transpiler_store); + if !context_id.post_concurrent_task(task) { + // Raced with `markTerminating()`; the job is already in + // `transpiler_store.queue` (inside the VM being freed). Reclaim + // only the freshly-allocated node. + // SAFETY: ownership was not transferred; `task` is the + // `ConcurrentTask::create_from` heap node above. + drop(unsafe { bun_core::heap::take(task.as_ptr()) }); + } } pub(crate) fn run_from_js_thread(&mut self) -> JsResult<()> { diff --git a/src/jsc/WorkTask.rs b/src/jsc/WorkTask.rs index aa90ad9373a0..5dd280339403 100644 --- a/src/jsc/WorkTask.rs +++ b/src/jsc/WorkTask.rs @@ -5,6 +5,7 @@ use bun_threading::{IntrusiveWorkTask as _, WorkPoolTask, work_pool::WorkPool}; use crate::JSGlobalObject; use crate::debugger::AsyncTaskTracker; use crate::event_loop::EventLoop; +use crate::js_global_object::ScriptExecutionContextIdentifier; use bun_ptr::BackRef; /// A generic task that runs work on a thread pool and executes a callback on the main JavaScript thread. @@ -34,9 +35,15 @@ pub trait WorkTaskContext: Sized { pub struct WorkTask { pub ctx: *mut Context, pub task: WorkPoolTask, - /// BACKREF — captured from the JS-thread VM at create time; the VM (and its - /// `EventLoop`) outlives every task scheduled on it. + /// BACKREF — captured from the JS-thread VM at create time. Only + /// dereferenced on the JS thread; the pool-thread completion goes through + /// [`Self::context_id`] so a worker VM freed by `terminate()` is never + /// touched. pub event_loop: BackRef, + /// Stable `u32` id for the originating `ScriptExecutionContext`. The + /// pool-thread `on_finish` posts the completion via this id under the C++ + /// contexts-map lock (serializing with worker `markTerminating()`). + pub context_id: ScriptExecutionContextIdentifier, // allocator field dropped — global mimalloc (see PORTING.md §Allocators) pub global_this: BackRef, pub concurrent_task: ConcurrentTask, @@ -64,6 +71,7 @@ impl WorkTask { let event_loop = BackRef::new(vm.event_loop_shared()); let mut this = Box::new(Self { event_loop, + context_id: global_this.script_execution_context_identifier(), ctx: value, global_this: BackRef::new(global_this), task: WorkPoolTask { @@ -129,14 +137,19 @@ impl WorkTask { // re-initializes it in place and returns the same address. Passing // `this_ptr` while holding `&mut *this` is sound because `from` only // stores the pointer (does not dereference it). - let event_loop = this.event_loop; + let context_id = this.context_id; let this_ptr: *mut Self = this; let task = core::ptr::NonNull::from( this.concurrent_task .from(this_ptr, AutoDeinit::ManualDeinit), ); - // `task` is the inline `concurrent_task` field of the live - // heap-allocated `*this`; `event_loop` is the JS-thread loop stored at init. - event_loop.enqueue_task_concurrent(task); + // Post by stable context id: the enqueue dereferences the target VM + // only under the contexts-map lock (serializes with worker + // `markTerminating()`). When the context is gone, `run_from_js` will + // never fire; `*this` (and its JSC-heap `global_this` / `ctx` + // `Strong` handles) cannot be reclaimed off-thread, so the box is + // leaked. The intrusive `task` is a field of `*this`, so no separate + // free is needed. + let _ = context_id.post_concurrent_task(task); } } diff --git a/src/jsc/any_task_job.rs b/src/jsc/any_task_job.rs index 870232d0fae8..71886a543d94 100644 --- a/src/jsc/any_task_job.rs +++ b/src/jsc/any_task_job.rs @@ -15,6 +15,7 @@ use bun_io::KeepAlive; use bun_threading::work_pool::{IntrusiveWorkTask as _, Task as WorkPoolTask, WorkPool}; use crate::event_loop::ConcurrentTask; +use crate::js_global_object::ScriptExecutionContextIdentifier; use crate::{JSGlobalObject, JsResult, VirtualMachineRef as VirtualMachine}; /// Per-job payload trait. Implementors own the off-thread work body and the @@ -49,6 +50,12 @@ pub trait AnyTaskJobCtx: Sized { /// e.g. a `JSPromiseStrong` field after scheduling. pub struct AnyTaskJob { vm: bun_ptr::BackRef, + /// Stable `u32` id for the originating `ScriptExecutionContext`. The + /// pool-thread [`Self::run_task`] posts the completion back via this id + /// under the C++ contexts-map lock, so `vm` (which embeds the target + /// `EventLoop`) is never dereferenced off-thread once a worker's + /// `markTerminating()` has returned. + context_id: ScriptExecutionContextIdentifier, task: WorkPoolTask, any_task: AnyTask, poll: KeepAlive, @@ -75,6 +82,7 @@ impl AnyTaskJob { let vm = bun_ptr::BackRef::new(global.bun_vm()); let job = bun_core::heap::into_raw(Box::new(Self { vm, + context_id: global.script_execution_context_identifier(), task: WorkPoolTask { node: Default::default(), callback: Self::run_task, @@ -137,14 +145,33 @@ impl AnyTaskJob { fn run_task(task: *mut WorkPoolTask) { // SAFETY: only reachable via the `WorkPoolTask::callback` slot wired // in `create`; `task` points to `Self.task` and the job is live until - // `run_from_js` reclaims it. + // `run_from_js` reclaims it (or is leaked on abandon below). let job = unsafe { &mut *Self::from_task_ptr(task) }; - let vm = job.vm; - job.ctx.run(vm.global); - // `ConcurrentTask::create` heap-allocates a fresh task; the queue takes - // ownership of it. - vm.event_loop_shared() - .enqueue_task_concurrent(ConcurrentTask::create(job.any_task.task())); + let context_id = job.context_id; + // Skip the work body when the owning context has already begun + // shutdown: several ctxs (e.g. `Scrypt`) write their output into a + // JSC-heap-backed `ArrayBuffer` that is freed by `teardownJSCVM`. + // `is_alive()` serializes with `markTerminating()` on the contexts-map + // lock. This is a fast-path drop only; the authoritative gate is the + // `post_concurrent_task` below. + if context_id.is_alive() { + let vm = job.vm; + job.ctx.run(vm.global); + } + // Post by stable context id: the enqueue dereferences the target VM + // only under the contexts-map lock (serializes with worker + // `markTerminating()`). + let task = ConcurrentTask::create(job.any_task.task()); + if context_id.post_concurrent_task(task) { + return; + } + // Target context gone or terminating. `run_from_js` will never fire; + // reclaim the heap `ConcurrentTaskItem`. The job box itself (and its + // `ctx`, which typically holds `JSPromiseStrong`/`Strong` handles into + // the dead JSC heap) cannot be released off-thread and is leaked. + // SAFETY: `post_concurrent_task` returned false so ownership was not + // transferred; `task` was `ConcurrentTask::create`-allocated above. + drop(unsafe { bun_core::heap::take(task.as_ptr()) }); } /// `AnyTask` callback — runs ON the JS thread. Reclaims the heap diff --git a/src/jsc/bindings/EventLoopTaskNoContext.cpp b/src/jsc/bindings/EventLoopTaskNoContext.cpp index 6f1f3d367ad9..028765551391 100644 --- a/src/jsc/bindings/EventLoopTaskNoContext.cpp +++ b/src/jsc/bindings/EventLoopTaskNoContext.cpp @@ -12,4 +12,9 @@ extern "C" void* Bun__EventLoopTaskNoContext__createdInBunVm(const EventLoopTask return task->createdInBunVm(); } +extern "C" uint32_t Bun__EventLoopTaskNoContext__contextId(const EventLoopTaskNoContext* task) +{ + return task->contextId(); +} + } // namespace Bun diff --git a/src/jsc/bindings/EventLoopTaskNoContext.h b/src/jsc/bindings/EventLoopTaskNoContext.h index fede33f2603c..c7df6a092ed5 100644 --- a/src/jsc/bindings/EventLoopTaskNoContext.h +++ b/src/jsc/bindings/EventLoopTaskNoContext.h @@ -1,6 +1,7 @@ #pragma once #include "ZigGlobalObject.h" +#include "ScriptExecutionContext.h" #include "root.h" namespace Bun { @@ -12,6 +13,7 @@ class EventLoopTaskNoContext { public: EventLoopTaskNoContext(JSC::JSGlobalObject* globalObject, Function&& task) : m_createdInBunVm(defaultGlobalObject(globalObject)->bunVM()) + , m_contextId(defaultGlobalObject(globalObject)->scriptExecutionContext()->identifier()) , m_task(WTF::move(task)) { } @@ -23,13 +25,16 @@ class EventLoopTaskNoContext { } void* createdInBunVm() const { return m_createdInBunVm; } + WebCore::ScriptExecutionContextIdentifier contextId() const { return m_contextId; } private: void* m_createdInBunVm; + WebCore::ScriptExecutionContextIdentifier m_contextId; Function m_task; }; extern "C" void Bun__EventLoopTaskNoContext__performTask(EventLoopTaskNoContext* task); extern "C" void* Bun__EventLoopTaskNoContext__createdInBunVm(const EventLoopTaskNoContext* task); +extern "C" uint32_t Bun__EventLoopTaskNoContext__contextId(const EventLoopTaskNoContext* task); } // namespace Bun diff --git a/src/jsc/bindings/ScriptExecutionContext.cpp b/src/jsc/bindings/ScriptExecutionContext.cpp index 60f4ca58b9ef..47f50a3ee280 100644 --- a/src/jsc/bindings/ScriptExecutionContext.cpp +++ b/src/jsc/bindings/ScriptExecutionContext.cpp @@ -302,6 +302,50 @@ extern "C" JSC::JSGlobalObject* ScriptExecutionContextIdentifier__getGlobalObjec return context->globalObject(); } +// True while the context exists and has not been marked terminating. Checked +// under the contexts-map lock so it serializes with markTerminating(). +extern "C" bool ScriptExecutionContext__isAlive(ScriptExecutionContextIdentifier id) +{ + Locker locker { allScriptExecutionContextsMapLock }; + auto* context = allScriptExecutionContextsMap().get(id); + return context && !context->isTerminating(); +} + +extern "C" void Bun__EventLoop__enqueueConcurrentTask(JSC::JSGlobalObject*, void* task); + +// postTaskTo() for a Rust-allocated ConcurrentTask. Returns true if the task +// was enqueued (ownership transferred to the target context's concurrent +// queue); false if the context is gone or terminating, in which case the +// caller retains ownership. The map lock is held across the enqueue, which is +// what serializes this with markTerminating(): either the poster's whole +// critical section runs before worker shutdown's markTerminating() (task +// enqueued, and shutdown's subsequent release_queued_tasks_for_shutdown() +// drain observes it), or after (isTerminating() true; poster drops the task +// without touching the freed VM). See markTerminating()'s comment for the +// ordering argument. +extern "C" bool ScriptExecutionContext__postConcurrentTask(ScriptExecutionContextIdentifier id, void* task) +{ + Locker locker { allScriptExecutionContextsMapLock }; + auto* context = allScriptExecutionContextsMap().get(id); + if (!context || context->isTerminating()) + return false; + Bun__EventLoop__enqueueConcurrentTask(context->globalObject(), task); + return true; +} + +// Decrement the target context's event-loop concurrent refcount, under the +// contexts-map lock so the bunVM pointer is dereferenced only while the +// context is known live. No-op (and no VM dereference) when the context is +// gone or terminating. +extern "C" void ScriptExecutionContext__unrefEventLoopConcurrently(ScriptExecutionContextIdentifier id) +{ + Locker locker { allScriptExecutionContextsMapLock }; + auto* context = allScriptExecutionContextsMap().get(id); + if (!context || context->isTerminating()) + return; + Bun__eventLoop__incrementRefConcurrently(WebCore::clientData(context->vm())->bunVM, -1); +} + extern "C" void ScriptExecutionContext__markTerminating(JSC::JSGlobalObject* globalObject) { if (auto* context = defaultGlobalObject(globalObject)->scriptExecutionContext()) diff --git a/src/jsc/virtual_machine_exports.rs b/src/jsc/virtual_machine_exports.rs index 749067bd49ce..461733674344 100644 --- a/src/jsc/virtual_machine_exports.rs +++ b/src/jsc/virtual_machine_exports.rs @@ -141,6 +141,26 @@ pub fn queue_task_concurrently(global: &JSGlobalObject, task: *mut crate::cpp_ta } } +/// Called from C++ `ScriptExecutionContext__postConcurrentTask` with the +/// contexts-map lock held, so `global`'s VM and its embedded `EventLoop` are +/// guaranteed live for the duration of this call (worker `markTerminating()` +/// serializes on the same lock before any VM dealloc). `task` is the +/// heap-allocated `ConcurrentTaskItem` the Rust caller passed in. +// HOST_EXPORT(Bun__EventLoop__enqueueConcurrentTask, c) +#[allow(clippy::not_unsafe_ptr_arg_deref)] +pub fn event_loop_enqueue_concurrent_task( + global: &JSGlobalObject, + task: *mut crate::event_loop::ConcurrentTaskItem, +) { + crate::mark_binding!(); + // SAFETY: see fn doc — VM/EventLoop live under the held contexts-map lock; + // `task` is the non-null heap allocation forwarded from Rust. + unsafe { + (*(*global.bun_vm_concurrently()).event_loop()) + .enqueue_task_concurrent(core::ptr::NonNull::new_unchecked(task)); + } +} + // HOST_EXPORT(Bun__handleRejectedPromise, c) pub fn handle_rejected_promise(global: &JSGlobalObject, promise: &mut JSPromise) { crate::mark_binding!(); diff --git a/src/runtime/api/Archive.rs b/src/runtime/api/Archive.rs index 39cbbdffc4d6..469f71340d58 100644 --- a/src/runtime/api/Archive.rs +++ b/src/runtime/api/Archive.rs @@ -11,6 +11,7 @@ use bun_event_loop::{TaskTag, Taskable, task_tag}; use bun_glob as glob; use bun_io::KeepAlive; use bun_jsc::ConcurrentTask::{AutoDeinit, ConcurrentTask}; +use bun_jsc::js_global_object::ScriptExecutionContextIdentifier; use bun_jsc::virtual_machine::VirtualMachine; use bun_jsc::{ self as jsc, CallFrame, JSGlobalObject, JSMap, JSPromise, JSPromiseStrong, JSValue, JsResult, @@ -682,7 +683,7 @@ pub trait TaskContext: Send { pub struct AsyncTask { ctx: C, promise: JSPromiseStrong, - vm: *mut VirtualMachine, + context_id: ScriptExecutionContextIdentifier, task: WorkPoolTask, concurrent_task: ConcurrentTask, keep_alive: KeepAlive, @@ -694,15 +695,10 @@ impl Taskable for AsyncTask { impl AsyncTask { fn create(global: &JSGlobalObject, ctx: C) -> Result<*mut Self, bun_alloc::AllocError> { - // `bun_vm_ptr()` returns `*mut VirtualMachine` with write provenance; valid for - // process lifetime. Do NOT launder `bun_vm()` (a `&VirtualMachine`) through - // `*const _ as *mut _` — that derives a writeable pointer from a shared - // reference and is UB under Stacked Borrows. - let vm: *mut VirtualMachine = global.bun_vm_ptr(); let this = Box::new(AsyncTask { ctx, promise: JSPromiseStrong::init(global), - vm, + context_id: global.script_execution_context_identifier(), task: WorkPoolTask { callback: Self::run_callback, node: Default::default(), @@ -746,12 +742,18 @@ impl AsyncTask { let this: *mut Self = unsafe { bun_core::from_field_ptr!(Self, task, work_task) }; // SAFETY: thread-pool has exclusive access to ctx until it enqueues the concurrent task. unsafe { (*this).ctx.run() }; - // SAFETY: vm points to the live owning VM; concurrent_task is intrusive on the same allocation. + // SAFETY: concurrent_task is intrusive on the same allocation. unsafe { + let context_id = (*this).context_id; let ct = core::ptr::NonNull::from( (*this).concurrent_task.from(this, AutoDeinit::ManualDeinit), ); - (*(*this).vm).enqueue_task_concurrent(ct); + // Post by stable context id so the target VM is only dereferenced + // under the contexts-map lock. If the context is gone the box is + // leaked — its `JSPromiseStrong` lives in the dead JSC heap and + // cannot be released off-thread; `ct` is a field of `*this`, so no + // separate free. + let _ = context_id.post_concurrent_task(ct); } } diff --git a/src/runtime/api/js_bundle_completion_task.rs b/src/runtime/api/js_bundle_completion_task.rs index 4932a07891c6..203cb80205ef 100644 --- a/src/runtime/api/js_bundle_completion_task.rs +++ b/src/runtime/api/js_bundle_completion_task.rs @@ -26,6 +26,7 @@ use bun_io::KeepAlive; use bun_jsc::AnyTask::AnyTask; use bun_jsc::WorkPool; use bun_jsc::event_loop::EventLoop; +use bun_jsc::js_global_object::ScriptExecutionContextIdentifier; use bun_jsc::{self as jsc, JSGlobalObject, JSPromise, JSValue}; use bun_options_types::WindowsOptions; use bun_options_types::schema::api; @@ -61,6 +62,7 @@ pub struct JSBundleCompletionTask { // BACKREF — the JS-thread `EventLoop` outlives every completion task; safe // `Deref` so call sites read `self.jsc_event_loop.enqueue_task_concurrent(..)`. pub jsc_event_loop: BackRef, + pub context_id: ScriptExecutionContextIdentifier, pub task: AnyTask, pub global_this: BackRef, pub promise: jsc::JSPromiseStrong, @@ -125,6 +127,7 @@ pub(crate) fn create_and_schedule_completion_task( // `event_loop` is the live JS-thread loop (caller derives it from // `vm.event_loop()`); never null once `Bun.build` is reachable. jsc_event_loop: BackRef::from(core::ptr::NonNull::new(event_loop).expect("event_loop")), + context_id: global_this.script_execution_context_identifier(), task: AnyTask::default(), global_this: BackRef::new(global_this), promise: jsc::JSPromiseStrong::default(), @@ -979,11 +982,15 @@ impl CompletionStruct for JSBundleCompletionTask { } fn complete_on_bundle_thread(&mut self) { - // `jsc_event_loop` is a `BackRef` — safe Deref. - // `ConcurrentTask::create` heap-allocates a fresh task; the - // queue takes ownership of it. - self.jsc_event_loop - .enqueue_task_concurrent(jsc::ConcurrentTask::create(self.task.task())); + let task = jsc::ConcurrentTask::create(self.task.task()); + if !self.context_id.post_concurrent_task(task) { + // SAFETY: ownership was not transferred; `task` is the fresh heap + // node from `ConcurrentTask::create` above. + drop(unsafe { bun_core::heap::take(task.as_ptr()) }); + // `on_complete_anytask` will never run; the box (promise/plugins/ + // KeepAlive) points into the dead VM and cannot be reclaimed + // off-thread. Bounded leak on worker terminate. + } } fn set_result(&mut self, result: BundleV2Result) { self.result = result; diff --git a/src/runtime/crypto/PasswordObject.rs b/src/runtime/crypto/PasswordObject.rs index 9fe555e60420..d093113f3734 100644 --- a/src/runtime/crypto/PasswordObject.rs +++ b/src/runtime/crypto/PasswordObject.rs @@ -9,12 +9,12 @@ use bun_jsc::{ }; // `bun_jsc::{AnyTask, ConcurrentTask, EventLoop}` are *modules* (re-exported from // `bun_event_loop`); pull the concrete types out by name. -use bun_jsc::event_loop::EventLoop; // JSC-side ZigString carries `to_js` (the `bun_core::ZigString` repr-twin // lives in `bun_jsc::zig_string`); used for ASCII→JS conversions only. use bun_jsc::AnyTask::{AnyTask, JsResult as AnyTaskJsResult}; use bun_jsc::ConcurrentTask::ConcurrentTask; use bun_jsc::ZigStringJsc as _; +use bun_jsc::js_global_object::ScriptExecutionContextIdentifier; use bun_jsc::zig_string::ZigString as JscZigString; use bun_jsc::{JSPromise, JSPromiseStrong}; use bun_threading::work_pool::WorkPool; @@ -549,7 +549,7 @@ struct PasswordJob { op: Op, password: Box<[u8]>, promise: JSPromiseStrong, - event_loop: *mut EventLoop, + context_id: ScriptExecutionContextIdentifier, global: *const JSGlobalObject, r#ref: KeepAlive, task: WorkPoolTask, @@ -585,13 +585,15 @@ impl PasswordJob { unsafe { (*result).task = AnyTask::from_typed(result, PasswordResult::::run_from_js_erased); } - // SAFETY: `event_loop` was stored from the JS-thread VM and outlives the - // job; ownership of `result` transfers to the event loop here. `task` is - // an intrusive field at a stable address. - unsafe { - (*self.event_loop).enqueue_task_concurrent(ConcurrentTask::create_from( - core::ptr::addr_of_mut!((*result).task), - )); + // SAFETY: `result.task` is an intrusive field at a stable heap address. + let node = ConcurrentTask::create_from(unsafe { core::ptr::addr_of_mut!((*result).task) }); + if !self.context_id.post_concurrent_task(node) { + // Target context is gone. Free the queue node and the result box; + // leak the JSPromiseStrong (its JSC heap is dead and cannot be + // released off-thread). + drop(unsafe { bun_core::heap::take(node.as_ptr()) }); + let mut r = unsafe { bun_core::heap::take(result) }; + core::mem::forget(core::mem::take(&mut r.promise)); } // `self: Box` drops here; Drop runs secure_zero on password (+op). } @@ -670,8 +672,7 @@ impl JSPasswordObject { op, password, promise, - // SAFETY: bun_vm() is non-null for a Bun-owned global; VM outlives the job. - event_loop: global_object.bun_vm().event_loop(), + context_id: global_object.script_execution_context_identifier(), global: std::ptr::from_ref(global_object), r#ref: KeepAlive::default(), task: WorkPoolTask::default(), diff --git a/src/runtime/node/node_fs.rs b/src/runtime/node/node_fs.rs index 3ee2ace9e788..6773cb11973e 100644 --- a/src/runtime/node/node_fs.rs +++ b/src/runtime/node/node_fs.rs @@ -17,6 +17,7 @@ use bun_io::KeepAlive; use bun_jsc::AbortSignal; use bun_jsc::EventLoopTaskPtr; use bun_jsc::debugger::AsyncTaskTracker; +use bun_jsc::js_global_object::ScriptExecutionContextIdentifier; use bun_jsc::virtual_machine::VirtualMachine; use bun_jsc::{EventLoopHandle, JSGlobalObject, JSValue, JsResult, Task, ThreadSafe, Unprotect}; use bun_paths::{self as paths, OSPathBuffer, OSPathChar, OSPathSliceZ, PathBuffer}; @@ -1238,6 +1239,7 @@ mod _async_tasks { /// Wrapped in [`ThreadSafe`] so the paired `unprotect()` runs on drop. pub args: ThreadSafe, pub global_object: bun_ptr::BackRef, + pub context_id: ScriptExecutionContextIdentifier, pub task: WorkPoolTask, pub result: Maybe, pub r#ref: KeepAlive, @@ -1281,6 +1283,7 @@ mod _async_tasks { // niche-optimised; never construct an all-zero `Result` value. result: Err(sys::Error::default()), global_object: bun_ptr::BackRef::new(global_object), + context_id: global_object.script_execution_context_identifier(), task: work_pool_task(Self::work_pool_callback), r#ref: KeepAlive::default(), tracker: AsyncTaskTracker::init(vm), @@ -1304,16 +1307,19 @@ mod _async_tasks { // `sys::Error::path` is `Box<[u8]>` boxed at the // `errno_sys_p` construction site, so no clone is needed — `node_fs` may drop. - // `bun_vm_concurrently()` skips the JS-thread debug assert and is the - // documented accessor for off-thread (work-pool) callers; the - // event-loop's concurrent queue is MPSC-safe. - let vm = this.global_object().bun_vm_concurrently(); - // SAFETY: VirtualMachine and its event loop are process-static - // (LIFETIMES.tsv); the concurrent queue is MPSC-safe. - unsafe { - (*(*vm).event_loop()).enqueue_task_concurrent(ConcurrentTask::create_from( - std::ptr::from_mut::(this), - )); + // Post by stable context id: the enqueue dereferences the target VM + // only under the contexts-map lock (serializes with worker + // `markTerminating()`). + let context_id = this.context_id; + let node = ConcurrentTask::create_from(std::ptr::from_mut::(this)); + if !context_id.post_concurrent_task(node) { + // Target context gone or terminating. `run_from_js_thread` will + // never fire; reclaim the heap `ConcurrentTask` node. `*this` + // holds `JSPromiseStrong`/`ThreadSafe` handles into the dead + // JSC heap and cannot be released off-thread, so the box is leaked. + // SAFETY: `post_concurrent_task` returned false so ownership was + // not transferred; `node` was `ConcurrentTask::create_from`-allocated above. + drop(unsafe { bun_core::heap::take(node.as_ptr()) }); } } @@ -1395,6 +1401,7 @@ mod _async_tasks { /// Wrapped in [`ThreadSafe`] so the paired `unprotect()` runs on drop. pub args: ThreadSafe, pub evtloop: EventLoopHandle, + pub context_id: ScriptExecutionContextIdentifier, 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 @@ -1582,6 +1589,7 @@ mod _async_tasks { result: core::cell::Cell::new(Ok(())), // `vm.event_loop` is the live per-thread `jsc::EventLoop` field. evtloop: EventLoopHandle::init(vm.event_loop.cast()), + context_id: global_object.script_execution_context_identifier(), task: work_pool_task(Self::work_pool_callback), r#ref: KeepAlive::default(), tracker: AsyncTaskTracker::init(vm), @@ -1615,6 +1623,7 @@ mod _async_tasks { // `has_result` CAS) before any read on the JS thread. result: core::cell::Cell::new(Ok(())), evtloop: EventLoopHandle::init_mini(mini), + context_id: ScriptExecutionContextIdentifier(0), task: work_pool_task(Self::work_pool_callback), r#ref: KeepAlive::default(), tracker: AsyncTaskTracker { id: 0 }, @@ -1690,14 +1699,18 @@ mod _async_tasks { // provenance from `Box::leak`, so the enqueued callback may safely // form `&mut *this` on the JS thread. if matches!(this_ref.evtloop, EventLoopHandle::Js { .. }) { - this_ref.evtloop.enqueue_task_concurrent(EventLoopTaskPtr { - js: ConcurrentTask::from_callback(this, |p| { - // SAFETY: `p` is the `Box::leak`'d task; subtask count hit zero so this - // JS-thread callback holds the only live reference (exclusive `&mut`). - unsafe { (&mut *p).run_from_js_thread().map_err(Into::into) } - }) - .as_ptr(), + let node = ConcurrentTask::from_callback(this, |p| { + // SAFETY: `p` is the `Box::leak`'d task; subtask count hit zero so this + // JS-thread callback holds the only live reference (exclusive `&mut`). + unsafe { (&mut *p).run_from_js_thread().map_err(Into::into) } }); + if !this_ref.context_id.post_concurrent_task(node) { + // Target context is gone; reclaim the node. `this` holds + // `JSPromiseStrong` into the dead JSC heap, so it is leaked. + // SAFETY: ownership was not transferred; `node` was + // `ConcurrentTask::from_callback`-allocated just above. + drop(unsafe { bun_core::heap::take(node.as_ptr()) }); + } } else { this_ref.evtloop.enqueue_task_concurrent(EventLoopTaskPtr { mini: AnyTaskWithExtraContext::from_callback_auto_deinit( @@ -2163,6 +2176,7 @@ mod _async_tasks { /// Wrapped in [`ThreadSafe`] so the paired `unprotect()` runs on drop. pub args: ThreadSafe, pub global_object: bun_ptr::BackRef, + pub context_id: ScriptExecutionContextIdentifier, pub task: WorkPoolTask, pub r#ref: KeepAlive, pub tracker: AsyncTaskTracker, @@ -2358,6 +2372,7 @@ mod _async_tasks { args: FsArgument::into_thread_safe(args), has_result: AtomicBool::new(false), global_object: bun_ptr::BackRef::new(global_object), + context_id: global_object.script_execution_context_identifier(), task: work_pool_task(Self::work_pool_callback), r#ref: KeepAlive::default(), tracker: AsyncTaskTracker::init(vm), @@ -2533,16 +2548,20 @@ mod _async_tasks { } } - // `bun_vm_concurrently()` skips the JS-thread debug assert and is the - // documented accessor for off-thread (work-pool) callers. - // SAFETY: `bun_vm_concurrently()` returns the process-singleton VM; - // sole `&mut` borrow at this point on the work-pool thread. - let vm = unsafe { &mut *self.global_object().bun_vm_concurrently() }; - // `ConcurrentTask::create` heap-allocates a fresh task; the - // queue takes ownership of it. - vm.enqueue_task_concurrent(ConcurrentTask::create(Task::init(std::ptr::from_mut::< - Self, - >(self)))); + // Post by stable context id: the enqueue dereferences the target VM + // only under the contexts-map lock (serializes with worker + // `markTerminating()`). + let context_id = self.context_id; + let node = ConcurrentTask::create(Task::init(std::ptr::from_mut::(self))); + if !context_id.post_concurrent_task(node) { + // Target context gone or terminating. `run_from_js_thread` will + // never fire; reclaim the heap `ConcurrentTask` node. `*self` + // holds `JSPromiseStrong` into the dead JSC heap and cannot be + // released off-thread, so the box is leaked. + // SAFETY: `post_concurrent_task` returned false so ownership was + // not transferred; `node` was `ConcurrentTask::create`-allocated above. + drop(unsafe { bun_core::heap::take(node.as_ptr()) }); + } } fn clear_result_list(&mut self) { diff --git a/src/runtime/node/node_zlib_binding.rs b/src/runtime/node/node_zlib_binding.rs index 773615da2ecb..f37e65ddb1e6 100644 --- a/src/runtime/node/node_zlib_binding.rs +++ b/src/runtime/node/node_zlib_binding.rs @@ -9,6 +9,7 @@ use bun_core::{String as BunString, ZigStringSlice}; use bun_event_loop::Taskable; use bun_io::KeepAlive; use bun_jsc::ConcurrentTask::{ConcurrentTask, Task}; +use bun_jsc::js_global_object::ScriptExecutionContextIdentifier; use bun_jsc::virtual_machine::VirtualMachine; use bun_jsc::{ self as jsc, CallFrame, ErrorCode, JSGlobalObject, JSValue, JsCell, JsResult, StringJsc as _, @@ -210,6 +211,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; + /// Captured on the JS thread at construction; used off-thread to post the + /// completion so a freed worker VM/global is never dereferenced. + fn context_id(&self) -> ScriptExecutionContextIdentifier; fn stream(&self) -> &JsCell; /// Write `(avail_out, avail_in)` into the JS-owned 2-element `Uint32Array` @@ -472,24 +476,22 @@ impl CompressionStream { // `ref_()` in `write()`); bodies use the `&self` accessor surface // (R-2). `ParentRef` Deref collapses the per-site raw deref. let this_ref = ParentRef::from(NonNull::new(this).expect("async_job_run: this")); - let global_this: &JSGlobalObject = this_ref.global_this(); - // `bun_vm_concurrently()` is the thread-safe accessor (skips the - // JS-thread debug assert; same backing pointer as `bun_vm()`). - // BACKREF — `bun_vm_concurrently()` never returns null for a Bun-owned - // global; wrap once so the `event_loop()` read below is safe Deref. - let vm = ParentRef::from( - NonNull::new(global_this.bun_vm_concurrently()).expect("bun_vm_concurrently"), - ); this_ref.stream().with_mut(|s| s.do_work()); - // SAFETY: `event_loop()` is a self-pointer into a live VM; the - // `enqueue_task_concurrent` body only touches the lock-free - // `concurrent_tasks` queue (thread-safe). `this` is the heap-allocated - // `m_ctx` payload — the matching `ref()` in `write()` keeps it alive - // until `run_from_js_thread` runs and calls `deref()`. - unsafe { - (*vm.event_loop()).enqueue_task_concurrent(ConcurrentTask::create(Task::init(this))); + // Post by stable context id: the enqueue dereferences the target VM + // only under the contexts-map lock (serializes with worker + // `markTerminating()`). `this` is the heap-allocated `m_ctx` payload — + // the matching `ref()` in `write()` keeps it alive until + // `run_from_js_thread` runs and calls `deref()`. + let node = ConcurrentTask::create(Task::init(this)); + if !this_ref.context_id().post_concurrent_task(node) { + // Target context gone or terminating. `run_from_js_thread` will + // never fire. `this` is the JSC-owned m_ctx holding `Strong`/ + // `KeepAlive`; its `deref()` is not safe off-thread, so leak it. + // SAFETY: ownership was not transferred; `node` was + // `ConcurrentTask::create`-allocated above. + drop(unsafe { bun_core::heap::take(node.as_ptr()) }); } } @@ -998,6 +1000,7 @@ macro_rules! __impl_compression_stream { type Stream = $ctx; #[inline] fn global_this(&self) -> &::bun_jsc::JSGlobalObject { self.global_this.get() } + #[inline] fn context_id(&self) -> ::bun_jsc::js_global_object::ScriptExecutionContextIdentifier { self.context_id } #[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 7a5b889ba935..b277253bed29 100644 --- a/src/runtime/node/zlib/NativeBrotli.rs +++ b/src/runtime/node/zlib/NativeBrotli.rs @@ -85,6 +85,7 @@ mod _impl { // JSC_BORROW backref; global outlives this m_ctx payload. `BackRef` // centralises the single unsafe deref so the trait impl is safe. pub global_this: bun_ptr::BackRef, + pub context_id: bun_jsc::js_global_object::ScriptExecutionContextIdentifier, 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) @@ -148,6 +149,7 @@ mod _impl { ref_count: Cell::new(1), // JSC_BORROW backref — the global outlives this m_ctx payload. global_this: bun_ptr::BackRef::new(global_this), + context_id: global_this.script_execution_context_identifier(), 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 2abe06797c9c..f1ac1afd98d1 100644 --- a/src/runtime/node/zlib/NativeZlib.rs +++ b/src/runtime/node/zlib/NativeZlib.rs @@ -42,6 +42,7 @@ mod _impl { // JSC_BORROW backref; global outlives this m_ctx payload. `BackRef` // centralises the single unsafe deref so the trait impl is safe. pub global_this: bun_ptr::BackRef, + pub context_id: bun_jsc::js_global_object::ScriptExecutionContextIdentifier, pub stream: JsCell, pub poll_ref: JsCell, pub this_value: JsCell, // jsc.Strong.Optional @@ -93,6 +94,7 @@ mod _impl { ref_count: Cell::new(1), // JSC_BORROW backref — the global outlives this m_ctx payload. global_this: bun_ptr::BackRef::new(global), + context_id: global.script_execution_context_identifier(), 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 8ca905cf12f4..6b2b2320963b 100644 --- a/src/runtime/node/zlib/NativeZstd.rs +++ b/src/runtime/node/zlib/NativeZstd.rs @@ -41,6 +41,7 @@ mod _impl { // LIFETIMES.tsv: JSC_BORROW. The global outlives this m_ctx payload; // `BackRef` centralises the single unsafe deref so the trait impl is safe. pub global_this: bun_ptr::BackRef, + pub context_id: bun_jsc::js_global_object::ScriptExecutionContextIdentifier, pub stream: JsCell, pub poll_ref: JsCell, pub this_value: JsCell, // jsc.Strong.Optional @@ -105,6 +106,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), + context_id: global.script_execution_context_identifier(), stream: JsCell::new(stream), poll_ref: JsCell::new(CountedKeepAlive::default()), this_value: JsCell::new(StrongOptional::empty()), diff --git a/src/runtime/webcore/fetch/FetchTasklet.rs b/src/runtime/webcore/fetch/FetchTasklet.rs index 9bff5a530a83..89a6be56772d 100644 --- a/src/runtime/webcore/fetch/FetchTasklet.rs +++ b/src/runtime/webcore/fetch/FetchTasklet.rs @@ -17,6 +17,7 @@ use bun_http::{ }; use bun_io::KeepAlive; use bun_jsc::debugger::AsyncTaskTracker; +use bun_jsc::js_global_object::ScriptExecutionContextIdentifier; use bun_jsc::virtual_machine::VirtualMachine; use bun_jsc::{ self as jsc, GlobalRef, JSGlobalObject, JSValue, JsResult, StringJsc, StrongOptional, @@ -69,6 +70,7 @@ pub struct FetchTasklet { pub metadata: Option, pub javascript_vm: &'static VirtualMachine, pub global_this: GlobalRef, + pub context_id: ScriptExecutionContextIdentifier, pub request_body: HTTPRequestBody, // ThreadSafeStreamBuffer is intrusively refcounted (`ref_count: AtomicU32`, // starts at 2) and shared with the HTTP thread via raw ptr; `Arc` can't be mutably @@ -297,15 +299,17 @@ impl FetchTasklet { /// Enqueue a concurrent task on the JS-thread event loop. /// - /// Centralises the `(*vm.event_loop()).enqueue_task_concurrent(..)` raw - /// deref. `event_loop()` returns a self-ptr into the VirtualMachine that - /// is valid for the VM's lifetime; `enqueue_task_concurrent` takes `&self` - /// and is thread-safe (lock-free MPSC push). `task` is a live - /// `ConcurrentTaskItem` that the queue takes ownership of via its - /// intrusive `next` link. + /// Routes through the contexts-map lock so the target `EventLoop` is only + /// dereferenced while its VM is known live. Returns `false` (caller keeps + /// ownership of `task`) when the context is gone or terminating. `task` is + /// a live `ConcurrentTaskItem` that the queue takes ownership of via its + /// intrusive `next` link on success. #[inline] - fn enqueue_concurrent(vm: &VirtualMachine, task: core::ptr::NonNull) { - vm.event_loop_shared().enqueue_task_concurrent(task); + fn enqueue_concurrent( + context_id: ScriptExecutionContextIdentifier, + task: core::ptr::NonNull, + ) -> bool { + context_id.post_concurrent_task(task) } /// Wrap a borrowed body chunk in a `StreamResult::Temporary*` for @@ -393,7 +397,7 @@ impl FetchTasklet { return; } let self_ = Self::from_raw_ref(this); - if self_.javascript_vm.is_shutting_down() { + if !self_.context_id.is_alive() { // SAFETY: last ref; exclusive access. `deinit()` would run // `clear_data()` + `Drop` for the JSC `Strong`/`Weak` fields, which // reach into the VM's HandleSet from this (HTTP) thread — not @@ -406,10 +410,14 @@ impl FetchTasklet { // lets make sure that we always call deinit from main thread // `from_callback` heap-allocates a fresh `ConcurrentTaskItem`; the queue // takes ownership of it. - Self::enqueue_concurrent( - self_.javascript_vm, - ConcurrentTask::from_callback(this, FetchTasklet::deinit_callback), - ); + let node = ConcurrentTask::from_callback(this, FetchTasklet::deinit_callback); + if !Self::enqueue_concurrent(self_.context_id, node) { + // Target VM raced to teardown between the check and the post; drop + // the freshly heap-allocated node and take the shutdown dealloc path. + drop(unsafe { bun_core::heap::take(node.as_ptr()) }); + // SAFETY: last ref; see the `!is_alive()` branch above. + unsafe { FetchTasklet::dealloc_for_shutdown(this) }; + } } // ConcurrentTask::from_callback takes `fn(*mut T) -> bun_event_loop::JsResult<()>` @@ -1879,6 +1887,7 @@ impl FetchTasklet { metadata: None, javascript_vm: jsc_vm, global_this: GlobalRef::from(global_this), + context_id: global_this.script_execution_context_identifier(), request_body: fetch_options.body, request_body_streaming_buffer: None, response_buffer: MutableString::default(), @@ -2131,17 +2140,20 @@ impl FetchTasklet { /// This is ALWAYS called from the http thread and we cannot touch the buffer here because is locked pub(crate) fn on_write_request_data_drain(this: *mut FetchTasklet) { let this_ref = Self::from_raw_ref(this); - if this_ref.javascript_vm.is_shutting_down() { + if !this_ref.context_id.is_alive() { return; } // ref until the main thread callback is called this_ref.ref_(); // `from_callback` heap-allocates a fresh `ConcurrentTaskItem`; the queue // takes ownership of it. - Self::enqueue_concurrent( - this_ref.javascript_vm, - ConcurrentTask::from_callback(this, FetchTasklet::resume_request_data_stream), - ); + let node = ConcurrentTask::from_callback(this, FetchTasklet::resume_request_data_stream); + if !Self::enqueue_concurrent(this_ref.context_id, node) { + // Target VM raced to teardown; drop the fresh node and undo the ref + // via the thread-safe path (which takes the shutdown dealloc branch). + drop(unsafe { bun_core::heap::take(node.as_ptr()) }); + FetchTasklet::deref_from_thread(this); + } } /// This is ALWAYS called from the main thread @@ -2451,36 +2463,31 @@ impl FetchTasklet { } } // will deinit when done with the http client (when is_done = true) - if task_ref.javascript_vm.is_shutting_down() { - // VM teardown: the JS-thread side will never drain this buffer (its - // on_progress_update bails the same way), so free the body bytes now. + // Shared cleanup for the "target VM is gone/terminating" path. Runs + // only the Rust-side work the JS-thread on_progress_update would never + // get to: free the body buffer, release the parked socket, undo the + // CAS flag, unlock, and (on the final result) drop both refs so the + // 1→0 transition takes `dealloc_for_shutdown`. + let abandon = |task_ref: &mut FetchTasklet| { task_ref.scheduled_response_buffer = MutableString::default(); - // The certificate will never be checked; release the parked - // socket instead of leaving it occupying an active request slot - // until the idle timeout. if task_ref.result.certificate_info.take().is_some() { if let Some(http_) = task_ref.http.as_mut() { http::http_thread().schedule_shutdown(http_); } } - // We won the `has_schedule_callback` CAS above but are not - // enqueueing the on_progress_update task; undo the flag so a later - // (final) callback can re-enter this branch instead of taking the - // already-scheduled early return. task_ref .has_schedule_callback .store(false, Ordering::Release); task_ref.mutex.unlock(); if is_done { - // No on_progress_update will ever run for this final result, so - // release the JS-side ref it would have dropped, then the - // HTTP-side ref. The 1→0 transition runs `dealloc_for_shutdown` - // (Rust boxes only — JSC handles are leaked to destructOnExit). // SAFETY: `task` is the live heap tasklet; both refs held. FetchTasklet::deref_from_thread(task); // SAFETY: second ref still held until this 1→0 transition. FetchTasklet::deref_from_thread(task); } + }; + if !task_ref.context_id.is_alive() { + abandon(task_ref); return; } let ct = core::ptr::NonNull::from( @@ -2490,7 +2497,10 @@ impl FetchTasklet { ); // `ct` is the inline `concurrent_task` field of the heap tasklet; the // queue takes ownership of its `next` link. - Self::enqueue_concurrent(task_ref.javascript_vm, ct); + if !Self::enqueue_concurrent(task_ref.context_id, ct) { + abandon(task_ref); + return; + } task_ref.mutex.unlock(); // we are done with the http client so we can deref our side diff --git a/src/runtime/webcore/s3/client.rs b/src/runtime/webcore/s3/client.rs index 80271e2b89eb..6c08cb0879cb 100644 --- a/src/runtime/webcore/s3/client.rs +++ b/src/runtime/webcore/s3/client.rs @@ -304,6 +304,9 @@ pub(crate) fn list_objects( callback: s3_simple_request::Callback::ListObjects(callback), headers, vm: Some(bun_ptr::BackRef::new(VirtualMachine::get())), + context_id: VirtualMachine::get() + .global() + .script_execution_context_identifier(), response_buffer: MutableString::default(), result: bun_http::HTTPClientResult::default(), concurrent_task: Default::default(), @@ -1011,6 +1014,9 @@ pub(crate) fn download_stream( headers, // `VirtualMachine::get()` returns the live per-thread VM singleton. vm: Some(bun_ptr::BackRef::new(VirtualMachine::get())), + context_id: VirtualMachine::get() + .global() + .script_execution_context_identifier(), has_schedule_callback: core::sync::atomic::AtomicBool::new(false), signal_store: Default::default(), signals: Default::default(), diff --git a/src/runtime/webcore/s3/download_stream.rs b/src/runtime/webcore/s3/download_stream.rs index 6863b5734e73..47e8e4387c0d 100644 --- a/src/runtime/webcore/s3/download_stream.rs +++ b/src/runtime/webcore/s3/download_stream.rs @@ -7,6 +7,7 @@ use bun_event_loop::ConcurrentTask::{AutoDeinit, ConcurrentTask}; use bun_event_loop::{TaskTag, Taskable, task_tag}; use bun_http::{AsyncHTTP, HTTPClientResult, Headers, Signals}; use bun_io::KeepAlive; +use bun_jsc::js_global_object::ScriptExecutionContextIdentifier; use bun_jsc::virtual_machine::VirtualMachine; use bun_s3_signing::credentials::SignResult; use bun_s3_signing::error::S3Error; @@ -21,6 +22,10 @@ pub struct S3HttpDownloadStreamingTask { /// JSC_BORROW: per-thread VM singleton, outlives every task. `None` only in /// the inert `Default` placeholder (overwritten before the task escapes). pub vm: Option>, + /// Stable id of the originating `ScriptExecutionContext`. The HTTP-thread + /// completion posts back via this id under the C++ contexts-map lock so a + /// worker VM freed by `terminate()` is never dereferenced. + pub context_id: ScriptExecutionContextIdentifier, pub sign_result: SignResult, pub headers: Headers, pub callback_context: NonNull<()>, @@ -62,6 +67,7 @@ impl Default for S3HttpDownloadStreamingTask { // never read — fully overwritten by `AsyncHTTP::init` before first use. http: core::mem::MaybeUninit::uninit(), vm: None, + context_id: ScriptExecutionContextIdentifier(0), sign_result: SignResult::default(), headers: Headers::default(), callback_context: NonNull::dangling(), @@ -344,15 +350,13 @@ impl S3HttpDownloadStreamingTask { let task = core::ptr::NonNull::from( self_.concurrent_task.from(this, AutoDeinit::ManualDeinit), ); - // `vm` is the live per-thread VM BackRef captured at task creation; event_loop - // is initialized for the request's lifetime and enqueue is thread-safe (`&self`). - // `task` is the inline `concurrent_task` field of this heap request; - // the queue takes ownership of its `next` link. - self_ - .vm - .expect("vm set at task creation") - .event_loop_shared() - .enqueue_task_concurrent(task); + // Post by stable context id under the C++ contexts-map lock so a + // worker VM freed by `terminate()` is never dereferenced. On + // abandon the heap task is leaked: `Drop` touches the VM event + // loop and must not run off-thread against a dead VM. `task` is + // the inline `concurrent_task` field of this heap request; the + // queue takes ownership of its `next` link. + let _ = self_.context_id.post_concurrent_task(task); } } } diff --git a/src/runtime/webcore/s3/simple_request.rs b/src/runtime/webcore/s3/simple_request.rs index b4114d0536f4..a5e04de1835e 100644 --- a/src/runtime/webcore/s3/simple_request.rs +++ b/src/runtime/webcore/s3/simple_request.rs @@ -10,6 +10,7 @@ use bun_http::{ Method, }; use bun_io::KeepAlive; +use bun_jsc::js_global_object::ScriptExecutionContextIdentifier; use bun_jsc::virtual_machine::VirtualMachine; use bun_picohttp as picohttp; use bun_s3_signing::acl::ACL; @@ -120,6 +121,10 @@ pub struct S3HttpSimpleTask { /// JSC_BORROW: per-thread VM singleton, outlives every task. `None` only in /// the inert `Default` placeholder (overwritten before the task escapes). pub vm: Option>, + /// Stable id of the originating `ScriptExecutionContext`. The HTTP-thread + /// completion posts back via this id under the C++ contexts-map lock so a + /// worker VM freed by `terminate()` is never dereferenced. + pub context_id: ScriptExecutionContextIdentifier, pub sign_result: SignResult, pub headers: Headers, pub callback_context: *mut c_void, @@ -156,6 +161,7 @@ impl Default for S3HttpSimpleTask { Self { http: core::mem::MaybeUninit::uninit(), vm: None, + context_id: ScriptExecutionContextIdentifier(0), sign_result: SignResult::default(), headers: Headers::default(), callback_context: core::ptr::null_mut(), @@ -487,14 +493,13 @@ impl S3HttpSimpleTask { this.concurrent_task .from(this_ptr, AutoDeinit::ManualDeinit), ); - // `vm` is the live per-thread VM BackRef captured at task creation; event_loop - // is set during VM init and outlives this task. `enqueue_task_concurrent` is `&self`. - // `task` is the inline `concurrent_task` field of this heap request; - // the queue takes ownership of its `next` link. - this.vm - .expect("vm set at task creation") - .event_loop_shared() - .enqueue_task_concurrent(task); + // Post by stable context id under the C++ contexts-map lock so a + // worker VM freed by `terminate()` is never dereferenced. On + // abandon the heap task is leaked: `Drop` touches the VM event + // loop and must not run off-thread against a dead VM. `task` is + // the inline `concurrent_task` field of this heap request; the + // queue takes ownership of its `next` link. + let _ = this.context_id.post_concurrent_task(task); } } } @@ -635,6 +640,9 @@ pub(crate) fn execute_simple_s3_request( range: options.range, headers, vm: Some(bun_ptr::BackRef::new(VirtualMachine::get())), + context_id: VirtualMachine::get() + .global() + .script_execution_context_identifier(), response_buffer: MutableString::default(), result: HTTPClientResult::default(), concurrent_task: ConcurrentTask::default(), diff --git a/test/js/web/workers/worker-terminate-lifetime.test.ts b/test/js/web/workers/worker-terminate-lifetime.test.ts index 9d476d1f3d53..81d74b450298 100644 --- a/test/js/web/workers/worker-terminate-lifetime.test.ts +++ b/test/js/web/workers/worker-terminate-lifetime.test.ts @@ -1,5 +1,5 @@ import { expect, test } from "bun:test"; -import { bunEnv, bunExe, isASAN, isDebug } from "harness"; +import { bunEnv, bunExe, isASAN, isDebug, tempDir } from "harness"; import { join } from "path"; // Worker VM startup/teardown is much slower under debug and/or ASAN; these @@ -176,3 +176,86 @@ test.skipIf(!isASAN)( }, timeout, ); + +// Regression: off-thread completions (WorkPool / HTTP thread / bundle thread) +// posted back to a worker's EventLoop via a raw BackRef/&VirtualMachine +// captured at schedule time. worker.terminate() freed the VM box before the +// pool thread ran, so the enqueue dereferenced freed memory. Every cross-thread +// source is now posted by ScriptExecutionContextIdentifier under the contexts- +// map lock (serializing with markTerminating). ASAN-only: release builds read +// freed memory without crashing; under ASAN the heap-use-after-free is +// deterministic on the first teardown. +test.skipIf(!isASAN)( + "terminate() while cross-thread WorkPool completions are in flight does not UAF on enqueue", + async () => { + // Worker body: arm one in-flight op per cross-thread source, signal the + // parent, then sit. All loopback-only; results ignored. + const body = ` + import { parentPort } from "node:worker_threads"; + import fs from "node:fs"; import fsp from "node:fs/promises"; + import zlib from "node:zlib"; import crypto from "node:crypto"; + import os from "node:os"; import path from "node:path"; + const sink = () => {}; const swallow = p => Promise.resolve(p).then(sink, sink); + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "wt-")); + const tf = path.join(tmp, "a.txt"); + fs.writeFileSync(tf, Buffer.alloc(1 << 14, "x").toString()); + // WorkTask + swallow(Bun.write(path.join(tmp, "b.txt"), Buffer.alloc(1 << 14, "y").toString())); + swallow(Bun.file(tf).text()); + // AsyncFSTask + swallow(fsp.readFile(tf)); swallow(fsp.stat(tf)); + swallow(fsp.readdir(tmp, { recursive: true })); + // AnyTaskJob (pbkdf2/scrypt/keygen/zstd) + crypto.pbkdf2("p", "s", 50000, 64, "sha512", sink); + crypto.scrypt("p", "saltsalt", 64, sink); + crypto.generateKeyPair("rsa", { modulusLength: 1024 }, sink); + swallow(Bun.zstdCompress(Buffer.alloc(1 << 14))); + // PasswordJob + swallow(Bun.password.hash("hunter2", { algorithm: "bcrypt", cost: 4 })); + // CompressionStream async_job_run + zlib.deflate(Buffer.alloc(1 << 16), sink); zlib.gzip(Buffer.alloc(1 << 16), sink); + // ConcurrentPromiseTask + swallow(new Bun.Transpiler({ loader: "ts" }).transform("const x: number = 1;")); + swallow(Array.fromAsync(new Bun.Glob("**/*").scan(tmp))); + // ConcurrentCppTask (WebCrypto) + swallow(crypto.subtle.digest("SHA-256", new Uint8Array(1 << 14))); + // JSBundleCompletionTask + const e = path.join(tmp, "e.ts"); fs.writeFileSync(e, "export const x=1;"); + swallow(Bun.build({ entrypoints: [e], target: "bun" })); + // FetchTasklet (HTTP thread) + const srv = Bun.serve({ port: 0, fetch: () => new Response(Buffer.alloc(1 << 12, "x")) }); + swallow(fetch("http://127.0.0.1:" + srv.port + "/").then(r => r.text())); + parentPort.postMessage("armed"); + setInterval(sink, 1 << 30); + `; + using dir = tempDir("worker-terminate-crossthread", { + "body.mjs": body, + "main.mjs": ` + import { Worker } from "node:worker_threads"; + const n = ${slow ? 4 : 12}; + for (let i = 0; i < n; i++) { + const w = new Worker(new URL("./body.mjs", import.meta.url)); + await new Promise((res, rej) => { + w.once("message", res); w.once("error", rej); + }); + await w.terminate(); + } + console.log("ok"); + `, + }); + await using proc = Bun.spawn({ + cmd: [bunExe(), "main.mjs"], + env: bunEnv, + cwd: String(dir), + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + // No UAF, no panic, no assert: stderr must be clean and the driver must + // have finished every round. + expect(stderr).toBe(""); + expect(stdout).toBe("ok\n"); + expect(exitCode).toBe(0); + }, + timeout * 2, +); From e60c51772eb6874c36c48ec81f765daa7181268b Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Sat, 25 Jul 2026 18:03:24 +0000 Subject: [PATCH 2/7] [autofix.ci] apply automated fixes --- docs/ROOT-B-SHUTDOWN-FENCE.md | 37 ++++++++++++++++++----------------- 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/docs/ROOT-B-SHUTDOWN-FENCE.md b/docs/ROOT-B-SHUTDOWN-FENCE.md index 57c446ce0c2a..eca9ce50d1ba 100644 --- a/docs/ROOT-B-SHUTDOWN-FENCE.md +++ b/docs/ROOT-B-SHUTDOWN-FENCE.md @@ -19,23 +19,23 @@ freed JSC heap. All reproduced cross-thread UAFs in this class converge on **`EventLoop::enqueue_task_concurrent`** (`src/jsc/event_loop.rs:997`). Putting -a check *inside* the funnel does not help: `&self` there is already a pointer +a check _inside_ the funnel does not help: `&self` there is already a pointer into the freed box. Several callers also "guard" with an off-thread `vm.is_shutting_down()` read; that read is itself a UAF face (the flag lives in the freed box). ## Shutdown map (reference) -| step | action | fences | -| ---- | -------------------------------------------------------------- | ----------------------------------- | -| 1 | `self.vm = null` under `vm_lock` | parent-thread readers | -| 2a | `is_shutting_down = true`, `on_exit`, drain timers/sockets/DNS | on-thread re-entry | -| 2b | `ScriptExecutionContext::markTerminating()` | C++ `postTaskTo` posters | -| 2c | `Bun__JSCTaskScheduler__markShuttingDown()` | `Atomics.notify` posters | -| 2d | `release_queued_tasks_for_shutdown()` | tasks already queued | -| 3 | `WebWorker__teardownJSCVM` | GC finalizers, JSC heap freed | -| 4 | `WebWorker__dispatchExit` | parent releases its ref | -| 5 | `vm.destroy()`; `dealloc(vm_ptr)`; free uws loop | VM box freed | +| step | action | fences | +| ---- | -------------------------------------------------------------- | ----------------------------- | +| 1 | `self.vm = null` under `vm_lock` | parent-thread readers | +| 2a | `is_shutting_down = true`, `on_exit`, drain timers/sockets/DNS | on-thread re-entry | +| 2b | `ScriptExecutionContext::markTerminating()` | C++ `postTaskTo` posters | +| 2c | `Bun__JSCTaskScheduler__markShuttingDown()` | `Atomics.notify` posters | +| 2d | `release_queued_tasks_for_shutdown()` | tasks already queued | +| 3 | `WebWorker__teardownJSCVM` | GC finalizers, JSC heap freed | +| 4 | `WebWorker__dispatchExit` | parent releases its ref | +| 5 | `vm.destroy()`; `dealloc(vm_ptr)`; free uws loop | VM box freed | Rust-side cross-thread posters were not serialized with any of 2b/2c/2d. @@ -101,12 +101,12 @@ about to be). The abandon path: Three generic helpers carry most of the surface: -| helper | users | -| ------------------------------ | ------------------------------------------------------------- | -| `WorkTask` | `ReadFile`, `WriteFile`, `GetAddrInfoRequest` | -| `ConcurrentPromiseTask` | `CopyFile`, `TransformTask`, `WalkTask`, `PipelineTask` | -| `AnyTaskJob` | `Pbkdf2Ctx`, `CryptoJob`, `ZstdCtx`, `SecretsCtx` | -| `ConcurrentCppTask` | WebCrypto (`PhonyWorkQueue::dispatch`) | +| helper | users | +| -------------------------- | ----------------------------------------------------------- | +| `WorkTask` | `ReadFile`, `WriteFile`, `GetAddrInfoRequest` | +| `ConcurrentPromiseTask` | `CopyFile`, `TransformTask`, `WalkTask`, `PipelineTask` | +| `AnyTaskJob` | `Pbkdf2Ctx`, `CryptoJob`, `ZstdCtx`, `SecretsCtx` | +| `ConcurrentCppTask` | WebCrypto (`PhonyWorkQueue::dispatch`) | Direct callers converted alongside: `FetchTasklet`, `PasswordJob`, `CompressionStream` (zlib/brotli/zstd), `AsyncFSTask` / `NewAsyncCpTask` / @@ -123,7 +123,8 @@ terminating-VM JS execution, the `serve.listen`/JS-re-entry assert zone. Refcount-deferred VM dealloc + a closed-flag at the funnel: each off-thread job takes an `Arc` clone of a per-VM gate and brackets its enqueue with a read lock; `shutdown` takes the write lock before dealloc. Same sweep, but: -- `shutdown` then *waits* on in-flight pool jobs (a slow argon2/RSA can stall + +- `shutdown` then _waits_ on in-flight pool jobs (a slow argon2/RSA can stall terminate for seconds); - more atomics on the hot enqueue path; - the gate must be `Arc`'d so it outlives the VM box anyway. From 0cdab210cbb449dab67015d8b073feaf163b4581 Mon Sep 17 00:00:00 2001 From: robobun Date: Sat, 25 Jul 2026 18:38:19 +0000 Subject: [PATCH 3/7] review: trim per-site comments, fix TranspilerStore SAFETY, guard id==0, capture global in AnyTaskJob - comment-cop: the per-call-site explanation is the same sentence everywhere; keep it on post_concurrent_task's doc and the design doc, drop the copies at each site. - RuntimeTranspilerStore: the SAFETY comment claimed is_alive() held the contexts-map lock across the (*vm).transpiler_store push; the Locker is RAII and releases before returning. State the residual race honestly. - ScriptExecutionContext.cpp: guard id==0 in the three new lookups (WTF::HashMap asserts on the empty-key in debug; this PR introduces ScriptExecutionContextIdentifier(0) as a sentinel). - AnyTaskJob: capture global on the JS thread so run_task never derefs the VM BackRef off-thread. --- src/jsc/ConcurrentPromiseTask.rs | 16 ++-------- src/jsc/CppTask.rs | 11 ++----- src/jsc/JSGlobalObject.rs | 33 +++++++------------- src/jsc/RuntimeTranspilerStore.rs | 20 +++++------- src/jsc/WorkTask.rs | 17 ++-------- src/jsc/any_task_job.rs | 31 ++++++------------ src/jsc/bindings/ScriptExecutionContext.cpp | 12 +++---- src/jsc/virtual_machine_exports.rs | 5 +-- src/runtime/api/Archive.rs | 6 +--- src/runtime/api/js_bundle_completion_task.rs | 7 ++--- src/runtime/crypto/PasswordObject.rs | 4 +-- src/runtime/node/node_fs.rs | 28 ++++------------- src/runtime/node/node_zlib_binding.rs | 16 +++------- src/runtime/webcore/fetch/FetchTasklet.rs | 21 +++---------- src/runtime/webcore/s3/download_stream.rs | 11 ++----- src/runtime/webcore/s3/simple_request.rs | 11 ++----- 16 files changed, 68 insertions(+), 181 deletions(-) diff --git a/src/jsc/ConcurrentPromiseTask.rs b/src/jsc/ConcurrentPromiseTask.rs index 69bd61694751..4757551b3679 100644 --- a/src/jsc/ConcurrentPromiseTask.rs +++ b/src/jsc/ConcurrentPromiseTask.rs @@ -32,14 +32,9 @@ pub struct ConcurrentPromiseTask<'a, Context: ConcurrentPromiseTaskContext> { // Owned here so dropping the task frees the context. pub ctx: Box, pub task: WorkPoolTask, - /// BACKREF — captured from the JS-thread VM at create time. Only - /// dereferenced on the JS thread; the pool-thread completion goes through - /// [`Self::context_id`] so a worker VM freed by `terminate()` is never - /// touched. + /// BACKREF — JS-thread only; pool-thread completion goes through `context_id`. pub event_loop: BackRef, - /// Stable `u32` id for the originating `ScriptExecutionContext`. The - /// pool-thread `on_finish` posts the completion via this id under the C++ - /// contexts-map lock (serializing with worker `markTerminating()`). + /// See [`ScriptExecutionContextIdentifier::post_concurrent_task`]. pub context_id: ScriptExecutionContextIdentifier, pub promise: JSPromiseStrong, pub global_this: &'a JSGlobalObject, @@ -122,12 +117,7 @@ impl<'a, Context: ConcurrentPromiseTaskContext> ConcurrentPromiseTask<'a, Contex .concurrent_task .from(this, AutoDeinit::ManualDeinit), ); - // Post by stable context id: the enqueue dereferences the target VM - // only under the contexts-map lock (serializes with worker - // `markTerminating()`). When the context is gone the box is leaked - // (its `JSPromiseStrong` and `global_this` point into the dead JSC - // heap and cannot be released off-thread); `task` is a field of - // `*this`, so no separate free. + // Abandon: JSC handles cannot drop off-thread, leak the box (task is intrusive). let _ = context_id.post_concurrent_task(task); } diff --git a/src/jsc/CppTask.rs b/src/jsc/CppTask.rs index a288aa12cc71..e3c9e508a278 100644 --- a/src/jsc/CppTask.rs +++ b/src/jsc/CppTask.rs @@ -58,10 +58,7 @@ impl EventLoopTaskNoContext { NonNull::new(Bun__EventLoopTaskNoContext__createdInBunVm(self)).map(bun_ptr::BackRef::from) } - /// Stable `u32` id of the `ScriptExecutionContext` that created this task, - /// captured on the JS thread at construction. Safe to use off-thread via - /// [`ScriptExecutionContextIdentifier::unref_event_loop_concurrently`] so - /// the VM is dereferenced only under the contexts-map lock. + /// See [`ScriptExecutionContextIdentifier::unref_event_loop_concurrently`]. pub fn context_id(&self) -> ScriptExecutionContextIdentifier { ScriptExecutionContextIdentifier(Bun__EventLoopTaskNoContext__contextId(self)) } @@ -88,10 +85,8 @@ impl ConcurrentCppTask { // 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. unsafe { EventLoopTaskNoContext::run(cpp_task) }; - // The task body (e.g. a WebCrypto crypto-thread lambda) posts its own - // result back via `ScriptExecutionContext::postTaskTo`, which already - // serializes with `markTerminating()`. The trailing `concurrent_ref` - // decrement dereferences the VM, so route it through the same lock. + // Task body posts its own result via `postTaskTo`; route the trailing + // `concurrent_ref` decrement through the same lock. context_id.unref_event_loop_concurrently(); } } diff --git a/src/jsc/JSGlobalObject.rs b/src/jsc/JSGlobalObject.rs index bde5994ef155..79810bccaf69 100644 --- a/src/jsc/JSGlobalObject.rs +++ b/src/jsc/JSGlobalObject.rs @@ -1704,24 +1704,20 @@ impl ScriptExecutionContextIdentifier { } /// `true` while the context exists and has not been marked terminating. - /// Serializes with `markTerminating()` on the C++ contexts-map lock, so - /// unlike [`Self::valid`] this is safe to consult off-thread for the - /// "skip a queued unit of work whose worker has begun shutdown" check. + /// Serializes with `markTerminating()` on the contexts-map lock. #[inline] pub fn is_alive(self) -> bool { ScriptExecutionContext__isAlive(self.0) } - /// Enqueue a heap-allocated [`ConcurrentTaskItem`] onto this context's JS - /// event loop from any thread, without holding any pointer into the target - /// VM. The enqueue is performed under the C++ contexts-map lock (the same - /// lock worker `markTerminating()` takes before its VM is freed), so the - /// target `EventLoop` is dereferenced only while known live. + /// Enqueue a heap-allocated [`ConcurrentTaskItem`] onto this context's + /// event loop from any thread. Runs under the contexts-map lock (same lock + /// as `markTerminating()`), so the target VM is dereferenced only while + /// known live. /// - /// Returns `true` if the task was enqueued (ownership transferred to the - /// queue). Returns `false` if the context is gone or terminating; the - /// caller retains ownership of `task` and must free it (without touching - /// the target VM or JSC heap). + /// Returns `true` if enqueued (ownership transferred). Returns `false` if + /// the context is gone or terminating; caller retains ownership of `task` + /// and must free it without touching the target VM/JSC heap. /// /// [`ConcurrentTaskItem`]: crate::event_loop::ConcurrentTaskItem #[inline] @@ -1734,9 +1730,7 @@ impl ScriptExecutionContextIdentifier { /// Decrement this context's event-loop `concurrent_ref` from off-thread /// under the contexts-map lock; no-op when the context is gone or - /// terminating. The paired increment happened on the JS thread before the - /// job was scheduled, so a lost decrement on a terminating worker is the - /// intended behavior (its loop will never tick again). + /// terminating. #[inline] pub fn unref_event_loop_concurrently(self) { ScriptExecutionContext__unrefEventLoopConcurrently(self.0); @@ -1758,12 +1752,9 @@ unsafe extern "C" { } impl JSGlobalObject { - /// The stable `u32` identifier for this global's `ScriptExecutionContext`. - /// Capture on the JS thread when scheduling off-thread work; the pool - /// thread posts its completion back via - /// [`ScriptExecutionContextIdentifier::post_concurrent_task`] instead of a - /// raw `&EventLoop`/`&VirtualMachine`, so a worker VM freed by - /// `terminate()` is never dereferenced. + /// The stable id for this global's `ScriptExecutionContext`. Capture on the + /// JS thread; post completions back via + /// [`ScriptExecutionContextIdentifier::post_concurrent_task`]. #[inline] pub fn script_execution_context_identifier(&self) -> ScriptExecutionContextIdentifier { ScriptExecutionContextIdentifier(ScriptExecutionContextIdentifier__forGlobalObject(self)) diff --git a/src/jsc/RuntimeTranspilerStore.rs b/src/jsc/RuntimeTranspilerStore.rs index 7e47b1c15086..b869d722cdfc 100644 --- a/src/jsc/RuntimeTranspilerStore.rs +++ b/src/jsc/RuntimeTranspilerStore.rs @@ -489,12 +489,9 @@ impl TranspilerJob { pub(crate) fn dispatch_to_main_thread(&mut self) { let context_id = self.context_id; - // The VM owns both `transpiler_store.queue` and the event loop; touch - // neither once the target context has begun teardown. `is_alive()` - // serializes with `markTerminating()` on the contexts-map lock. + // VM owns both `transpiler_store.queue` and the event loop; touch neither after teardown. if !context_id.is_alive() { - // Reclaim pure-Rust heap owned by this job; leak the JSC-heap - // handles (`promise`, `fetcher`, specifiers, `resolved_source`). + // Reclaim pure-Rust heap; leak JSC-heap handles. let old_path = core::mem::take(&mut self.path); if !old_path.text.is_empty() { // SAFETY: `text` is the `Box<[u8]>` produced by @@ -508,8 +505,10 @@ impl TranspilerJob { return; } let vm = self.vm; - // SAFETY: `is_alive()` held the contexts-map lock; the VM is live for - // this queue push (BACKREF — VM owns the store). + // SAFETY: `is_alive()` is best-effort (lock released before this deref); + // `transpiler_store` is a VM field, so this and the push race the narrow + // is_alive→dealloc window, same as every `(*vm).*` read in `run()`. The + // authoritative fence is `post_concurrent_task` below. let transpiler_store: *mut RuntimeTranspilerStore = unsafe { ptr::addr_of_mut!((*vm).transpiler_store) }; let job = NonNull::from(&mut *self); @@ -518,11 +517,8 @@ impl TranspilerJob { // Another thread may free `self` at any time after .push, so we cannot use it any more. let task = ConcurrentTask::create_from(transpiler_store); if !context_id.post_concurrent_task(task) { - // Raced with `markTerminating()`; the job is already in - // `transpiler_store.queue` (inside the VM being freed). Reclaim - // only the freshly-allocated node. - // SAFETY: ownership was not transferred; `task` is the - // `ConcurrentTask::create_from` heap node above. + // Raced with teardown; job already in the VM-owned queue, reclaim only the node. + // SAFETY: ownership not transferred; `task` is the `create_from` heap node above. drop(unsafe { bun_core::heap::take(task.as_ptr()) }); } } diff --git a/src/jsc/WorkTask.rs b/src/jsc/WorkTask.rs index 5dd280339403..f59ec12453c9 100644 --- a/src/jsc/WorkTask.rs +++ b/src/jsc/WorkTask.rs @@ -35,14 +35,9 @@ pub trait WorkTaskContext: Sized { pub struct WorkTask { pub ctx: *mut Context, pub task: WorkPoolTask, - /// BACKREF — captured from the JS-thread VM at create time. Only - /// dereferenced on the JS thread; the pool-thread completion goes through - /// [`Self::context_id`] so a worker VM freed by `terminate()` is never - /// touched. + /// BACKREF — JS-thread only; pool-thread completion goes through `context_id`. pub event_loop: BackRef, - /// Stable `u32` id for the originating `ScriptExecutionContext`. The - /// pool-thread `on_finish` posts the completion via this id under the C++ - /// contexts-map lock (serializing with worker `markTerminating()`). + /// See [`ScriptExecutionContextIdentifier::post_concurrent_task`]. pub context_id: ScriptExecutionContextIdentifier, // allocator field dropped — global mimalloc (see PORTING.md §Allocators) pub global_this: BackRef, @@ -143,13 +138,7 @@ impl WorkTask { this.concurrent_task .from(this_ptr, AutoDeinit::ManualDeinit), ); - // Post by stable context id: the enqueue dereferences the target VM - // only under the contexts-map lock (serializes with worker - // `markTerminating()`). When the context is gone, `run_from_js` will - // never fire; `*this` (and its JSC-heap `global_this` / `ctx` - // `Strong` handles) cannot be reclaimed off-thread, so the box is - // leaked. The intrusive `task` is a field of `*this`, so no separate - // free is needed. + // Abandon: JSC handles cannot drop off-thread, leak the box (task is intrusive). let _ = context_id.post_concurrent_task(task); } } diff --git a/src/jsc/any_task_job.rs b/src/jsc/any_task_job.rs index 71886a543d94..717e5dc005f5 100644 --- a/src/jsc/any_task_job.rs +++ b/src/jsc/any_task_job.rs @@ -50,12 +50,10 @@ pub trait AnyTaskJobCtx: Sized { /// e.g. a `JSPromiseStrong` field after scheduling. pub struct AnyTaskJob { vm: bun_ptr::BackRef, - /// Stable `u32` id for the originating `ScriptExecutionContext`. The - /// pool-thread [`Self::run_task`] posts the completion back via this id - /// under the C++ contexts-map lock, so `vm` (which embeds the target - /// `EventLoop`) is never dereferenced off-thread once a worker's - /// `markTerminating()` has returned. + /// See [`ScriptExecutionContextIdentifier::post_concurrent_task`]. context_id: ScriptExecutionContextIdentifier, + /// Captured on the JS thread so the pool thread never derefs `vm`. + global: *mut JSGlobalObject, task: WorkPoolTask, any_task: AnyTask, poll: KeepAlive, @@ -83,6 +81,7 @@ impl AnyTaskJob { let job = bun_core::heap::into_raw(Box::new(Self { vm, context_id: global.script_execution_context_identifier(), + global: core::ptr::from_ref(global).cast_mut(), task: WorkPoolTask { node: Default::default(), callback: Self::run_task, @@ -148,29 +147,17 @@ impl AnyTaskJob { // `run_from_js` reclaims it (or is leaked on abandon below). let job = unsafe { &mut *Self::from_task_ptr(task) }; let context_id = job.context_id; - // Skip the work body when the owning context has already begun - // shutdown: several ctxs (e.g. `Scrypt`) write their output into a - // JSC-heap-backed `ArrayBuffer` that is freed by `teardownJSCVM`. - // `is_alive()` serializes with `markTerminating()` on the contexts-map - // lock. This is a fast-path drop only; the authoritative gate is the - // `post_concurrent_task` below. + // Skip the work body on shutdown: some ctxs write into JSC-heap-backed + // `ArrayBuffer`s. Fast-path only; `post_concurrent_task` below is the gate. if context_id.is_alive() { - let vm = job.vm; - job.ctx.run(vm.global); + job.ctx.run(job.global); } - // Post by stable context id: the enqueue dereferences the target VM - // only under the contexts-map lock (serializes with worker - // `markTerminating()`). let task = ConcurrentTask::create(job.any_task.task()); if context_id.post_concurrent_task(task) { return; } - // Target context gone or terminating. `run_from_js` will never fire; - // reclaim the heap `ConcurrentTaskItem`. The job box itself (and its - // `ctx`, which typically holds `JSPromiseStrong`/`Strong` handles into - // the dead JSC heap) cannot be released off-thread and is leaked. - // SAFETY: `post_concurrent_task` returned false so ownership was not - // transferred; `task` was `ConcurrentTask::create`-allocated above. + // Abandon: JSC handles cannot drop off-thread, leak the job box. + // SAFETY: ownership not transferred; `task` was `ConcurrentTask::create`-allocated above. drop(unsafe { bun_core::heap::take(task.as_ptr()) }); } diff --git a/src/jsc/bindings/ScriptExecutionContext.cpp b/src/jsc/bindings/ScriptExecutionContext.cpp index 47f50a3ee280..892d23c0ac68 100644 --- a/src/jsc/bindings/ScriptExecutionContext.cpp +++ b/src/jsc/bindings/ScriptExecutionContext.cpp @@ -302,10 +302,10 @@ extern "C" JSC::JSGlobalObject* ScriptExecutionContextIdentifier__getGlobalObjec return context->globalObject(); } -// True while the context exists and has not been marked terminating. Checked -// under the contexts-map lock so it serializes with markTerminating(). +// True while the context exists and has not been marked terminating. extern "C" bool ScriptExecutionContext__isAlive(ScriptExecutionContextIdentifier id) { + if (!id) return false; Locker locker { allScriptExecutionContextsMapLock }; auto* context = allScriptExecutionContextsMap().get(id); return context && !context->isTerminating(); @@ -325,6 +325,7 @@ extern "C" void Bun__EventLoop__enqueueConcurrentTask(JSC::JSGlobalObject*, void // ordering argument. extern "C" bool ScriptExecutionContext__postConcurrentTask(ScriptExecutionContextIdentifier id, void* task) { + if (!id) return false; Locker locker { allScriptExecutionContextsMapLock }; auto* context = allScriptExecutionContextsMap().get(id); if (!context || context->isTerminating()) @@ -333,12 +334,11 @@ extern "C" bool ScriptExecutionContext__postConcurrentTask(ScriptExecutionContex return true; } -// Decrement the target context's event-loop concurrent refcount, under the -// contexts-map lock so the bunVM pointer is dereferenced only while the -// context is known live. No-op (and no VM dereference) when the context is -// gone or terminating. +// Decrement the target context's event-loop concurrent refcount under the +// contexts-map lock; no-op when the context is gone or terminating. extern "C" void ScriptExecutionContext__unrefEventLoopConcurrently(ScriptExecutionContextIdentifier id) { + if (!id) return; Locker locker { allScriptExecutionContextsMapLock }; auto* context = allScriptExecutionContextsMap().get(id); if (!context || context->isTerminating()) diff --git a/src/jsc/virtual_machine_exports.rs b/src/jsc/virtual_machine_exports.rs index 461733674344..1e84cee2346a 100644 --- a/src/jsc/virtual_machine_exports.rs +++ b/src/jsc/virtual_machine_exports.rs @@ -142,10 +142,7 @@ pub fn queue_task_concurrently(global: &JSGlobalObject, task: *mut crate::cpp_ta } /// Called from C++ `ScriptExecutionContext__postConcurrentTask` with the -/// contexts-map lock held, so `global`'s VM and its embedded `EventLoop` are -/// guaranteed live for the duration of this call (worker `markTerminating()` -/// serializes on the same lock before any VM dealloc). `task` is the -/// heap-allocated `ConcurrentTaskItem` the Rust caller passed in. +/// contexts-map lock held, so `global`'s VM/`EventLoop` are live for this call. // HOST_EXPORT(Bun__EventLoop__enqueueConcurrentTask, c) #[allow(clippy::not_unsafe_ptr_arg_deref)] pub fn event_loop_enqueue_concurrent_task( diff --git a/src/runtime/api/Archive.rs b/src/runtime/api/Archive.rs index 469f71340d58..3bbff9c9fae2 100644 --- a/src/runtime/api/Archive.rs +++ b/src/runtime/api/Archive.rs @@ -748,11 +748,7 @@ impl AsyncTask { let ct = core::ptr::NonNull::from( (*this).concurrent_task.from(this, AutoDeinit::ManualDeinit), ); - // Post by stable context id so the target VM is only dereferenced - // under the contexts-map lock. If the context is gone the box is - // leaked — its `JSPromiseStrong` lives in the dead JSC heap and - // cannot be released off-thread; `ct` is a field of `*this`, so no - // separate free. + // Abandon: JSC handles cannot drop off-thread, leak the box (ct is intrusive). let _ = context_id.post_concurrent_task(ct); } } diff --git a/src/runtime/api/js_bundle_completion_task.rs b/src/runtime/api/js_bundle_completion_task.rs index 203cb80205ef..d31a393ed02d 100644 --- a/src/runtime/api/js_bundle_completion_task.rs +++ b/src/runtime/api/js_bundle_completion_task.rs @@ -984,12 +984,9 @@ impl CompletionStruct for JSBundleCompletionTask { fn complete_on_bundle_thread(&mut self) { let task = jsc::ConcurrentTask::create(self.task.task()); if !self.context_id.post_concurrent_task(task) { - // SAFETY: ownership was not transferred; `task` is the fresh heap - // node from `ConcurrentTask::create` above. + // Abandon: JSC handles cannot drop off-thread, leak the box. + // SAFETY: ownership not transferred; `task` is the fresh `create` heap node above. drop(unsafe { bun_core::heap::take(task.as_ptr()) }); - // `on_complete_anytask` will never run; the box (promise/plugins/ - // KeepAlive) points into the dead VM and cannot be reclaimed - // off-thread. Bounded leak on worker terminate. } } fn set_result(&mut self, result: BundleV2Result) { diff --git a/src/runtime/crypto/PasswordObject.rs b/src/runtime/crypto/PasswordObject.rs index d093113f3734..eee4d40ec0fb 100644 --- a/src/runtime/crypto/PasswordObject.rs +++ b/src/runtime/crypto/PasswordObject.rs @@ -588,9 +588,7 @@ impl PasswordJob { // SAFETY: `result.task` is an intrusive field at a stable heap address. let node = ConcurrentTask::create_from(unsafe { core::ptr::addr_of_mut!((*result).task) }); if !self.context_id.post_concurrent_task(node) { - // Target context is gone. Free the queue node and the result box; - // leak the JSPromiseStrong (its JSC heap is dead and cannot be - // released off-thread). + // Abandon: free node + result box; leak the `JSPromiseStrong` (JSC heap is dead). drop(unsafe { bun_core::heap::take(node.as_ptr()) }); let mut r = unsafe { bun_core::heap::take(result) }; core::mem::forget(core::mem::take(&mut r.promise)); diff --git a/src/runtime/node/node_fs.rs b/src/runtime/node/node_fs.rs index 6773cb11973e..115630a949b9 100644 --- a/src/runtime/node/node_fs.rs +++ b/src/runtime/node/node_fs.rs @@ -1307,18 +1307,11 @@ mod _async_tasks { // `sys::Error::path` is `Box<[u8]>` boxed at the // `errno_sys_p` construction site, so no clone is needed — `node_fs` may drop. - // Post by stable context id: the enqueue dereferences the target VM - // only under the contexts-map lock (serializes with worker - // `markTerminating()`). let context_id = this.context_id; let node = ConcurrentTask::create_from(std::ptr::from_mut::(this)); if !context_id.post_concurrent_task(node) { - // Target context gone or terminating. `run_from_js_thread` will - // never fire; reclaim the heap `ConcurrentTask` node. `*this` - // holds `JSPromiseStrong`/`ThreadSafe` handles into the dead - // JSC heap and cannot be released off-thread, so the box is leaked. - // SAFETY: `post_concurrent_task` returned false so ownership was - // not transferred; `node` was `ConcurrentTask::create_from`-allocated above. + // Abandon: JSC handles cannot drop off-thread, leak the box. + // SAFETY: ownership not transferred; `node` was `create_from`-allocated above. drop(unsafe { bun_core::heap::take(node.as_ptr()) }); } } @@ -1705,10 +1698,8 @@ mod _async_tasks { unsafe { (&mut *p).run_from_js_thread().map_err(Into::into) } }); if !this_ref.context_id.post_concurrent_task(node) { - // Target context is gone; reclaim the node. `this` holds - // `JSPromiseStrong` into the dead JSC heap, so it is leaked. - // SAFETY: ownership was not transferred; `node` was - // `ConcurrentTask::from_callback`-allocated just above. + // Abandon: JSC handles cannot drop off-thread, leak the box. + // SAFETY: ownership not transferred; `node` was `from_callback`-allocated above. drop(unsafe { bun_core::heap::take(node.as_ptr()) }); } } else { @@ -2548,18 +2539,11 @@ mod _async_tasks { } } - // Post by stable context id: the enqueue dereferences the target VM - // only under the contexts-map lock (serializes with worker - // `markTerminating()`). let context_id = self.context_id; let node = ConcurrentTask::create(Task::init(std::ptr::from_mut::(self))); if !context_id.post_concurrent_task(node) { - // Target context gone or terminating. `run_from_js_thread` will - // never fire; reclaim the heap `ConcurrentTask` node. `*self` - // holds `JSPromiseStrong` into the dead JSC heap and cannot be - // released off-thread, so the box is leaked. - // SAFETY: `post_concurrent_task` returned false so ownership was - // not transferred; `node` was `ConcurrentTask::create`-allocated above. + // Abandon: JSC handles cannot drop off-thread, leak the box. + // SAFETY: ownership not transferred; `node` was `create`-allocated above. drop(unsafe { bun_core::heap::take(node.as_ptr()) }); } } diff --git a/src/runtime/node/node_zlib_binding.rs b/src/runtime/node/node_zlib_binding.rs index f37e65ddb1e6..335dfec74dc8 100644 --- a/src/runtime/node/node_zlib_binding.rs +++ b/src/runtime/node/node_zlib_binding.rs @@ -211,8 +211,7 @@ 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; - /// Captured on the JS thread at construction; used off-thread to post the - /// completion so a freed worker VM/global is never dereferenced. + /// See [`ScriptExecutionContextIdentifier::post_concurrent_task`]. fn context_id(&self) -> ScriptExecutionContextIdentifier; fn stream(&self) -> &JsCell; @@ -479,18 +478,11 @@ impl CompressionStream { this_ref.stream().with_mut(|s| s.do_work()); - // Post by stable context id: the enqueue dereferences the target VM - // only under the contexts-map lock (serializes with worker - // `markTerminating()`). `this` is the heap-allocated `m_ctx` payload — - // the matching `ref()` in `write()` keeps it alive until - // `run_from_js_thread` runs and calls `deref()`. + // `this` is the heap `m_ctx` payload; the `ref()` in `write()` keeps it alive. let node = ConcurrentTask::create(Task::init(this)); if !this_ref.context_id().post_concurrent_task(node) { - // Target context gone or terminating. `run_from_js_thread` will - // never fire. `this` is the JSC-owned m_ctx holding `Strong`/ - // `KeepAlive`; its `deref()` is not safe off-thread, so leak it. - // SAFETY: ownership was not transferred; `node` was - // `ConcurrentTask::create`-allocated above. + // Abandon: `deref()` is not safe off-thread, leak `this`. + // SAFETY: ownership not transferred; `node` was `create`-allocated above. drop(unsafe { bun_core::heap::take(node.as_ptr()) }); } } diff --git a/src/runtime/webcore/fetch/FetchTasklet.rs b/src/runtime/webcore/fetch/FetchTasklet.rs index 89a6be56772d..56161ab0acb1 100644 --- a/src/runtime/webcore/fetch/FetchTasklet.rs +++ b/src/runtime/webcore/fetch/FetchTasklet.rs @@ -297,13 +297,7 @@ impl FetchTasklet { unsafe { &*this } } - /// Enqueue a concurrent task on the JS-thread event loop. - /// - /// Routes through the contexts-map lock so the target `EventLoop` is only - /// dereferenced while its VM is known live. Returns `false` (caller keeps - /// ownership of `task`) when the context is gone or terminating. `task` is - /// a live `ConcurrentTaskItem` that the queue takes ownership of via its - /// intrusive `next` link on success. + /// See [`ScriptExecutionContextIdentifier::post_concurrent_task`]. #[inline] fn enqueue_concurrent( context_id: ScriptExecutionContextIdentifier, @@ -412,8 +406,7 @@ impl FetchTasklet { // takes ownership of it. let node = ConcurrentTask::from_callback(this, FetchTasklet::deinit_callback); if !Self::enqueue_concurrent(self_.context_id, node) { - // Target VM raced to teardown between the check and the post; drop - // the freshly heap-allocated node and take the shutdown dealloc path. + // Raced with teardown; drop the fresh node and take the shutdown dealloc path. drop(unsafe { bun_core::heap::take(node.as_ptr()) }); // SAFETY: last ref; see the `!is_alive()` branch above. unsafe { FetchTasklet::dealloc_for_shutdown(this) }; @@ -2149,8 +2142,7 @@ impl FetchTasklet { // takes ownership of it. let node = ConcurrentTask::from_callback(this, FetchTasklet::resume_request_data_stream); if !Self::enqueue_concurrent(this_ref.context_id, node) { - // Target VM raced to teardown; drop the fresh node and undo the ref - // via the thread-safe path (which takes the shutdown dealloc branch). + // Raced with teardown; drop the fresh node and undo the ref. drop(unsafe { bun_core::heap::take(node.as_ptr()) }); FetchTasklet::deref_from_thread(this); } @@ -2463,11 +2455,8 @@ impl FetchTasklet { } } // will deinit when done with the http client (when is_done = true) - // Shared cleanup for the "target VM is gone/terminating" path. Runs - // only the Rust-side work the JS-thread on_progress_update would never - // get to: free the body buffer, release the parked socket, undo the - // CAS flag, unlock, and (on the final result) drop both refs so the - // 1→0 transition takes `dealloc_for_shutdown`. + // Abandon path: Rust-side cleanup the JS-thread `on_progress_update` would + // never reach (body buffer, parked socket, CAS flag, unlock, final derefs). let abandon = |task_ref: &mut FetchTasklet| { task_ref.scheduled_response_buffer = MutableString::default(); if task_ref.result.certificate_info.take().is_some() { diff --git a/src/runtime/webcore/s3/download_stream.rs b/src/runtime/webcore/s3/download_stream.rs index 47e8e4387c0d..fa1fe00a848c 100644 --- a/src/runtime/webcore/s3/download_stream.rs +++ b/src/runtime/webcore/s3/download_stream.rs @@ -22,9 +22,7 @@ pub struct S3HttpDownloadStreamingTask { /// JSC_BORROW: per-thread VM singleton, outlives every task. `None` only in /// the inert `Default` placeholder (overwritten before the task escapes). pub vm: Option>, - /// Stable id of the originating `ScriptExecutionContext`. The HTTP-thread - /// completion posts back via this id under the C++ contexts-map lock so a - /// worker VM freed by `terminate()` is never dereferenced. + /// See [`ScriptExecutionContextIdentifier::post_concurrent_task`]. pub context_id: ScriptExecutionContextIdentifier, pub sign_result: SignResult, pub headers: Headers, @@ -350,12 +348,7 @@ impl S3HttpDownloadStreamingTask { let task = core::ptr::NonNull::from( self_.concurrent_task.from(this, AutoDeinit::ManualDeinit), ); - // Post by stable context id under the C++ contexts-map lock so a - // worker VM freed by `terminate()` is never dereferenced. On - // abandon the heap task is leaked: `Drop` touches the VM event - // loop and must not run off-thread against a dead VM. `task` is - // the inline `concurrent_task` field of this heap request; the - // queue takes ownership of its `next` link. + // Abandon: `Drop` touches the VM event loop, leak the box (task is intrusive). let _ = self_.context_id.post_concurrent_task(task); } } diff --git a/src/runtime/webcore/s3/simple_request.rs b/src/runtime/webcore/s3/simple_request.rs index a5e04de1835e..353b8fcfc6a3 100644 --- a/src/runtime/webcore/s3/simple_request.rs +++ b/src/runtime/webcore/s3/simple_request.rs @@ -121,9 +121,7 @@ pub struct S3HttpSimpleTask { /// JSC_BORROW: per-thread VM singleton, outlives every task. `None` only in /// the inert `Default` placeholder (overwritten before the task escapes). pub vm: Option>, - /// Stable id of the originating `ScriptExecutionContext`. The HTTP-thread - /// completion posts back via this id under the C++ contexts-map lock so a - /// worker VM freed by `terminate()` is never dereferenced. + /// See [`ScriptExecutionContextIdentifier::post_concurrent_task`]. pub context_id: ScriptExecutionContextIdentifier, pub sign_result: SignResult, pub headers: Headers, @@ -493,12 +491,7 @@ impl S3HttpSimpleTask { this.concurrent_task .from(this_ptr, AutoDeinit::ManualDeinit), ); - // Post by stable context id under the C++ contexts-map lock so a - // worker VM freed by `terminate()` is never dereferenced. On - // abandon the heap task is leaked: `Drop` touches the VM event - // loop and must not run off-thread against a dead VM. `task` is - // the inline `concurrent_task` field of this heap request; the - // queue takes ownership of its `next` link. + // Abandon: `Drop` touches the VM event loop, leak the box (task is intrusive). let _ = this.context_id.post_concurrent_task(task); } } From ae2db332b3d393d7e2a7b3560c1c230ccc326e68 Mon Sep 17 00:00:00 2001 From: robobun Date: Sat, 25 Jul 2026 18:59:21 +0000 Subject: [PATCH 4/7] review: convert COMPLETION_VTABLE plugin enqueue, drop jsc_event_loop, root test scratch under harness tempDir COMPLETION_VTABLE.enqueue_task_concurrent (the bundler's off-thread onLoad/onResolve plugin dispatch) was still derefing jsc_event_loop; route it through context_id.post_concurrent_task like complete_on_bundle_thread. The jsc_event_loop field (and the event_loop parameter on create_and_schedule_completion_task) are now dead and removed. Worker test scratch dir rooted under body.mjs's directory (the harness tempDir), so 'using dir' reclaims it instead of leaking into $TMPDIR. repro/verify.mjs now owns one scratch root and removes it on exit. Test's Bun.build grows a plugin so the COMPLETION_VTABLE path is armed. --- repro/rootB-verify/verify.mjs | 6 +++++ repro/rootB-verify/worker-body.mjs | 9 ++++--- src/runtime/api/JSBundler.rs | 3 +-- src/runtime/api/js_bundle_completion_task.rs | 24 +++++++------------ src/runtime/server/HTMLBundle.rs | 3 +-- .../workers/worker-terminate-lifetime.test.ts | 15 ++++++++---- 6 files changed, 33 insertions(+), 27 deletions(-) diff --git a/repro/rootB-verify/verify.mjs b/repro/rootB-verify/verify.mjs index bca2cbe13112..354683a7fbc1 100644 --- a/repro/rootB-verify/verify.mjs +++ b/repro/rootB-verify/verify.mjs @@ -7,9 +7,15 @@ // bun bd repro/rootB-verify/verify.mjs import { Worker } from "node:worker_threads"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; const ITERATIONS = Number(process.env.ROOTB_ITER ?? 100); const body = new URL("./worker-body.mjs", import.meta.url); +const scratch = fs.mkdtempSync(path.join(os.tmpdir(), "rootB-verify-")); +process.env.ROOTB_SCRATCH = scratch; +process.on("exit", () => fs.rmSync(scratch, { recursive: true, force: true })); for (let i = 0; i < ITERATIONS; i++) { const w = new Worker(body); diff --git a/repro/rootB-verify/worker-body.mjs b/repro/rootB-verify/worker-body.mjs index ca3b56c84cb6..89fd53236c34 100644 --- a/repro/rootB-verify/worker-body.mjs +++ b/repro/rootB-verify/worker-body.mjs @@ -4,7 +4,7 @@ // point is to have work airborne on a process-global thread when the VM dies, // not to observe the result. -import { parentPort } from "node:worker_threads"; +import { parentPort, threadId } from "node:worker_threads"; import fs from "node:fs"; import fsp from "node:fs/promises"; import zlib from "node:zlib"; @@ -16,9 +16,12 @@ import path from "node:path"; const sink = () => {}; const swallow = (p) => Promise.resolve(p).then(sink, sink); -const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "rootB-")); +// One scratch root for all iterations (verify.mjs removes it on exit). +const root = process.env.ROOTB_SCRATCH ?? path.join(os.tmpdir(), "rootB-verify"); +const tmp = path.join(root, String(threadId)); +fs.mkdirSync(tmp, { recursive: true }); const tmpFile = path.join(tmp, "a.txt"); -fs.writeFileSync(tmpFile, "x".repeat(1 << 16)); +fs.writeFileSync(tmpFile, Buffer.alloc(1 << 16, "x").toString()); // fetch / HTMLRewriter / TLS (HTTP thread -> FetchTasklet) { diff --git a/src/runtime/api/JSBundler.rs b/src/runtime/api/JSBundler.rs index cfbd22382da3..06703fafdb78 100644 --- a/src/runtime/api/JSBundler.rs +++ b/src/runtime/api/JSBundler.rs @@ -1339,7 +1339,7 @@ pub mod js_bundler { let mut plugins: Option<*mut Plugin> = None; let config = Config::from_js(global_this, arguments[0], &mut plugins)?; - let event_loop = vm.event_loop(); + let _ = vm; // `BundleV2.generateFromJavaScript` — the completion-task struct lives in // `crate::api::js_bundle_completion_task` (bun_runtime owns it because its @@ -1350,7 +1350,6 @@ pub mod js_bundler { config, plugins.and_then(core::ptr::NonNull::new), global_this, - event_loop, ) .map_err(|_| JsError::OutOfMemory)?; // SAFETY: `completion` is the freshly-boxed allocation returned above; diff --git a/src/runtime/api/js_bundle_completion_task.rs b/src/runtime/api/js_bundle_completion_task.rs index d31a393ed02d..3c1b7e5b6422 100644 --- a/src/runtime/api/js_bundle_completion_task.rs +++ b/src/runtime/api/js_bundle_completion_task.rs @@ -25,7 +25,7 @@ use bun_core::env::OperatingSystem; use bun_io::KeepAlive; use bun_jsc::AnyTask::AnyTask; use bun_jsc::WorkPool; -use bun_jsc::event_loop::EventLoop; + use bun_jsc::js_global_object::ScriptExecutionContextIdentifier; use bun_jsc::{self as jsc, JSGlobalObject, JSPromise, JSValue}; use bun_options_types::WindowsOptions; @@ -59,9 +59,6 @@ pub struct JSBundleCompletionTask { // `unsafe impl Send` below for the thread-affinity constraint this imposes. pub ref_count: RefCount, pub config: JSBundlerConfig, - // BACKREF — the JS-thread `EventLoop` outlives every completion task; safe - // `Deref` so call sites read `self.jsc_event_loop.enqueue_task_concurrent(..)`. - pub jsc_event_loop: BackRef, pub context_id: ScriptExecutionContextIdentifier, pub task: AnyTask, pub global_this: BackRef, @@ -117,16 +114,12 @@ pub(crate) fn create_and_schedule_completion_task( config: JSBundlerConfig, plugins: Option>, global_this: &JSGlobalObject, - event_loop: *mut EventLoop, ) -> crate::Result<*mut JSBundleCompletionTask> { let vm = global_this.bun_vm_ptr(); let env = global_this.bun_vm().transpiler.env; let completion = bun_core::heap::into_raw(Box::new(JSBundleCompletionTask { ref_count: RefCount::init(), config, - // `event_loop` is the live JS-thread loop (caller derives it from - // `vm.event_loop()`); never null once `Bun.build` is reachable. - jsc_event_loop: BackRef::from(core::ptr::NonNull::new(event_loop).expect("event_loop")), context_id: global_this.script_execution_context_identifier(), task: AnyTask::default(), global_this: BackRef::new(global_this), @@ -764,13 +757,14 @@ fn from_completion_handle<'a>(c: NonNull) -> &'a JSBundleCo static COMPLETION_VTABLE: dispatch::CompletionDispatch = dispatch::CompletionDispatch { result_is_err: |c| matches!(from_completion_handle(c).result, BundleV2Result::Err(_)), enqueue_task_concurrent: |c, task| { - // `jsc_event_loop` is a `BackRef` — safe Deref. - // SAFETY: `task` is a fresh heap-allocated non-null `ConcurrentTaskItem` - // passed through from the bundler vtable; the queue takes ownership. - unsafe { - from_completion_handle(c) - .jsc_event_loop - .enqueue_task_concurrent(core::ptr::NonNull::new_unchecked(task)) + // SAFETY: `task` is a fresh heap-allocated non-null `ConcurrentTaskItem`. + let node = unsafe { core::ptr::NonNull::new_unchecked(task) }; + if !from_completion_handle(c) + .context_id + .post_concurrent_task(node) + { + // SAFETY: ownership not transferred; reclaim the heap node. + drop(unsafe { bun_core::heap::take(node.as_ptr()) }); } }, }; diff --git a/src/runtime/server/HTMLBundle.rs b/src/runtime/server/HTMLBundle.rs index 937785d53a27..28de23af4537 100644 --- a/src/runtime/server/HTMLBundle.rs +++ b/src/runtime/server/HTMLBundle.rs @@ -498,8 +498,7 @@ impl Route { } config.source_map = bundler_options::SourceMapOption::Linked; - let completion_task = - create_and_schedule_completion_task(config, plugins, global, vm.event_loop())?; + let completion_task = create_and_schedule_completion_task(config, plugins, global)?; // SAFETY: `completion_task` is the freshly-boxed allocation (refcount==1); sole owner. unsafe { (*completion_task).started_at_ns = diff --git a/test/js/web/workers/worker-terminate-lifetime.test.ts b/test/js/web/workers/worker-terminate-lifetime.test.ts index 81d74b450298..a368839d5c8f 100644 --- a/test/js/web/workers/worker-terminate-lifetime.test.ts +++ b/test/js/web/workers/worker-terminate-lifetime.test.ts @@ -191,12 +191,14 @@ test.skipIf(!isASAN)( // Worker body: arm one in-flight op per cross-thread source, signal the // parent, then sit. All loopback-only; results ignored. const body = ` - import { parentPort } from "node:worker_threads"; + import { parentPort, threadId } from "node:worker_threads"; import fs from "node:fs"; import fsp from "node:fs/promises"; import zlib from "node:zlib"; import crypto from "node:crypto"; - import os from "node:os"; import path from "node:path"; + import path from "node:path"; const sink = () => {}; const swallow = p => Promise.resolve(p).then(sink, sink); - const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "wt-")); + // Scratch rooted under body.mjs's dir (the harness tempDir) so cleanup is automatic. + const tmp = path.join(path.dirname(new URL(import.meta.url).pathname), "scratch-" + threadId); + fs.mkdirSync(tmp, { recursive: true }); const tf = path.join(tmp, "a.txt"); fs.writeFileSync(tf, Buffer.alloc(1 << 14, "x").toString()); // WorkTask @@ -219,9 +221,12 @@ test.skipIf(!isASAN)( swallow(Array.fromAsync(new Bun.Glob("**/*").scan(tmp))); // ConcurrentCppTask (WebCrypto) swallow(crypto.subtle.digest("SHA-256", new Uint8Array(1 << 14))); - // JSBundleCompletionTask + // JSBundleCompletionTask (+ COMPLETION_VTABLE.enqueue_task_concurrent via plugins) const e = path.join(tmp, "e.ts"); fs.writeFileSync(e, "export const x=1;"); - swallow(Bun.build({ entrypoints: [e], target: "bun" })); + swallow(Bun.build({ + entrypoints: [e], target: "bun", + plugins: [{ name: "p", setup(b) { b.onLoad({ filter: /\\.ts$/ }, () => undefined); } }], + })); // FetchTasklet (HTTP thread) const srv = Bun.serve({ port: 0, fetch: () => new Response(Buffer.alloc(1 << 12, "x")) }); swallow(fetch("http://127.0.0.1:" + srv.port + "/").then(r => r.text())); From a9a1475ca823077ce22c2ceb25ed49c7f3a38fe7 Mon Sep 17 00:00:00 2001 From: robobun Date: Sat, 25 Jul 2026 19:27:53 +0000 Subject: [PATCH 5/7] review: zlib do_work() is_alive() parity, ConcurrentTask::destroy_from_callback, drop dead event_loop fields - node_zlib_binding: wrap do_work() in is_alive() (writes into the pinned JS ArrayBuffer), same best-effort skip as AnyTaskJob::run_task. - ConcurrentTask::destroy_from_callback: from_callback allocates two boxes (outer ConcurrentTask + inner ManagedTask); the abandon paths at FetchTasklet deref_from_thread/on_write_request_data_drain and NewAsyncCpTask now reclaim both. - WorkTask / ConcurrentPromiseTask: event_loop field was write-only after on_finish moved to context_id; removed with its imports. --- src/event_loop/ConcurrentTask.rs | 15 +++++++++++++++ src/jsc/ConcurrentPromiseTask.rs | 9 --------- src/jsc/WorkTask.rs | 5 ----- src/runtime/node/node_fs.rs | 7 ++++--- src/runtime/node/node_zlib_binding.rs | 8 ++++++-- src/runtime/webcore/fetch/FetchTasklet.rs | 8 ++++---- 6 files changed, 29 insertions(+), 23 deletions(-) diff --git a/src/event_loop/ConcurrentTask.rs b/src/event_loop/ConcurrentTask.rs index f88145557f66..94a32b3458cf 100644 --- a/src/event_loop/ConcurrentTask.rs +++ b/src/event_loop/ConcurrentTask.rs @@ -309,6 +309,21 @@ impl ConcurrentTask { Self::create(ManagedTask::ManagedTask::new(ptr, callback)) } + /// Reclaim a node produced by [`Self::from_callback`] that was never + /// enqueued. Frees both the outer `ConcurrentTask` and the inner + /// `ManagedTask` box; does not call the callback. + /// + /// # Safety + /// `node` must be a `from_callback`-allocated node whose ownership was not + /// transferred to a queue. + pub unsafe fn destroy_from_callback(node: core::ptr::NonNull) { + // SAFETY: caller contract — `from_callback` wraps a heap `ManagedTask`. + let outer = unsafe { bun_core::heap::take(node.as_ptr()) }; + debug_assert!(outer.task.tag == task_tag::ManagedTask); + // SAFETY: `ManagedTask::new` produced the inner box via `heap::into_raw`. + drop(unsafe { bun_core::heap::take(outer.task.ptr.cast::()) }); + } + pub fn from( &mut self, of: *mut T, diff --git a/src/jsc/ConcurrentPromiseTask.rs b/src/jsc/ConcurrentPromiseTask.rs index 4757551b3679..5ee3af114db9 100644 --- a/src/jsc/ConcurrentPromiseTask.rs +++ b/src/jsc/ConcurrentPromiseTask.rs @@ -2,12 +2,9 @@ use bun_event_loop::ConcurrentTask::{AutoDeinit, ConcurrentTask, TaskTag, Taskab use bun_io::{self as Async, KeepAlive}; use bun_threading::{IntrusiveWorkTask as _, WorkPoolTask, work_pool::WorkPool}; -use crate::event_loop::EventLoop; use crate::js_global_object::ScriptExecutionContextIdentifier; use crate::js_promise::{JSPromise, Strong as JSPromiseStrong}; -use crate::virtual_machine::VirtualMachine; use crate::{JSGlobalObject, JsTerminated}; -use bun_ptr::BackRef; /// The `Context` type parameter for [`ConcurrentPromiseTask`] must implement this trait: /// - `run(&mut self)` — performs the work on the thread pool @@ -32,8 +29,6 @@ pub struct ConcurrentPromiseTask<'a, Context: ConcurrentPromiseTaskContext> { // Owned here so dropping the task frees the context. pub ctx: Box, pub task: WorkPoolTask, - /// BACKREF — JS-thread only; pool-thread completion goes through `context_id`. - pub event_loop: BackRef, /// See [`ScriptExecutionContextIdentifier::post_concurrent_task`]. pub context_id: ScriptExecutionContextIdentifier, pub promise: JSPromiseStrong, @@ -59,11 +54,7 @@ impl Taskable for ConcurrentPromiseTask<' impl<'a, Context: ConcurrentPromiseTaskContext> ConcurrentPromiseTask<'a, Context> { pub fn create_on_js_thread(global_this: &'a JSGlobalObject, value: Box) -> Box { - // `VirtualMachine::get()` returns the JS-thread singleton; the VM and - // its `EventLoop` outlive every task scheduled on it. - let event_loop = BackRef::new(VirtualMachine::get().as_mut().event_loop_shared()); let mut this = Box::new(Self { - event_loop, context_id: global_this.script_execution_context_identifier(), ctx: value, task: WorkPoolTask { diff --git a/src/jsc/WorkTask.rs b/src/jsc/WorkTask.rs index f59ec12453c9..fd90cc09878c 100644 --- a/src/jsc/WorkTask.rs +++ b/src/jsc/WorkTask.rs @@ -4,7 +4,6 @@ use bun_threading::{IntrusiveWorkTask as _, WorkPoolTask, work_pool::WorkPool}; use crate::JSGlobalObject; use crate::debugger::AsyncTaskTracker; -use crate::event_loop::EventLoop; use crate::js_global_object::ScriptExecutionContextIdentifier; use bun_ptr::BackRef; @@ -35,8 +34,6 @@ pub trait WorkTaskContext: Sized { pub struct WorkTask { pub ctx: *mut Context, pub task: WorkPoolTask, - /// BACKREF — JS-thread only; pool-thread completion goes through `context_id`. - pub event_loop: BackRef, /// See [`ScriptExecutionContextIdentifier::post_concurrent_task`]. pub context_id: ScriptExecutionContextIdentifier, // allocator field dropped — global mimalloc (see PORTING.md §Allocators) @@ -63,9 +60,7 @@ impl Taskable for WorkTask { impl WorkTask { pub fn create_on_js_thread(global_this: &JSGlobalObject, value: *mut Context) -> *mut Self { let vm = global_this.bun_vm().as_mut(); - let event_loop = BackRef::new(vm.event_loop_shared()); let mut this = Box::new(Self { - event_loop, context_id: global_this.script_execution_context_identifier(), ctx: value, global_this: BackRef::new(global_this), diff --git a/src/runtime/node/node_fs.rs b/src/runtime/node/node_fs.rs index 115630a949b9..17c6a833f0a4 100644 --- a/src/runtime/node/node_fs.rs +++ b/src/runtime/node/node_fs.rs @@ -1698,9 +1698,10 @@ mod _async_tasks { unsafe { (&mut *p).run_from_js_thread().map_err(Into::into) } }); if !this_ref.context_id.post_concurrent_task(node) { - // Abandon: JSC handles cannot drop off-thread, leak the box. - // SAFETY: ownership not transferred; `node` was `from_callback`-allocated above. - drop(unsafe { bun_core::heap::take(node.as_ptr()) }); + // SAFETY: ownership not transferred; `node` is a `from_callback` allocation. + unsafe { + bun_event_loop::ConcurrentTask::ConcurrentTask::destroy_from_callback(node) + }; } } else { this_ref.evtloop.enqueue_task_concurrent(EventLoopTaskPtr { diff --git a/src/runtime/node/node_zlib_binding.rs b/src/runtime/node/node_zlib_binding.rs index 335dfec74dc8..bc71e96c62e4 100644 --- a/src/runtime/node/node_zlib_binding.rs +++ b/src/runtime/node/node_zlib_binding.rs @@ -475,12 +475,16 @@ impl CompressionStream { // `ref_()` in `write()`); bodies use the `&self` accessor surface // (R-2). `ParentRef` Deref collapses the per-site raw deref. let this_ref = ParentRef::from(NonNull::new(this).expect("async_job_run: this")); + let context_id = this_ref.context_id(); - this_ref.stream().with_mut(|s| s.do_work()); + // `do_work()` writes into the pinned JS ArrayBuffer; best-effort skip on shutdown. + if context_id.is_alive() { + this_ref.stream().with_mut(|s| s.do_work()); + } // `this` is the heap `m_ctx` payload; the `ref()` in `write()` keeps it alive. let node = ConcurrentTask::create(Task::init(this)); - if !this_ref.context_id().post_concurrent_task(node) { + if !context_id.post_concurrent_task(node) { // Abandon: `deref()` is not safe off-thread, leak `this`. // SAFETY: ownership not transferred; `node` was `create`-allocated above. drop(unsafe { bun_core::heap::take(node.as_ptr()) }); diff --git a/src/runtime/webcore/fetch/FetchTasklet.rs b/src/runtime/webcore/fetch/FetchTasklet.rs index 56161ab0acb1..67422cb6c9d0 100644 --- a/src/runtime/webcore/fetch/FetchTasklet.rs +++ b/src/runtime/webcore/fetch/FetchTasklet.rs @@ -406,8 +406,8 @@ impl FetchTasklet { // takes ownership of it. let node = ConcurrentTask::from_callback(this, FetchTasklet::deinit_callback); if !Self::enqueue_concurrent(self_.context_id, node) { - // Raced with teardown; drop the fresh node and take the shutdown dealloc path. - drop(unsafe { bun_core::heap::take(node.as_ptr()) }); + // SAFETY: ownership not transferred; `node` is a `from_callback` allocation. + unsafe { ConcurrentTask::destroy_from_callback(node) }; // SAFETY: last ref; see the `!is_alive()` branch above. unsafe { FetchTasklet::dealloc_for_shutdown(this) }; } @@ -2142,8 +2142,8 @@ impl FetchTasklet { // takes ownership of it. let node = ConcurrentTask::from_callback(this, FetchTasklet::resume_request_data_stream); if !Self::enqueue_concurrent(this_ref.context_id, node) { - // Raced with teardown; drop the fresh node and undo the ref. - drop(unsafe { bun_core::heap::take(node.as_ptr()) }); + // SAFETY: ownership not transferred; `node` is a `from_callback` allocation. + unsafe { ConcurrentTask::destroy_from_callback(node) }; FetchTasklet::deref_from_thread(this); } } From b32298fe97be0a5a6b0987c5ce3a4b48bf2d55bb Mon Sep 17 00:00:00 2001 From: robobun Date: Sat, 25 Jul 2026 19:46:16 +0000 Subject: [PATCH 6/7] review: drop no-op let _ = vm; name Windows mkdirp completions as deferred in design doc --- docs/ROOT-B-SHUTDOWN-FENCE.md | 7 +++++++ src/runtime/api/JSBundler.rs | 2 -- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/docs/ROOT-B-SHUTDOWN-FENCE.md b/docs/ROOT-B-SHUTDOWN-FENCE.md index eca9ce50d1ba..0f4de9099f7f 100644 --- a/docs/ROOT-B-SHUTDOWN-FENCE.md +++ b/docs/ROOT-B-SHUTDOWN-FENCE.md @@ -113,6 +113,13 @@ Direct callers converted alongside: `FetchTasklet`, `PasswordJob`, `AsyncReaddirRecursiveTask`, `S3HttpSimpleTask` / `S3HttpDownloadStreamingTask`, `Archive::AsyncTask`, `JSBundleCompletionTask`, `TranspilerJob`. +Same enqueue shape, deferred to follow-up (not in the verify harness): +`napi_async_work` / `ThreadSafeFunction`, `fs.watch` / `fs.watchFile` +(PathWatcherManager reader thread), the shell WorkPool tasks, `AsyncModule` +package-manager wake, and the Windows-only `WriteFileWindows` / `CopyFileWindows` +`AsyncMkdirp` completion (the POSIX `WriteFile`/`CopyFile` paths above go +through the converted `WorkTask`/`ConcurrentPromiseTask`). + Explicitly **not** enqueue-shaped and left for their own fixes: nested-worker child-init reading a freed parent VM, `node:quic` finalizer ordering, `RedisClient::finalize` free-then-read, `Bun.SQL` handle crashes during diff --git a/src/runtime/api/JSBundler.rs b/src/runtime/api/JSBundler.rs index 06703fafdb78..c9d810473d72 100644 --- a/src/runtime/api/JSBundler.rs +++ b/src/runtime/api/JSBundler.rs @@ -1339,8 +1339,6 @@ pub mod js_bundler { let mut plugins: Option<*mut Plugin> = None; let config = Config::from_js(global_this, arguments[0], &mut plugins)?; - let _ = vm; - // `BundleV2.generateFromJavaScript` — the completion-task struct lives in // `crate::api::js_bundle_completion_task` (bun_runtime owns it because its // fields name `Config`/`Plugin`/`HTMLBundle::Route`; lower-tier crates From 96e26edc25a7fbd5095c31e610dc0c1ff09e119e Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 25 Jul 2026 21:16:57 +0000 Subject: [PATCH 7/7] review: drop write-only S3HttpDownloadStreamingTask.vm field --- src/runtime/webcore/s3/client.rs | 2 -- src/runtime/webcore/s3/download_stream.rs | 5 ----- 2 files changed, 7 deletions(-) diff --git a/src/runtime/webcore/s3/client.rs b/src/runtime/webcore/s3/client.rs index 6c08cb0879cb..5be541d367d9 100644 --- a/src/runtime/webcore/s3/client.rs +++ b/src/runtime/webcore/s3/client.rs @@ -1012,8 +1012,6 @@ pub(crate) fn download_stream( callback, range: range.map(Vec::into_boxed_slice), headers, - // `VirtualMachine::get()` returns the live per-thread VM singleton. - vm: Some(bun_ptr::BackRef::new(VirtualMachine::get())), context_id: VirtualMachine::get() .global() .script_execution_context_identifier(), diff --git a/src/runtime/webcore/s3/download_stream.rs b/src/runtime/webcore/s3/download_stream.rs index fa1fe00a848c..4f9c51710f18 100644 --- a/src/runtime/webcore/s3/download_stream.rs +++ b/src/runtime/webcore/s3/download_stream.rs @@ -8,7 +8,6 @@ use bun_event_loop::{TaskTag, Taskable, task_tag}; use bun_http::{AsyncHTTP, HTTPClientResult, Headers, Signals}; use bun_io::KeepAlive; use bun_jsc::js_global_object::ScriptExecutionContextIdentifier; -use bun_jsc::virtual_machine::VirtualMachine; use bun_s3_signing::credentials::SignResult; use bun_s3_signing::error::S3Error; use bun_threading::Mutex; @@ -19,9 +18,6 @@ pub struct S3HttpDownloadStreamingTask { // `MaybeUninit` because `AsyncHTTP` contains non-null references, so // `mem::zeroed()` can't be used here (mirrors `S3HttpSimpleTask`). pub http: core::mem::MaybeUninit>, - /// JSC_BORROW: per-thread VM singleton, outlives every task. `None` only in - /// the inert `Default` placeholder (overwritten before the task escapes). - pub vm: Option>, /// See [`ScriptExecutionContextIdentifier::post_concurrent_task`]. pub context_id: ScriptExecutionContextIdentifier, pub sign_result: SignResult, @@ -64,7 +60,6 @@ impl Default for S3HttpDownloadStreamingTask { Self { // never read — fully overwritten by `AsyncHTTP::init` before first use. http: core::mem::MaybeUninit::uninit(), - vm: None, context_id: ScriptExecutionContextIdentifier(0), sign_result: SignResult::default(), headers: Headers::default(),