Skip to content
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.

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

Support for realms/contexts in deno_core (in preparation for ShadowRealm) #911

Open
9 of 13 tasks
andreubotella opened this issue Dec 30, 2021 · 11 comments
Open
9 of 13 tasks
Labels

Comments

@andreubotella
Copy link
Contributor

andreubotella commented Dec 30, 2021

In JS, code execution is always tied to some realm (or "context" in V8 terminology), which provides a global object and a set of built-ins (Object, Array, Error, etc.) which is different from those of other realms. Multiple realms can share an event loop (= a thread), and those that do share one may be able to access objects from a different realm. In the web, a same-origin <iframe> is a different realm from the top-level page (iframe.contentWindow.Array !== window.Array). Currently there is no way to create realms or run code in them purely with JS built-ins, that's something that host environments (the web / Node.js / Deno) would have to provide, so it's fine for Deno to not support them.

But there is currently a stage-3 TC39 proposal called ShadowRealm that would allow creating realms, as a way of sandboxing untrusted code. This proposal is not close to shipping anywhere, and it's only now starting to get implemented in V8, but currently Deno assumes that there's a single V8 context available at all times, and it's better to start refactoring things to support realms with plenty of time to spare.

Since ShadowRealms are meant to be a sandbox primitive, there would be no way to make objects from the parent realm available inside the ShadowRealm and vice versa. This simplifies to some extent the requirements needed, because that way there is no need to comb through the JS code looking for wrong uses of instanceof. That said, these are the requirements that seem to be needed for ShadowRealm (and I'm sure I'm forgetting some):

@andreubotella andreubotella changed the title Support for realms/contexts in deno_core (in preparation for ShadowRealm) Support for realms/contexts in deno_core (in preparation for ShadowRealm) Dec 30, 2021
@andreubotella
Copy link
Contributor Author

