From 47c89630fb3f4c50743ce78492f0696b4ad969cf Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 21 Jun 2026 16:38:03 +0000 Subject: [PATCH 1/4] hot: GC-protect pending_internal_promise on every store path The promise returned by JSModuleLoader::loadModule is not retained by JSC once it settles; the --hot run loop polls pending_internal_promise every tick via report_exception_in_hot_reloaded_module_if_needed and reload(), so the stored cell pointer must be a GC root. Only the preload path protected it; the common loadAndEvaluateModule / resolvedPromise / setOverrideModuleRunMainPromise paths stored the raw pointer with is_protected = false, leaving a window where a collection between ticks frees the cell and the next status() read dereferences a dead JSPromise (BUN-38CT: SIGSEGV in JSPromise::status -> WriteBarrierBase::get). Route every store through a set_pending_internal_promise helper that unprotects the outgoing value and protects the incoming one. The protect count is balanced across reloads (stays at 1). --- src/jsc/JSPromise.rs | 11 +++--- src/jsc/VirtualMachine.rs | 60 ++++++++++++++++---------------- src/runtime/hw_exports.rs | 3 +- src/runtime/jsc_hooks.rs | 11 +++--- test/cli/hot/hot.test.ts | 72 +++++++++++++++++++++++++++++++++++++++ 5 files changed, 113 insertions(+), 44 deletions(-) diff --git a/src/jsc/JSPromise.rs b/src/jsc/JSPromise.rs index 72b9b2cfcd18..7fd0346c0629 100644 --- a/src/jsc/JSPromise.rs +++ b/src/jsc/JSPromise.rs @@ -378,14 +378,13 @@ 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. + /// heap cell; the caller must hold a GC root for `p` (a stack local, + /// `JSValue::protect`, or a `Strong`) for the duration of the call. + /// Centralizes the per-call-site `unsafe { (*p).status() }` deref so + /// callers don't open-code it. #[inline] pub 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). + // `p` is a non-null GC-managed cell the caller holds a root for. JSPromise::opaque_ref(p).status() } diff --git a/src/jsc/VirtualMachine.rs b/src/jsc/VirtualMachine.rs index 2dc03fbd4246..573e66591f25 100644 --- a/src/jsc/VirtualMachine.rs +++ b/src/jsc/VirtualMachine.rs @@ -2285,6 +2285,26 @@ impl VirtualMachine { } } + /// Store (or clear) the entry-point evaluation promise. The promise is + /// polled across event-loop ticks by the `--hot` run loop + /// ([`report_exception_in_hot_reloaded_module_if_needed`] and [`reload`]), + /// and the module loader drops its own reference once the promise settles, + /// so it must be a GC root while stored here. Balances the previous + /// `protect()` before taking the new one. + pub fn set_pending_internal_promise(&mut self, promise: Option<*mut JSInternalPromise>) { + if self.pending_internal_promise_is_protected { + if let Some(prev) = self.pending_internal_promise { + JSValue::from_cell(prev).unprotect(); + } + self.pending_internal_promise_is_protected = false; + } + self.pending_internal_promise = promise; + if let Some(p) = promise { + JSValue::from_cell(p).protect(); + self.pending_internal_promise_is_protected = true; + } + } + /// `reloadEntryPoint(entry_path)` — set `main`, generate the synthetic /// `bun:main` entry, run preloads, and kick off module evaluation. pub fn reload_entry_point( @@ -2353,9 +2373,7 @@ impl VirtualMachine { 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); } } @@ -2363,8 +2381,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 = jsc::bun_string_jsc::create_utf8_for_js(global_ref, MAIN_FILE_NAME) .map_err(|_| crate::CrateError::JSError)?; @@ -2378,8 +2395,7 @@ impl VirtualMachine { return Ok(stored); } let resolved = JSC__JSInternalPromise__resolvedPromise(global_ref, ret); - self.pending_internal_promise = Some(resolved); - self.pending_internal_promise_is_protected = false; + self.set_pending_internal_promise(Some(resolved)); return Ok(resolved); } } @@ -2403,8 +2419,7 @@ impl VirtualMachine { p }; - self.pending_internal_promise = Some(promise); - self.pending_internal_promise_is_protected = false; + self.set_pending_internal_promise(Some(promise)); JSValue::from_cell(promise).ensure_still_alive(); Ok(promise) } else { @@ -2414,8 +2429,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; + self.set_pending_internal_promise(Some(promise)); JSValue::from_cell(promise).ensure_still_alive(); Ok(promise) } @@ -3556,13 +3570,8 @@ 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. + // reload_entry_point() stores into pending_internal_promise on every + // return path; that store unprotects the outgoing promise. let main = self.main; // Note: reshaped for borrowck — copy the `RawSlice` first to avoid // overlapping `&self`/`&mut self` borrows. @@ -4573,9 +4582,7 @@ impl VirtualMachine { 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); } } @@ -4587,8 +4594,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; + self.set_pending_internal_promise(Some(promise)); JSValue::from_cell(promise).ensure_still_alive(); Ok(promise) } @@ -4759,13 +4765,7 @@ impl VirtualMachine { self.overridden_main.deinit(); self.entry_point_result.value.deinit(); self.entry_point_result.cjs_set_value = 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 dd343b1a658c..8d4c4d16a3ab 100644 --- a/src/runtime/hw_exports.rs +++ b/src/runtime/hw_exports.rs @@ -102,8 +102,7 @@ pub fn set_override_module_run_main_promise( promise: *mut JSInternalPromise, ) { if vm.pending_internal_promise.is_none() { - vm.pending_internal_promise = Some(promise); - vm.pending_internal_promise_is_protected = false; + vm.set_pending_internal_promise(Some(promise)); } } diff --git a/src/runtime/jsc_hooks.rs b/src/runtime/jsc_hooks.rs index deafdd440bff..38155c99f454 100644 --- a/src/runtime/jsc_hooks.rs +++ b/src/runtime/jsc_hooks.rs @@ -661,8 +661,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 `(*vm)` accesses would otherwise + // alias the guard's capture. let vm_for_guard = vm; scopeguard::defer! { // SAFETY: per fn contract. @@ -774,9 +774,9 @@ unsafe fn load_preloads(vm: *mut VirtualMachine) -> bun_jsc::CrateResult<*mut JS } }; - // SAFETY: per fn contract. - unsafe { (*vm).pending_internal_promise = Some(promise) }; - let _protected = JSValue::from_cell(promise).protected(); + // SAFETY: per fn contract — short-lived `&mut *vm` for the field store; + // `promise` is a live JSC heap cell from `import_ptr` just above. + unsafe { (*vm).set_pending_internal_promise(Some(promise)) }; // ── wait ──────────────────────────────────────────────────────── // HMR `pending_internal_promise` swap loop; non-watcher path uses @@ -825,7 +825,6 @@ unsafe fn load_preloads(vm: *mut VirtualMachine) -> bun_jsc::CrateResult<*mut JS if unsafe { &*promise }.status() == PromiseStatus::Rejected { return Ok(promise); } - // `_protected` drops here → unprotect. } // Under --isolate each test file gets diff --git a/test/cli/hot/hot.test.ts b/test/cli/hot/hot.test.ts index df1af3e01bd2..6ef0ca821f21 100644 --- a/test/cli/hot/hot.test.ts +++ b/test/cli/hot/hot.test.ts @@ -766,3 +766,75 @@ ${Buffer.alloc(counter * 2, " ").toString()}throw new Error(${counter});`, }, longTimeout, ); + +// JSModuleLoader::loadModule's returned promise is not retained by JSC once +// it settles. pending_internal_promise holds it as a raw cell pointer and is +// polled every tick by the --hot run loop (via +// report_exception_in_hot_reloaded_module_if_needed and reload()). Without a +// GC root, a collection between ticks can free the cell and the next poll +// reads a dead JSPromise (Sentry BUN-38CT: SIGSEGV in JSPromise::status -> +// WriteBarrierBase::get). Conservative stack scanning usually keeps it alive +// on the caller's stack, which is why the crash is rare and the race is not +// deterministically reproducible in-process; this test instead asserts the +// invariant the fix establishes: the stored promise is always GC-protected. +it( + "roots the entry-point evaluation promise so per-tick status reads can't see a freed cell", + async () => { + const root = join(cwd, "pip-root.js"); + // reload_entry_point stores pending_internal_promise before module + // evaluation runs, so heapStats() inside the entry module observes whether + // that store took a protect(). A later setImmediate snapshot confirms the + // root persists once the promise has settled and nothing in JSC references + // it anymore. + writeFileSync( + root, + ` + const { heapStats } = require("bun:jsc"); + function protectedPromises() { + const counts = heapStats().protectedObjectTypeCounts; + let n = 0; + for (const [k, v] of Object.entries(counts)) { + if (k.includes("Promise")) n += v; + } + return { n, counts }; + } + const during = protectedPromises(); + setImmediate(() => { + const after = protectedPromises(); + console.log(JSON.stringify({ during, after })); + process.exit(0); + }); + `, + ); + + await using proc = spawn({ + cmd: [bunExe(), "--hot", "--no-clear-screen", "run", root], + env: bunEnv, + cwd, + stdout: "pipe", + stderr: "pipe", + stdin: "ignore", + }); + + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + + const line = stdout.split("\n").find(l => l.startsWith("{")); + if (!line) { + throw new Error(`no JSON line in stdout.\nstdout: ${JSON.stringify(stdout)}\nstderr: ${stderr}`); + } + const { during, after } = JSON.parse(line); + // At minimum the entry-point evaluation promise must be protected while + // stored in pending_internal_promise, both during evaluation and on the + // next tick (the window the run loop reads it in). + expect({ during: during.n, after: after.n, duringCounts: during.counts, afterCounts: after.counts }).toEqual({ + during: expect.any(Number), + after: expect.any(Number), + duringCounts: expect.any(Object), + afterCounts: expect.any(Object), + }); + expect(during.n).toBeGreaterThanOrEqual(1); + expect(after.n).toBeGreaterThanOrEqual(1); + expect(exitCode).toBe(0); + }, + timeout, +); From 087b5dc93b988b1ec1b66c97c6ca2eb31abbd451 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 21 Jun 2026 17:08:19 +0000 Subject: [PATCH 2/4] review: keep scoped protect in load_preloads; drop redundant ensure_still_alive; make test assertion non-vacuous --- src/jsc/VirtualMachine.rs | 5 ----- src/runtime/jsc_hooks.rs | 6 ++++++ test/cli/hot/hot.test.ts | 17 +++++++++++------ 3 files changed, 17 insertions(+), 11 deletions(-) diff --git a/src/jsc/VirtualMachine.rs b/src/jsc/VirtualMachine.rs index 573e66591f25..afe86f8a47c6 100644 --- a/src/jsc/VirtualMachine.rs +++ b/src/jsc/VirtualMachine.rs @@ -2372,7 +2372,6 @@ impl VirtualMachine { // SAFETY: hook contract. let p = unsafe { (hooks.load_preloads)(self) }?; if !p.is_null() { - JSValue::from_cell(p).ensure_still_alive(); self.set_pending_internal_promise(Some(p)); return Ok(p); } @@ -2420,7 +2419,6 @@ impl VirtualMachine { }; self.set_pending_internal_promise(Some(promise)); - JSValue::from_cell(promise).ensure_still_alive(); Ok(promise) } else { let global = self.global; @@ -2430,7 +2428,6 @@ impl VirtualMachine { .map(NonNull::as_ptr) .ok_or(crate::CrateError::JSError)?; self.set_pending_internal_promise(Some(promise)); - JSValue::from_cell(promise).ensure_still_alive(); Ok(promise) } } @@ -4581,7 +4578,6 @@ impl VirtualMachine { // SAFETY: hook contract. let p = unsafe { (hooks.load_preloads)(self) }?; if !p.is_null() { - JSValue::from_cell(p).ensure_still_alive(); self.set_pending_internal_promise(Some(p)); return Ok(p); } @@ -4595,7 +4591,6 @@ impl VirtualMachine { .map(NonNull::as_ptr) .ok_or(crate::CrateError::JSError)?; self.set_pending_internal_promise(Some(promise)); - JSValue::from_cell(promise).ensure_still_alive(); Ok(promise) } diff --git a/src/runtime/jsc_hooks.rs b/src/runtime/jsc_hooks.rs index 38155c99f454..cf8d033672ae 100644 --- a/src/runtime/jsc_hooks.rs +++ b/src/runtime/jsc_hooks.rs @@ -777,6 +777,12 @@ unsafe fn load_preloads(vm: *mut VirtualMachine) -> bun_jsc::CrateResult<*mut JS // SAFETY: per fn contract — short-lived `&mut *vm` for the field store; // `promise` is a live JSC heap cell from `import_ptr` just above. unsafe { (*vm).set_pending_internal_promise(Some(promise)) }; + // The wait loop below tolerates `pending_internal_promise` being + // swapped by HMR inside `tick()`/`auto_tick()`; that swap unprotects + // this local. Keep an independent scoped protect so the + // `.unwrap_or(promise)` fallback and the post-loop `status()` read + // stay rooted across a swap (JSC protect is refcounted). + let _protected = JSValue::from_cell(promise).protected(); // ── wait ──────────────────────────────────────────────────────── // HMR `pending_internal_promise` swap loop; non-watcher path uses diff --git a/test/cli/hot/hot.test.ts b/test/cli/hot/hot.test.ts index 6ef0ca821f21..824c7b8d5ff8 100644 --- a/test/cli/hot/hot.test.ts +++ b/test/cli/hot/hot.test.ts @@ -825,15 +825,20 @@ it( const { during, after } = JSON.parse(line); // At minimum the entry-point evaluation promise must be protected while // stored in pending_internal_promise, both during evaluation and on the - // next tick (the window the run loop reads it in). - expect({ during: during.n, after: after.n, duringCounts: during.counts, afterCounts: after.counts }).toEqual({ - during: expect.any(Number), - after: expect.any(Number), + // next tick (the window the run loop reads it in). Assert on a combined + // object so the full protectedObjectTypeCounts map appears in the failure + // diff when either >=1 check fails. + expect({ + duringProtectedPromises: during.n >= 1, + afterProtectedPromises: after.n >= 1, + duringCounts: during.counts, + afterCounts: after.counts, + }).toEqual({ + duringProtectedPromises: true, + afterProtectedPromises: true, duringCounts: expect.any(Object), afterCounts: expect.any(Object), }); - expect(during.n).toBeGreaterThanOrEqual(1); - expect(after.n).toBeGreaterThanOrEqual(1); expect(exitCode).toBe(0); }, timeout, From 8f3c88465fa8beec85f3b23df1f7ac70b30b39ca Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 21 Jun 2026 17:37:02 +0000 Subject: [PATCH 3/4] review: drop the now-redundant pending_internal_promise_is_protected field Every store goes through set_pending_internal_promise, so the invariant is_protected == pending_internal_promise.is_some() always holds and the bool mirrors recoverable information. The previous readers in reload() and swap_global_for_test_isolation() were removed in 9ea827964d, and VirtualMachine is repr(Rust) so there is no layout contract. --- src/jsc/VirtualMachine.rs | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/jsc/VirtualMachine.rs b/src/jsc/VirtualMachine.rs index afe86f8a47c6..285ac59d0c23 100644 --- a/src/jsc/VirtualMachine.rs +++ b/src/jsc/VirtualMachine.rs @@ -288,8 +288,10 @@ pub struct VirtualMachine { pub proxy_env_storage: crate::rare_data::ProxyEnvStorage, pub resolved_path_dups: Vec>, pub is_us_loop_entered: bool, + /// Entry-point evaluation promise. Always GC-protected while `Some`; + /// every write goes through [`set_pending_internal_promise`] which + /// balances the previous `protect()`. pub pending_internal_promise: Option<*mut JSInternalPromise>, - pub pending_internal_promise_is_protected: bool, pub pending_internal_promise_reported_at: u32, pub hot_reload_deferred: bool, pub entry_point_result: EntryPointResult, @@ -2292,16 +2294,12 @@ impl VirtualMachine { /// so it must be a GC root while stored here. Balances the previous /// `protect()` before taking the new one. pub fn set_pending_internal_promise(&mut self, promise: Option<*mut JSInternalPromise>) { - if self.pending_internal_promise_is_protected { - if let Some(prev) = self.pending_internal_promise { - JSValue::from_cell(prev).unprotect(); - } - self.pending_internal_promise_is_protected = false; + if let Some(prev) = self.pending_internal_promise { + JSValue::from_cell(prev).unprotect(); } self.pending_internal_promise = promise; if let Some(p) = promise { JSValue::from_cell(p).protect(); - self.pending_internal_promise_is_protected = true; } } From 211a2cd5e7cdddbbb35289a0b0686dce5add0158 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 21 Jun 2026 18:11:09 +0000 Subject: [PATCH 4/4] ci: retrigger