From 51737506dd12610bfe332799a6de8692e0e74941 Mon Sep 17 00:00:00 2001 From: Ry Dahl Date: Mon, 23 Dec 2019 18:09:03 -0500 Subject: [PATCH] make InstantiateModule work (#124) --- src/binding.cc | 26 +++++++- src/context.rs | 14 ++++- src/module.rs | 41 ++++++++++++- src/primitives.rs | 9 ++- tests/test_api.rs | 147 +++++++++++++++++++++++++++++++++++++--------- 5 files changed, 199 insertions(+), 38 deletions(-) diff --git a/src/binding.cc b/src/binding.cc index 8ebe6577..6f44da4b 100644 --- a/src/binding.cc +++ b/src/binding.cc @@ -778,10 +778,32 @@ int v8__Module__GetIdentityHash(const v8::Module& self) { return self.GetIdentityHash(); } +// This is an extern C calling convention compatible version of +// v8::Module::ResolveCallback. +typedef v8::Module* (*v8__Module__ResolveCallback)( + v8::Local context, v8::Local specifier, + v8::Local referrer); + MaybeBool v8__Module__InstantiateModule(v8::Module& self, v8::Local context, - v8::Module::ResolveCallback callback) { - return maybe_to_maybe_bool(self.InstantiateModule(context, callback)); + v8__Module__ResolveCallback c_cb) { + static v8__Module__ResolveCallback static_cb = nullptr; + assert(static_cb == nullptr); + static_cb = c_cb; + auto cxx_cb = [](v8::Local context, + v8::Local specifier, + v8::Local referrer) { + v8::Module* m = static_cb(context, specifier, referrer); + if (m == nullptr) { + return v8::MaybeLocal(); + } else { + return v8::MaybeLocal(ptr_to_local(m)); + } + }; + + auto r = maybe_to_maybe_bool(self.InstantiateModule(context, cxx_cb)); + static_cb = nullptr; + return r; } v8::Value* v8__Module__Evaluate(v8::Module& self, diff --git a/src/context.rs b/src/context.rs index 4e8c10a4..a3b04d50 100644 --- a/src/context.rs +++ b/src/context.rs @@ -9,7 +9,7 @@ extern "C" { fn v8__Context__New(isolate: &Isolate) -> *mut Context; fn v8__Context__Enter(this: &mut Context); fn v8__Context__Exit(this: &mut Context); - fn v8__Context__GetIsolate(this: &mut Context) -> *mut Isolate; + fn v8__Context__GetIsolate<'sc>(this: &'sc Context) -> &'sc mut Isolate; fn v8__Context__Global(this: *mut Context) -> *mut Object; } @@ -54,3 +54,15 @@ impl Context { unsafe { v8__Context__Exit(self) }; } } + +impl AsRef for Context { + fn as_ref(&self) -> &Isolate { + unsafe { v8__Context__GetIsolate(self) } + } +} + +impl AsMut for Context { + fn as_mut(&mut self) -> &mut Isolate { + unsafe { v8__Context__GetIsolate(self) } + } +} diff --git a/src/module.rs b/src/module.rs index 09c7cbe5..95d9d32a 100644 --- a/src/module.rs +++ b/src/module.rs @@ -7,9 +7,16 @@ use crate::String; use crate::Value; use std::mem::MaybeUninit; -type ResolveCallback = +#[allow(non_camel_case_types)] +type v8__Module__ResolveCallback = extern "C" fn(Local, Local, Local) -> *mut Module; +type ResolveCallback = fn( + Local, + Local, + Local, +) -> Option>; + extern "C" { fn v8__Module__GetStatus(this: *const Module) -> ModuleStatus; fn v8__Module__GetException(this: *const Module) -> *mut Value; @@ -25,7 +32,7 @@ extern "C" { fn v8__Module__InstantiateModule( this: *mut Module, context: Local, - callback: ResolveCallback, + callback: v8__Module__ResolveCallback, ) -> MaybeBool; fn v8__Module__Evaluate( this: *mut Module, @@ -117,7 +124,35 @@ impl Module { context: Local, callback: ResolveCallback, ) -> Option { - unsafe { v8__Module__InstantiateModule(self, context, callback) }.into() + use std::sync::Mutex; + lazy_static! { + static ref RESOLVE_CALLBACK: Mutex> = + Mutex::new(None); + static ref INSTANTIATE_LOCK: Mutex<()> = Mutex::new(()); + } + let instantiate_guard = INSTANTIATE_LOCK.lock().unwrap(); + + { + let mut guard = RESOLVE_CALLBACK.lock().unwrap(); + *guard = Some(callback); + } + + extern "C" fn c_cb( + context: Local, + specifier: Local, + referrer: Local, + ) -> *mut Module { + let guard = RESOLVE_CALLBACK.lock().unwrap(); + let cb = guard.unwrap(); + match cb(context, specifier, referrer) { + None => std::ptr::null_mut(), + Some(mut p) => &mut *p, + } + } + let r = + unsafe { v8__Module__InstantiateModule(self, context, c_cb) }.into(); + drop(instantiate_guard); + r } /// Evaluates the module and its dependencies. diff --git a/src/primitives.rs b/src/primitives.rs index 8f84649e..bc636a60 100644 --- a/src/primitives.rs +++ b/src/primitives.rs @@ -2,7 +2,6 @@ use std::ops::Deref; use crate::isolate::Isolate; use crate::support::Opaque; -use crate::HandleScope; use crate::Local; use crate::Value; @@ -29,21 +28,21 @@ extern "C" { fn v8__False(isolate: *mut Isolate) -> *mut Boolean; } -pub fn new_null<'sc>(scope: &mut HandleScope<'sc>) -> Local<'sc, Primitive> { +pub fn new_null<'sc>(scope: &mut impl AsMut) -> Local<'sc, Primitive> { unsafe { Local::from_raw(v8__Null(scope.as_mut())) }.unwrap() } pub fn new_undefined<'sc>( - scope: &mut HandleScope<'sc>, + scope: &mut impl AsMut, ) -> Local<'sc, Primitive> { unsafe { Local::from_raw(v8__Undefined(scope.as_mut())) }.unwrap() } -pub fn new_true<'sc>(scope: &mut HandleScope<'sc>) -> Local<'sc, Boolean> { +pub fn new_true<'sc>(scope: &mut impl AsMut) -> Local<'sc, Boolean> { unsafe { Local::from_raw(v8__True(scope.as_mut())) }.unwrap() } -pub fn new_false<'sc>(scope: &mut HandleScope<'sc>) -> Local<'sc, Boolean> { +pub fn new_false<'sc>(scope: &mut impl AsMut) -> Local<'sc, Boolean> { unsafe { Local::from_raw(v8__False(scope.as_mut())) }.unwrap() } diff --git a/tests/test_api.rs b/tests/test_api.rs index 3c629308..b74c0e66 100644 --- a/tests/test_api.rs +++ b/tests/test_api.rs @@ -209,25 +209,25 @@ fn array_buffer() { } fn v8_str<'sc>( - scope: &mut HandleScope<'sc>, + isolate: &mut impl AsMut, s: &str, ) -> v8::Local<'sc, v8::String> { - v8::String::new(scope, s).unwrap() + v8::String::new(isolate, s).unwrap() +} + +fn eval<'sc>( + scope: &mut HandleScope<'sc>, + context: Local, + code: &'static str, +) -> Option> { + let source = v8_str(scope, code); + let mut script = + v8::Script::compile(&mut *scope, context, source, None).unwrap(); + script.run(scope, context) } #[test] fn try_catch() { - fn eval<'sc>( - scope: &mut HandleScope<'sc>, - context: Local, - code: &'static str, - ) -> Option> { - let source = v8_str(scope, code); - let mut script = - v8::Script::compile(&mut *scope, context, source, None).unwrap(); - script.run(scope, context) - }; - let _g = setup(); let mut params = v8::Isolate::create_params(); params.set_array_buffer_allocator(v8::Allocator::new_default_allocator()); @@ -736,17 +736,18 @@ fn set_promise_reject_callback() { } fn mock_script_origin<'sc>( - scope: &mut HandleScope<'sc>, + isolate: &mut impl AsMut, + resource_name_: &str, ) -> v8::ScriptOrigin<'sc> { - let resource_name = v8_str(scope, "foo.js"); - let resource_line_offset = v8::Integer::new(scope, 0); - let resource_column_offset = v8::Integer::new(scope, 0); - let resource_is_shared_cross_origin = v8::new_true(scope); - let script_id = v8::Integer::new(scope, 123); - let source_map_url = v8_str(scope, "source_map_url"); - let resource_is_opaque = v8::new_true(scope); - let is_wasm = v8::new_false(scope); - let is_module = v8::new_true(scope); + let resource_name = v8_str(isolate, resource_name_); + let resource_line_offset = v8::Integer::new(isolate, 0); + let resource_column_offset = v8::Integer::new(isolate, 0); + let resource_is_shared_cross_origin = v8::new_true(isolate); + let script_id = v8::Integer::new(isolate, 123); + let source_map_url = v8_str(isolate, "source_map_url"); + let resource_is_opaque = v8::new_true(isolate); + let is_wasm = v8::new_false(isolate); + let is_module = v8::new_true(isolate); v8::ScriptOrigin::new( resource_name.into(), resource_line_offset, @@ -774,7 +775,7 @@ fn script_compiler_source() { context.enter(); let source = "1+2"; - let script_origin = mock_script_origin(scope); + let script_origin = mock_script_origin(scope, "foo.js"); let source = v8::script_compiler::Source::new(v8_str(scope, source), &script_origin); @@ -810,10 +811,10 @@ fn module_instantiation_failures1() { "import './foo.js';\n\ export {} from './bar.js';", ); - let origin = mock_script_origin(scope); + let origin = mock_script_origin(scope, "foo.js"); let source = v8::script_compiler::Source::new(source_text, &origin); - let module = v8::script_compiler::compile_module( + let mut module = v8::script_compiler::compile_module( &isolate, source, v8::script_compiler::CompileOptions::NoCompileOptions, @@ -839,7 +840,99 @@ fn module_instantiation_failures1() { assert_eq!(1, loc.get_line_number()); assert_eq!(15, loc.get_column_number()); - // TODO(ry) Instantiation should fail. + // Instantiation should fail. + { + let mut try_catch = v8::TryCatch::new(scope); + let tc = try_catch.enter(); + fn resolve_callback( + mut context: v8::Local, + _specifier: v8::Local, + _referrer: v8::Local, + ) -> Option> { + let isolate: &mut v8::Isolate = context.as_mut(); + let e = v8_str(isolate, "boom"); + isolate.throw_exception(e.into()); + None + } + let result = module.instantiate_module(context, resolve_callback); + assert!(result.is_none()); + assert!(tc.has_caught()); + assert!(tc + .exception() + .unwrap() + .strict_equals(v8_str(scope, "boom").into())); + assert_eq!(v8::ModuleStatus::Uninstantiated, module.get_status()); + } + + context.exit(); + }); + drop(locker); + isolate.exit(); + drop(g); +} + +#[test] +fn module_evaluation() { + let g = setup(); + let mut params = v8::Isolate::create_params(); + params.set_array_buffer_allocator(v8::Allocator::new_default_allocator()); + let mut isolate = v8::Isolate::new(params); + isolate.enter(); + let mut locker = v8::Locker::new(&isolate); + v8::HandleScope::enter(&mut locker, |scope| { + let mut context = v8::Context::new(scope); + context.enter(); + + let source_text = v8_str( + scope, + "import 'Object.expando = 5';\n\ + import 'Object.expando *= 2';", + ); + let origin = mock_script_origin(scope, "foo.js"); + let source = v8::script_compiler::Source::new(source_text, &origin); + + let mut module = v8::script_compiler::compile_module( + &isolate, + source, + v8::script_compiler::CompileOptions::NoCompileOptions, + v8::script_compiler::NoCacheReason::NoReason, + ) + .unwrap(); + assert_eq!(v8::ModuleStatus::Uninstantiated, module.get_status()); + + fn resolve_callback( + mut context: v8::Local, + specifier: v8::Local, + _referrer: v8::Local, + ) -> Option> { + let isolate_: &mut v8::Isolate = context.as_mut(); + let module_ = { + let mut escapable_scope = v8::EscapableHandleScope::new(isolate_); + let origin = mock_script_origin(isolate_, "module.js"); + let source = v8::script_compiler::Source::new(specifier, &origin); + let module = v8::script_compiler::compile_module( + isolate_, + source, + v8::script_compiler::CompileOptions::NoCompileOptions, + v8::script_compiler::NoCacheReason::NoReason, + ) + .unwrap(); + escapable_scope.escape(cast(module)) + }; + Some(cast(module_)) + } + let result = module.instantiate_module(context, resolve_callback); + assert!(result.unwrap()); + assert_eq!(v8::ModuleStatus::Instantiated, module.get_status()); + + let result = module.evaluate(context); + assert!(result.is_some()); + assert_eq!(v8::ModuleStatus::Evaluated, module.get_status()); + + let result = eval(scope, context, "Object.expando").unwrap(); + assert!(result.is_number()); + let expected = v8::Number::new(scope, 10.); + assert!(result.strict_equals(expected.into())); context.exit(); });