From 583df13217a0a66352606b435c4207d7fe5c9a04 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 2 Sep 2026 01:06:59 +0000 Subject: [PATCH 1/7] Keep the entry-point promise alive while the hot-reload loop polls it The VM stores the promise that the module loader returns for the entry point in pending_internal_promise, and the --hot loop reads its status on every tick. Once that promise settles nothing else refers to it, so a collection can free the cell and the next read sees a dead promise. A dead cell reads as Pending, which defers every later reload, or as garbage, which panics with "invalid JSPromise status 255". Protect the promise for as long as the slot holds it. Every writer goes through set_pending_internal_promise, which protects the new value and unprotects the old one, so the separate is_protected flag goes away. --- src/jsc/VirtualMachine.rs | 66 +++++++++++++++++------------------- src/runtime/hw_exports.rs | 5 ++- src/runtime/jsc_hooks.rs | 12 ++++--- test/cli/hot/hot.test.ts | 71 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 111 insertions(+), 43 deletions(-) diff --git a/src/jsc/VirtualMachine.rs b/src/jsc/VirtualMachine.rs index e1171eb8df47..1651304a0657 100644 --- a/src/jsc/VirtualMachine.rs +++ b/src/jsc/VirtualMachine.rs @@ -297,8 +297,9 @@ pub struct VirtualMachine { pub rare_data: Option>, pub proxy_env_storage: crate::rare_data::ProxyEnvStorage, pub(crate) resolved_path_dups: Vec>, - pub pending_internal_promise: Option<*mut JSInternalPromise>, - pub pending_internal_promise_is_protected: bool, + /// The entry-point promise the watcher loops poll. Always GC-protected + /// while stored: write it through `set_pending_internal_promise`. + pending_internal_promise: Option<*mut JSInternalPromise>, pub pending_internal_promise_reported_at: u32, pub(crate) hot_reload_deferred: bool, pub entry_point_result: EntryPointResult, @@ -2905,10 +2906,7 @@ impl VirtualMachine { // SAFETY: hook contract. let p = unsafe { (hooks.load_preloads)(self) }?; if !p.is_null() { - JSValue::from_cell(p).ensure_still_alive(); - JSValue::from_cell(p).protect(); - self.pending_internal_promise = Some(p); - self.pending_internal_promise_is_protected = true; + self.set_pending_internal_promise(Some(p)); return Ok(p); } } @@ -2916,8 +2914,7 @@ impl VirtualMachine { // Check if Module.runMain was patched. if self.has_patched_run_main { bun_core::hint::cold(); - self.pending_internal_promise = None; - self.pending_internal_promise_is_protected = false; + self.set_pending_internal_promise(None); let global_ref = self.global(); let argv1 = bun_string_jsc::create_utf8_for_js(global_ref, MAIN_FILE_NAME) .map_err(|_| crate::CrateError::JSError)?; @@ -2936,8 +2933,7 @@ impl VirtualMachine { JSC__JSInternalPromise__resolvedPromise(global_ref, ret) }) .map_err(|_| crate::CrateError::JSError)?; - self.pending_internal_promise = Some(resolved); - self.pending_internal_promise_is_protected = false; + self.set_pending_internal_promise(Some(resolved)); return Ok(resolved); } } @@ -2964,9 +2960,7 @@ impl VirtualMachine { p }; - self.pending_internal_promise = Some(promise); - self.pending_internal_promise_is_protected = false; - JSValue::from_cell(promise).ensure_still_alive(); + self.set_pending_internal_promise(Some(promise)); Ok(promise) } else { self.entry_evaluation_started = false; @@ -2976,13 +2970,30 @@ impl VirtualMachine { jsc::JSModuleLoader::load_and_evaluate_module_ptr(global, Some(&main_str)) .map(NonNull::as_ptr) .ok_or(crate::CrateError::JSError)?; - self.pending_internal_promise = Some(promise); - self.pending_internal_promise_is_protected = false; - JSValue::from_cell(promise).ensure_still_alive(); + self.set_pending_internal_promise(Some(promise)); Ok(promise) } } + /// The entry-point promise that the watcher loops poll, if one is stored. + #[inline] + pub fn pending_internal_promise(&self) -> Option<*mut JSInternalPromise> { + self.pending_internal_promise + } + + /// Stores the entry-point promise and keeps it alive for as long as it is + /// stored. The module loader returns a plain GC cell that nothing refers to + /// once its reaction chain has settled, and the hot-reload loop reads the + /// slot on every tick. + pub fn set_pending_internal_promise(&mut self, promise: Option<*mut JSInternalPromise>) { + if let Some(p) = promise { + JSValue::from_cell(p).protect(); + } + if let Some(old) = core::mem::replace(&mut self.pending_internal_promise, promise) { + JSValue::from_cell(old).unprotect(); + } + } + /// `loadEntryPoint(entry_path)` — `reload_entry_point` + spin until the /// returned promise settles. pub fn load_entry_point( @@ -4013,12 +4024,6 @@ impl VirtualMachine { // the JSC module loader registry. self.global().reload().expect("Failed to reload"); self.hot_reload_counter += 1; - if self.pending_internal_promise_is_protected { - if let Some(p) = self.pending_internal_promise { - JSValue::from_cell(p).unprotect(); - } - self.pending_internal_promise_is_protected = false; - } // reload_entry_point() stores into pending_internal_promise on every return path. let main = self.main; // Note: reshaped for borrowck — copy the `RawSlice` first to avoid @@ -4962,10 +4967,7 @@ impl VirtualMachine { // SAFETY: hook contract. let p = unsafe { (hooks.load_preloads)(self) }?; if !p.is_null() { - JSValue::from_cell(p).ensure_still_alive(); - self.pending_internal_promise = Some(p); - JSValue::from_cell(p).protect(); - self.pending_internal_promise_is_protected = true; + self.set_pending_internal_promise(Some(p)); return Ok(p); } } @@ -4977,9 +4979,7 @@ impl VirtualMachine { let promise = jsc::JSModuleLoader::load_and_evaluate_module_ptr(global, Some(&main_str)) .map(NonNull::as_ptr) .ok_or(crate::CrateError::JSError)?; - self.pending_internal_promise = Some(promise); - self.pending_internal_promise_is_protected = false; - JSValue::from_cell(promise).ensure_still_alive(); + self.set_pending_internal_promise(Some(promise)); Ok(promise) } @@ -5198,13 +5198,7 @@ impl VirtualMachine { self.entry_point_result.value.deinit(); self.entry_point_result.cjs_set_value = false; self.entry_point_result.evaluated_as_cjs = false; - if let Some(promise) = self.pending_internal_promise { - if self.pending_internal_promise_is_protected { - JSValue::from_cell(promise).unprotect(); - self.pending_internal_promise_is_protected = false; - } - self.pending_internal_promise = None; - } + self.set_pending_internal_promise(None); self.has_patched_run_main = false; self.set_main(b""); self.main_hash = 0; diff --git a/src/runtime/hw_exports.rs b/src/runtime/hw_exports.rs index 8773e10d9d11..2253e90b2e2a 100644 --- a/src/runtime/hw_exports.rs +++ b/src/runtime/hw_exports.rs @@ -103,9 +103,8 @@ pub fn set_override_module_run_main_promise( vm: &mut VirtualMachine, promise: *mut JSInternalPromise, ) { - if vm.pending_internal_promise.is_none() { - vm.pending_internal_promise = Some(promise); - vm.pending_internal_promise_is_protected = false; + if vm.pending_internal_promise().is_none() { + vm.set_pending_internal_promise(Some(promise)); } } diff --git a/src/runtime/jsc_hooks.rs b/src/runtime/jsc_hooks.rs index 029b356845de..9ad172b0521d 100644 --- a/src/runtime/jsc_hooks.rs +++ b/src/runtime/jsc_hooks.rs @@ -740,7 +740,7 @@ unsafe fn load_preloads(vm: *mut VirtualMachine) -> bun_jsc::CrateResult<*mut JS // SAFETY: per fn contract — `vm` is the live per-thread VM. unsafe { (*vm).is_in_preload = true }; // Note: copy the raw ptr into a guard-owned local so the defer body - // doesn't borrow the fn param — later `(*vm).pending_internal_promise = …` + // doesn't borrow the fn param — later `(*vm).set_pending_internal_promise(…)` // would otherwise alias the guard's capture. let vm_for_guard = vm; scopeguard::defer! { @@ -852,7 +852,7 @@ unsafe fn load_preloads(vm: *mut VirtualMachine) -> bun_jsc::CrateResult<*mut JS }; // SAFETY: per fn contract. - unsafe { (*vm).pending_internal_promise = Some(promise) }; + unsafe { (*vm).set_pending_internal_promise(Some(promise)) }; let _protected = JSValue::from_cell(promise).protected(); // ── wait ──────────────────────────────────────────────────────── @@ -869,7 +869,9 @@ unsafe fn load_preloads(vm: *mut VirtualMachine) -> bun_jsc::CrateResult<*mut JS // SAFETY: `pending_internal_promise` was set just above (or // swapped by HMR to another live cell); `status()` is a // read-only FFI call on a live JSC heap cell. - let pip = unsafe { &*vm }.pending_internal_promise.unwrap_or(promise); + let pip = unsafe { &*vm } + .pending_internal_promise() + .unwrap_or(promise); // SAFETY: `pip` is a live JSC heap cell (set just above or // the protected `promise` fallback). if unsafe { &*pip }.status() != PromiseStatus::Pending { @@ -878,7 +880,9 @@ unsafe fn load_preloads(vm: *mut VirtualMachine) -> bun_jsc::CrateResult<*mut JS // SAFETY: `el` is the live per-thread event loop. unsafe { (*el).tick() }; // SAFETY: per fn contract — `vm` is the live per-thread VM. - let pip = unsafe { &*vm }.pending_internal_promise.unwrap_or(promise); + let pip = unsafe { &*vm } + .pending_internal_promise() + .unwrap_or(promise); // SAFETY: `pip` is a live JSC heap cell (see above). if unsafe { &*pip }.status() == PromiseStatus::Pending { // SAFETY: per fn contract — short-lived `&mut *vm` for the diff --git a/test/cli/hot/hot.test.ts b/test/cli/hot/hot.test.ts index 8ab6f31dd9e6..07904387376d 100644 --- a/test/cli/hot/hot.test.ts +++ b/test/cli/hot/hot.test.ts @@ -192,6 +192,77 @@ it( timeout, ); +it( + "should keep reloading after a GC runs between reloads", + async () => { + // Once the entry point's load promise settles, the only reference left to + // it is the VM slot the --hot loop polls on every tick. After a reload, + // collect that promise from an fs callback and fill the heap with pending + // promises so its cell is reused. If the slot does not root the promise, + // it now reads "pending" and every later reload is deferred forever. + const root = join(cwd, "hot-entry-promise-gc.js"); + const source = ` + import { readFile } from "fs"; + globalThis.counter ??= 0; + const n = ++globalThis.counter; + console.write(\`[#!root] Reloaded: \${n}\\n\`); + if (n > 1) { + readFile(import.meta.path, () => { + Bun.gc(true); + globalThis.pending = Array.from({ length: 20_000 }, () => new Promise(() => {})); + console.write(\`[#!root] gc done: \${n}\\n\`); + }); + } + setTimeout(() => {}, 9_999_999); + `; + writeFileSync(root, source); + + await using runner = spawn({ + cmd: [bunExe(), "--hot", "run", root], + env: bunEnv, + cwd, + stdout: "pipe", + stderr: "pipe", + stdin: "ignore", + }); + + let stdout = ""; + let stderr = ""; + const drained = Promise.all([ + (async () => { + for await (const chunk of runner.stdout) stdout += new TextDecoder().decode(chunk); + })(), + (async () => { + for await (const chunk of runner.stderr) stderr += new TextDecoder().decode(chunk); + })(), + ]); + const reloads = () => [...stdout.matchAll(/\[#!root\] Reloaded: (\d+)\n/g)].map(m => Number(m[1])); + const stepTimeout = isDebug ? 120_000 : 20_000; + async function waitForOutput(what: string, done: () => boolean) { + const deadline = Date.now() + stepTimeout; + while (!done()) { + if (Date.now() > deadline) { + throw new Error(`timed out waiting for ${what}\nstdout:\n${stdout}\nstderr:\n${stderr}`); + } + await Bun.sleep(20); + } + } + + await waitForOutput("the first load", () => reloads().length > 0); + writeFileSync(root, source); + // A save can fire more than one watcher event, so count reloads instead of + // expecting exactly one. + await waitForOutput("the GC after a reload", () => stdout.includes("[#!root] gc done: ")); + const before = Math.max(...reloads()); + writeFileSync(root, source); + await waitForOutput(`a reload after reload ${before}`, () => Math.max(...reloads()) > before); + + runner.kill(); + await drained; + }, + longTimeout, +); + it.each(["hot-file-loader.file", "hot-file-loader.css"])( "should hot reload when `%s` is overwritten", async (targetFilename: string) => { From 163da1f1ff8e5a644ad735c93f5c5b96b96baba4 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 2 Sep 2026 01:28:03 +0000 Subject: [PATCH 2/7] Store the entry-point promise in a WriteBarrier on the global object The global that loads the entry point owns the promise for that load. A WriteBarrier slot that visitChildren marks ties the promise's lifetime to the global's, instead of a gcProtect entry the VM has to balance by hand. The Rust side reads and writes the slot through two FFI calls, and the test isolation swap no longer clears it: the slot goes away with the old global. The test also fails at once when the child exits instead of waiting out the step deadline. --- src/jsc/JSPromise.rs | 11 +++--- src/jsc/VirtualMachine.rs | 57 ++++++++++++++++------------ src/jsc/bindings/ZigGlobalObject.cpp | 18 +++++++++ src/jsc/bindings/ZigGlobalObject.h | 5 +++ test/cli/hot/hot.test.ts | 4 ++ 5 files changed, 65 insertions(+), 30 deletions(-) diff --git a/src/jsc/JSPromise.rs b/src/jsc/JSPromise.rs index 3ab99c92c7b3..b5ddea407ae4 100644 --- a/src/jsc/JSPromise.rs +++ b/src/jsc/JSPromise.rs @@ -251,14 +251,15 @@ impl JSPromise { } /// Safe `status()` for the common `*mut JSPromise`-stored case - /// (`vm.pending_internal_promise` etc.). `JSPromise` is a GC-managed JSC - /// heap cell; pointers to it are kept alive by the VM's strong-ref slots, - /// not by Rust ownership. Centralizes the per-call-site - /// `unsafe { (*p).status() }` deref so callers don't open-code it. + /// (`vm.pending_internal_promise()` etc.). `JSPromise` is a GC-managed JSC + /// heap cell; pointers to it are kept alive by a GC slot that marking + /// visits (the global's `WriteBarrier`, a `Strong`), not by Rust + /// ownership. Centralizes the per-call-site `unsafe { (*p).status() }` + /// deref so callers don't open-code it. #[inline] pub(crate) fn status_ptr(p: *mut JSPromise) -> Status { // `p` is a non-null GC-managed cell tracked by the VM (caller obtained - // it from a strong-ref VM field or a fresh + // it from a rooted VM slot or a fresh // `JSInternalPromise__resolvedPromise` return value). JSPromise::opaque_ref(p).status() } diff --git a/src/jsc/VirtualMachine.rs b/src/jsc/VirtualMachine.rs index 1651304a0657..e03b9522edb9 100644 --- a/src/jsc/VirtualMachine.rs +++ b/src/jsc/VirtualMachine.rs @@ -297,9 +297,6 @@ pub struct VirtualMachine { pub rare_data: Option>, pub proxy_env_storage: crate::rare_data::ProxyEnvStorage, pub(crate) resolved_path_dups: Vec>, - /// The entry-point promise the watcher loops poll. Always GC-protected - /// while stored: write it through `set_pending_internal_promise`. - pending_internal_promise: Option<*mut JSInternalPromise>, pub pending_internal_promise_reported_at: u32, pub(crate) hot_reload_deferred: bool, pub entry_point_result: EntryPointResult, @@ -410,6 +407,16 @@ unsafe extern "C" { safe fn Bun__closeAllNodeSqliteDatabasesForTermination(global: &JSGlobalObject); safe fn Bun__WebView__closeAllForTermination(); safe fn Zig__GlobalObject__prepareForDestruction(global: &JSGlobalObject); + // safe: the global stores `promise` in a `WriteBarrier` slot that its + // `visitChildren` marks; null clears the slot. The returned cell is alive + // for as long as the slot holds it. + safe fn Bun__GlobalObject__pendingInternalPromise( + global: &JSGlobalObject, + ) -> *mut JSInternalPromise; + safe fn Bun__GlobalObject__setPendingInternalPromise( + global: &JSGlobalObject, + promise: *mut JSInternalPromise, + ); safe fn Zig__GlobalObject__forbidExecution(global: &JSGlobalObject); safe fn Zig__GlobalObject__stopActiveDOMObjectsForTestIsolation(global: &JSGlobalObject); safe fn Zig__GlobalObject__destructOnExit(global: &JSGlobalObject); @@ -2924,7 +2931,7 @@ impl VirtualMachine { .map_err(|_| crate::CrateError::JSError)?; // If the override stored a promise itself, use that; otherwise // wrap its return value. - if let Some(stored) = self.pending_internal_promise { + if let Some(stored) = self.pending_internal_promise() { return Ok(stored); } // `Promise.resolve(ret)` reads `ret.constructor` / `ret.then`, @@ -2975,23 +2982,22 @@ impl VirtualMachine { } } - /// The entry-point promise that the watcher loops poll, if one is stored. + /// The promise for the current entry-point load, if one is stored. The + /// watcher loops poll it. #[inline] pub fn pending_internal_promise(&self) -> Option<*mut JSInternalPromise> { - self.pending_internal_promise + NonNull::new(Bun__GlobalObject__pendingInternalPromise(self.global())).map(NonNull::as_ptr) } - /// Stores the entry-point promise and keeps it alive for as long as it is - /// stored. The module loader returns a plain GC cell that nothing refers to - /// once its reaction chain has settled, and the hot-reload loop reads the - /// slot on every tick. + /// Stores the entry-point promise in a GC slot on the global object, so it + /// lives as long as the global that is loading it. The module loader + /// returns a plain cell that nothing refers to once its reaction chain has + /// settled, and the hot-reload loop reads the slot on every tick. pub fn set_pending_internal_promise(&mut self, promise: Option<*mut JSInternalPromise>) { - if let Some(p) = promise { - JSValue::from_cell(p).protect(); - } - if let Some(old) = core::mem::replace(&mut self.pending_internal_promise, promise) { - JSValue::from_cell(old).unprotect(); - } + Bun__GlobalObject__setPendingInternalPromise( + self.global(), + promise.unwrap_or(core::ptr::null_mut()), + ); } /// `loadEntryPoint(entry_path)` — `reload_entry_point` + spin until the @@ -3005,7 +3011,7 @@ impl VirtualMachine { // pending_internal_promise can change if hot module reloading is enabled if self.is_watcher_enabled() { loop { - let Some(p) = self.pending_internal_promise else { + let Some(p) = self.pending_internal_promise() else { break; }; // SAFETY: `p` is a live JSC heap cell tracked by the VM. @@ -3013,7 +3019,7 @@ impl VirtualMachine { break; } self.event_loop_mut().tick(); - let Some(p) = self.pending_internal_promise else { + let Some(p) = self.pending_internal_promise() else { break; }; // SAFETY: see above. @@ -3029,7 +3035,7 @@ impl VirtualMachine { let _ = self.wait_for_promise(jsc::AnyPromise::Internal(promise)); } - Ok(self.pending_internal_promise.unwrap_or(promise)) + Ok(self.pending_internal_promise().unwrap_or(promise)) } } @@ -3875,7 +3881,7 @@ impl VirtualMachine { /// After a hot reload, surfaces the entry-point promise's rejection (if any) and re-arms the watcher. pub fn report_exception_in_hot_reloaded_module_if_needed(&mut self) { - let promise = match self.pending_internal_promise { + let promise = match self.pending_internal_promise() { Some(p) => p, None => { self.add_main_to_watcher_if_needed(); @@ -3986,7 +3992,7 @@ impl VirtualMachine { bun_core::reload_process(should_clear_terminal, false); } - if let Some(p) = self.pending_internal_promise { + if let Some(p) = self.pending_internal_promise() { // SAFETY: `p` is a live JSC heap cell tracked by the VM. match crate::JSPromise::status_ptr(p) { crate::js_promise::Status::Pending => { @@ -5012,7 +5018,7 @@ impl VirtualMachine { // pending_internal_promise can change if hot module reloading is enabled if self.is_watcher_enabled() { loop { - let Some(p) = self.pending_internal_promise else { + let Some(p) = self.pending_internal_promise() else { break; }; // SAFETY: `p` is a live JSC heap cell tracked by the VM. @@ -5020,7 +5026,7 @@ impl VirtualMachine { break; } self.event_loop_mut().tick(); - let Some(p) = self.pending_internal_promise else { + let Some(p) = self.pending_internal_promise() else { break; }; // SAFETY: see above. @@ -5039,7 +5045,7 @@ impl VirtualMachine { // Pre-arm the waker so this settled-promise tick cannot park (#36450). self.wakeup(); self.auto_tick(); - Ok(self.pending_internal_promise.unwrap()) + Ok(self.pending_internal_promise().unwrap()) } /// Tracks a listening socket so watch-mode reloads can close it. @@ -5198,7 +5204,8 @@ impl VirtualMachine { self.entry_point_result.value.deinit(); self.entry_point_result.cjs_set_value = false; self.entry_point_result.evaluated_as_cjs = false; - self.set_pending_internal_promise(None); + // The entry-point promise slot lives on the global, so the swap below + // leaves it behind with the old global. self.has_patched_run_main = false; self.set_main(b""); self.main_hash = 0; diff --git a/src/jsc/bindings/ZigGlobalObject.cpp b/src/jsc/bindings/ZigGlobalObject.cpp index 6971d225ee6c..a1fe93561310 100644 --- a/src/jsc/bindings/ZigGlobalObject.cpp +++ b/src/jsc/bindings/ZigGlobalObject.cpp @@ -4270,6 +4270,24 @@ void GlobalObject::setNodeWorkerEntryEvaluatedHook(JSObject* hook) m_nodeWorkerEntryEvaluatedHook.clear(); } +void GlobalObject::setPendingInternalPromise(JSC::JSPromise* promise) +{ + if (promise) + m_pendingInternalPromise.set(vm(), this, promise); + else + m_pendingInternalPromise.clear(); +} + +extern "C" JSC::JSPromise* Bun__GlobalObject__pendingInternalPromise(Zig::GlobalObject* globalObject) +{ + return globalObject->pendingInternalPromise(); +} + +extern "C" void Bun__GlobalObject__setPendingInternalPromise(Zig::GlobalObject* globalObject, JSC::JSPromise* promise) +{ + globalObject->setPendingInternalPromise(promise); +} + extern "C" void Bun__InspectorConnection__disconnectAllOnExit(Zig::GlobalObject*); void GlobalObject::setNodeParentPort(WebCore::MessagePort* port) diff --git a/src/jsc/bindings/ZigGlobalObject.h b/src/jsc/bindings/ZigGlobalObject.h index 7837e1107c83..34a407c4d7a4 100644 --- a/src/jsc/bindings/ZigGlobalObject.h +++ b/src/jsc/bindings/ZigGlobalObject.h @@ -524,6 +524,9 @@ class GlobalObject : public Bun::GlobalScope { /* node:worker_threads worker: { stdin?, stdout, stderr } MessagePorts from the parent Worker; */ \ /* process.stdin/stdout/stderr are built over these lazily (BunProcess.cpp constructStd*). */ \ V(private, WriteBarrier, m_nodeWorkerStdioPorts) \ + /* The module loader's promise for the current entry-point load. The --hot loop polls it on every */ \ + /* tick, and once it settles nothing else refers to it (VirtualMachine::pending_internal_promise). */ \ + V(private, WriteBarrier, m_pendingInternalPromise) \ \ /* The original, unmodified Error.prepareStackTrace. */ \ /* */ \ @@ -764,6 +767,8 @@ class GlobalObject : public Bun::GlobalScope { void nodeWorkerEntryDidSettle(); JSObject* nodeWorkerEntryEvaluatedHook() { return m_nodeWorkerEntryEvaluatedHook.get(); } void setNodeWorkerEntryEvaluatedHook(JSObject* hook); + JSC::JSPromise* pendingInternalPromise() { return m_pendingInternalPromise.get(); } + void setPendingInternalPromise(JSC::JSPromise* promise); Bun::MarkdownTagStrings& markdownTagStrings() { return m_markdownTagStrings; } #include "ZigGeneratedClasses+lazyStructureHeader.h" diff --git a/test/cli/hot/hot.test.ts b/test/cli/hot/hot.test.ts index 07904387376d..3c381d63d7a6 100644 --- a/test/cli/hot/hot.test.ts +++ b/test/cli/hot/hot.test.ts @@ -241,6 +241,10 @@ it( async function waitForOutput(what: string, done: () => boolean) { const deadline = Date.now() + stepTimeout; while (!done()) { + const exit = runner.exitCode ?? runner.signalCode; + if (exit !== null) { + throw new Error(`child exited (${exit}) while waiting for ${what}\nstdout:\n${stdout}\nstderr:\n${stderr}`); + } if (Date.now() > deadline) { throw new Error(`timed out waiting for ${what}\nstdout:\n${stdout}\nstderr:\n${stderr}`); } From fb78b9d5210781897ce7bc55f97fc51d22dc270c Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 2 Sep 2026 01:30:50 +0000 Subject: [PATCH 3/7] Trim the comments around the entry-point promise slot --- src/jsc/JSPromise.rs | 11 ++--------- src/jsc/VirtualMachine.rs | 16 +++++----------- src/jsc/bindings/ZigGlobalObject.h | 3 +-- 3 files changed, 8 insertions(+), 22 deletions(-) diff --git a/src/jsc/JSPromise.rs b/src/jsc/JSPromise.rs index b5ddea407ae4..fa077a7cbdc3 100644 --- a/src/jsc/JSPromise.rs +++ b/src/jsc/JSPromise.rs @@ -250,17 +250,10 @@ impl JSPromise { } } - /// Safe `status()` for the common `*mut JSPromise`-stored case - /// (`vm.pending_internal_promise()` etc.). `JSPromise` is a GC-managed JSC - /// heap cell; pointers to it are kept alive by a GC slot that marking - /// visits (the global's `WriteBarrier`, a `Strong`), not by Rust - /// ownership. Centralizes the per-call-site `unsafe { (*p).status() }` - /// deref so callers don't open-code it. + /// `status()` for a non-null `*mut JSPromise` that a GC slot keeps alive + /// (`vm.pending_internal_promise()` etc.). #[inline] pub(crate) fn status_ptr(p: *mut JSPromise) -> Status { - // `p` is a non-null GC-managed cell tracked by the VM (caller obtained - // it from a rooted VM slot or a fresh - // `JSInternalPromise__resolvedPromise` return value). JSPromise::opaque_ref(p).status() } diff --git a/src/jsc/VirtualMachine.rs b/src/jsc/VirtualMachine.rs index e03b9522edb9..ea43bb725c40 100644 --- a/src/jsc/VirtualMachine.rs +++ b/src/jsc/VirtualMachine.rs @@ -407,9 +407,7 @@ unsafe extern "C" { safe fn Bun__closeAllNodeSqliteDatabasesForTermination(global: &JSGlobalObject); safe fn Bun__WebView__closeAllForTermination(); safe fn Zig__GlobalObject__prepareForDestruction(global: &JSGlobalObject); - // safe: the global stores `promise` in a `WriteBarrier` slot that its - // `visitChildren` marks; null clears the slot. The returned cell is alive - // for as long as the slot holds it. + // safe: a `WriteBarrier` slot on the global; null clears it. safe fn Bun__GlobalObject__pendingInternalPromise( global: &JSGlobalObject, ) -> *mut JSInternalPromise; @@ -2982,17 +2980,14 @@ impl VirtualMachine { } } - /// The promise for the current entry-point load, if one is stored. The - /// watcher loops poll it. + /// The promise for the current entry-point load. The watcher loops poll it. #[inline] pub fn pending_internal_promise(&self) -> Option<*mut JSInternalPromise> { NonNull::new(Bun__GlobalObject__pendingInternalPromise(self.global())).map(NonNull::as_ptr) } - /// Stores the entry-point promise in a GC slot on the global object, so it - /// lives as long as the global that is loading it. The module loader - /// returns a plain cell that nothing refers to once its reaction chain has - /// settled, and the hot-reload loop reads the slot on every tick. + /// Stores the entry-point promise in a GC slot on the global, which keeps + /// it alive: once it settles nothing else refers to it. pub fn set_pending_internal_promise(&mut self, promise: Option<*mut JSInternalPromise>) { Bun__GlobalObject__setPendingInternalPromise( self.global(), @@ -5204,8 +5199,7 @@ impl VirtualMachine { self.entry_point_result.value.deinit(); self.entry_point_result.cjs_set_value = false; self.entry_point_result.evaluated_as_cjs = false; - // The entry-point promise slot lives on the global, so the swap below - // leaves it behind with the old global. + // The entry-point promise slot is on the global and goes with it. self.has_patched_run_main = false; self.set_main(b""); self.main_hash = 0; diff --git a/src/jsc/bindings/ZigGlobalObject.h b/src/jsc/bindings/ZigGlobalObject.h index 34a407c4d7a4..49cf7c1c3d39 100644 --- a/src/jsc/bindings/ZigGlobalObject.h +++ b/src/jsc/bindings/ZigGlobalObject.h @@ -524,8 +524,7 @@ class GlobalObject : public Bun::GlobalScope { /* node:worker_threads worker: { stdin?, stdout, stderr } MessagePorts from the parent Worker; */ \ /* process.stdin/stdout/stderr are built over these lazily (BunProcess.cpp constructStd*). */ \ V(private, WriteBarrier, m_nodeWorkerStdioPorts) \ - /* The module loader's promise for the current entry-point load. The --hot loop polls it on every */ \ - /* tick, and once it settles nothing else refers to it (VirtualMachine::pending_internal_promise). */ \ + /* The current entry-point load's promise (VirtualMachine::pending_internal_promise). */ \ V(private, WriteBarrier, m_pendingInternalPromise) \ \ /* The original, unmodified Error.prepareStackTrace. */ \ From 3a0de545edf65ee19250cdebc819e7b4a2eee4ff Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 17 Sep 2026 19:53:51 +0000 Subject: [PATCH 4/7] Test that bun test prints a build error when a GC runs before the report --- test/cli/test/bun-test.test.ts | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/test/cli/test/bun-test.test.ts b/test/cli/test/bun-test.test.ts index 49a4905e9b14..1011f7264ef8 100644 --- a/test/cli/test/bun-test.test.ts +++ b/test/cli/test/bun-test.test.ts @@ -2092,3 +2092,31 @@ describe.concurrent("test file discovery (scanner)", () => { expect(exitCode).toBe(0); }); }); + +// The preload runs a full collection at the start of every event loop iteration, so one +// runs after the load promise rejects and before its BuildMessage is printed. Without a +// root on that promise, an optimized ASAN build reports a read of the freed BuildMessage. +// A debug build keeps the promise in a stack slot and passes either way. +test("prints a test file's build error when a GC runs before it is reported", async () => { + using dir = tempDir("bun-test-build-error-gc", { + "gc.cjs": `(function loop() { Bun.gc(true); setImmediate(loop); })();`, + "broken.test.mjs": ` + import { test } from "bun:test"; + console.log(typeof module, typeof exports); + test("t", () => {}); + `, + }); + + await using proc = Bun.spawn({ + cmd: [bunExe(), "test", "--preload", "./gc.cjs", "./broken.test.mjs"], + env: bunEnv, + cwd: String(dir), + stdout: "ignore", + stderr: "pipe", + }); + const [stderr, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]); + + expect(stderr).toContain("error: Cannot use import statement with CommonJS-only features"); + expect(stderr).toContain(" 1 error"); + expect(exitCode).toBe(1); +}); From b0f0f08270e76807e59e4b0f6abeaf8fe92c582a Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 30 Sep 2026 09:46:23 +0000 Subject: [PATCH 5/7] Remove the entry-point promise slot from Zig::GlobalObject REVIEW.md keeps per-VM state on VirtualMachine and allows no new fields on ZigGlobalObject. The next commit roots the promise in a slot that the VM owns. --- src/jsc/JSPromise.rs | 10 +++- src/jsc/VirtualMachine.rs | 85 +++++++++++++++------------- src/jsc/bindings/ZigGlobalObject.cpp | 18 ------ src/jsc/bindings/ZigGlobalObject.h | 4 -- src/runtime/hw_exports.rs | 5 +- src/runtime/jsc_hooks.rs | 12 ++-- 6 files changed, 60 insertions(+), 74 deletions(-) diff --git a/src/jsc/JSPromise.rs b/src/jsc/JSPromise.rs index d5fcddc93de6..7028f946d150 100644 --- a/src/jsc/JSPromise.rs +++ b/src/jsc/JSPromise.rs @@ -241,10 +241,16 @@ impl JSPromise { } } - /// `status()` for a non-null `*mut JSPromise` that a GC slot keeps alive - /// (`vm.pending_internal_promise()` etc.). + /// Safe `status()` for the common `*mut JSPromise`-stored case + /// (`vm.pending_internal_promise` etc.). `JSPromise` is a GC-managed JSC + /// heap cell; pointers to it are kept alive by the VM's strong-ref slots, + /// not by Rust ownership. Centralizes the per-call-site + /// `unsafe { (*p).status() }` deref so callers don't open-code it. #[inline] pub(crate) fn status_ptr(p: *mut JSPromise) -> Status { + // `p` is a non-null GC-managed cell tracked by the VM (caller obtained + // it from a strong-ref VM field or a fresh + // `JSInternalPromise__resolvedPromise` return value). JSPromise::opaque_ref(p).status() } diff --git a/src/jsc/VirtualMachine.rs b/src/jsc/VirtualMachine.rs index 51f639929261..a0395b3e401c 100644 --- a/src/jsc/VirtualMachine.rs +++ b/src/jsc/VirtualMachine.rs @@ -292,6 +292,8 @@ pub struct VirtualMachine { pub rare_data: Option>, pub proxy_env_storage: crate::rare_data::ProxyEnvStorage, pub(crate) resolved_path_dups: Vec>, + pub pending_internal_promise: Option<*mut JSInternalPromise>, + pub pending_internal_promise_is_protected: bool, pub pending_internal_promise_reported_at: u32, pub(crate) hot_reload_deferred: bool, pub entry_point_result: EntryPointResult, @@ -456,14 +458,6 @@ unsafe extern "C" { safe fn Bun__closeAllNodeSqliteDatabasesForTermination(global: &JSGlobalObject); safe fn Bun__WebView__closeAllForTermination(); safe fn Zig__GlobalObject__prepareForDestruction(global: &JSGlobalObject); - // safe: a `WriteBarrier` slot on the global; null clears it. - safe fn Bun__GlobalObject__pendingInternalPromise( - global: &JSGlobalObject, - ) -> *mut JSInternalPromise; - safe fn Bun__GlobalObject__setPendingInternalPromise( - global: &JSGlobalObject, - promise: *mut JSInternalPromise, - ); safe fn Zig__GlobalObject__forbidExecution(global: &JSGlobalObject); safe fn Zig__GlobalObject__stopActiveDOMObjectsForTestIsolation(global: &JSGlobalObject); safe fn Zig__GlobalObject__retireForTestIsolation(global: &JSGlobalObject); @@ -3476,7 +3470,10 @@ impl VirtualMachine { // SAFETY: hook contract. let p = unsafe { (hooks.load_preloads)(self) }?; if !p.is_null() { - self.set_pending_internal_promise(Some(p)); + JSValue::from_cell(p).ensure_still_alive(); + JSValue::from_cell(p).protect(); + self.pending_internal_promise = Some(p); + self.pending_internal_promise_is_protected = true; return Ok(p); } } @@ -3484,7 +3481,8 @@ impl VirtualMachine { // Check if Module.runMain was patched. if self.has_patched_run_main { bun_core::hint::cold(); - self.set_pending_internal_promise(None); + self.pending_internal_promise = None; + self.pending_internal_promise_is_protected = false; let global_ref = self.global(); let argv1 = bun_string_jsc::create_utf8_for_js(global_ref, MAIN_FILE_NAME) .map_err(|_| crate::CrateError::JSError)?; @@ -3494,7 +3492,7 @@ impl VirtualMachine { .map_err(|_| crate::CrateError::JSError)?; // If the override stored a promise itself, use that; otherwise // wrap its return value. - if let Some(stored) = self.pending_internal_promise() { + if let Some(stored) = self.pending_internal_promise { return Ok(stored); } // `Promise.resolve(ret)` reads `ret.constructor` / `ret.then`, @@ -3503,7 +3501,8 @@ impl VirtualMachine { JSC__JSInternalPromise__resolvedPromise(global_ref, ret) }) .map_err(|_| crate::CrateError::JSError)?; - self.set_pending_internal_promise(Some(resolved)); + self.pending_internal_promise = Some(resolved); + self.pending_internal_promise_is_protected = false; return Ok(resolved); } } @@ -3530,7 +3529,9 @@ impl VirtualMachine { p }; - self.set_pending_internal_promise(Some(promise)); + self.pending_internal_promise = Some(promise); + self.pending_internal_promise_is_protected = false; + JSValue::from_cell(promise).ensure_still_alive(); Ok(promise) } else { self.entry_evaluation_started = false; @@ -3540,26 +3541,13 @@ impl VirtualMachine { jsc::JSModuleLoader::load_and_evaluate_module_ptr(global, Some(&main_str)) .map(NonNull::as_ptr) .ok_or(crate::CrateError::JSError)?; - self.set_pending_internal_promise(Some(promise)); + self.pending_internal_promise = Some(promise); + self.pending_internal_promise_is_protected = false; + JSValue::from_cell(promise).ensure_still_alive(); Ok(promise) } } - /// The promise for the current entry-point load. The watcher loops poll it. - #[inline] - pub fn pending_internal_promise(&self) -> Option<*mut JSInternalPromise> { - NonNull::new(Bun__GlobalObject__pendingInternalPromise(self.global())).map(NonNull::as_ptr) - } - - /// Stores the entry-point promise in a GC slot on the global, which keeps - /// it alive: once it settles nothing else refers to it. - pub fn set_pending_internal_promise(&mut self, promise: Option<*mut JSInternalPromise>) { - Bun__GlobalObject__setPendingInternalPromise( - self.global(), - promise.unwrap_or(core::ptr::null_mut()), - ); - } - /// `loadEntryPoint(entry_path)` — `reload_entry_point` + spin until the /// returned promise settles. pub fn load_entry_point( @@ -3571,7 +3559,7 @@ impl VirtualMachine { // pending_internal_promise can change if hot module reloading is enabled if self.is_watcher_enabled() { loop { - let Some(p) = self.pending_internal_promise() else { + let Some(p) = self.pending_internal_promise else { break; }; // SAFETY: `p` is a live JSC heap cell tracked by the VM. @@ -3579,7 +3567,7 @@ impl VirtualMachine { break; } self.event_loop_mut().tick(); - let Some(p) = self.pending_internal_promise() else { + let Some(p) = self.pending_internal_promise else { break; }; // SAFETY: see above. @@ -3595,7 +3583,7 @@ impl VirtualMachine { let _ = self.wait_for_promise(jsc::AnyPromise::Internal(promise)); } - Ok(self.pending_internal_promise().unwrap_or(promise)) + Ok(self.pending_internal_promise.unwrap_or(promise)) } } @@ -4465,7 +4453,7 @@ impl VirtualMachine { /// After a hot reload, surfaces the entry-point promise's rejection (if any) and re-arms the watcher. pub fn report_exception_in_hot_reloaded_module_if_needed(&mut self) { - let promise = match self.pending_internal_promise() { + let promise = match self.pending_internal_promise { Some(p) => p, None => { self.add_main_to_watcher_if_needed(); @@ -4576,7 +4564,7 @@ impl VirtualMachine { bun_core::reload_process(should_clear_terminal, false); } - if let Some(p) = self.pending_internal_promise() { + if let Some(p) = self.pending_internal_promise { // SAFETY: `p` is a live JSC heap cell tracked by the VM. match crate::JSPromise::status_ptr(p) { crate::js_promise::Status::Pending => { @@ -4614,6 +4602,12 @@ impl VirtualMachine { // the JSC module loader registry. self.global().reload().expect("Failed to reload"); self.hot_reload_counter += 1; + if self.pending_internal_promise_is_protected { + if let Some(p) = self.pending_internal_promise { + JSValue::from_cell(p).unprotect(); + } + self.pending_internal_promise_is_protected = false; + } // reload_entry_point() stores into pending_internal_promise on every return path. let main = self.main; // Note: reshaped for borrowck — copy the `RawSlice` first to avoid @@ -5552,7 +5546,10 @@ impl VirtualMachine { // SAFETY: hook contract. let p = unsafe { (hooks.load_preloads)(self) }?; if !p.is_null() { - self.set_pending_internal_promise(Some(p)); + JSValue::from_cell(p).ensure_still_alive(); + self.pending_internal_promise = Some(p); + JSValue::from_cell(p).protect(); + self.pending_internal_promise_is_protected = true; return Ok(p); } } @@ -5564,7 +5561,9 @@ impl VirtualMachine { let promise = jsc::JSModuleLoader::load_and_evaluate_module_ptr(global, Some(&main_str)) .map(NonNull::as_ptr) .ok_or(crate::CrateError::JSError)?; - self.set_pending_internal_promise(Some(promise)); + self.pending_internal_promise = Some(promise); + self.pending_internal_promise_is_protected = false; + JSValue::from_cell(promise).ensure_still_alive(); Ok(promise) } @@ -5597,7 +5596,7 @@ impl VirtualMachine { // pending_internal_promise can change if hot module reloading is enabled if self.is_watcher_enabled() { loop { - let Some(p) = self.pending_internal_promise() else { + let Some(p) = self.pending_internal_promise else { break; }; // SAFETY: `p` is a live JSC heap cell tracked by the VM. @@ -5605,7 +5604,7 @@ impl VirtualMachine { break; } self.event_loop_mut().tick(); - let Some(p) = self.pending_internal_promise() else { + let Some(p) = self.pending_internal_promise else { break; }; // SAFETY: see above. @@ -5624,7 +5623,7 @@ impl VirtualMachine { // Pre-arm the waker so this settled-promise tick cannot park (#36450). self.wakeup(); self.auto_tick(); - Ok(self.pending_internal_promise().unwrap()) + Ok(self.pending_internal_promise.unwrap()) } /// Tracks a listening socket so watch-mode reloads can close it. @@ -5789,7 +5788,13 @@ impl VirtualMachine { self.entry_point_result.value.deinit(); self.entry_point_result.cjs_set_value = false; self.entry_point_result.evaluated_as_cjs = false; - // The entry-point promise slot is on the global and goes with it. + if let Some(promise) = self.pending_internal_promise { + if self.pending_internal_promise_is_protected { + JSValue::from_cell(promise).unprotect(); + self.pending_internal_promise_is_protected = false; + } + self.pending_internal_promise = None; + } self.has_patched_run_main = false; self.set_main(b""); self.main_hash = 0; diff --git a/src/jsc/bindings/ZigGlobalObject.cpp b/src/jsc/bindings/ZigGlobalObject.cpp index 2ca43fb53840..73443d4f0cc5 100644 --- a/src/jsc/bindings/ZigGlobalObject.cpp +++ b/src/jsc/bindings/ZigGlobalObject.cpp @@ -4607,24 +4607,6 @@ void GlobalObject::setNodeWorkerEntryEvaluatedHook(JSObject* hook) m_nodeWorkerEntryEvaluatedHook.clear(); } -void GlobalObject::setPendingInternalPromise(JSC::JSPromise* promise) -{ - if (promise) - m_pendingInternalPromise.set(vm(), this, promise); - else - m_pendingInternalPromise.clear(); -} - -extern "C" JSC::JSPromise* Bun__GlobalObject__pendingInternalPromise(Zig::GlobalObject* globalObject) -{ - return globalObject->pendingInternalPromise(); -} - -extern "C" void Bun__GlobalObject__setPendingInternalPromise(Zig::GlobalObject* globalObject, JSC::JSPromise* promise) -{ - globalObject->setPendingInternalPromise(promise); -} - extern "C" void Bun__InspectorConnection__disconnectAllOnExit(Zig::GlobalObject*); void GlobalObject::setNodeParentPort(WebCore::MessagePort* port) diff --git a/src/jsc/bindings/ZigGlobalObject.h b/src/jsc/bindings/ZigGlobalObject.h index bccc0766635a..3d4eb50b199a 100644 --- a/src/jsc/bindings/ZigGlobalObject.h +++ b/src/jsc/bindings/ZigGlobalObject.h @@ -538,8 +538,6 @@ class GlobalObject : public Bun::GlobalScope { /* node:worker_threads worker: { stdin?, stdout, stderr } MessagePorts from the parent Worker; */ \ /* process.stdin/stdout/stderr are built over these lazily (BunProcess.cpp constructStd*). */ \ V(private, WriteBarrier, m_nodeWorkerStdioPorts) \ - /* The current entry-point load's promise (VirtualMachine::pending_internal_promise). */ \ - V(private, WriteBarrier, m_pendingInternalPromise) \ V(private, LazyPropertyOfGlobalObject, m_moduleGraphFrameStructure) \ \ /* The original, unmodified Error.prepareStackTrace. */ \ @@ -782,8 +780,6 @@ class GlobalObject : public Bun::GlobalScope { void nodeWorkerEntryDidSettle(); JSObject* nodeWorkerEntryEvaluatedHook() { return m_nodeWorkerEntryEvaluatedHook.get(); } void setNodeWorkerEntryEvaluatedHook(JSObject* hook); - JSC::JSPromise* pendingInternalPromise() { return m_pendingInternalPromise.get(); } - void setPendingInternalPromise(JSC::JSPromise* promise); Bun::MarkdownTagStrings& markdownTagStrings() { return m_markdownTagStrings; } #include "ZigGeneratedClasses+lazyStructureHeader.h" diff --git a/src/runtime/hw_exports.rs b/src/runtime/hw_exports.rs index 350996358d76..e2236c2db093 100644 --- a/src/runtime/hw_exports.rs +++ b/src/runtime/hw_exports.rs @@ -109,8 +109,9 @@ pub(crate) fn set_override_module_run_main_promise( vm: &mut VirtualMachine, promise: *mut JSInternalPromise, ) { - if vm.pending_internal_promise().is_none() { - vm.set_pending_internal_promise(Some(promise)); + if vm.pending_internal_promise.is_none() { + vm.pending_internal_promise = Some(promise); + vm.pending_internal_promise_is_protected = false; } } diff --git a/src/runtime/jsc_hooks.rs b/src/runtime/jsc_hooks.rs index 2b6b206a8a47..e8425d5a5134 100644 --- a/src/runtime/jsc_hooks.rs +++ b/src/runtime/jsc_hooks.rs @@ -708,7 +708,7 @@ unsafe fn load_preloads(vm: *mut VirtualMachine) -> bun_jsc::CrateResult<*mut JS // SAFETY: per fn contract — `vm` is the live per-thread VM. unsafe { (*vm).is_in_preload = true }; // Note: copy the raw ptr into a guard-owned local so the defer body - // doesn't borrow the fn param — later `(*vm).set_pending_internal_promise(…)` + // doesn't borrow the fn param — later `(*vm).pending_internal_promise = …` // would otherwise alias the guard's capture. let vm_for_guard = vm; scopeguard::defer! { @@ -820,7 +820,7 @@ unsafe fn load_preloads(vm: *mut VirtualMachine) -> bun_jsc::CrateResult<*mut JS }; // SAFETY: per fn contract. - unsafe { (*vm).set_pending_internal_promise(Some(promise)) }; + unsafe { (*vm).pending_internal_promise = Some(promise) }; let _protected = JSValue::from_cell(promise).protected(); // ── wait ──────────────────────────────────────────────────────── @@ -837,9 +837,7 @@ unsafe fn load_preloads(vm: *mut VirtualMachine) -> bun_jsc::CrateResult<*mut JS // SAFETY: `pending_internal_promise` was set just above (or // swapped by HMR to another live cell); `status()` is a // read-only FFI call on a live JSC heap cell. - let pip = unsafe { &*vm } - .pending_internal_promise() - .unwrap_or(promise); + let pip = unsafe { &*vm }.pending_internal_promise.unwrap_or(promise); // SAFETY: `pip` is a live JSC heap cell (set just above or // the protected `promise` fallback). if unsafe { &*pip }.status() != PromiseStatus::Pending { @@ -848,9 +846,7 @@ unsafe fn load_preloads(vm: *mut VirtualMachine) -> bun_jsc::CrateResult<*mut JS // SAFETY: `el` is the live per-thread event loop. unsafe { (*el).tick() }; // SAFETY: per fn contract — `vm` is the live per-thread VM. - let pip = unsafe { &*vm } - .pending_internal_promise() - .unwrap_or(promise); + let pip = unsafe { &*vm }.pending_internal_promise.unwrap_or(promise); // SAFETY: `pip` is a live JSC heap cell (see above). if unsafe { &*pip }.status() == PromiseStatus::Pending { // SAFETY: per fn contract — short-lived `&mut *vm` for the From 6964fa4e72b4fd6af438769bd379d9df3428d482 Mon Sep 17 00:00:00 2001 From: Koray Date: Tue, 29 Sep 2026 17:23:18 +0300 Subject: [PATCH 6/7] hot: root the entry-point promise on VirtualMachine The hot loop keeps polling the entry-point evaluation promise after module evaluation settles. A raw pointer can outlive its last GC edge and later alias an unrelated handled rejection after the cell is reused. Store the promise in a VM-owned strong slot and route every read and write through that rooted owner. Add a GC-stress hot-reload regression that verifies handled rejections stay handled. --- src/jsc/VirtualMachine.rs | 83 ++++++++++++-------------- src/runtime/hw_exports.rs | 5 +- src/runtime/jsc_hooks.rs | 21 ++++--- test/cli/hot/hot.test.ts | 121 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 173 insertions(+), 57 deletions(-) diff --git a/src/jsc/VirtualMachine.rs b/src/jsc/VirtualMachine.rs index a0395b3e401c..7bea06592f22 100644 --- a/src/jsc/VirtualMachine.rs +++ b/src/jsc/VirtualMachine.rs @@ -292,8 +292,7 @@ pub struct VirtualMachine { pub rare_data: Option>, pub proxy_env_storage: crate::rare_data::ProxyEnvStorage, pub(crate) resolved_path_dups: Vec>, - pub pending_internal_promise: Option<*mut JSInternalPromise>, - pub pending_internal_promise_is_protected: bool, + pending_internal_promise: crate::strong::Optional, pub pending_internal_promise_reported_at: u32, pub(crate) hot_reload_deferred: bool, pub entry_point_result: EntryPointResult, @@ -3404,6 +3403,23 @@ impl VirtualMachine { } } + #[inline] + pub fn pending_internal_promise(&self) -> Option<*mut JSInternalPromise> { + self.pending_internal_promise.get()?.as_promise() + } + + /// Replace the entry-point promise while keeping the published pointer + /// rooted for as long as the VM slot holds it. + pub fn set_pending_internal_promise(&mut self, promise: *mut JSInternalPromise) { + debug_assert!(!promise.is_null()); + self.pending_internal_promise + .set(self.global(), JSValue::from_cell(promise)); + } + + pub(crate) fn clear_pending_internal_promise(&mut self) { + self.pending_internal_promise.clear_without_deallocation(); + } + /// `reloadEntryPoint(entry_path)` — set `main`, generate the synthetic /// `bun:main` entry, run preloads, and kick off module evaluation. pub(crate) fn reload_entry_point( @@ -3470,10 +3486,7 @@ impl VirtualMachine { // SAFETY: hook contract. let p = unsafe { (hooks.load_preloads)(self) }?; if !p.is_null() { - JSValue::from_cell(p).ensure_still_alive(); - JSValue::from_cell(p).protect(); - self.pending_internal_promise = Some(p); - self.pending_internal_promise_is_protected = true; + self.set_pending_internal_promise(p); return Ok(p); } } @@ -3481,8 +3494,7 @@ impl VirtualMachine { // Check if Module.runMain was patched. if self.has_patched_run_main { bun_core::hint::cold(); - self.pending_internal_promise = None; - self.pending_internal_promise_is_protected = false; + self.clear_pending_internal_promise(); let global_ref = self.global(); let argv1 = bun_string_jsc::create_utf8_for_js(global_ref, MAIN_FILE_NAME) .map_err(|_| crate::CrateError::JSError)?; @@ -3492,7 +3504,7 @@ impl VirtualMachine { .map_err(|_| crate::CrateError::JSError)?; // If the override stored a promise itself, use that; otherwise // wrap its return value. - if let Some(stored) = self.pending_internal_promise { + if let Some(stored) = self.pending_internal_promise() { return Ok(stored); } // `Promise.resolve(ret)` reads `ret.constructor` / `ret.then`, @@ -3501,8 +3513,7 @@ impl VirtualMachine { JSC__JSInternalPromise__resolvedPromise(global_ref, ret) }) .map_err(|_| crate::CrateError::JSError)?; - self.pending_internal_promise = Some(resolved); - self.pending_internal_promise_is_protected = false; + self.set_pending_internal_promise(resolved); return Ok(resolved); } } @@ -3529,9 +3540,7 @@ impl VirtualMachine { p }; - self.pending_internal_promise = Some(promise); - self.pending_internal_promise_is_protected = false; - JSValue::from_cell(promise).ensure_still_alive(); + self.set_pending_internal_promise(promise); Ok(promise) } else { self.entry_evaluation_started = false; @@ -3541,9 +3550,7 @@ impl VirtualMachine { jsc::JSModuleLoader::load_and_evaluate_module_ptr(global, Some(&main_str)) .map(NonNull::as_ptr) .ok_or(crate::CrateError::JSError)?; - self.pending_internal_promise = Some(promise); - self.pending_internal_promise_is_protected = false; - JSValue::from_cell(promise).ensure_still_alive(); + self.set_pending_internal_promise(promise); Ok(promise) } } @@ -3559,7 +3566,7 @@ impl VirtualMachine { // pending_internal_promise can change if hot module reloading is enabled if self.is_watcher_enabled() { loop { - let Some(p) = self.pending_internal_promise else { + let Some(p) = self.pending_internal_promise() else { break; }; // SAFETY: `p` is a live JSC heap cell tracked by the VM. @@ -3567,7 +3574,7 @@ impl VirtualMachine { break; } self.event_loop_mut().tick(); - let Some(p) = self.pending_internal_promise else { + let Some(p) = self.pending_internal_promise() else { break; }; // SAFETY: see above. @@ -3583,7 +3590,7 @@ impl VirtualMachine { let _ = self.wait_for_promise(jsc::AnyPromise::Internal(promise)); } - Ok(self.pending_internal_promise.unwrap_or(promise)) + Ok(self.pending_internal_promise().unwrap_or(promise)) } } @@ -4453,7 +4460,7 @@ impl VirtualMachine { /// After a hot reload, surfaces the entry-point promise's rejection (if any) and re-arms the watcher. pub fn report_exception_in_hot_reloaded_module_if_needed(&mut self) { - let promise = match self.pending_internal_promise { + let promise = match self.pending_internal_promise() { Some(p) => p, None => { self.add_main_to_watcher_if_needed(); @@ -4474,7 +4481,7 @@ impl VirtualMachine { let global_ref = self.global(); // `JSPromise` is an `opaque_ffi!` ZST handle; `opaque_mut` // is the centralised non-null deref proof (live JSC heap - // cell tracked by the VM's strong-ref slot). + // cell tracked by the VM's strong pending-entry slot). let result = crate::JSPromise::opaque_mut(promise).result(global_ref.vm()); let promise_js = JSValue::from_cell(promise); self.unhandled_rejection(global_ref, result, promise_js); @@ -4564,7 +4571,7 @@ impl VirtualMachine { bun_core::reload_process(should_clear_terminal, false); } - if let Some(p) = self.pending_internal_promise { + if let Some(p) = self.pending_internal_promise() { // SAFETY: `p` is a live JSC heap cell tracked by the VM. match crate::JSPromise::status_ptr(p) { crate::js_promise::Status::Pending => { @@ -4602,12 +4609,7 @@ impl VirtualMachine { // the JSC module loader registry. self.global().reload().expect("Failed to reload"); self.hot_reload_counter += 1; - if self.pending_internal_promise_is_protected { - if let Some(p) = self.pending_internal_promise { - JSValue::from_cell(p).unprotect(); - } - self.pending_internal_promise_is_protected = false; - } + self.clear_pending_internal_promise(); // reload_entry_point() stores into pending_internal_promise on every return path. let main = self.main; // Note: reshaped for borrowck — copy the `RawSlice` first to avoid @@ -5546,10 +5548,7 @@ impl VirtualMachine { // SAFETY: hook contract. let p = unsafe { (hooks.load_preloads)(self) }?; if !p.is_null() { - JSValue::from_cell(p).ensure_still_alive(); - self.pending_internal_promise = Some(p); - JSValue::from_cell(p).protect(); - self.pending_internal_promise_is_protected = true; + self.set_pending_internal_promise(p); return Ok(p); } } @@ -5561,9 +5560,7 @@ impl VirtualMachine { let promise = jsc::JSModuleLoader::load_and_evaluate_module_ptr(global, Some(&main_str)) .map(NonNull::as_ptr) .ok_or(crate::CrateError::JSError)?; - self.pending_internal_promise = Some(promise); - self.pending_internal_promise_is_protected = false; - JSValue::from_cell(promise).ensure_still_alive(); + self.set_pending_internal_promise(promise); Ok(promise) } @@ -5596,7 +5593,7 @@ impl VirtualMachine { // pending_internal_promise can change if hot module reloading is enabled if self.is_watcher_enabled() { loop { - let Some(p) = self.pending_internal_promise else { + let Some(p) = self.pending_internal_promise() else { break; }; // SAFETY: `p` is a live JSC heap cell tracked by the VM. @@ -5604,7 +5601,7 @@ impl VirtualMachine { break; } self.event_loop_mut().tick(); - let Some(p) = self.pending_internal_promise else { + let Some(p) = self.pending_internal_promise() else { break; }; // SAFETY: see above. @@ -5623,7 +5620,7 @@ impl VirtualMachine { // Pre-arm the waker so this settled-promise tick cannot park (#36450). self.wakeup(); self.auto_tick(); - Ok(self.pending_internal_promise.unwrap()) + Ok(self.pending_internal_promise().unwrap()) } /// Tracks a listening socket so watch-mode reloads can close it. @@ -5788,13 +5785,7 @@ impl VirtualMachine { self.entry_point_result.value.deinit(); self.entry_point_result.cjs_set_value = false; self.entry_point_result.evaluated_as_cjs = false; - if let Some(promise) = self.pending_internal_promise { - if self.pending_internal_promise_is_protected { - JSValue::from_cell(promise).unprotect(); - self.pending_internal_promise_is_protected = false; - } - self.pending_internal_promise = None; - } + self.clear_pending_internal_promise(); self.has_patched_run_main = false; self.set_main(b""); self.main_hash = 0; diff --git a/src/runtime/hw_exports.rs b/src/runtime/hw_exports.rs index e2236c2db093..6aa799534550 100644 --- a/src/runtime/hw_exports.rs +++ b/src/runtime/hw_exports.rs @@ -109,9 +109,8 @@ pub(crate) fn set_override_module_run_main_promise( vm: &mut VirtualMachine, promise: *mut JSInternalPromise, ) { - if vm.pending_internal_promise.is_none() { - vm.pending_internal_promise = Some(promise); - vm.pending_internal_promise_is_protected = false; + if vm.pending_internal_promise().is_none() { + vm.set_pending_internal_promise(promise); } } diff --git a/src/runtime/jsc_hooks.rs b/src/runtime/jsc_hooks.rs index e8425d5a5134..c2c29d03f648 100644 --- a/src/runtime/jsc_hooks.rs +++ b/src/runtime/jsc_hooks.rs @@ -708,8 +708,8 @@ unsafe fn load_preloads(vm: *mut VirtualMachine) -> bun_jsc::CrateResult<*mut JS // SAFETY: per fn contract — `vm` is the live per-thread VM. unsafe { (*vm).is_in_preload = true }; // Note: copy the raw ptr into a guard-owned local so the defer body - // doesn't borrow the fn param — later `(*vm).pending_internal_promise = …` - // would otherwise alias the guard's capture. + // doesn't borrow the fn param — later mutable VM access would otherwise + // alias the guard's capture. let vm_for_guard = vm; scopeguard::defer! { // SAFETY: per fn contract. @@ -820,7 +820,7 @@ unsafe fn load_preloads(vm: *mut VirtualMachine) -> bun_jsc::CrateResult<*mut JS }; // SAFETY: per fn contract. - unsafe { (*vm).pending_internal_promise = Some(promise) }; + unsafe { (*vm).set_pending_internal_promise(promise) }; let _protected = JSValue::from_cell(promise).protected(); // ── wait ──────────────────────────────────────────────────────── @@ -837,16 +837,20 @@ unsafe fn load_preloads(vm: *mut VirtualMachine) -> bun_jsc::CrateResult<*mut JS // SAFETY: `pending_internal_promise` was set just above (or // swapped by HMR to another live cell); `status()` is a // read-only FFI call on a live JSC heap cell. - let pip = unsafe { &*vm }.pending_internal_promise.unwrap_or(promise); + let pip = unsafe { &*vm } + .pending_internal_promise() + .unwrap_or(promise); // SAFETY: `pip` is a live JSC heap cell (set just above or - // the protected `promise` fallback). + // the `_protected` promise fallback). if unsafe { &*pip }.status() != PromiseStatus::Pending { break; } // SAFETY: `el` is the live per-thread event loop. unsafe { (*el).tick() }; // SAFETY: per fn contract — `vm` is the live per-thread VM. - let pip = unsafe { &*vm }.pending_internal_promise.unwrap_or(promise); + let pip = unsafe { &*vm } + .pending_internal_promise() + .unwrap_or(promise); // SAFETY: `pip` is a live JSC heap cell (see above). if unsafe { &*pip }.status() == PromiseStatus::Pending { // SAFETY: per fn contract — short-lived `&mut *vm` for the @@ -862,7 +866,8 @@ unsafe fn load_preloads(vm: *mut VirtualMachine) -> bun_jsc::CrateResult<*mut JS } } - // SAFETY: `promise` is a live (still-protected) JSC heap cell. + // SAFETY: `promise` is rooted by `_protected` even if HMR swapped the + // VM's pending-entry slot while the loop ran. if unsafe { &*promise }.status() == PromiseStatus::Rejected { return Ok(promise); } @@ -872,7 +877,7 @@ unsafe fn load_preloads(vm: *mut VirtualMachine) -> bun_jsc::CrateResult<*mut JS if !unsafe { &*vm }.script_allowed() { return Ok(core::ptr::null_mut()); } - // `_protected` drops here → unprotect. + // `_protected` drops here. } // Under --isolate each test file gets diff --git a/test/cli/hot/hot.test.ts b/test/cli/hot/hot.test.ts index d10ec3519c18..68fa8d18341a 100644 --- a/test/cli/hot/hot.test.ts +++ b/test/cli/hot/hot.test.ts @@ -137,6 +137,127 @@ it("preload not found should exit with code 1 and not time out", async () => { expect(await new Response(runner.stderr).text()).toContain("preload not found"); }); +it( + "does not report handled rejections after hot reload collects the entry promise", + async () => { + const entry = join(cwd, "entry-promise-gc.js"); + const revisionFile = join(cwd, "entry-promise-gc-revision.js"); + // LTO can determine when the stale pointer stops being conservatively rooted. + // Repeat enough reloads to reliably force collection and cell reuse in release builds. + const totalBoots = 100; + writeFileSync(revisionFile, "export const revision = 0;\n"); + writeFileSync( + entry, + `import { revision } from "./entry-promise-gc-revision.js"; + +const state = (globalThis.__entryPromiseGC ??= { + boots: 0, + caught: new Set(), + owners: new Map(), + unhandled: [], +}); + +if (state.boots === 0) { + process.on("unhandledRejection", (reason, promise) => { + const failure = { + kind: "unhandled", + caught: state.caught.has(reason), + owned: state.owners.has(promise), + sameReason: state.owners.get(promise) === reason, + }; + state.unhandled.push(failure); + console.log(JSON.stringify(failure)); + }); +} + +const boot = ++state.boots; +setTimeout(async () => { + state.caught.clear(); + state.owners.clear(); + for (let wave = 0; wave < 3; wave++) { + Bun.gc(true); + const observed = []; + for (let i = 0; i < 5000; i++) { + const reason = new Error("caught-" + boot + "-" + wave + "-" + i); + const promise = Promise.reject(reason); + state.owners.set(promise, reason); + observed.push(promise.catch(error => state.caught.add(error))); + } + await Promise.all(observed); + await Bun.sleep(1); + } + console.log(JSON.stringify({ kind: "ready", boot, revision, unhandled: state.unhandled.length })); + if (state.unhandled.length > 0 || boot === ${totalBoots}) { + setTimeout(() => { + console.log(JSON.stringify({ kind: "result", boots: state.boots, unhandled: state.unhandled.length })); + process.exit(state.unhandled.length === 0 ? 0 : 70); + }, 0); + } +}, 0); +`, + ); + + await using runner = spawn({ + cmd: [bunExe(), "--hot", "--no-clear-screen", entry], + env: bunEnv, + cwd, + stdout: "pipe", + stderr: "pipe", + stdin: "ignore", + }); + + type Message = + | { kind: "unhandled"; caught: boolean; owned: boolean; sameReason: boolean } + | { kind: "ready"; boot: number; revision: number; unhandled: number } + | { kind: "result"; boots: number; unhandled: number }; + const messages: Message[] = []; + const driveReload = (async () => { + const decoder = new TextDecoder(); + let buffered = ""; + for await (const chunk of runner.stdout) { + buffered += decoder.decode(chunk, { stream: true }); + let newline; + while ((newline = buffered.indexOf("\n")) !== -1) { + const line = buffered.slice(0, newline); + buffered = buffered.slice(newline + 1); + if (!line) continue; + const message = JSON.parse(line) as Message; + messages.push(message); + if (message.kind === "ready" && message.boot < totalBoots) { + writeFileSync(revisionFile, `export const revision = ${message.boot};\n`); + } + } + } + buffered += decoder.decode(); + if (buffered) messages.push(JSON.parse(buffered) as Message); + })(); + + const [, stderr, exitCode] = await Promise.all([driveReload, new Response(runner.stderr).text(), runner.exited]); + const unhandled = messages.filter(message => message.kind === "unhandled"); + const ready = messages.filter(message => message.kind === "ready"); + const result = messages.find(message => message.kind === "result"); + + expect({ + exitCode, + stderr, + unhandled, + readyCount: ready.length, + readySequenceIsValid: ready.every( + (message, index) => message.boot === index + 1 && message.revision === index && message.unhandled === 0, + ), + result, + }).toEqual({ + exitCode: 0, + stderr: "", + unhandled: [], + readyCount: totalBoots, + readySequenceIsValid: true, + result: { kind: "result", boots: totalBoots, unhandled: 0 }, + }); + }, + longTimeout, +); + it( "should hot reload when file is overwritten", async () => { From 449404e7727b209780b6325d6207bb0019ad29b7 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 30 Sep 2026 10:13:57 +0000 Subject: [PATCH 7/7] Run the handled-rejection test for 20 boots and allow the debug reload notice With 100 boots the test takes 39 to 49 seconds on a release ASAN build in the CI environment, and the file's timeout there is 30 seconds. On a build without the root, every measured run gave its first false report within 12 boots. A debug build prints "DEBUG: Reloading..." to stderr on each reload, so the test now ignores that line and still rejects any other output. --- test/cli/hot/hot.test.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/test/cli/hot/hot.test.ts b/test/cli/hot/hot.test.ts index 68fa8d18341a..7bf4f9413fe3 100644 --- a/test/cli/hot/hot.test.ts +++ b/test/cli/hot/hot.test.ts @@ -144,7 +144,8 @@ it( const revisionFile = join(cwd, "entry-promise-gc-revision.js"); // LTO can determine when the stale pointer stops being conservatively rooted. // Repeat enough reloads to reliably force collection and cell reuse in release builds. - const totalBoots = 100; + // Every measured run without the root failed within 12 boots. A debug boot takes about 4 seconds. + const totalBoots = isDebug ? 6 : 20; writeFileSync(revisionFile, "export const revision = 0;\n"); writeFileSync( entry, @@ -239,7 +240,8 @@ setTimeout(async () => { expect({ exitCode, - stderr, + // A debug build prints this line on every reload. + stderr: stderr.split("\n").filter(line => line !== "" && line !== "DEBUG: Reloading..."), unhandled, readyCount: ready.length, readySequenceIsValid: ready.every( @@ -248,7 +250,7 @@ setTimeout(async () => { result, }).toEqual({ exitCode: 0, - stderr: "", + stderr: [], unhandled: [], readyCount: totalBoots, readySequenceIsValid: true,