refactor(core): JsRuntime doesn't defer to OwnedIsolate (#7853)

Remove Deref and DeferMut implementations for JsRuntime.

cli/inspector.rs

@@ -394,7 +394,7 @@ impl DenoInspector {
     server: Option<Arc<InspectorServer>>,
   ) -> Box<Self> {
     let context = isolate.global_context();
-    let scope = &mut v8::HandleScope::new(&mut **isolate);
+    let scope = &mut v8::HandleScope::new(isolate.v8_isolate());
 
     let (new_websocket_tx, new_websocket_rx) =
       mpsc::unbounded::<WebSocketProxy>();

cli/worker.rs

@@ -428,7 +428,7 @@ impl WebWorker {
     );
 
     let terminated = Arc::new(AtomicBool::new(false));
-    let isolate_handle = worker.isolate.thread_safe_handle();
+    let isolate_handle = worker.isolate.v8_isolate().thread_safe_handle();
     let (terminate_tx, terminate_rx) = mpsc::channel::<()>(1);
 
     let handle = WebWorkerHandle {

core/modules.rs

@@ -686,7 +686,7 @@ mod tests {
       ]
     );
 
-    let state_rc = JsRuntime::state(&runtime);
+    let state_rc = JsRuntime::state(runtime.v8_isolate());
     let state = state_rc.borrow();
     let modules = &state.modules;
     assert_eq!(modules.get_id("file:///a.js"), Some(a_id));
@@ -753,7 +753,7 @@ mod tests {
         ]
       );
 
-      let state_rc = JsRuntime::state(&runtime);
+      let state_rc = JsRuntime::state(runtime.v8_isolate());
       let state = state_rc.borrow();
       let modules = &state.modules;
 
@@ -829,7 +829,7 @@ mod tests {
         ]
       );
 
-      let state_rc = JsRuntime::state(&runtime);
+      let state_rc = JsRuntime::state(runtime.v8_isolate());
       let state = state_rc.borrow();
       let modules = &state.modules;
 
@@ -976,7 +976,7 @@ mod tests {
       vec!["file:///b.js", "file:///c.js", "file:///d.js"]
     );
 
-    let state_rc = JsRuntime::state(&runtime);
+    let state_rc = JsRuntime::state(runtime.v8_isolate());
     let state = state_rc.borrow();
     let modules = &state.modules;
 

core/runtime.rs

@@ -37,8 +37,6 @@ use std::collections::HashMap;
 use std::convert::TryFrom;
 use std::ffi::c_void;
 use std::mem::forget;
-use std::ops::Deref;
-use std::ops::DerefMut;
 use std::option::Option;
 use std::pin::Pin;
 use std::rc::Rc;
@@ -117,19 +115,6 @@ pub(crate) struct JsRuntimeState {
   waker: AtomicWaker,
 }
 
