From 83f6def3c62e7f336516d59881e8f9f7846d9024 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Wed, 7 Oct 2020 15:56:52 +0200 Subject: [PATCH] refactor(core): JsRuntime doesn't defer to OwnedIsolate (#7853) Remove Deref and DeferMut implementations for JsRuntime. --- cli/inspector.rs | 2 +- cli/worker.rs | 2 +- core/modules.rs | 8 ++--- core/runtime.rs | 83 +++++++++++++++++++++--------------------------- 4 files changed, 42 insertions(+), 53 deletions(-) diff --git a/cli/inspector.rs b/cli/inspector.rs index 4a6fe7a142..6992e9855e 100644 --- a/cli/inspector.rs +++ b/cli/inspector.rs @@ -394,7 +394,7 @@ impl DenoInspector { server: Option>, ) -> Box { 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::(); diff --git a/cli/worker.rs b/cli/worker.rs index 47e5c47618..0a50b5624e 100644 --- a/cli/worker.rs +++ b/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 { diff --git a/core/modules.rs b/core/modules.rs index a0e4fad958..1038dd84f5 100644 --- a/core/modules.rs +++ b/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; diff --git a/core/runtime.rs b/core/runtime.rs index 2474d18872..ac0e812b7d 100644 --- a/core/runtime.rs +++ b/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 { - let state = Self::state(self); + pub fn global_context(&mut self) -> v8::Global { + 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> { - 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>, 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 { - 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>, 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> { - 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> { - 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 { 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);