Skip to content

--hot: fix a use-after-free of the entry point's promise #44350

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
dylan-conway merged 3 commits into from
Oct 1, 2026
Merged

--hot: fix a use-after-free of the entry point's promise #44350

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
853225e
The VM holds the promise of the entry point with a Strong instead of …
dylan-conway Oct 1, 2026
c0dd158
Two tests that count strong handles subtract what the process holds a…
dylan-conway Oct 1, 2026
df2d5ed
Test that --hot does not take another promise for that of the entry p…
dylan-conway Oct 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 34 additions & 45 deletions src/jsc/VirtualMachine.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -292,8 +292,7 @@ pub struct VirtualMachine {
pub rare_data: Option<Box<RareData>>,
pub proxy_env_storage: crate::rare_data::ProxyEnvStorage,
pub(crate) resolved_path_dups: Vec<Box<[u8]>>,
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,
Expand Down Expand Up @@ -3470,19 +3469,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 = bun_string_jsc::create_utf8_for_js(global_ref, MAIN_FILE_NAME)
.map_err(|_| crate::CrateError::JSError)?;
Expand All @@ -3492,7 +3487,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`,
Expand All @@ -3501,8 +3496,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);
}
}
Expand All @@ -3529,9 +3523,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;
Expand All @@ -3541,9 +3533,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)
}
}
Expand All @@ -3559,15 +3549,15 @@ 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.
if crate::JSPromise::status_ptr(p) != crate::js_promise::Status::Pending {
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.
Expand All @@ -3583,7 +3573,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))
}
}

Expand Down Expand Up @@ -4451,9 +4441,24 @@ impl VirtualMachine {
(self.on_unhandled_rejection)(self, global_object, reason);
}

/// The promise of the latest load of the entry point. `--hot` looks at it on every tick,
/// long after it has settled and the module loader has let go of it.
pub fn pending_internal_promise(&self) -> Option<*mut JSInternalPromise> {
Some(self.pending_internal_promise.get()?.to_cell()?.cast())
}

pub fn set_pending_internal_promise(&mut self, promise: Option<*mut JSInternalPromise>) {
match promise {
Some(promise) => self
.pending_internal_promise
.set(self.global(), JSValue::from_cell(promise)),
None => self.pending_internal_promise.clear_without_deallocation(),
}
}

/// 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();
Expand Down Expand Up @@ -4564,7 +4569,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 => {
Expand Down Expand Up @@ -4602,12 +4607,6 @@ impl VirtualMachine {
// the JSC module loader registry.
self.global().reload().expect("Failed to reload");
Comment thread
dylan-conway marked this conversation as resolved.
Resolved
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
Expand Down Expand Up @@ -5474,6 +5473,7 @@ impl VirtualMachine {
drop(core::mem::take(&mut self.main_resolved_path));

self.overridden_main.deinit();
self.pending_internal_promise.deinit();

// `timer`/`entry_point` live in the high-tier `RuntimeState` box, so
// dispatch the reclaim through the hook.
Expand Down Expand Up @@ -5546,10 +5546,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);
}
}
Expand All @@ -5561,9 +5558,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)
}

Expand Down Expand Up @@ -5596,15 +5591,15 @@ 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.
if crate::JSPromise::status_ptr(p) != crate::js_promise::Status::Pending {
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.
Expand All @@ -5623,7 +5618,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.
Expand Down Expand Up @@ -5788,13 +5783,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;
Expand Down
5 changes: 2 additions & 3 deletions src/runtime/hw_exports.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(Some(promise));
}
}

Expand Down
10 changes: 7 additions & 3 deletions src/runtime/jsc_hooks.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(Some(promise)) };
let _protected = JSValue::from_cell(promise).protected();

// ── wait ────────────────────────────────────────────────────────
Expand All @@ -837,7 +837,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 {
Expand All @@ -846,7 +848,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
Expand Down
100 changes: 100 additions & 0 deletions test/cli/hot/hot.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -828,3 +828,103 @@ it(
},
timeout,
);

it("holds the promise of the entry point itself, which it looks at on every tick", async () => {
const source = (comment: string) => `
globalThis.loads = (globalThis.loads ?? 0) + 1;
const load = globalThis.loads;
setTimeout(() => {
Bun.gc(true);
const { nodes, nodeClassNames, edges } = require("bun:jsc").generateHeapSnapshotForDebugging();
const className = new Map();
for (let i = 0; i < nodes.length; i += 7) className.set(nodes[i], nodeClassNames[nodes[i + 2]]);
let held = 0;
for (let i = 0; i < edges.length; i += 4)
if (className.get(edges[i]) === "StrongRootBlock" && className.get(edges[i + 1]) === "Promise") held++;
console.log("load " + load + ": " + held + " held");
}, 0);
// ${comment}
`;
using dir = tempDir("hot-entry-promise", { "main.js": source("first") });
await using runner = spawn({
cmd: [bunExe(), "--hot", "--no-clear-screen", "main.js"],
env: bunEnv,
cwd: String(dir),
stdout: "pipe",
stderr: "inherit",
stdin: "ignore",
});
const reader = runner.stdout.getReader();
let stdout = "";
const line = async (prefix: string) => {
for (;;) {
const found = stdout
.split("\n")
.slice(0, -1)
.find(line => line.startsWith(prefix));
if (found) return found;
const { value, done } = await reader.read();
if (done) return stdout;
stdout += Buffer.from(value).toString();
}
};
expect(await line("load 1:")).toBe("load 1: 1 held");
writeFileSync(join(String(dir), "main.js"), source("second"));
expect(await line("load 2:")).toBe("load 2: 1 held");
});

// The cell of a collected promise goes to the next promise that is made.
it.each([
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
["pending", `new Promise(() => {})`],
["rejected and handled", `Promise.reject(new Error("not the entry point's"))`],
])("does not take a promise of the program's, %s, for that of the entry point", async (_, promise) => {
using dir = tempDir("hot-entry-promise-reused", { "main.js": `console.log("first load");` });
await using runner = spawn({
cmd: [bunExe(), "--hot", "--no-clear-screen", "main.js"],
env: bunEnv,
cwd: String(dir),
stdout: "pipe",
stderr: "pipe",
stdin: "ignore",
});
const reader = runner.stdout.getReader();
let stdout = "";
const line = async (expected: string) => {
while (!stdout.split("\n").slice(0, -1).includes(expected)) {
const { value, done } = await reader.read();
if (done) break;
stdout += Buffer.from(value).toString();
}
};
await line("first load");
// What is left on the stack can keep it alive, and differs with where a collection starts from.
writeFileSync(
join(String(dir), "main.js"),
`
globalThis.kept = [];
require("fs").readFile(__filename, () => {
Bun.gc(true);
setImmediate(() => {
Bun.gc(true);
require("crypto").randomBytes(8, () => {
Bun.gc(true);
for (let i = 0; i < 20000; i++) {
const promise = ${promise};
promise.catch(() => {});
kept.push(promise);
}
console.log("collected");
});
});
});
`,
);
await line("collected");
Comment on lines +900 to +922

@claude claude Bot Oct 1, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 nit (optional): maintainers get a regression test that can pass on the unfixed build, since nothing checks the entry-point promise was actually collected before the 20k promises are made. The fixture at test/cli/hot/hot.test.ts:900-919 relies on three Bun.gc(true) calls from different stack depths to free the second load's promise, and the comment at line 900 says the result depends on what the stack holds; CLAUDE.md rule 15 names that exact approach as blaming the conservative scanner without proof. If the promise survives, the third reload is clean on base too and the toEqual at line 926 passes vacuously. Fix: assert the precondition the test depends on, or keep only the deterministic heap-edge test at line 832 and state that this one is best-effort.

Why this was flagged

The it.each test at test/cli/hot/hot.test.ts:877-930 is meant to fail on the base branch by making the collected entry-point promise's cell be reused by one of 20,000 program promises. It only asserts clean stdout and stderr after the third load (line 926-929); it never checks that the second load's promise was freed by the Bun.gc(true) calls at lines 906, 908 and 910. The comment at line 900 states the collection depends on what is left on the stack, and the PR description reports runs where the promise was not collected at all. On such a run the unfixed build reloads cleanly and the test passes, so it does not guard the fix. CLAUDE.md rule 15 says running GC from a different stack depth or more times is not a valid way to work around retention and is blaming the scanner without proof. The first new test at line 832 (counting StrongRootBlock -> Promise edges) is the deterministic one; this one needs a precondition assertion or a note that it is best-effort.

Verification: nit. On the unfixed build, whenever one of the three Bun.gc(true) calls does not free the second load's entry-point promise, the third load prints clean output and the test passes anyway. Mechanism verified in /home/claude/bun/test/cli/hot/hot.test.ts: the only assertions are lines 926-929. Nothing asserts the precondition that the entry-point promise was reclaimed before the churn.

writeFileSync(join(String(dir), "main.js"), `console.log("third load"); process.exit(0);`);
await line("third load");
const stderr = (await runner.stderr.text()).split("\n").filter(line => line && !line.startsWith("DEBUG: "));
expect({ stdout: stdout.split("\n"), stderr }).toEqual({
stdout: ["first load", "collected", "third load", ""],
stderr: [],
});
});
5 changes: 4 additions & 1 deletion test/js/bun/module-graph/module-graph-isolation.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1086,6 +1086,9 @@ const dir = String(
`,
"response-bodies-of-disposed-graphs.mjs": `
import { heapStats } from "bun:jsc";
const protectedPromises = () => heapStats().protectedObjectTypeCounts.Promise ?? 0;
// What the process holds by itself, like the promise of this entry point.
const before = protectedPromises();
// A body that never ends: a chunk per pull, the next one when the host says so.
let release = () => {};
const server = Bun.serve({ port: 0, fetch: request => new URL(request.url).pathname === "/turn" ? new Response("turn") : new Response(new ReadableStream({ async pull(controller) { controller.enqueue(new Uint8Array(1024)); await new Promise(resolve => (release = resolve)); } })) });
Expand All @@ -1109,7 +1112,7 @@ const dir = String(
}
await hostTurn();
Bun.gc(true);
console.log(JSON.stringify({ text: Bun.peek.status(text), protectedPromises: heapStats().protectedObjectTypeCounts.Promise ?? 0 }));
console.log(JSON.stringify({ text: Bun.peek.status(text), protectedPromises: protectedPromises() - before }));
process.exit(0);
`,
"errors-of-a-graph-made-by-a-graph.mjs": `
Expand Down
Loading
Loading