diff options
| -rw-r--r-- | Cargo.lock | 4 | ||||
| -rw-r--r-- | core/Cargo.toml | 2 | ||||
| -rw-r--r-- | core/bindings.rs | 16 | ||||
| -rw-r--r-- | core/es_isolate.rs | 104 | ||||
| -rw-r--r-- | core/isolate.rs | 454 |
5 files changed, 289 insertions, 291 deletions
diff --git a/Cargo.lock b/Cargo.lock index 00d199c12..fbd7a4618 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1883,9 +1883,9 @@ dependencies = [ [[package]] name = "rusty_v8" -version = "0.2.1" +version = "0.3.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8ed9530934df58a5be648916e142858b473558a0df6356b59e083c1c3c2f7d32" +checksum = "cbefe28096120d07dc9a70ff402e1eddff9fcf7921362dd2b79d6b32aa3e298f" dependencies = [ "bitflags", "cargo_gn", diff --git a/core/Cargo.toml b/core/Cargo.toml index 0c1fb21ce..fc19813a3 100644 --- a/core/Cargo.toml +++ b/core/Cargo.toml @@ -21,7 +21,7 @@ libc = "0.2.66" log = "0.4.8" serde_json = "1.0.44" url = "2.1.0" -rusty_v8 = "0.2.1" +rusty_v8 = "0.3.3" [[example]] name = "deno_core_http_bench" diff --git a/core/bindings.rs b/core/bindings.rs index 6ea0b9b91..3db95d896 100644 --- a/core/bindings.rs +++ b/core/bindings.rs @@ -1,6 +1,8 @@ // Copyright 2018-2020 the Deno authors. All rights reserved. MIT license. use crate::es_isolate::EsIsolate; +use crate::isolate::encode_message_as_json; +use crate::isolate::handle_exception; use crate::isolate::Isolate; use crate::isolate::ZeroCopyBuf; @@ -278,13 +280,19 @@ pub extern "C" fn message_callback( unsafe { &mut *(scope.isolate().get_data(0) as *mut Isolate) }; // TerminateExecution was called - if scope.isolate().is_execution_terminating() { + // TODO(piscisaureus): rusty_v8 should implement the + // `is_execution_terminating()` method on struct `Isolate` also. + if scope + .isolate() + .thread_safe_handle() + .is_execution_terminating() + { let undefined = v8::undefined(scope).into(); - deno_isolate.handle_exception(scope, undefined); + handle_exception(scope, undefined, &mut deno_isolate.last_exception); return; } - let json_str = deno_isolate.encode_message_as_json(scope, message); + let json_str = encode_message_as_json(scope, message); deno_isolate.last_exception = Some(json_str); } @@ -668,7 +676,7 @@ pub fn encode_message_as_object<'a>( s: &mut impl v8::ToLocal<'a>, message: v8::Local<v8::Message>, ) -> v8::Local<'a, v8::Object> { - let context = s.isolate().get_current_context(); + let context = s.get_current_context().unwrap(); let json_obj = v8::Object::new(s); let exception_str = message.get(s); diff --git a/core/es_isolate.rs b/core/es_isolate.rs index 152805f3f..0746804f1 100644 --- a/core/es_isolate.rs +++ b/core/es_isolate.rs @@ -25,6 +25,8 @@ use std::rc::Rc; use std::task::Context; use std::task::Poll; +use crate::isolate::attach_handle_to_error; +use crate::isolate::exception_to_err_result; use crate::isolate::Isolate; use crate::isolate::StartupData; use crate::module_specifier::ModuleSpecifier; @@ -80,25 +82,10 @@ impl DerefMut for EsIsolate { } } +// TODO(ry): a V8 Isolate cannot actually be moved between threads without the +// use of a Locker, therefore EsIsolate should not implement the `Send` trait. unsafe impl Send for EsIsolate {} -impl Drop for EsIsolate { - fn drop(&mut self) { - let isolate = self.core_isolate.v8_isolate.as_ref().unwrap(); - // Clear persistent handles we own. - { - let mut locker = v8::Locker::new(&isolate); - let scope = locker.enter(); - for module in self.modules.info.values_mut() { - module.handle.reset(scope); - } - for handle in self.dyn_import_map.values_mut() { - handle.reset(scope); - } - } - } -} - impl EsIsolate { pub fn new( loader: Rc<dyn Loader + Unpin>, @@ -107,11 +94,11 @@ impl EsIsolate { ) -> Box<Self> { let mut core_isolate = Isolate::new(startup_data, will_snapshot); { - let isolate = core_isolate.v8_isolate.as_mut().unwrap(); - isolate.set_host_initialize_import_meta_object_callback( + let v8_isolate = core_isolate.v8_isolate.as_mut().unwrap(); + v8_isolate.set_host_initialize_import_meta_object_callback( bindings::host_initialize_import_meta_object_callback, ); - isolate.set_host_import_module_dynamically_callback( + v8_isolate.set_host_import_module_dynamically_callback( bindings::host_import_module_dynamically_callback, ); } @@ -147,13 +134,14 @@ impl EsIsolate { name: &str, source: &str, ) -> Result<ModuleId, ErrBox> { - let isolate = self.core_isolate.v8_isolate.as_ref().unwrap(); - let mut locker = v8::Locker::new(&isolate); + let core_isolate = &mut self.core_isolate; + let v8_isolate = core_isolate.v8_isolate.as_mut().unwrap(); + let js_error_create_fn = &*core_isolate.js_error_create_fn; - let mut hs = v8::HandleScope::new(locker.enter()); + let mut hs = v8::HandleScope::new(v8_isolate); let scope = hs.enter(); - assert!(!self.core_isolate.global_context.is_empty()); - let context = self.core_isolate.global_context.get(scope).unwrap(); + assert!(!core_isolate.global_context.is_empty()); + let context = core_isolate.global_context.get(scope).unwrap(); let mut cs = v8::ContextScope::new(scope, context); let scope = cs.enter(); @@ -166,13 +154,15 @@ impl EsIsolate { let mut try_catch = v8::TryCatch::new(scope); let tc = try_catch.enter(); - let maybe_module = v8::script_compiler::compile_module(&isolate, source); + let maybe_module = v8::script_compiler::compile_module(scope, source); if tc.has_caught() { assert!(maybe_module.is_none()); - return self - .core_isolate - .exception_to_err_result(scope, tc.exception().unwrap()); + return exception_to_err_result( + scope, + tc.exception().unwrap(), + js_error_create_fn, + ); } let module = maybe_module.unwrap(); @@ -201,9 +191,10 @@ impl EsIsolate { /// the V8 exception. By default this type is CoreJSError, however it may be a /// different type if Isolate::set_js_error_create() has been used. fn mod_instantiate(&mut self, id: ModuleId) -> Result<(), ErrBox> { - let isolate = self.core_isolate.v8_isolate.as_ref().unwrap(); - let mut locker = v8::Locker::new(isolate); - let mut hs = v8::HandleScope::new(locker.enter()); + let v8_isolate = self.core_isolate.v8_isolate.as_mut().unwrap(); + let js_error_create_fn = &*self.core_isolate.js_error_create_fn; + + let mut hs = v8::HandleScope::new(v8_isolate); let scope = hs.enter(); assert!(!self.core_isolate.global_context.is_empty()); let context = self.core_isolate.global_context.get(scope).unwrap(); @@ -221,7 +212,11 @@ impl EsIsolate { let mut module = module_info.handle.get(scope).unwrap(); if module.get_status() == v8::ModuleStatus::Errored { - self.exception_to_err_result(scope, module.get_exception())? + exception_to_err_result( + scope, + module.get_exception(), + js_error_create_fn, + )? } let result = @@ -230,7 +225,7 @@ impl EsIsolate { Some(_) => Ok(()), None => { let exception = tc.exception().unwrap(); - self.exception_to_err_result(scope, exception) + exception_to_err_result(scope, exception, js_error_create_fn) } } } @@ -241,9 +236,10 @@ impl EsIsolate { &mut self, id: ModuleId, ) -> Result<(), ErrBox> { - let isolate = self.core_isolate.v8_isolate.as_ref().unwrap(); - let mut locker = v8::Locker::new(isolate); - let mut hs = v8::HandleScope::new(locker.enter()); + let v8_isolate = self.core_isolate.v8_isolate.as_mut().unwrap(); + let js_error_create_fn = &*self.core_isolate.js_error_create_fn; + + let mut hs = v8::HandleScope::new(v8_isolate); let scope = hs.enter(); assert!(!self.core_isolate.global_context.is_empty()); let context = self.core_isolate.global_context.get(scope).unwrap(); @@ -271,10 +267,9 @@ impl EsIsolate { match status { v8::ModuleStatus::Evaluated => Ok(()), v8::ModuleStatus::Errored => { - let i = &mut self.core_isolate; let exception = module.get_exception(); - i.exception_to_err_result(scope, exception) - .map_err(|err| i.attach_handle_to_error(scope, err, exception)) + exception_to_err_result(scope, exception, js_error_create_fn) + .map_err(|err| attach_handle_to_error(scope, err, exception)) } other => panic!("Unexpected module status {:?}", other), } @@ -286,12 +281,14 @@ impl EsIsolate { /// the V8 exception. By default this type is CoreJSError, however it may be a /// different type if Isolate::set_js_error_create() has been used. pub fn mod_evaluate(&mut self, id: ModuleId) -> Result<(), ErrBox> { - let isolate = self.core_isolate.v8_isolate.as_ref().unwrap(); - let mut locker = v8::Locker::new(isolate); - let mut hs = v8::HandleScope::new(locker.enter()); + let core_isolate = &mut self.core_isolate; + let v8_isolate = core_isolate.v8_isolate.as_mut().unwrap(); + let js_error_create_fn = &*core_isolate.js_error_create_fn; + + let mut hs = v8::HandleScope::new(v8_isolate); let scope = hs.enter(); - assert!(!self.core_isolate.global_context.is_empty()); - let context = self.core_isolate.global_context.get(scope).unwrap(); + assert!(!core_isolate.global_context.is_empty()); + let context = core_isolate.global_context.get(scope).unwrap(); let mut cs = v8::ContextScope::new(scope, context); let scope = cs.enter(); @@ -316,9 +313,8 @@ impl EsIsolate { match status { v8::ModuleStatus::Evaluated => Ok(()), v8::ModuleStatus::Errored => { - let i = &mut self.core_isolate; let exception = module.get_exception(); - i.exception_to_err_result(scope, exception) + exception_to_err_result(scope, exception, js_error_create_fn) } other => panic!("Unexpected module status {:?}", other), } @@ -362,11 +358,12 @@ impl EsIsolate { id: DynImportId, err: ErrBox, ) -> Result<(), ErrBox> { - let isolate = self.core_isolate.v8_isolate.as_ref().unwrap(); - let mut locker = v8::Locker::new(isolate); - let mut hs = v8::HandleScope::new(locker.enter()); + let core_isolate = &mut self.core_isolate; + let v8_isolate = core_isolate.v8_isolate.as_mut().unwrap(); + + let mut hs = v8::HandleScope::new(v8_isolate); let scope = hs.enter(); - let context = self.core_isolate.global_context.get(scope).unwrap(); + let context = core_isolate.global_context.get(scope).unwrap(); let mut cs = v8::ContextScope::new(scope, context); let scope = cs.enter(); @@ -403,9 +400,8 @@ impl EsIsolate { ) -> Result<(), ErrBox> { debug!("dyn_import_done {} {:?}", id, mod_id); assert!(mod_id != 0); - let isolate = self.core_isolate.v8_isolate.as_ref().unwrap(); - let mut locker = v8::Locker::new(isolate); - let mut hs = v8::HandleScope::new(locker.enter()); + let v8_isolate = self.core_isolate.v8_isolate.as_mut().unwrap(); + let mut hs = v8::HandleScope::new(v8_isolate); let scope = hs.enter(); assert!(!self.core_isolate.global_context.is_empty()); let context = self.core_isolate.global_context.get(scope).unwrap(); diff --git a/core/isolate.rs b/core/isolate.rs index d67ed9331..4a1c1bbd4 100644 --- a/core/isolate.rs +++ b/core/isolate.rs @@ -26,6 +26,7 @@ use std::collections::HashMap; use std::convert::From; use std::error::Error; use std::fmt; +use std::mem::forget; use std::ops::{Deref, DerefMut}; use std::option::Option; use std::pin::Pin; @@ -169,7 +170,7 @@ pub struct Isolate { pub(crate) js_recv_cb: v8::Global<v8::Function>, pub(crate) pending_promise_exceptions: HashMap<i32, v8::Global<v8::Value>>, shared_isolate_handle: Arc<Mutex<Option<*mut v8::Isolate>>>, - js_error_create: Arc<JSErrorCreateFn>, + pub(crate) js_error_create_fn: Arc<JSErrorCreateFn>, needs_init: bool, pub(crate) shared: SharedQueue, pending_ops: FuturesUnordered<PendingOpFuture>, @@ -181,42 +182,28 @@ pub struct Isolate { error_handler: Option<Box<IsolateErrorHandleFn>>, } -// TODO(ry) this shouldn't be necessary, v8::OwnedIsolate should impl Send. +// TODO(ry): a V8 Isolate cannot actually be moved between threads without the +// use of a Locker, therefore Isolate should not implement the `Send` trait. unsafe impl Send for Isolate {} impl Drop for Isolate { fn drop(&mut self) { - // remove shared_libdeno_isolate reference - *self.shared_isolate_handle.lock().unwrap() = None; - - // TODO Too much boiler plate. - // <Boilerplate> - let isolate = self.v8_isolate.take().unwrap(); - // Clear persistent handles we own. - { - let mut locker = v8::Locker::new(&isolate); - let mut hs = v8::HandleScope::new(locker.enter()); - let scope = hs.enter(); - // </Boilerplate> - self.global_context.reset(scope); - self.shared_ab.reset(scope); - self.js_recv_cb.reset(scope); - for (_key, handle) in self.pending_promise_exceptions.iter_mut() { - handle.reset(scope); - } - } if let Some(creator) = self.snapshot_creator.take() { + // TODO(ry): in rusty_v8, `SnapShotCreator::get_owned_isolate()` returns + // a `struct OwnedIsolate` which is not actually owned, hence the need + // here to leak the `OwnedIsolate` in order to avoid a double free and + // the segfault that it causes. + let v8_isolate = self.v8_isolate.take().unwrap(); + forget(v8_isolate); + // TODO(ry) V8 has a strange assert which prevents a SnapshotCreator from // being deallocated if it hasn't created a snapshot yet. // https://github.com/v8/v8/blob/73212783fbd534fac76cc4b66aac899c13f71fc8/src/api.cc#L603 // If that assert is removed, this if guard could be removed. // WARNING: There may be false positive LSAN errors here. - std::mem::forget(isolate); if self.has_snapshotted { drop(creator); } - } else { - drop(isolate); } } } @@ -272,12 +259,9 @@ impl Isolate { let mut creator = v8::SnapshotCreator::new(Some(&bindings::EXTERNAL_REFERENCES)); let isolate = unsafe { creator.get_owned_isolate() }; - let isolate = Isolate::setup_isolate(isolate); - - let mut locker = v8::Locker::new(&isolate); - let scope = locker.enter(); + let mut isolate = Isolate::setup_isolate(isolate); - let mut hs = v8::HandleScope::new(scope); + let mut hs = v8::HandleScope::new(&mut isolate); let scope = hs.enter(); let context = bindings::initialize_context(scope); @@ -294,12 +278,9 @@ impl Isolate { } let isolate = v8::Isolate::new(params); - let isolate = Isolate::setup_isolate(isolate); + let mut isolate = Isolate::setup_isolate(isolate); - let mut locker = v8::Locker::new(&isolate); - let scope = locker.enter(); - - let mut hs = v8::HandleScope::new(scope); + let mut hs = v8::HandleScope::new(&mut isolate); let scope = hs.enter(); let context = match load_snapshot { @@ -329,7 +310,7 @@ impl Isolate { snapshot: load_snapshot, has_snapshotted: false, shared_isolate_handle: Arc::new(Mutex::new(None)), - js_error_create: Arc::new(CoreJSError::from_v8_exception), + js_error_create_fn: Arc::new(CoreJSError::from_v8_exception), shared, needs_init, pending_ops: FuturesUnordered::new(), @@ -362,62 +343,6 @@ impl Isolate { isolate } - pub fn exception_to_err_result<'a, T>( - &mut self, - scope: &mut (impl v8::ToLocal<'a> + v8::InContext), - exception: v8::Local<v8::Value>, - ) -> Result<T, ErrBox> { - self.handle_exception(scope, exception); - self.check_last_exception().map(|_| unreachable!()) - } - - pub fn handle_exception<'a>( - &mut self, - scope: &mut (impl v8::ToLocal<'a> + v8::InContext), - exception: v8::Local<v8::Value>, - ) { - // Use a HandleScope because the functions below create a lot of - // local handles (in particular, `encode_message_as_json()` does). - let mut hs = v8::HandleScope::new(scope); - let scope = hs.enter(); - - let is_terminating_exception = scope.isolate().is_execution_terminating(); - let mut exception = exception; - - if is_terminating_exception { - // TerminateExecution was called. Cancel exception termination so that the - // exception can be created.. - scope.isolate().cancel_terminate_execution(); - - // Maybe make a new exception object. - if exception.is_null_or_undefined() { - let exception_str = - v8::String::new(scope, "execution terminated").unwrap(); - exception = v8::Exception::error(scope, exception_str); - } - } - - let message = v8::Exception::create_message(scope, exception); - let json_str = self.encode_message_as_json(scope, message); - self.last_exception = Some(json_str); - - if is_terminating_exception { - // Re-enable exception termination. - scope.isolate().terminate_execution(); - } - } - - pub fn encode_message_as_json<'a>( - &mut self, - scope: &mut (impl v8::ToLocal<'a> + v8::InContext), - message: v8::Local<v8::Message>, - ) -> String { - let context = scope.isolate().get_current_context(); - let json_obj = bindings::encode_message_as_object(scope, message); - let json_string = v8::json::stringify(context, json_obj.into()).unwrap(); - json_string.to_rust_string_lossy(scope) - } - /// Defines the how Deno.core.dispatch() acts. /// Called whenever Deno.core.dispatch() is called in JavaScript. zero_copy_buf /// corresponds to the second argument of Deno.core.dispatch(). @@ -437,14 +362,7 @@ impl Isolate { where F: Fn(V8Exception) -> ErrBox + 'static, { - self.js_error_create = Arc::new(f); - } - - /// Get a thread safe handle on the isolate. - pub fn shared_isolate_handle(&mut self) -> IsolateHandle { - IsolateHandle { - shared_isolate: self.shared_isolate_handle.clone(), - } + self.js_error_create_fn = Arc::new(f); } /// Executes a bit of built-in JavaScript to provide Deno.sharedQueue. @@ -463,7 +381,7 @@ impl Isolate { pub fn dispatch_op<'s>( &mut self, - scope: &mut (impl v8::ToLocal<'s> + v8::InContext), + scope: &mut impl v8::ToLocal<'s>, op_id: OpId, control_buf: &[u8], zero_copy_buf: Option<ZeroCopyBuf>, @@ -516,11 +434,12 @@ impl Isolate { ) -> Result<(), ErrBox> { self.shared_init(); - let isolate = self.v8_isolate.as_ref().unwrap(); - let mut locker = v8::Locker::new(isolate); - assert!(!self.global_context.is_empty()); - let mut hs = v8::HandleScope::new(locker.enter()); + let js_error_create_fn = &*self.js_error_create_fn; + let v8_isolate = self.v8_isolate.as_mut().unwrap(); + + let mut hs = v8::HandleScope::new(v8_isolate); let scope = hs.enter(); + assert!(!self.global_context.is_empty()); let context = self.global_context.get(scope).unwrap(); let mut cs = v8::ContextScope::new(scope, context); let scope = cs.enter(); @@ -539,75 +458,8 @@ impl Isolate { None => { assert!(tc.has_caught()); let exception = tc.exception().unwrap(); - self.exception_to_err_result(scope, exception) - } - } - } - - pub(crate) fn check_last_exception(&mut self) -> Result<(), ErrBox> { - match self.last_exception.take() { - None => Ok(()), - Some(json_str) => { - let v8_exception = V8Exception::from_json(&json_str).unwrap(); - let js_error = (self.js_error_create)(v8_exception); - Err(js_error) - } - } - } - - pub(crate) fn attach_handle_to_error( - &mut self, - scope: &mut impl v8::InIsolate, - err: ErrBox, - handle: v8::Local<v8::Value>, - ) -> ErrBox { - ErrWithV8Handle::new(scope, err, handle).into() - } - - fn check_promise_exceptions<'s>( - &mut self, - scope: &mut (impl v8::ToLocal<'s> + v8::InContext), - ) -> Result<(), ErrBox> { - if let Some(&key) = self.pending_promise_exceptions.keys().next() { - let mut handle = self.pending_promise_exceptions.remove(&key).unwrap(); - let exception = handle.get(scope).expect("empty error handle"); - handle.reset(scope); - self.exception_to_err_result(scope, exception) - } else { - Ok(()) - } - } - - fn async_op_response<'s>( - &mut self, - scope: &mut (impl v8::ToLocal<'s> + v8::InContext), - maybe_buf: Option<(OpId, Box<[u8]>)>, - ) -> Result<(), ErrBox> { - let context = scope.isolate().get_current_context(); - let global: v8::Local<v8::Value> = context.global(scope).into(); - let js_recv_cb = self - .js_recv_cb - .get(scope) - .expect("Deno.core.recv has not been called."); - - // TODO(piscisaureus): properly integrate TryCatch in the scope chain. - let mut try_catch = v8::TryCatch::new(scope); - let tc = try_catch.enter(); - - match maybe_buf { - Some((op_id, buf)) => { - let op_id: v8::Local<v8::Value> = - v8::Integer::new(scope, op_id as i32).into(); - let ui8: v8::Local<v8::Value> = - bindings::boxed_slice_to_uint8array(scope, buf).into(); - js_recv_cb.call(scope, context, global, &[op_id, ui8]) + exception_to_err_result(scope, exception, js_error_create_fn) } - None => js_recv_cb.call(scope, context, global, &[]), - }; - - match tc.exception() { - None => Ok(()), - Some(exception) => self.exception_to_err_result(scope, exception), } } @@ -620,9 +472,11 @@ impl Isolate { pub fn snapshot(&mut self) -> Result<v8::OwnedStartupData, ErrBox> { assert!(self.snapshot_creator.is_some()); - let isolate = self.v8_isolate.as_ref().unwrap(); - let mut locker = v8::Locker::new(isolate); - let mut hs = v8::HandleScope::new(locker.enter()); + let v8_isolate = self.v8_isolate.as_mut().unwrap(); + let js_error_create_fn = &*self.js_error_create_fn; + let last_exception = &mut self.last_exception; + + let mut hs = v8::HandleScope::new(v8_isolate); let scope = hs.enter(); self.global_context.reset(scope); @@ -631,7 +485,8 @@ impl Isolate { .create_blob(v8::FunctionCodeHandling::Keep) .unwrap(); self.has_snapshotted = true; - self.check_last_exception().map(|_| snapshot) + + check_last_exception(last_exception, js_error_create_fn).map(|_| snapshot) } } @@ -643,14 +498,22 @@ impl Future for Isolate { inner.waker.register(cx.waker()); inner.shared_init(); - let mut locker = v8::Locker::new(&*inner.v8_isolate.as_mut().unwrap()); - let mut hs = v8::HandleScope::new(locker.enter()); + let v8_isolate = inner.v8_isolate.as_mut().unwrap(); + let js_error_create_fn = &*inner.js_error_create_fn; + let js_recv_cb = &inner.js_recv_cb; + let pending_promise_exceptions = &mut inner.pending_promise_exceptions; + + let mut hs = v8::HandleScope::new(v8_isolate); let scope = hs.enter(); let context = inner.global_context.get(scope).unwrap(); let mut cs = v8::ContextScope::new(scope, context); let scope = cs.enter(); - inner.check_promise_exceptions(scope)?; + check_promise_exceptions( + scope, + pending_promise_exceptions, + js_error_create_fn, + )?; let mut overflow_response: Option<(OpId, Buf)> = None; @@ -678,17 +541,26 @@ impl Future for Isolate { } if inner.shared.size() > 0 { - inner.async_op_response(scope, None)?; + async_op_response(scope, None, js_recv_cb, js_error_create_fn)?; // The other side should have shifted off all the messages. assert_eq!(inner.shared.size(), 0); } if overflow_response.is_some() { let (op_id, buf) = overflow_response.take().unwrap(); - inner.async_op_response(scope, Some((op_id, buf)))?; + async_op_response( + scope, + Some((op_id, buf)), + js_recv_cb, + js_error_create_fn, + )?; } - inner.check_promise_exceptions(scope)?; + check_promise_exceptions( + scope, + pending_promise_exceptions, + js_error_create_fn, + )?; // We're idle if pending_ops is empty. if inner.pending_ops.is_empty() { @@ -702,26 +574,147 @@ impl Future for Isolate { } } -/// IsolateHandle is a thread safe handle on an Isolate. It exposed thread safe V8 functions. -#[derive(Clone)] -pub struct IsolateHandle { - shared_isolate: Arc<Mutex<Option<*mut v8::Isolate>>>, +fn async_op_response<'s>( + scope: &mut impl v8::ToLocal<'s>, + maybe_buf: Option<(OpId, Box<[u8]>)>, + js_recv_cb: &v8::Global<v8::Function>, + js_error_create_fn: &JSErrorCreateFn, +) -> Result<(), ErrBox> { + let context = scope.get_current_context().unwrap(); + let global: v8::Local<v8::Value> = context.global(scope).into(); + let js_recv_cb = js_recv_cb + .get(scope) + .expect("Deno.core.recv has not been called."); + + // TODO(piscisaureus): properly integrate TryCatch in the scope chain. + let mut try_catch = v8::TryCatch::new(scope); + let tc = try_catch.enter(); + + match maybe_buf { + Some((op_id, buf)) => { + let op_id: v8::Local<v8::Value> = + v8::Integer::new(scope, op_id as i32).into(); + let ui8: v8::Local<v8::Value> = + bindings::boxed_slice_to_uint8array(scope, buf).into(); + js_recv_cb.call(scope, context, global, &[op_id, ui8]) + } + None => js_recv_cb.call(scope, context, global, &[]), + }; + + match tc.exception() { + None => Ok(()), + Some(exception) => { + exception_to_err_result(scope, exception, js_error_create_fn) + } + } } -unsafe impl Send for IsolateHandle {} +pub(crate) fn attach_handle_to_error( + scope: &mut impl v8::InIsolate, + err: ErrBox, + handle: v8::Local<v8::Value>, +) -> ErrBox { + ErrWithV8Handle::new(scope, err, handle).into() +} -impl IsolateHandle { - /// Terminate the execution of any currently running javascript. - /// After terminating execution it is probably not wise to continue using - /// the isolate. - pub fn terminate_execution(&self) { - if let Some(isolate) = *self.shared_isolate.lock().unwrap() { - let isolate = unsafe { &mut *isolate }; - isolate.terminate_execution(); +pub(crate) fn exception_to_err_result<'a, T>( + scope: &mut impl v8::ToLocal<'a>, + exception: v8::Local<v8::Value>, + js_error_create_fn: &JSErrorCreateFn, +) -> Result<T, ErrBox> { + let mut last_exception = Option::<String>::None; + handle_exception(scope, exception, &mut last_exception); + check_last_exception(&mut last_exception, js_error_create_fn) + .map(|_| unreachable!()) +} + +pub(crate) fn handle_exception<'a>( + scope: &mut impl v8::ToLocal<'a>, + exception: v8::Local<v8::Value>, + last_exception: &mut Option<String>, // Out parameter. +) { + // Use a HandleScope because the functions below create a lot of + // local handles (in particular, `encode_message_as_json()` does). + let mut hs = v8::HandleScope::new(scope); + let scope = hs.enter(); + + // TODO(piscisaureus): in rusty_v8, `is_execution_terminating()` should + // also be implemented on `struct Isolate`. + let is_terminating_exception = scope + .isolate() + .thread_safe_handle() + .is_execution_terminating(); + let mut exception = exception; + + if is_terminating_exception { + // TerminateExecution was called. Cancel exception termination so that the + // exception can be created.. + // TODO(piscisaureus): in rusty_v8, `cancel_terminate_execution()` should + // also be implemented on `struct Isolate`. + scope + .isolate() + .thread_safe_handle() + .cancel_terminate_execution(); + + // Maybe make a new exception object. + if exception.is_null_or_undefined() { + let exception_str = + v8::String::new(scope, "execution terminated").unwrap(); + exception = v8::Exception::error(scope, exception_str); + } + } + + let message = v8::Exception::create_message(scope, exception); + let json_str = encode_message_as_json(scope, message); + let prev_last_exception = last_exception.replace(json_str); + assert_eq!(prev_last_exception, None); + + if is_terminating_exception { + // Re-enable exception termination. + // TODO(piscisaureus): in rusty_v8, `terminate_execution()` should also + // be implemented on `struct Isolate`. + scope.isolate().thread_safe_handle().terminate_execution(); + } +} + +pub(crate) fn check_last_exception( + last_exception: &mut Option<String>, + js_error_create_fn: &JSErrorCreateFn, +) -> Result<(), ErrBox> { + match last_exception.take() { + None => Ok(()), + Some(json_str) => { + let v8_exception = V8Exception::from_json(&json_str).unwrap(); + let js_error = (js_error_create_fn)(v8_exception); + Err(js_error) } } } +fn check_promise_exceptions<'s>( + scope: &mut impl v8::ToLocal<'s>, + pending_promise_exceptions: &mut HashMap<i32, v8::Global<v8::Value>>, + js_error_create_fn: &JSErrorCreateFn, +) -> Result<(), ErrBox> { + if let Some(&key) = pending_promise_exceptions.keys().next() { + let handle = pending_promise_exceptions.remove(&key).unwrap(); + let exception = handle.get(scope).expect("empty error handle"); + exception_to_err_result(scope, exception, js_error_create_fn) + } else { + Ok(()) + } +} + +pub fn encode_message_as_json<'a>( + scope: &mut impl v8::ToLocal<'a>, + message: v8::Local<v8::Message>, +) -> String { + let context = scope.get_current_context().unwrap(); + let json_obj = bindings::encode_message_as_object(scope, message); + let json_string = v8::json::stringify(context, json_obj.into()).unwrap(); + json_string.to_rust_string_lossy(scope) +} + pub fn js_check<T>(r: Result<T, ErrBox>) -> T { if let Err(e) = r { panic!(e.to_string()); @@ -940,60 +933,61 @@ pub mod tests { #[test] fn terminate_execution() { - let (tx, rx) = std::sync::mpsc::channel::<bool>(); - let tx_clone = tx.clone(); - let (mut isolate, _dispatch_count) = setup(Mode::Async); - let shared = isolate.shared_isolate_handle(); + // TODO(piscisaureus): in rusty_v8, the `thread_safe_handle()` method + // should not require a mutable reference to `struct rusty_v8::Isolate`. + let v8_isolate_handle = + isolate.v8_isolate.as_mut().unwrap().thread_safe_handle(); - let t1 = std::thread::spawn(move || { + let terminator_thread = std::thread::spawn(move || { // allow deno to boot and run std::thread::sleep(std::time::Duration::from_millis(100)); // terminate execution - shared.terminate_execution(); - - // allow shutdown - std::thread::sleep(std::time::Duration::from_millis(200)); - - // unless reported otherwise the test should fail after this point - tx_clone.send(false).ok(); + let ok = v8_isolate_handle.terminate_execution(); + assert!(ok, true); }); - let t2 = std::thread::spawn(move || { - // Rn an infinite loop, which should be terminated. - match isolate.execute("infinite_loop.js", "for(;;) {}") { - Ok(_) => panic!("execution should be terminated"), - Err(e) => { - assert_eq!(e.to_string(), "Uncaught Error: execution terminated") - } - }; - - // `execute()` returned, which means `terminate_execution()` worked. - tx.send(true).ok(); - - // Make sure the isolate unusable again. - isolate - .execute("simple.js", "1 + 1") - .expect("execution should be possible again"); - }); + // Rn an infinite loop, which should be terminated. + match isolate.execute("infinite_loop.js", "for(;;) {}") { + Ok(_) => panic!("execution should be terminated"), + Err(e) => { + assert_eq!(e.to_string(), "Uncaught Error: execution terminated") + } + }; - rx.recv().expect("execution should be terminated"); + // Cancel the execution-terminating exception in order to allow script + // execution again. + // TODO(piscisaureus): in rusty_v8, `cancel_terminate_execution()` should + // also be implemented on `struct Isolate`. + let ok = isolate + .v8_isolate + .as_mut() + .unwrap() + .thread_safe_handle() + .cancel_terminate_execution(); + assert!(ok); + + // Verify that the isolate usable again. + isolate + .execute("simple.js", "1 + 1") + .expect("execution should be possible again"); - t1.join().unwrap(); - t2.join().unwrap(); + terminator_thread.join().unwrap(); } #[test] fn dangling_shared_isolate() { - let shared = { + let v8_isolate_handle = { // isolate is dropped at the end of this block let (mut isolate, _dispatch_count) = setup(Mode::Async); - isolate.shared_isolate_handle() + // TODO(piscisaureus): in rusty_v8, the `thread_safe_handle()` method + // should not require a mutable reference to `struct rusty_v8::Isolate`. + isolate.v8_isolate.as_mut().unwrap().thread_safe_handle() }; // this should not SEGFAULT - shared.terminate_execution(); + v8_isolate_handle.terminate_execution(); } #[test] |