hot: GC-protect pending_internal_promise on every store path

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()
     }
 

src/jsc/VirtualMachine.rs

@@ -288,8 +288,10 @@ pub struct VirtualMachine {
     pub proxy_env_storage: crate::rare_data::ProxyEnvStorage,
     pub resolved_path_dups: Vec<Box<[u8]>>,
     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,
@@ -2285,6 +2287,22 @@ 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 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();
+        }
+    }
+
     /// `reloadEntryPoint(entry_path)` — set `main`, generate the synthetic
     /// `bun:main` entry, run preloads, and kick off module evaluation.
     pub fn reload_entry_point(
@@ -2352,19 +2370,15 @@ 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);
                     }
                 }
 
                 // 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 +2392,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,9 +2416,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 {
             let global = self.global;
@@ -2414,9 +2425,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(Some(promise));
             Ok(promise)
         }
     }
@@ -3556,13 +3565,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.
@@ -4572,10 +4576,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);
                 }
             }
@@ -4587,9 +4588,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)
     }
 
@@ -4759,13 +4758,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;

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));
     }
 }
 

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,8 +774,14 @@ unsafe fn load_preloads(vm: *mut VirtualMachine) -> bun_jsc::CrateResult<*mut JS
             }
         };
 
-        // SAFETY: per fn contract.
-        unsafe { (*vm).pending_internal_promise = Some(promise) };
+        // 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 ────────────────────────────────────────────────────────
@@ -825,7 +831,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

test/cli/hot/hot.test.ts

@@ -766,3 +766,80 @@ ${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). 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(exitCode).toBe(0);
+  },
+  timeout,
+);