deno
deno copied to clipboard
denoland
`JsRuntime::create_realm` should be a static method
The JsRuntime::create_realm method, introduced, in #14019, made it possible to create V8 contexts that had the same set of extensions and ops loaded, and it has since been used to write tests for deno_core's realm support. However, this method takes a &mut self, so it can only be called from code embedding the Deno runtime, as opposed to from the runtime itself.
Given that the whole intention of having deno_core support realms was to make it possible to implement the upcoming ShadowRealm TC39 proposal (see #13239), it must be possible to create such a realm from a callback passed to V8.
Making this method static would require making JsRuntime::init_extension_js and JsRuntime::init_cbs static as well, plus probably moving the extension field from JsRuntime to JsRuntimeState.
As I was trying this out, I realized that since JsRuntime::handle_scope and JsRealm::handle_scope create a brand new scope rather than adding to the existing scope stack, JsRealm::execute_script can't be used in a V8 callback since there's an existing scope stack. This means that you can't really run initialization code inside the context from the ShadowRealm constructor – or at least not without building on the existing scopes.
I either made such methods static, or moved them to JsRealm, in #16211. ~~But this seems not to run into the issue I mentioned in the second comment, even though I'm pretty sure it used to when I tried months ago.~~
Edit: Never mind, it does run into that issue, but JsRealm::execute_script only runs on realm creation when the isolate hasn't been built from a snapshot, so you don't end up seeing this in Deno CLI builds (which was what I was first using for testing), but you do see it in deno_core's unit tests.
@andreubotella is this something you still want to pursue?
I'm not sure at the moment. I'll try rebasing #16211 to see if it can be done without this.