For the general case with arbitrary realms, not just those with the security properties of ShadowRealm, there are more requirements and questions:

  • Comb through the JS runtime code to make sure instanceof checks are sound in the presence of objects from a different realm.
  • HTML and WebIDL have convoluted requirements regarding in which realm JS code gets executed, and in which realm the return values of web APIs are constructed. (I confess I don't fully understand those myself.)
  • serde_v8 and the bindings must support inputs from a different realm.
  • Creating contexts with an empty global, even though the isolate was created from a non-empty snapshot. Also, creating contexts from arbitrary existing objects. (Both needed to implement Node.js's vm module.)

@bartlomieju
Copy link
Member

Inspector support for multiple contexts should also be considered when working on this feature. What you already proposed is gonna be rather large undertaking, I expect even more non-trivial engineering effort for inspector integration.

@andreubotella
Copy link
Contributor Author

Recently denoland/deno#13861 has radically changed how ops work internally, such that op synchronization is no longer needed. Also, denoland/deno#13993, although still experimental, should guarantee that async ops resolve the right promise on the right realm. Both of this PRs simplify the work needed for refactoring deno_core to support realms, and as a side effect, reduce the number of JS callbacks that need to be kept in a per-context weakmap.

andreubotella referenced this issue in andreubotella/deno Mar 17, 2022
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 #13239.
andreubotella referenced this issue in andreubotella/deno 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 #13239.
andreubotella referenced this issue in andreubotella/deno 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 #13239.

Co-authored-by: Luis Malheiro <luismalheiro@gmail.com>
andreubotella referenced this issue in andreubotella/deno 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 #13239.

Co-authored-by: Luis Malheiro <luismalheiro@gmail.com>
@stale
Copy link

stale bot commented Jul 21, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

@andreubotella
Copy link
Contributor Author

Not stale. This is being worked on, slowly.

@stale stale bot closed this as completed Jul 30, 2022
@crowlKats crowlKats reopened this Jul 30, 2022
@crowlKats crowlKats added the feat label Jul 30, 2022
andreubotella referenced this issue in andreubotella/deno Aug 25, 2022
…textState`

The `JsRuntimeState` struct stores a number of JS callbacks that are
used either in the event loop or when interacting with V8. Some of
these callback fields are vectors of callbacks, and therefore could
plausibly store at least one callback per realm. However, some of
those fields are `Option<v8::Global<v8::Function>>`, which would make
the callbacks set by a realm override the one that might have been set
by a different realm.

As it turns out, all of the current such optional callbacks
(`js_promise_reject_cb`, `js_format_exception_cb` and
`js_wasm_streaming_cb`) are only used from inside a realm, and
therefore this change makes it so such callbacks can only be set from
inside a realm, and will only affect that realm.

Towards #13239.
andreubotella referenced this issue in andreubotella/deno Oct 8, 2022
andreubotella referenced this issue in andreubotella/deno Oct 20, 2022
@andreubotella
Copy link
Contributor Author

andreubotella commented Dec 28, 2022

Status update: in October, many of the PRs that I landed related to realms were reverted in denoland/deno#16366 because they were blocking some planned optimizations in deno_core. The plan was for those optimizations to take about two weeks to be implemented, and to start working on relanding those patches afterwards. Those optimizations took way longer than that, but I am now starting to work on relanding those PRs.

I have updated the status of the bullet points in the OP to mark whether they are currently reverted or relanded.

andreubotella referenced this issue in andreubotella/deno Jan 14, 2023
…textState`

The `JsRuntimeState` struct stores a number of JS callbacks that are
used either in the event loop or when interacting with V8. Some of
these callback fields are vectors of callbacks, and therefore could
plausibly store at least one callback per realm. However, some of
those fields are `Option<v8::Global<v8::Function>>`, which would make
the callbacks set by a realm override the one that might have been set
by a different realm.

As it turns out, all of the current such optional callbacks
(`js_promise_reject_cb`, `js_format_exception_cb` and
`js_wasm_streaming_cb`) are only used from inside a realm, and
therefore this change makes it so such callbacks can only be set from
inside a realm, and will only affect that realm.

This is a reland of denoland#15599.

Towards #13239.
littledivy referenced this issue in denoland/deno Jan 15, 2023
…textState` (#17422)

The `JsRuntimeState` struct stores a number of JS callbacks that are
used either in the event loop or when interacting with V8. Some of these
callback fields are vectors of callbacks, and therefore could plausibly
store at least one callback per realm. However, some of those fields are
`Option<v8::Global<v8::Function>>`, which would make the callbacks set
by a realm override the one that might have been set by a different
realm.

As it turns out, all of the current such optional callbacks
(`js_promise_reject_cb`, `js_format_exception_cb` and
`js_wasm_streaming_cb`) are only used from inside a realm, and therefore
this change makes it so such callbacks can only be set from inside a
realm, and will only affect that realm.

This is a reland of #15599.

Towards #13239.
bartlomieju referenced this issue in denoland/deno Jan 16, 2023
…textState` (#17422)

The `JsRuntimeState` struct stores a number of JS callbacks that are
used either in the event loop or when interacting with V8. Some of these
callback fields are vectors of callbacks, and therefore could plausibly
store at least one callback per realm. However, some of those fields are
`Option<v8::Global<v8::Function>>`, which would make the callbacks set
by a realm override the one that might have been set by a different
realm.

As it turns out, all of the current such optional callbacks
(`js_promise_reject_cb`, `js_format_exception_cb` and
`js_wasm_streaming_cb`) are only used from inside a realm, and therefore
this change makes it so such callbacks can only be set from inside a
realm, and will only affect that realm.

This is a reland of #15599.

Towards #13239.
andreubotella referenced this issue in andreubotella/deno Apr 8, 2023
This change makes realms other than the main one support modules by
having a module map associated to each realm, rather than one per
module. As part of this, it also:

- Adds an argument to `JsRuntime::create_realm` to set the realm's
  module loader. This allows different realms to have different module
  loading strategies (different import maps, for example).

- Moves all of the methods of `JsRuntime` related to module loading
  into `JsRealm`. To minimize changing unrelated code, the public and
  crate-private methods in `JsRuntime` are kept, with their
  implementation replaced by a call to the corresponding method of the
  main realm's `JsRealm`.

- Removes the `module_map` argument to the `EventLoopPendingState`
  constructor, instead accessing each realm's corresponding module map
  as part of the existing iteration.

- Changes the parts of `JsRuntime::poll_event_loop` that deal with
  module evaluation and detecting stalled top-level awaits to support
  multiple module maps and multiple top-level module evaluations at
  the same time.

- Moves `pending_mod_evaluate` and `pending_dyn_mod_evaluate` from
  `JsRuntimeState` to `ContextState`.

Towards #13239.
@bartlomieju
Copy link
Member

@andreubotella can we close this issue now?

@andreubotella
Copy link
Contributor Author

I intended this to be a meta-issue covering everything up to shipping ShadowRealm, and although realms now work fine for deno_core embedders, there's still some work needed before the ShadowRealm constructor can be implemented (see #101). And after that there's still the work of properly boostrapping the realm in deno_runtime and integration testing.

@bartlomieju
Copy link
Member

Sounds good, let's keep it open then.

@spolu
Copy link

spolu commented Jul 3, 2024

Any update on this work? In particular, is there a valid supported way to enforce isolation of execution in the current deno_core/jsruntime APIs?

@lucacasonato lucacasonato transferred this issue from denoland/deno Sep 18, 2024
@caramboleyo-no2fa
Copy link

@spolu the only way were workers quite some time, but deno crashes if you have open to much of them. i needed every http request to take a worker and (even with reusing them) had deno crash around 5k workers on a 8t/32GB machine.

Now it seems to support node:vm https://docs.deno.com/api/node/vm/

i switched to bun a while ago and use ShadowRealms there. Debugging is a pain, cause the error throwing is incompletely implemented, but the context separation from the main thread works perfectly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants