From 71140b4cc486af25e34fb2e8e5bebce5eb36d307 Mon Sep 17 00:00:00 2001 From: Ryan Dahl Date: Thu, 19 Dec 2019 16:58:39 -0500 Subject: [PATCH] Change HandleScope::enter to take &Isolate, make v8::Locker into opaque blob --- src/binding.cc | 3 ++ src/function.rs | 8 ++--- src/handle_scope.rs | 12 +++---- src/locker.rs | 14 +------- src/object.rs | 2 +- tests/test_api.rs | 78 ++++++++++++++++++++++++++------------------- 6 files changed, 61 insertions(+), 56 deletions(-) diff --git a/src/binding.cc b/src/binding.cc index eccf4a0f..2bb1e096 100644 --- a/src/binding.cc +++ b/src/binding.cc @@ -21,6 +21,9 @@ static_assert(sizeof(HandleScope) == sizeof(size_t) * 3, static_assert(sizeof(v8::PromiseRejectMessage) == sizeof(size_t) * 3, "PromiseRejectMessage size mismatch"); +static_assert(sizeof(v8::Locker) == sizeof(size_t) * 2, + "Locker size mismatch"); + extern "C" { void v8__V8__SetFlagsFromCommandLine(int* argc, char** argv) { diff --git a/src/function.rs b/src/function.rs index 8b2db7a1..8d9ee210 100644 --- a/src/function.rs +++ b/src/function.rs @@ -37,7 +37,7 @@ extern "C" { fn v8__ReturnValue__Set(rv: *mut ReturnValue, value: *mut Value) -> (); fn v8__ReturnValue__Get(rv: *mut ReturnValue) -> *mut Value; - fn v8__ReturnValue__GetIsolate(rv: *mut ReturnValue) -> *mut Isolate; + fn v8__ReturnValue__GetIsolate(rv: &ReturnValue) -> *mut Isolate; } #[repr(C)] @@ -55,8 +55,8 @@ impl ReturnValue { } /// Convenience getter for Isolate - pub fn get_isolate(&mut self) -> *mut Isolate { - unsafe { v8__ReturnValue__GetIsolate(&mut *self) } + pub fn get_isolate(&self) -> &Isolate { + unsafe { v8__ReturnValue__GetIsolate(self).as_ref().unwrap() } } /// Getter. Creates a new Local<> so it comes with a certain performance @@ -84,7 +84,7 @@ impl FunctionCallbackInfo { } /// The current Isolate. - pub fn get_isolate(&self) -> &mut Isolate { + pub fn get_isolate(&self) -> &Isolate { unsafe { v8__FunctionCallbackInfo__GetIsolate(self) } } diff --git a/src/handle_scope.rs b/src/handle_scope.rs index b0481729..13a4458a 100644 --- a/src/handle_scope.rs +++ b/src/handle_scope.rs @@ -7,7 +7,7 @@ use crate::isolate::LockedIsolate; extern "C" { fn v8__HandleScope__CONSTRUCT( buf: &mut MaybeUninit, - isolate: &mut Isolate, + isolate: &Isolate, ); fn v8__HandleScope__DESTRUCT(this: &mut HandleScope); fn v8__HandleScope__GetIsolate<'sc>( @@ -19,12 +19,12 @@ extern "C" { pub struct HandleScope<'sc>([usize; 3], PhantomData<&'sc mut ()>); impl<'sc> HandleScope<'sc> { - pub fn enter

(parent: &mut P, mut f: impl FnMut(&mut HandleScope<'_>) -> ()) - where - P: LockedIsolate, - { + pub fn enter( + isolate: &Isolate, + mut f: impl FnMut(&mut HandleScope<'_>) -> (), + ) { let mut scope: MaybeUninit = MaybeUninit::uninit(); - unsafe { v8__HandleScope__CONSTRUCT(&mut scope, parent.cxx_isolate()) }; + unsafe { v8__HandleScope__CONSTRUCT(&mut scope, isolate) }; let scope = unsafe { &mut *(scope.as_mut_ptr()) }; f(scope); diff --git a/src/locker.rs b/src/locker.rs index 93c27f0f..42c06998 100644 --- a/src/locker.rs +++ b/src/locker.rs @@ -2,7 +2,6 @@ use std::marker::PhantomData; use std::mem::MaybeUninit; use crate::isolate::Isolate; -use crate::isolate::LockedIsolate; // class Locker { // public: @@ -18,12 +17,7 @@ extern "C" { } #[repr(C)] -pub struct Locker<'a> { - has_lock: bool, - top_level: bool, - isolate: &'a mut Isolate, - phantom: PhantomData<&'a Isolate>, -} +pub struct Locker<'sc>([usize; 2], PhantomData<&'sc mut ()>); impl<'a> Locker<'a> { pub fn new(isolate: &Isolate) -> Self { @@ -40,9 +34,3 @@ impl<'a> Drop for Locker<'a> { unsafe { v8__Locker__DESTRUCT(self) } } } - -impl<'a> LockedIsolate for Locker<'a> { - fn cxx_isolate(&mut self) -> &mut Isolate { - self.isolate - } -} diff --git a/src/object.rs b/src/object.rs index 4c3be15f..368fc590 100644 --- a/src/object.rs +++ b/src/object.rs @@ -61,7 +61,7 @@ impl Object { } /// Return the isolate to which the Object belongs to. - pub fn get_isolate(&self) -> &mut Isolate { + pub fn get_isolate(&self) -> &Isolate { unsafe { v8__Object__GetIsolate(self) } } } diff --git a/tests/test_api.rs b/tests/test_api.rs index d9c1d282..ac6a6148 100644 --- a/tests/test_api.rs +++ b/tests/test_api.rs @@ -46,10 +46,11 @@ fn handle_scope_nested() { v8::array_buffer::Allocator::new_default_allocator(), ); let isolate = v8::Isolate::new(params); - let mut locker = v8::Locker::new(&isolate); - v8::HandleScope::enter(&mut locker, |scope| { - v8::HandleScope::enter(scope, |_scope| {}); + let locker = v8::Locker::new(&isolate); + v8::HandleScope::enter(&isolate, |_scope1| { + v8::HandleScope::enter(&isolate, |_scope2| {}); }); + drop(locker); drop(g); } @@ -62,11 +63,11 @@ fn handle_scope_numbers() { v8::array_buffer::Allocator::new_default_allocator(), ); let isolate = v8::Isolate::new(params); - let mut locker = v8::Locker::new(&isolate); - v8::HandleScope::enter(&mut locker, |scope| { + let locker = v8::Locker::new(&isolate); + v8::HandleScope::enter(&isolate, |scope| { let l1 = v8::Integer::new(scope, -123); let l2 = v8::Integer::new_from_unsigned(scope, 456); - v8::HandleScope::enter(scope, |scope2| { + v8::HandleScope::enter(&isolate, |scope2| { let l3 = v8::Number::new(scope2, 78.9); assert_eq!(l1.value(), -123); assert_eq!(l2.value(), 456); @@ -75,6 +76,7 @@ fn handle_scope_numbers() { assert_eq!(v8::Number::value(&l2), 456f64); }); }); + drop(locker); drop(g); } @@ -86,8 +88,8 @@ fn test_string() { v8::array_buffer::Allocator::new_default_allocator(), ); let isolate = v8::Isolate::new(params); - let mut locker = v8::Locker::new(&isolate); - v8::HandleScope::enter(&mut locker, |scope| { + let locker = v8::Locker::new(&isolate); + v8::HandleScope::enter(&isolate, |scope| { let reference = "Hello 🦕 world!"; let local = v8::String::new(scope, reference, v8::NewStringType::Normal).unwrap(); @@ -95,6 +97,7 @@ fn test_string() { assert_eq!(17, local.utf8_length(scope)); assert_eq!(reference, local.to_rust_string_lossy(scope)); }); + drop(locker); } #[test] @@ -117,9 +120,9 @@ fn script_compile_and_run() { v8::array_buffer::Allocator::new_default_allocator(), ); let isolate = v8::Isolate::new(params); - let mut locker = v8::Locker::new(&isolate); + let locker = v8::Locker::new(&isolate); - v8::HandleScope::enter(&mut locker, |s| { + v8::HandleScope::enter(&isolate, |s| { let mut context = v8::Context::new(s); context.enter(); let source = @@ -134,6 +137,7 @@ fn script_compile_and_run() { assert_eq!(result.to_rust_string_lossy(s), "Hello 13th planet"); context.exit(); }); + drop(locker); } #[test] @@ -144,9 +148,9 @@ fn script_origin() { v8::array_buffer::Allocator::new_default_allocator(), ); let isolate = v8::Isolate::new(params); - let mut locker = v8::Locker::new(&isolate); + let locker = v8::Locker::new(&isolate); - v8::HandleScope::enter(&mut locker, |s| { + v8::HandleScope::enter(&isolate, |s| { let mut context = v8::Context::new(s); context.enter(); @@ -181,6 +185,7 @@ fn script_origin() { let _result = script.run(s, context).unwrap(); context.exit(); }); + drop(locker); } #[test] @@ -237,8 +242,8 @@ fn test_primitives() { v8::array_buffer::Allocator::new_default_allocator(), ); let isolate = v8::Isolate::new(params); - let mut locker = v8::Locker::new(&isolate); - v8::HandleScope::enter(&mut locker, |scope| { + let locker = v8::Locker::new(&isolate); + v8::HandleScope::enter(&isolate, |scope| { let null = v8::new_null(scope); assert!(!null.is_undefined()); assert!(null.is_null()); @@ -259,6 +264,7 @@ fn test_primitives() { assert!(!false_.is_null()); assert!(!false_.is_null_or_undefined()); }); + drop(locker); } #[test] @@ -269,9 +275,9 @@ fn exception() { v8::array_buffer::Allocator::new_default_allocator(), ); let mut isolate = v8::Isolate::new(params); - let mut locker = v8::Locker::new(&isolate); + let locker = v8::Locker::new(&isolate); isolate.enter(); - v8::HandleScope::enter(&mut locker, |scope| { + v8::HandleScope::enter(&isolate, |scope| { let mut context = v8::Context::new(scope); context.enter(); let reference = "This is a test error"; @@ -292,6 +298,7 @@ fn exception() { assert!(v8::Exception::GetStackTrace(exception).is_none()); context.exit(); }); + drop(locker); isolate.exit(); } @@ -303,8 +310,8 @@ fn json() { v8::array_buffer::Allocator::new_default_allocator(), ); let isolate = v8::Isolate::new(params); - let mut locker = v8::Locker::new(&isolate); - v8::HandleScope::enter(&mut locker, |s| { + let locker = v8::Locker::new(&isolate); + v8::HandleScope::enter(&isolate, |s| { let mut context = v8::Context::new(s); context.enter(); let json_string = @@ -319,6 +326,7 @@ fn json() { assert_eq!("{\"a\":1,\"b\":2}".to_string(), rust_str); context.exit(); }); + drop(locker); } // TODO Safer casts https://github.com/denoland/rusty_v8/issues/51 @@ -335,8 +343,8 @@ fn object() { v8::array_buffer::Allocator::new_default_allocator(), ); let isolate = v8::Isolate::new(params); - let mut locker = v8::Locker::new(&isolate); - v8::HandleScope::enter(&mut locker, |scope| { + let locker = v8::Locker::new(&isolate); + v8::HandleScope::enter(&isolate, |scope| { let mut context = v8::Context::new(scope); context.enter(); let null: v8::Local = new_null(scope).into(); @@ -352,6 +360,7 @@ fn object() { assert!(!object.is_null_or_undefined()); context.exit(); }); + drop(locker); } #[test] @@ -362,8 +371,8 @@ fn promise_resolved() { v8::array_buffer::Allocator::new_default_allocator(), ); let isolate = v8::Isolate::new(params); - let mut locker = v8::Locker::new(&isolate); - v8::HandleScope::enter(&mut locker, |scope| { + let locker = v8::Locker::new(&isolate); + v8::HandleScope::enter(&isolate, |scope| { let mut context = v8::Context::new(scope); context.enter(); let maybe_resolver = v8::PromiseResolver::new(context); @@ -391,6 +400,7 @@ fn promise_resolved() { assert_eq!(result_str.to_rust_string_lossy(scope), "test".to_string()); context.exit(); }); + drop(locker); } #[test] @@ -401,8 +411,8 @@ fn promise_rejected() { v8::array_buffer::Allocator::new_default_allocator(), ); let isolate = v8::Isolate::new(params); - let mut locker = v8::Locker::new(&isolate); - v8::HandleScope::enter(&mut locker, |scope| { + let locker = v8::Locker::new(&isolate); + v8::HandleScope::enter(&isolate, |scope| { let mut context = v8::Context::new(scope); context.enter(); let maybe_resolver = v8::PromiseResolver::new(context); @@ -431,12 +441,13 @@ fn promise_rejected() { assert_eq!(result_str.to_rust_string_lossy(scope), "test".to_string()); context.exit(); }); + drop(locker); } extern "C" fn fn_callback(info: &FunctionCallbackInfo) { assert_eq!(info.length(), 0); - let mut locker = v8::Locker::new(info.get_isolate()); - v8::HandleScope::enter(&mut locker, |scope| { + let isolate = info.get_isolate(); + v8::HandleScope::enter(&isolate, |scope| { let mut context = v8::Context::new(scope); context.enter(); let s = @@ -459,8 +470,8 @@ fn function() { v8::array_buffer::Allocator::new_default_allocator(), ); let isolate = v8::Isolate::new(params); - let mut locker = v8::Locker::new(&isolate); - v8::HandleScope::enter(&mut locker, |scope| { + let locker = v8::Locker::new(&isolate); + v8::HandleScope::enter(&isolate, |scope| { let mut context = v8::Context::new(scope); context.enter(); let global = context.global(); @@ -482,6 +493,7 @@ fn function() { assert_eq!(rust_str, "Hello callback!".to_string()); context.exit(); }); + drop(locker); } extern "C" fn promise_reject_callback(msg: v8::PromiseRejectMessage) { @@ -492,12 +504,13 @@ extern "C" fn promise_reject_callback(msg: v8::PromiseRejectMessage) { let promise_obj: v8::Local = cast(promise); let isolate = promise_obj.get_isolate(); let value = msg.get_value(); - let mut locker = v8::Locker::new(isolate); - v8::HandleScope::enter(&mut locker, |scope| { + let locker = v8::Locker::new(isolate); + v8::HandleScope::enter(&isolate, |scope| { let value_str: v8::Local = cast(value); let rust_str = value_str.to_rust_string_lossy(scope); assert_eq!(rust_str, "promise rejected".to_string()); }); + drop(locker); } #[test] @@ -510,8 +523,8 @@ fn set_promise_reject_callback() { let mut isolate = v8::Isolate::new(params); isolate.set_promise_reject_callback(promise_reject_callback); isolate.enter(); - let mut locker = v8::Locker::new(&isolate); - v8::HandleScope::enter(&mut locker, |scope| { + let locker = v8::Locker::new(&isolate); + v8::HandleScope::enter(&isolate, |scope| { let mut context = v8::Context::new(scope); context.enter(); let mut resolver = v8::PromiseResolver::new(context).unwrap(); @@ -522,5 +535,6 @@ fn set_promise_reject_callback() { resolver.reject(context, value); context.exit(); }); + drop(locker); isolate.exit(); }