Skip to content
Closed
83 changes: 37 additions & 46 deletions src/jsc/VirtualMachine.rs
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 @@ -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.
Comment on lines +3411 to +3412

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.

If you need a paragraph-long comment to justify why the workaround is OK, the code is wrong — fix the code

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(
Expand Down Expand Up @@ -3470,19 +3486,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(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.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)?;
Expand All @@ -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`,
Expand All @@ -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);
}
}
Expand All @@ -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;
Expand All @@ -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)
}
}
Expand All @@ -3559,15 +3566,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 +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))
}
}

Expand Down Expand Up @@ -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();
Expand All @@ -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);
Expand Down Expand Up @@ -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 => {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
}
}
Expand All @@ -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)
}

Expand Down Expand Up @@ -5596,15 +5593,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 +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.
Expand Down Expand Up @@ -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;
Expand Down
5 changes: 2 additions & 3 deletions src/runtime/hw_exports.rs
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(promise);
}
}

Expand Down
21 changes: 13 additions & 8 deletions src/runtime/jsc_hooks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Comment on lines +711 to +712

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.

If you need a paragraph-long comment to justify why the workaround is OK, the code is wrong — fix the code

let vm_for_guard = vm;
scopeguard::defer! {
// SAFETY: per fn contract.
Expand Down 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(promise) };
let _protected = JSValue::from_cell(promise).protected();

// ── wait ────────────────────────────────────────────────────────
Expand All @@ -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
Expand All @@ -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);
}
Expand All @@ -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
Expand Down
Loading
Loading