Skip to content

Commit

Permalink
feat(core): Reland support for async ops in realms (denoland#17204)
Browse files Browse the repository at this point in the history
Currently realms are supported on `deno_core`, but there was no support
for async ops anywhere other than the main realm. The main issue is that
the `js_recv_cb` callback, which resolves promises corresponding to
async ops, was only set for the main realm, so async ops in other realms
would never resolve. Furthermore, promise ID's are specific to each
realm, which meant that async ops from other realms would result in a
wrong promise from the main realm being resolved.

This change takes the `ContextState` struct added in denoland#17050, and adds to
it a `js_recv_cb` callback for each realm. Combined with the fact that
that same PR also added a list of known realms to `JsRuntimeState`, and
that denoland#17174 made `OpCtx` instances realm-specific and had them include
an index into that list of known realms, this makes it possible to know
the current realm in the `queue_async_op` and `queue_fast_async_op`
methods, and therefore to send the results of promises for each realm to
that realm, and prevent the ID's from getting mixed up.

Additionally, since promise ID's are no longer unique to the isolate,
having a single set of unrefed ops doesn't work. This change therefore
also moves `unrefed_ops` from `JsRuntimeState` to `ContextState`, and
adds the lengths of the unrefed op sets for all known realms to get the
total number of unrefed ops to compare in the event loop.

This PR is a reland of denoland#14734 after it was reverted in denoland#16366, except
that `ContextState` and `JsRuntimeState::known_realms` were previously
relanded in denoland#17050. Another significant difference with the original PR
is passing around an index into `JsRuntimeState::known_realms` instead
of a `v8::Global<v8::Context>` to identify the realm, because async op
queuing in fast calls cannot call into V8, and therefore cannot have
access to V8 globals. This also simplified the implementation of
`resolve_async_ops`.

Co-authored-by: Luis Malheiro <[email protected]>
  • Loading branch information
andreubotella and lmalheiro committed Jan 14, 2023
1 parent b9cd19a commit 6878234
Show file tree
Hide file tree
Showing 9 changed files with 338 additions and 67 deletions.
3 changes: 2 additions & 1 deletion core/ops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ where
}
}

pub type RealmIdx = usize;
pub type PromiseId = i32;
pub type OpAsyncFuture = OpCall<(PromiseId, OpId, OpResult)>;
pub type OpFn =
Expand Down Expand Up @@ -156,7 +157,7 @@ pub struct OpCtx {
pub decl: Rc<OpDecl>,
pub runtime_state: Weak<RefCell<JsRuntimeState>>,
// Index of the current realm into `JsRuntimeState::known_realms`.
pub realm_idx: usize,
pub realm_idx: RealmIdx,
}

/// Maintains the resources and ops inside a JS runtime.
Expand Down
8 changes: 4 additions & 4 deletions core/ops_builtin_v8.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,14 +76,14 @@ fn to_v8_local_fn(

#[op(v8)]
fn op_ref_op(scope: &mut v8::HandleScope, promise_id: i32) {
let state_rc = JsRuntime::state(scope);
state_rc.borrow_mut().unrefed_ops.remove(&promise_id);
let context_state = JsRealm::state_from_scope(scope);
context_state.borrow_mut().unrefed_ops.remove(&promise_id);
}

#[op(v8)]
fn op_unref_op(scope: &mut v8::HandleScope, promise_id: i32) {
let state_rc = JsRuntime::state(scope);
state_rc.borrow_mut().unrefed_ops.insert(promise_id);
let context_state = JsRealm::state_from_scope(scope);
context_state.borrow_mut().unrefed_ops.insert(promise_id);
}

#[op(v8)]
Expand Down
Loading

0 comments on commit 6878234

Please sign in to comment.