-impl Deref for JsRuntime {
-  type Target = v8::Isolate;
-  fn deref(&self) -> &v8::Isolate {
-    self.v8_isolate.as_ref().unwrap()
-  }
-}
-
-impl DerefMut for JsRuntime {
-  fn deref_mut(&mut self) -> &mut v8::Isolate {
-    self.v8_isolate.as_mut().unwrap()
-  }
-}
-
 impl Drop for JsRuntime {
   fn drop(&mut self) {
     if let Some(creator) = self.snapshot_creator.take() {
@@ -315,13 +300,13 @@ impl JsRuntime {
     }
   }
 
-  pub fn global_context(&self) -> v8::Global<v8::Context> {
+  pub fn global_context(&mut self) -> v8::Global<v8::Context> {
-    let state = Self::state(self);
+    let state = Self::state(self.v8_isolate());
     let state = state.borrow();
     state.global_context.clone().unwrap()
   }
 
-  fn v8_isolate(&mut self) -> &mut v8::OwnedIsolate {
+  pub fn v8_isolate(&mut self) -> &mut v8::OwnedIsolate {
     self.v8_isolate.as_mut().unwrap()
   }
 
@@ -351,7 +336,7 @@ impl JsRuntime {
   }
 
   pub fn op_state(&mut self) -> Rc<RefCell<OpState>> {
-    let state_rc = Self::state(self);
+    let state_rc = Self::state(self.v8_isolate());
     let state = state_rc.borrow();
     state.op_state.clone()
   }
@@ -404,7 +389,7 @@ impl JsRuntime {
   /// be a different type if `RuntimeOptions::js_error_create_fn` has been set.
   pub fn snapshot(&mut self) -> v8::StartupData {
     assert!(self.snapshot_creator.is_some());
-    let state = Self::state(self);
+    let state = Self::state(self.v8_isolate());
 
     // Note: create_blob() method must not be called from within a HandleScope.
     // TODO(piscisaureus): The rusty_v8 type system should enforce this.
@@ -425,7 +410,7 @@ impl JsRuntime {
   where
     F: Fn(Rc<RefCell<OpState>>, BufVec) -> Op + 'static,
   {
-    Self::state(self)
+    Self::state(self.v8_isolate())
       .borrow_mut()
       .op_state
       .borrow_mut()
@@ -489,7 +474,7 @@ impl Future for JsRuntime {
     let runtime = self.get_mut();
     runtime.shared_init();
 
-    let state_rc = Self::state(runtime);
+    let state_rc = Self::state(runtime.v8_isolate());
     {
       let state = state_rc.borrow();
       state.waker.register(cx.waker());
@@ -630,7 +615,7 @@ impl JsRuntime {
     name: &str,
     source: &str,
   ) -> Result<ModuleId, AnyError> {
-    let state_rc = Self::state(self);
+    let state_rc = Self::state(self.v8_isolate());
     let context = self.global_context();
     let scope = &mut v8::HandleScope::with_context(self.v8_isolate(), context);
 
@@ -684,7 +669,7 @@ impl JsRuntime {
   /// about the V8 exception. By default this type is `JsError`, however it may
   /// be a different type if `RuntimeOptions::js_error_create_fn` has been set.
   fn mod_instantiate(&mut self, id: ModuleId) -> Result<(), AnyError> {
-    let state_rc = Self::state(self);
+    let state_rc = Self::state(self.v8_isolate());
     let context = self.global_context();
 
     let scope = &mut v8::HandleScope::with_context(self.v8_isolate(), context);
@@ -725,7 +710,7 @@ impl JsRuntime {
   ) -> Result<(), AnyError> {
     self.shared_init();
 
-    let state_rc = Self::state(self);
+    let state_rc = Self::state(self.v8_isolate());
     let context = self.global_context();
     let context1 = self.global_context();
 
@@ -738,7 +723,8 @@ impl JsRuntime {
       .clone();
 
     let status = {
-      let scope = &mut v8::HandleScope::with_context(&mut **self, context);
+      let scope =
+        &mut v8::HandleScope::with_context(self.v8_isolate(), context);
       let module = module_handle.get(scope);
       module.get_status()
     };
@@ -760,7 +746,8 @@ impl JsRuntime {
       // For more details see:
       // https://github.com/denoland/deno/issues/4908
       // https://v8.dev/features/top-level-await#module-execution-order
-      let scope = &mut v8::HandleScope::with_context(&mut **self, context1);
+      let scope =
+        &mut v8::HandleScope::with_context(self.v8_isolate(), context1);
       let module = v8::Local::new(scope, &module_handle);
       let maybe_value = module.evaluate(scope);
 
@@ -805,7 +792,7 @@ impl JsRuntime {
   ) -> Result<mpsc::Receiver<Result<(), AnyError>>, AnyError> {
     self.shared_init();
 
-    let state_rc = Self::state(self);
+    let state_rc = Self::state(self.v8_isolate());
     let context = self.global_context();
 
     let scope = &mut v8::HandleScope::with_context(self.v8_isolate(), context);
@@ -883,7 +870,7 @@ impl JsRuntime {
     id: ModuleLoadId,
     err: AnyError,
   ) -> Result<(), AnyError> {
-    let state_rc = Self::state(self);
+    let state_rc = Self::state(self.v8_isolate());
     let context = self.global_context();
 
     let scope = &mut v8::HandleScope::with_context(self.v8_isolate(), context);
@@ -914,7 +901,7 @@ impl JsRuntime {
     id: ModuleLoadId,
     mod_id: ModuleId,
   ) -> Result<(), AnyError> {
-    let state_rc = Self::state(self);
+    let state_rc = Self::state(self.v8_isolate());
     let context = self.global_context();
 
     debug!("dyn_import_done {} {:?}", id, mod_id);
@@ -949,7 +936,7 @@ impl JsRuntime {
     &mut self,
     cx: &mut Context,
   ) -> Poll<Result<(), AnyError>> {
-    let state_rc = Self::state(self);
+    let state_rc = Self::state(self.v8_isolate());
 
     if state_rc.borrow().preparing_dyn_imports.is_empty() {
       return Poll::Ready(Ok(()));
@@ -987,7 +974,7 @@ impl JsRuntime {
     &mut self,
     cx: &mut Context,
   ) -> Poll<Result<(), AnyError>> {
-    let state_rc = Self::state(self);
+    let state_rc = Self::state(self.v8_isolate());
 
     if state_rc.borrow().pending_dyn_imports.is_empty() {
       return Poll::Ready(Ok(()));
@@ -1044,11 +1031,12 @@ impl JsRuntime {
   }
 
   fn poll_mod_evaluate(&mut self, _cx: &mut Context) -> Result<(), AnyError> {
-    let state_rc = Self::state(self);
+    let state_rc = Self::state(self.v8_isolate());
 
     let context = self.global_context();
     {
-      let scope = &mut v8::HandleScope::with_context(&mut **self, context);
+      let scope =
+        &mut v8::HandleScope::with_context(self.v8_isolate(), context);
 
       let mut state = state_rc.borrow_mut();
 
@@ -1090,12 +1078,13 @@ impl JsRuntime {
     &mut self,
     _cx: &mut Context,
   ) -> Result<(), AnyError> {
-    let state_rc = Self::state(self);
+    let state_rc = Self::state(self.v8_isolate());
 
     loop {
       let context = self.global_context();
       let maybe_result = {
-        let scope = &mut v8::HandleScope::with_context(&mut **self, context);
+        let scope =
+          &mut v8::HandleScope::with_context(self.v8_isolate(), context);
 
         let mut state = state_rc.borrow_mut();
         if let Some(&dyn_import_id) =
@@ -1169,7 +1158,7 @@ impl JsRuntime {
     let referrer_specifier =
       ModuleSpecifier::resolve_url(&module_url_found).unwrap();
 
-    let state_rc = Self::state(self);
+    let state_rc = Self::state(self.v8_isolate());
     // #A There are 3 cases to handle at this moment:
     // 1. Source code resolved result have the same module name as requested
     //    and is not yet registered
@@ -1208,14 +1197,14 @@ impl JsRuntime {
 
     // Now we must iterate over all imports of the module and load them.
     let imports = {
-      let state_rc = Self::state(self);
+      let state_rc = Self::state(self.v8_isolate());
       let state = state_rc.borrow();
       state.modules.get_children(module_id).unwrap().clone()
     };
 
     for module_specifier in imports {
       let is_registered = {
-        let state_rc = Self::state(self);
+        let state_rc = Self::state(self.v8_isolate());
         let state = state_rc.borrow();
         state.modules.is_registered(&module_specifier)
       };
@@ -1249,7 +1238,7 @@ impl JsRuntime {
   ) -> Result<ModuleId, AnyError> {
     self.shared_init();
     let loader = {
-      let state_rc = Self::state(self);
+      let state_rc = Self::state(self.v8_isolate());
       let state = state_rc.borrow();
       state.loader.clone()
     };
@@ -1277,7 +1266,7 @@ impl JsRuntime {
     &mut self,
     cx: &mut Context,
   ) -> Option<(OpId, Box<[u8]>)> {
-    let state_rc = Self::state(self);
+    let state_rc = Self::state(self.v8_isolate());
     let mut overflow_response: Option<(OpId, Box<[u8]>)> = None;
 
     loop {
@@ -1326,7 +1315,7 @@ impl JsRuntime {
   }
 
   fn check_promise_exceptions(&mut self) -> Result<(), AnyError> {
-    let state_rc = Self::state(self);
+    let state_rc = Self::state(self.v8_isolate());
     let mut state = state_rc.borrow_mut();
 
     if state.pending_promise_exceptions.is_empty() {
@@ -1349,7 +1338,7 @@ impl JsRuntime {
     &mut self,
     maybe_overflown_response: Option<(OpId, Box<[u8]>)>,
   ) -> Result<(), AnyError> {
-    let state_rc = Self::state(self);
+    let state_rc = Self::state(self.v8_isolate());
 
     let shared_queue_size = state_rc.borrow().shared.size();
 
@@ -1398,7 +1387,7 @@ impl JsRuntime {
 
   fn drain_macrotasks(&mut self) -> Result<(), AnyError> {
     let js_macrotask_cb_handle =
-      match &Self::state(self).borrow().js_macrotask_cb {
+      match &Self::state(self.v8_isolate()).borrow().js_macrotask_cb {
         Some(handle) => handle.clone(),
         None => return Ok(()),
       };
@@ -2034,7 +2023,7 @@ pub mod tests {
       heap_limits: Some(heap_limits),
       ..Default::default()
     });
-    let cb_handle = runtime.thread_safe_handle();
+    let cb_handle = runtime.v8_isolate().thread_safe_handle();
 
     let callback_invoke_count = Rc::new(AtomicUsize::default());
     let inner_invoke_count = Rc::clone(&callback_invoke_count);
@@ -2080,7 +2069,7 @@ pub mod tests {
       heap_limits: Some(heap_limits),
       ..Default::default()
     });
-    let cb_handle = runtime.thread_safe_handle();
+    let cb_handle = runtime.v8_isolate().thread_safe_handle();
 
     let callback_invoke_count_first = Rc::new(AtomicUsize::default());
     let inner_invoke_count_first = Rc::clone(&callback_invoke_count_first);
@@ -2199,7 +2188,7 @@ pub mod tests {
       .unwrap();
     assert_eq!(dispatch_count.load(Ordering::Relaxed), 0);
 
-    let state_rc = JsRuntime::state(&runtime);
+    let state_rc = JsRuntime::state(runtime.v8_isolate());
     {
       let state = state_rc.borrow();
       let imports = state.modules.get_children(mod_a);