Skip to content

Commit

Permalink
refactor(core): Move pending_promise_rejections to ContextState (d…
Browse files Browse the repository at this point in the history
…enoland#17447)

This is a requirement before supporting modules in realms.
  • Loading branch information
andreubotella committed Jan 16, 2023
1 parent 6da958d commit f0c79a6
Show file tree
Hide file tree
Showing 3 changed files with 65 additions and 39 deletions.
16 changes: 9 additions & 7 deletions core/bindings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -455,10 +455,12 @@ pub extern "C" fn promise_reject_callback(message: v8::PromiseRejectMessage) {
let scope = &mut unsafe { v8::CallbackScope::new(&message) };

let context_state_rc = JsRealm::state_from_scope(scope);
let mut context_state = context_state_rc.borrow_mut();

if let Some(js_promise_reject_cb) = context_state.js_promise_reject_cb.clone()
{
drop(context_state);

let promise_reject_cb =
context_state_rc.borrow().js_promise_reject_cb.clone();
if let Some(js_promise_reject_cb) = promise_reject_cb {
let tc_scope = &mut v8::TryCatch::new(scope);
let undefined: v8::Local<v8::Value> = v8::undefined(tc_scope).into();
let type_ = v8::Integer::new(tc_scope, message.get_event() as i32);
Expand Down Expand Up @@ -496,20 +498,20 @@ pub extern "C" fn promise_reject_callback(message: v8::PromiseRejectMessage) {
}
}
} else {
let state_rc = JsRuntime::state(scope);
let mut state = state_rc.borrow_mut();
let promise = message.get_promise();
let promise_global = v8::Global::new(scope, promise);
match message.get_event() {
PromiseRejectWithNoHandler => {
let error = message.get_value().unwrap();
let error_global = v8::Global::new(scope, error);
state
context_state
.pending_promise_rejections
.insert(promise_global, error_global);
}
PromiseHandlerAddedAfterReject => {
state.pending_promise_rejections.remove(&promise_global);
context_state
.pending_promise_rejections
.remove(&promise_global);
}
PromiseRejectAfterResolved => {}
PromiseResolveAfterResolved => {
Expand Down
20 changes: 11 additions & 9 deletions core/ops_builtin_v8.rs
Original file line number Diff line number Diff line change
Expand Up @@ -864,13 +864,13 @@ fn op_store_pending_promise_rejection<'a>(
promise: serde_v8::Value<'a>,
reason: serde_v8::Value<'a>,
) {
let state_rc = JsRuntime::state(scope);
let mut state = state_rc.borrow_mut();
let context_state_rc = JsRealm::state_from_scope(scope);
let mut context_state = context_state_rc.borrow_mut();
let promise_value =
v8::Local::<v8::Promise>::try_from(promise.v8_value).unwrap();
let promise_global = v8::Global::new(scope, promise_value);
let error_global = v8::Global::new(scope, reason.v8_value);
state
context_state
.pending_promise_rejections
.insert(promise_global, error_global);
}
Expand All @@ -880,25 +880,27 @@ fn op_remove_pending_promise_rejection<'a>(
scope: &mut v8::HandleScope<'a>,
promise: serde_v8::Value<'a>,
) {
let state_rc = JsRuntime::state(scope);
let mut state = state_rc.borrow_mut();
let context_state_rc = JsRealm::state_from_scope(scope);
let mut context_state = context_state_rc.borrow_mut();
let promise_value =
v8::Local::<v8::Promise>::try_from(promise.v8_value).unwrap();
let promise_global = v8::Global::new(scope, promise_value);
state.pending_promise_rejections.remove(&promise_global);
context_state
.pending_promise_rejections
.remove(&promise_global);
}

#[op(v8)]
fn op_has_pending_promise_rejection<'a>(
scope: &mut v8::HandleScope<'a>,
promise: serde_v8::Value<'a>,
) -> bool {
let state_rc = JsRuntime::state(scope);
let state = state_rc.borrow();
let context_state_rc = JsRealm::state_from_scope(scope);
let context_state = context_state_rc.borrow();
let promise_value =
v8::Local::<v8::Promise>::try_from(promise.v8_value).unwrap();
let promise_global = v8::Global::new(scope, promise_value);
state
context_state
.pending_promise_rejections
.contains_key(&promise_global)
}
Expand Down
68 changes: 45 additions & 23 deletions core/runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,8 @@ pub(crate) struct ContextState {
pub(crate) js_promise_reject_cb: Option<v8::Global<v8::Function>>,
pub(crate) js_format_exception_cb: Option<v8::Global<v8::Function>>,
pub(crate) js_wasm_streaming_cb: Option<v8::Global<v8::Function>>,
pub(crate) pending_promise_rejections:
HashMap<v8::Global<v8::Promise>, v8::Global<v8::Value>>,
pub(crate) unrefed_ops: HashSet<i32>,
// We don't explicitly re-read this prop but need the slice to live alongside
// the context
Expand All @@ -169,8 +171,6 @@ pub struct JsRuntimeState {
pub(crate) js_macrotask_cbs: Vec<v8::Global<v8::Function>>,
pub(crate) js_nexttick_cbs: Vec<v8::Global<v8::Function>>,
pub(crate) has_tick_scheduled: bool,
pub(crate) pending_promise_rejections:
HashMap<v8::Global<v8::Promise>, v8::Global<v8::Value>>,
pub(crate) pending_dyn_mod_evaluate: Vec<DynImportModEvaluate>,
pub(crate) pending_mod_evaluate: Option<ModEvaluate>,
/// A counter used to delay our dynamic import deadlock detection by one spin
Expand Down Expand Up @@ -384,7 +384,6 @@ impl JsRuntime {
unsafe { std::alloc::alloc(layout) as *mut _ };

let state_rc = Rc::new(RefCell::new(JsRuntimeState {
pending_promise_rejections: HashMap::new(),
pending_dyn_mod_evaluate: vec![],
pending_mod_evaluate: None,
dyn_module_evaluate_idle_counter: 0,
Expand Down Expand Up @@ -1605,6 +1604,7 @@ impl JsRuntime {
&mut self,
id: ModuleId,
) -> oneshot::Receiver<Result<(), Error>> {
let global_realm = self.global_realm();
let state_rc = self.state.clone();
let module_map_rc = Self::module_map(self.v8_isolate());
let scope = &mut self.handle_scope();
Expand Down Expand Up @@ -1688,7 +1688,11 @@ impl JsRuntime {
.handled_promise_rejections
.contains(&promise_global);
if !pending_rejection_was_already_handled {
state.pending_promise_rejections.remove(&promise_global);
global_realm
.state(tc_scope)
.borrow_mut()
.pending_promise_rejections
.remove(&promise_global);
}
}
let promise_global = v8::Global::new(tc_scope, promise);
Expand Down Expand Up @@ -2127,26 +2131,14 @@ impl JsRuntime {
}

fn check_promise_rejections(&mut self) -> Result<(), Error> {
let mut state = self.state.borrow_mut();

if state.pending_promise_rejections.is_empty() {
return Ok(());
let known_realms = self.state.borrow().known_realms.clone();
let isolate = self.v8_isolate();
for weak_context in known_realms {
if let Some(context) = weak_context.to_global(isolate) {
JsRealm(context).check_promise_rejections(isolate)?;
}
}

let key = {
state
.pending_promise_rejections
.keys()
.next()
.unwrap()
.clone()
};
let handle = state.pending_promise_rejections.remove(&key).unwrap();
drop(state);

let scope = &mut self.handle_scope();
let exception = v8::Local::new(scope, handle);
exception_to_err_result(scope, exception, true)
Ok(())
}

// Send finished responses to JS
Expand Down Expand Up @@ -2497,6 +2489,36 @@ impl JsRealm {
}

// TODO(andreubotella): `mod_evaluate`, `load_main_module`, `load_side_module`

fn check_promise_rejections(
&self,
isolate: &mut v8::Isolate,
) -> Result<(), Error> {
let context_state_rc = self.state(isolate);
let mut context_state = context_state_rc.borrow_mut();

if context_state.pending_promise_rejections.is_empty() {
return Ok(());
}

let key = {
context_state
.pending_promise_rejections
.keys()
.next()
.unwrap()
.clone()
};
let handle = context_state
.pending_promise_rejections
.remove(&key)
.unwrap();
drop(context_state);

let scope = &mut self.handle_scope(isolate);
let exception = v8::Local::new(scope, handle);
exception_to_err_result(scope, exception, true)
}
}

#[inline]
Expand Down

0 comments on commit f0c79a6

Please sign in to comment.