feat(core): Add initial support for realms #14019
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This handles bindings, extension JS and sync ops. It also adds the `JsRealm` API that adds methods like `JsRuntime`'s `handle_scope`, `global_object` and `execute_script` specific to the realm. Towards denoland#13239.
13 tasks
andreubotella
changed the title
deno_coredeno_core
lucacasonato
approved these changes
Mar 21, 2022
There was a problem hiding this comment.
Looks good to me. Thanks for the work @andreubotella.
I do wonder if JsRealm's lifetime could not be bound to JsRuntime, so it could store a reference to the JsRuntime in the JsRealm struct. That would alleviate the need to pass the runtime to all the methods on the JsRealm.
I think it might be a bad idea to store a mutable reference to |
bartlomieju
reviewed
Mar 21, 2022
bartlomieju
changed the title
deno_core
|
Yikes, sorry it takes so long @andreubotella, could you rebase the PR? |
Done. |
andreubotella
added a commit
to andreubotella/deno
that referenced
this pull request
May 26, 2022
Pull request denoland#14019 enabled initial support for realms, but it did not include support for async ops anywhere other than the main realm. The main issue was 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 creates a `ContextState` struct, similar to `JsRuntimeState` but stored in a slot of each `v8::Context`, which contains a `js_recv_cb` callback for each realm. Combined with a new list of known realms, which stores them as `v8::Weak<v8::Context>`, and a change in the `#[op]` macro to pass the current context to `queue_async_op`, this makes it possible to send the results of promises for different realms to their 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. Towards denoland#13239.
andreubotella
added a commit
to andreubotella/deno
that referenced
this pull request
May 28, 2022
This behavior was introduced in denoland#14019 but wasn't properly tested in that PR.
andreubotella
added a commit
to andreubotella/deno
that referenced
this pull request
Jul 10, 2022
Pull request denoland#14019 enabled initial support for realms, but it did not include support for async ops anywhere other than the main realm. The main issue was 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 creates a `ContextState` struct, similar to `JsRuntimeState` but stored in a slot of each `v8::Context`, which contains a `js_recv_cb` callback for each realm. Combined with a new list of known realms, which stores them as `v8::Weak<v8::Context>`, and a change in the `#[op]` macro to pass the current context to `queue_async_op`, this makes it possible to send the results of promises for different realms to their 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. Towards denoland#13239. Co-authored-by: Luis Malheiro <[email protected]>
andreubotella
added a commit
to andreubotella/deno
that referenced
this pull request
Jul 11, 2022
Pull request denoland#14019 enabled initial support for realms, but it did not include support for async ops anywhere other than the main realm. The main issue was 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 creates a `ContextState` struct, similar to `JsRuntimeState` but stored in a slot of each `v8::Context`, which contains a `js_recv_cb` callback for each realm. Combined with a new list of known realms, which stores them as `v8::Weak<v8::Context>`, and a change in the `#[op]` macro to pass the current context to `queue_async_op`, this makes it possible to send the results of promises for different realms to their 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. Towards denoland#13239. Co-authored-by: Luis Malheiro <[email protected]>
bartlomieju
pushed a commit
that referenced
this pull request
Aug 10, 2022
Pull request #14019 enabled initial support for realms, but it did not include support for async ops anywhere other than the main realm. The main issue was 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 creates a `ContextState` struct, similar to `JsRuntimeState` but stored in a slot of each `v8::Context`, which contains a `js_recv_cb` callback for each realm. Combined with a new list of known realms, which stores them as `v8::Weak<v8::Context>`, and a change in the `#[op]` macro to pass the current context to `queue_async_op`, this makes it possible to send the results of promises for different realms to their 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. Co-authored-by: Luis Malheiro <[email protected]>
bartlomieju
pushed a commit
to littledivy/deno
that referenced
this pull request
Aug 11, 2022
Pull request denoland#14019 enabled initial support for realms, but it did not include support for async ops anywhere other than the main realm. The main issue was 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 creates a `ContextState` struct, similar to `JsRuntimeState` but stored in a slot of each `v8::Context`, which contains a `js_recv_cb` callback for each realm. Combined with a new list of known realms, which stores them as `v8::Weak<v8::Context>`, and a change in the `#[op]` macro to pass the current context to `queue_async_op`, this makes it possible to send the results of promises for different realms to their 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. Co-authored-by: Luis Malheiro <[email protected]>
dsherret
pushed a commit
that referenced
this pull request
Aug 11, 2022
Pull request #14019 enabled initial support for realms, but it did not include support for async ops anywhere other than the main realm. The main issue was 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 creates a `ContextState` struct, similar to `JsRuntimeState` but stored in a slot of each `v8::Context`, which contains a `js_recv_cb` callback for each realm. Combined with a new list of known realms, which stores them as `v8::Weak<v8::Context>`, and a change in the `#[op]` macro to pass the current context to `queue_async_op`, this makes it possible to send the results of promises for different realms to their 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. Co-authored-by: Luis Malheiro <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This handles bindings, extension JS and sync ops. It also adds the
JsRealmAPI that adds methods likeJsRuntime'shandle_scope,global_objectandexecute_scriptspecific to the realm.Towards #13239.
@bartlomieju @AaronO @littledivy PTAL. Does this conflict with some of the remaining work to do for ops?