From faa64edaf409757549a7df85812f6ea4f368051c Mon Sep 17 00:00:00 2001 From: Bert Belder Date: Sat, 18 Jul 2020 22:32:11 +0200 Subject: [PATCH] Upgrade to rusty_v8 0.7.0 (#6801) --- Cargo.lock | 4 +- cli/inspector.rs | 6 ++- core/Cargo.toml | 2 +- core/bindings.rs | 113 +++++++++++++++++++++------------------ core/core_isolate.rs | 78 ++++++++++++--------------- core/errors.rs | 9 ++-- core/es_isolate.rs | 122 ++++++++++++++++++------------------------- 7 files changed, 162 insertions(+), 172 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 126dbb0121..1f1aeb62eb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1880,9 +1880,9 @@ dependencies = [ [[package]] name = "rusty_v8" -version = "0.6.0" +version = "0.7.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "da4dd2d680bf9812b3156b44f89fdd7b508c1369fbc69a14150972c6919c84d1" +checksum = "390f2a21e9855f0cf6113a076cabdfd880b13c055391d61474d5eebf2c1ed491" dependencies = [ "bitflags", "cargo_gn", diff --git a/cli/inspector.rs b/cli/inspector.rs index 6c08aad24c..b4d377cefb 100644 --- a/cli/inspector.rs +++ b/cli/inspector.rs @@ -415,7 +415,11 @@ impl DenoInspector { }); // Tell the inspector about the global context. - let context = core_state.global_context.get(scope).unwrap(); + let context = core_state + .global_context + .as_ref() + .map(|context| v8::Local::new(scope, context)) + .unwrap(); let context_name = v8::inspector::StringView::from(&b"global context"[..]); self_.context_created(context, Self::CONTEXT_GROUP_ID, context_name); diff --git a/core/Cargo.toml b/core/Cargo.toml index 1a0ef879fd..d1bc4ce934 100644 --- a/core/Cargo.toml +++ b/core/Cargo.toml @@ -19,7 +19,7 @@ futures = { version = "0.3.5", features = ["thread-pool", "compat"] } lazy_static = "1.4.0" libc = "0.2.71" log = "0.4.8" -rusty_v8 = "0.6.0" +rusty_v8 = "0.7.0" serde_json = "1.0.55" smallvec = "1.4.0" url = "2.1.1" diff --git a/core/bindings.rs b/core/bindings.rs index 0d0a633a58..1c76cf56f5 100644 --- a/core/bindings.rs +++ b/core/bindings.rs @@ -1,6 +1,7 @@ // Copyright 2018-2020 the Deno authors. All rights reserved. MIT license. use crate::CoreIsolate; +use crate::CoreIsolateState; use crate::EsIsolate; use crate::JSError; use crate::ZeroCopyBuf; @@ -236,9 +237,7 @@ pub extern "C" fn host_import_module_dynamically_callback( let resolver = v8::PromiseResolver::new(scope).unwrap(); let promise = resolver.get_promise(scope); - let mut resolver_handle = v8::Global::new(); - resolver_handle.set(scope, resolver); - + let resolver_handle = v8::Global::new(scope, resolver); { let state_rc = EsIsolate::state(scope); let mut state = state_rc.borrow_mut(); @@ -283,18 +282,13 @@ pub extern "C" fn promise_reject_callback(message: v8::PromiseRejectMessage) { match message.get_event() { v8::PromiseRejectEvent::PromiseRejectWithNoHandler => { let error = message.get_value(); - let mut error_global = v8::Global::::new(); - error_global.set(scope, error); + let error_global = v8::Global::new(scope, error); state .pending_promise_exceptions .insert(promise_id, error_global); } v8::PromiseRejectEvent::PromiseHandlerAddedAfterReject => { - if let Some(mut handle) = - state.pending_promise_exceptions.remove(&promise_id) - { - handle.reset(scope); - } + state.pending_promise_exceptions.remove(&promise_id); } v8::PromiseRejectEvent::PromiseRejectAfterResolved => {} v8::PromiseRejectEvent::PromiseResolveAfterResolved => { @@ -364,14 +358,17 @@ fn recv( let state_rc = CoreIsolate::state(scope); let mut state = state_rc.borrow_mut(); - if !state.js_recv_cb.is_empty() { - let msg = v8::String::new(scope, "Deno.core.recv already called.").unwrap(); - scope.throw_exception(msg.into()); - return; - } + let cb = match v8::Local::::try_from(args.get(0)) { + Ok(cb) => cb, + Err(err) => return throw_type_error(scope, err.to_string()), + }; - let recv_fn = v8::Local::::try_from(args.get(0)).unwrap(); - state.js_recv_cb.set(scope, recv_fn); + let slot = match &mut state.js_recv_cb { + slot @ None => slot, + _ => return throw_type_error(scope, "Deno.core.recv() already called"), + }; + + slot.replace(v8::Global::new(scope, cb)); } fn send( @@ -391,7 +388,6 @@ fn send( let state_rc = CoreIsolate::state(scope); let mut state = state_rc.borrow_mut(); - assert!(!state.global_context.is_empty()); let buf_iter = (1..args.length()).map(|idx| { v8::Local::::try_from(args.get(idx)) @@ -433,17 +429,22 @@ fn set_macrotask_callback( let state_rc = CoreIsolate::state(scope); let mut state = state_rc.borrow_mut(); - if !state.js_macrotask_cb.is_empty() { - let msg = - v8::String::new(scope, "Deno.core.setMacrotaskCallback already called.") - .unwrap(); - scope.throw_exception(msg.into()); - return; - } + let cb = match v8::Local::::try_from(args.get(0)) { + Ok(cb) => cb, + Err(err) => return throw_type_error(scope, err.to_string()), + }; - let macrotask_cb_fn = - v8::Local::::try_from(args.get(0)).unwrap(); - state.js_macrotask_cb.set(scope, macrotask_cb_fn); + let slot = match &mut state.js_macrotask_cb { + slot @ None => slot, + _ => { + return throw_type_error( + scope, + "Deno.core.setMacrotaskCallback() already called", + ); + } + }; + + slot.replace(v8::Global::new(scope, cb)); } fn eval_context( @@ -668,18 +669,23 @@ fn shared_getter( ) { let state_rc = CoreIsolate::state(scope); let mut state = state_rc.borrow_mut(); + let CoreIsolateState { + shared_ab, shared, .. + } = &mut *state; // Lazily initialize the persistent external ArrayBuffer. - if state.shared_ab.is_empty() { - let ab = v8::SharedArrayBuffer::with_backing_store( - scope, - state.shared.get_backing_store(), - ); - state.shared_ab.set(scope, ab); - } - - let shared_ab = state.shared_ab.get(scope).unwrap(); - rv.set(shared_ab.into()); + let shared_ab = match shared_ab { + Some(ref ab) => v8::Local::new(scope, ab), + slot @ None => { + let ab = v8::SharedArrayBuffer::with_backing_store( + scope, + shared.get_backing_store(), + ); + slot.replace(v8::Global::new(scope, ab)); + ab + } + }; + rv.set(shared_ab.into()) } pub fn module_resolve_callback<'s>( @@ -709,19 +715,17 @@ pub fn module_resolve_callback<'s>( if req_str == specifier_str { let id = state.module_resolve_cb(&req_str, referrer_id); - let maybe_info = state.modules.get_info(id); - - if maybe_info.is_none() { - let msg = format!( - "Cannot resolve module \"{}\" from \"{}\"", - req_str, referrer_name - ); - let msg = v8::String::new(scope, &msg).unwrap(); - scope.throw_exception(msg.into()); - break; + match state.modules.get_info(id) { + Some(info) => return Some(v8::Local::new(scope, &info.handle)), + None => { + let msg = format!( + r#"Cannot resolve module "{}" from "{}""#, + req_str, referrer_name + ); + throw_type_error(scope, msg); + return None; + } } - - return maybe_info.and_then(|i| i.handle.get(scope)); } } @@ -775,3 +779,12 @@ fn get_promise_details( } } } + +fn throw_type_error<'s>( + scope: &mut v8::HandleScope<'s>, + message: impl AsRef, +) { + let message = v8::String::new(scope, message.as_ref()).unwrap(); + let exception = v8::Exception::type_error(scope, message); + scope.throw_exception(exception); +} diff --git a/core/core_isolate.rs b/core/core_isolate.rs index c9e718d2db..74bb2f545f 100644 --- a/core/core_isolate.rs +++ b/core/core_isolate.rs @@ -95,10 +95,10 @@ pub struct CoreIsolate { /// embedder slots. pub struct CoreIsolateState { pub resource_table: Rc>, - pub global_context: v8::Global, - pub(crate) shared_ab: v8::Global, - pub(crate) js_recv_cb: v8::Global, - pub(crate) js_macrotask_cb: v8::Global, + pub global_context: Option>, + pub(crate) shared_ab: Option>, + pub(crate) js_recv_cb: Option>, + pub(crate) js_macrotask_cb: Option>, pub(crate) pending_promise_exceptions: HashMap>, pub(crate) js_error_create_fn: Box, pub(crate) shared: SharedQueue, @@ -177,7 +177,7 @@ impl CoreIsolate { StartupData::None => (None, None), }; - let mut global_context = v8::Global::::new(); + let global_context; let (mut isolate, maybe_snapshot_creator) = if will_snapshot { // TODO(ry) Support loading snapshots before snapshotting. assert!(startup_snapshot.is_none()); @@ -188,7 +188,7 @@ impl CoreIsolate { { let scope = &mut v8::HandleScope::new(&mut isolate); let context = bindings::initialize_context(scope); - global_context.set(scope, context); + global_context = v8::Global::new(scope, context); creator.set_default_context(context); } (isolate, Some(creator)) @@ -217,18 +217,18 @@ impl CoreIsolate { // main source code and source maps. bindings::initialize_context(scope) }; - global_context.set(scope, context); + global_context = v8::Global::new(scope, context); } (isolate, None) }; isolate.set_slot(Rc::new(RefCell::new(CoreIsolateState { - global_context, + global_context: Some(global_context), resource_table: Rc::new(RefCell::new(ResourceTable::default())), pending_promise_exceptions: HashMap::new(), - shared_ab: v8::Global::::new(), - js_recv_cb: v8::Global::::new(), - js_macrotask_cb: v8::Global::::new(), + shared_ab: None, + js_recv_cb: None, + js_macrotask_cb: None, js_error_create_fn: Box::new(JSError::create), shared: SharedQueue::new(RECOMMENDED_SIZE), pending_ops: FuturesUnordered::new(), @@ -285,9 +285,10 @@ impl CoreIsolate { let state_rc = Self::state(self); let state = state_rc.borrow(); - let scope = &mut v8::HandleScope::new(self.v8_isolate.as_mut().unwrap()); - let context = state.global_context.get(scope).unwrap(); - let scope = &mut v8::ContextScope::new(scope, context); + let scope = &mut v8::HandleScope::with_context( + self.v8_isolate.as_mut().unwrap(), + state.global_context.as_ref().unwrap(), + ); drop(state); @@ -326,13 +327,8 @@ impl CoreIsolate { let state = Self::state(self); // Note: create_blob() method must not be called from within a HandleScope. - // The HandleScope created here is exited at the end of the block. // TODO(piscisaureus): The rusty_v8 type system should enforce this. - { - let v8_isolate = self.v8_isolate.as_mut().unwrap(); - let scope = &mut v8::HandleScope::new(v8_isolate); - state.borrow_mut().global_context.reset(scope); - } + state.borrow_mut().global_context.take(); let snapshot_creator = self.snapshot_creator.as_mut().unwrap(); let snapshot = snapshot_creator @@ -372,12 +368,10 @@ impl Future for CoreIsolate { state.waker.register(cx.waker()); } - let scope = &mut v8::HandleScope::new(&mut **core_isolate); - let context = { - let state = state_rc.borrow(); - state.global_context.get(scope).unwrap() - }; - let scope = &mut v8::ContextScope::new(scope, context); + let scope = &mut v8::HandleScope::with_context( + &mut **core_isolate, + state_rc.borrow().global_context.as_ref().unwrap(), + ); check_promise_exceptions(scope)?; @@ -528,15 +522,12 @@ fn async_op_response<'s>( ) -> Result<(), ErrBox> { let context = scope.get_current_context(); let global: v8::Local = context.global(scope).into(); - - let state_rc = CoreIsolate::state(scope); - let state = state_rc.borrow_mut(); - - let js_recv_cb = state + let js_recv_cb = CoreIsolate::state(scope) + .borrow() .js_recv_cb - .get(scope) + .as_ref() + .map(|cb| v8::Local::new(scope, cb)) .expect("Deno.core.recv has not been called."); - drop(state); let tc_scope = &mut v8::TryCatch::new(scope); @@ -561,22 +552,21 @@ fn drain_macrotasks<'s>(scope: &mut v8::HandleScope<'s>) -> Result<(), ErrBox> { let context = scope.get_current_context(); let global: v8::Local = context.global(scope).into(); - let js_macrotask_cb = { - let state_rc = CoreIsolate::state(scope); - let state = state_rc.borrow_mut(); - state.js_macrotask_cb.get(scope) + let js_macrotask_cb = match CoreIsolate::state(scope) + .borrow_mut() + .js_macrotask_cb + .as_ref() + { + Some(cb) => v8::Local::new(scope, cb), + None => return Ok(()), }; - if js_macrotask_cb.is_none() { - return Ok(()); - } - let js_macrotask_cb = js_macrotask_cb.unwrap(); // Repeatedly invoke macrotask callback until it returns true (done), // such that ready microtasks would be automatically run before // next macrotask is processed. - loop { - let tc_scope = &mut v8::TryCatch::new(scope); + let tc_scope = &mut v8::TryCatch::new(scope); + loop { let is_done = js_macrotask_cb.call(tc_scope, global, &[]); if let Some(exception) = tc_scope.exception() { @@ -641,7 +631,7 @@ fn check_promise_exceptions<'s>( if let Some(&key) = state.pending_promise_exceptions.keys().next() { let handle = state.pending_promise_exceptions.remove(&key).unwrap(); drop(state); - let exception = handle.get(scope).expect("empty error handle"); + let exception = v8::Local::new(scope, handle); exception_to_err_result(scope, exception) } else { Ok(()) diff --git a/core/errors.rs b/core/errors.rs index 27c08fac5a..218d7c6197 100644 --- a/core/errors.rs +++ b/core/errors.rs @@ -378,12 +378,15 @@ impl ErrWithV8Handle { err: ErrBox, handle: v8::Local, ) -> Self { - let handle = v8::Global::new_from(scope, handle); + let handle = v8::Global::new(scope, handle); Self { err, handle } } - pub fn get_handle(&self) -> &v8::Global { - &self.handle + pub fn get_handle<'s>( + &self, + scope: &mut v8::HandleScope<'s>, + ) -> v8::Local<'s, v8::Value> { + v8::Local::new(scope, &self.handle) } } diff --git a/core/es_isolate.rs b/core/es_isolate.rs index a59ba7e92d..35e215cd7c 100644 --- a/core/es_isolate.rs +++ b/core/es_isolate.rs @@ -112,10 +112,10 @@ impl EsIsolate { ) -> Result { let state_rc = Self::state(self); let core_state_rc = CoreIsolate::state(self); - let core_state = core_state_rc.borrow(); - let scope = &mut v8::HandleScope::new(&mut *self.0); - let context = core_state.global_context.get(scope).unwrap(); - let scope = &mut v8::ContextScope::new(scope, context); + let scope = &mut v8::HandleScope::with_context( + &mut *self.0, + core_state_rc.borrow().global_context.as_ref().unwrap(), + ); let name_str = v8::String::new(scope, name).unwrap(); let source_str = v8::String::new(scope, source).unwrap(); @@ -146,15 +146,14 @@ impl EsIsolate { import_specifiers.push(module_specifier); } - let mut handle = v8::Global::::new(); - handle.set(tc_scope, module); + state_rc.borrow_mut().modules.register( + id, + name, + main, + v8::Global::::new(tc_scope, module), + import_specifiers, + ); - { - let mut state = state_rc.borrow_mut(); - state - .modules - .register(id, name, main, handle, import_specifiers); - } Ok(id) } @@ -164,23 +163,19 @@ impl EsIsolate { /// the V8 exception. By default this type is JSError, however it may be a /// different type if CoreIsolate::set_js_error_create_fn() has been used. fn mod_instantiate(&mut self, id: ModuleId) -> Result<(), ErrBox> { - let state_rc = Self::state(self); - let state = state_rc.borrow(); - let core_state_rc = CoreIsolate::state(self); let core_state = core_state_rc.borrow(); - let scope = &mut v8::HandleScope::new(&mut *self.0); - let context = core_state.global_context.get(scope).unwrap(); - let scope = &mut v8::ContextScope::new(scope, context); + let scope = &mut v8::HandleScope::with_context( + &mut *self.0, + core_state.global_context.as_ref().unwrap(), + ); let tc_scope = &mut v8::TryCatch::new(scope); - let module_info = match state.modules.get_info(id) { - Some(info) => info, + let module = match Self::state(tc_scope).borrow().modules.get_info(id) { + Some(info) => v8::Local::new(tc_scope, &info.handle), None if id == 0 => return Ok(()), _ => panic!("module id {} not found in module table", id), }; - let module = module_info.handle.get(tc_scope).unwrap(); - drop(state); if module.get_status() == v8::ModuleStatus::Errored { exception_to_err_result(tc_scope, module.get_exception())? @@ -204,22 +199,22 @@ impl EsIsolate { /// different type if CoreIsolate::set_js_error_create_fn() has been used. pub fn mod_evaluate(&mut self, id: ModuleId) -> Result<(), ErrBox> { self.shared_init(); - let state_rc = Self::state(self); - let state = state_rc.borrow(); let core_state_rc = CoreIsolate::state(self); - let scope = &mut v8::HandleScope::new(&mut *self.0); - let context = { - let core_state = core_state_rc.borrow(); - core_state.global_context.get(scope).unwrap() - }; - let scope = &mut v8::ContextScope::new(scope, context); + let scope = &mut v8::HandleScope::with_context( + &mut *self.0, + core_state_rc.borrow().global_context.as_ref().unwrap(), + ); - let info = state.modules.get_info(id).expect("ModuleInfo not found"); - let module = info.handle.get(scope).expect("Empty module handle"); + let module = Self::state(scope) + .borrow() + .modules + .get_info(id) + .map(|info| v8::Local::new(scope, &info.handle)) + .expect("ModuleInfo not found"); let mut status = module.get_status(); - drop(state); + if status == v8::ModuleStatus::Instantiated { // IMPORTANT: Top-level-await is enabled, which means that return value // of module evaluation is a promise. @@ -251,11 +246,7 @@ impl EsIsolate { .expect("Expected to get promise as module evaluation result"); let promise_id = promise.get_identity_hash(); let mut core_state = core_state_rc.borrow_mut(); - if let Some(mut handle) = - core_state.pending_promise_exceptions.remove(&promise_id) - { - handle.reset(scope); - } + core_state.pending_promise_exceptions.remove(&promise_id); } else { assert!(status == v8::ModuleStatus::Errored); } @@ -278,29 +269,23 @@ impl EsIsolate { err: ErrBox, ) -> Result<(), ErrBox> { let state_rc = Self::state(self); - let mut state = state_rc.borrow_mut(); - let core_state_rc = CoreIsolate::state(self); - let core_state = core_state_rc.borrow(); - let scope = &mut v8::HandleScope::new(&mut *self.0); - let context = core_state.global_context.get(scope).unwrap(); - let scope = &mut v8::ContextScope::new(scope, context); + let scope = &mut v8::HandleScope::with_context( + &mut *self.0, + core_state_rc.borrow().global_context.as_ref().unwrap(), + ); - drop(core_state); - - let mut resolver_handle = state + let resolver_handle = state_rc + .borrow_mut() .dyn_import_map .remove(&id) .expect("Invalid dyn import id"); - let resolver = resolver_handle.get(scope).unwrap(); - resolver_handle.reset(scope); - - drop(state); + let resolver = resolver_handle.get(scope); let exception = err .downcast_ref::() - .and_then(|err| err.get_handle().get(scope)) + .map(|err| err.get_handle(scope)) .unwrap_or_else(|| { let message = err.to_string(); let message = v8::String::new(scope, &message).unwrap(); @@ -323,32 +308,27 @@ impl EsIsolate { debug!("dyn_import_done {} {:?}", id, mod_id); assert!(mod_id != 0); - let scope = &mut v8::HandleScope::new(&mut *self.0); - let context = { - let core_state = core_state_rc.borrow(); - core_state.global_context.get(scope).unwrap() - }; - let scope = &mut v8::ContextScope::new(scope, context); + let scope = &mut v8::HandleScope::with_context( + &mut *self.0, + core_state_rc.borrow().global_context.as_ref().unwrap(), + ); - let mut resolver_handle = { - let mut state = state_rc.borrow_mut(); - state - .dyn_import_map - .remove(&id) - .expect("Invalid dyn import id") - }; - let resolver = resolver_handle.get(scope).unwrap(); - resolver_handle.reset(scope); + let resolver_handle = state_rc + .borrow_mut() + .dyn_import_map + .remove(&id) + .expect("Invalid dyn import id"); + let resolver = resolver_handle.get(scope); let module = { let state = state_rc.borrow(); - let info = state + state .modules .get_info(mod_id) - .expect("Dyn import module info not found"); - // Resolution success - info.handle.get(scope).unwrap() + .map(|info| v8::Local::new(scope, &info.handle)) + .expect("Dyn import module info not found") }; + // Resolution success assert_eq!(module.get_status(), v8::ModuleStatus::Evaluated); let module_namespace = module.get_module_namespace();