refactor(core): Move optional callbacks from `JsRuntimeState` to `ContextState` (#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.

1 files changed, 59 insertions, 16 deletions

diff --git a/core/runtime.rs b/core/runtime.rs
index 2b5974d43..0a5404fa1 100644
--- a/core/runtime.rs
+++ b/core/runtime.rs
@@ -152,8 +152,9 @@ pub type CompiledWasmModuleStore = CrossIsolateStore<v8::CompiledWasmModule>;
 pub(crate) struct ContextState {
   js_recv_cb: Option<v8::Global<v8::Function>>,
   pub(crate) js_build_custom_error_cb: Option<v8::Global<v8::Function>>,
-  // TODO(andreubotella): Move the rest of Option<Global<Function>> fields from
-  // JsRuntimeState to this struct.
+  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) unrefed_ops: HashSet<i32>,
   // We don't explicitly re-read this prop but need the slice to live alongside
   // the context
@@ -167,10 +168,7 @@ pub struct JsRuntimeState {
   known_realms: Vec<v8::Weak<v8::Context>>,
   pub(crate) js_macrotask_cbs: Vec<v8::Global<v8::Function>>,
   pub(crate) js_nexttick_cbs: Vec<v8::Global<v8::Function>>,
-  pub(crate) js_promise_reject_cb: Option<v8::Global<v8::Function>>,
-  pub(crate) js_format_exception_cb: Option<v8::Global<v8::Function>>,
   pub(crate) has_tick_scheduled: bool,
-  pub(crate) js_wasm_streaming_cb: Option<v8::Global<v8::Function>>,
   pub(crate) pending_promise_exceptions:
     HashMap<v8::Global<v8::Promise>, v8::Global<v8::Value>>,
   pub(crate) pending_dyn_mod_evaluate: Vec<DynImportModEvaluate>,
@@ -392,10 +390,7 @@ impl JsRuntime {
       dyn_module_evaluate_idle_counter: 0,
       js_macrotask_cbs: vec![],
       js_nexttick_cbs: vec![],
-      js_promise_reject_cb: None,
-      js_format_exception_cb: None,
       has_tick_scheduled: false,
-      js_wasm_streaming_cb: None,
       source_map_getter: options.source_map_getter,
       source_map_cache: Default::default(),
       pending_ops: FuturesUnordered::new(),
@@ -932,19 +927,18 @@ impl JsRuntime {
         let v8_isolate = self.v8_isolate();
         if let Some(context) = weak_context.to_global(v8_isolate) {
           let realm = JsRealm::new(context.clone());
-          let realm_state = realm.state(v8_isolate);
-          std::mem::take(&mut realm_state.borrow_mut().js_recv_cb);
-          std::mem::take(
-            &mut realm_state.borrow_mut().js_build_custom_error_cb,
-          );
+          let realm_state_rc = realm.state(v8_isolate);
+          let mut realm_state = realm_state_rc.borrow_mut();
+          std::mem::take(&mut realm_state.js_recv_cb);
+          std::mem::take(&mut realm_state.js_build_custom_error_cb);
+          std::mem::take(&mut realm_state.js_promise_reject_cb);
+          std::mem::take(&mut realm_state.js_format_exception_cb);
+          std::mem::take(&mut realm_state.js_wasm_streaming_cb);
           context.open(v8_isolate).clear_all_slots(v8_isolate);
         }
       }
 
       let mut state = self.state.borrow_mut();
-      std::mem::take(&mut state.js_promise_reject_cb);
-      std::mem::take(&mut state.js_format_exception_cb);
-      std::mem::take(&mut state.js_wasm_streaming_cb);
       state.js_macrotask_cbs.clear();
       state.js_nexttick_cbs.clear();
       state.known_realms.clear();
@@ -4081,6 +4075,55 @@ Deno.core.ops.op_async_serialize_object_with_numbers_as_keys({
   }
 
   #[tokio::test]
+  async fn test_set_promise_reject_callback_realms() {
+    let mut runtime = JsRuntime::new(RuntimeOptions::default());
+    let global_realm = runtime.global_realm();
+    let realm1 = runtime.create_realm().unwrap();
+    let realm2 = runtime.create_realm().unwrap();
+
+    let realm_expectations = &[
+      (&global_realm, "global_realm", 42),
+      (&realm1, "realm1", 140),
+      (&realm2, "realm2", 720),
+    ];
+
+    // Set up promise reject callbacks.
+    for (realm, realm_name, number) in realm_expectations {
+      realm
+        .execute_script(
+          runtime.v8_isolate(),
+          "",
+          &format!(
+            r#"
+              Deno.core.initializeAsyncOps();
+              globalThis.rejectValue = undefined;
+              Deno.core.setPromiseRejectCallback((_type, _promise, reason) => {{
+                globalThis.rejectValue = `{realm_name}/${{reason}}`;
+              }});
+              Deno.core.ops.op_void_async().then(() => Promise.reject({number}));
+            "#,
+            realm_name=realm_name,
+            number=number
+          ),
+        )
+        .unwrap();
+    }
+
+    runtime.run_event_loop(false).await.unwrap();
+
+    for (realm, realm_name, number) in realm_expectations {
+      let reject_value = realm
+        .execute_script(runtime.v8_isolate(), "", "globalThis.rejectValue")
+        .unwrap();
+      let scope = &mut realm.handle_scope(runtime.v8_isolate());
+      let reject_value = v8::Local::new(scope, reject_value);
+      assert!(reject_value.is_string());
+      let reject_value_string = reject_value.to_rust_string_lossy(scope);
+      assert_eq!(reject_value_string, format!("{}/{}", realm_name, number));
+    }
+  }
+
+  #[tokio::test]
   async fn test_set_promise_reject_callback_top_level_await() {
     static PROMISE_REJECT: AtomicUsize = AtomicUsize::new(0);