diff --git a/src/isolate.rs b/src/isolate.rs index 2457b254..dc4811c6 100644 --- a/src/isolate.rs +++ b/src/isolate.rs @@ -6,6 +6,7 @@ use crate::isolate_create_params::raw; use crate::isolate_create_params::CreateParams; use crate::promise::PromiseRejectMessage; use crate::scope::data::ScopeData; +use crate::snapshot::SnapshotCreator; use crate::support::MapFnFrom; use crate::support::MapFnTo; use crate::support::Opaque; @@ -17,8 +18,10 @@ use crate::Array; use crate::CallbackScope; use crate::Context; use crate::Data; +use crate::ExternalReferences; use crate::FixedArray; use crate::Function; +use crate::FunctionCodeHandling; use crate::HandleScope; use crate::Local; use crate::Message; @@ -26,6 +29,7 @@ use crate::Module; use crate::Object; use crate::Promise; use crate::PromiseResolver; +use crate::StartupData; use crate::String; use crate::Value; @@ -532,6 +536,13 @@ impl Isolate { owned_isolate } + #[allow(clippy::new_ret_no_self)] + pub fn snapshot_creator( + external_references: Option<&'static ExternalReferences>, + ) -> OwnedIsolate { + SnapshotCreator::new(external_references) + } + /// Initial configuration parameters for a new Isolate. #[inline(always)] pub fn create_params() -> CreateParams { @@ -587,6 +598,17 @@ impl Isolate { unsafe { &mut *annex_ptr } } + pub(crate) fn set_snapshot_creator( + &mut self, + snapshot_creator: SnapshotCreator, + ) { + let prev = self + .get_annex_mut() + .maybe_snapshot_creator + .replace(snapshot_creator); + assert!(prev.is_none()); + } + pub(crate) fn get_finalizer_map(&self) -> &FinalizerMap { &self.get_annex().finalizer_map } @@ -1031,9 +1053,7 @@ impl Isolate { unsafe { v8__Isolate__HasPendingBackgroundTasks(self) } } - /// Disposes the isolate. The isolate must not be entered by any - /// thread to be disposable. - unsafe fn dispose(&mut self) { + unsafe fn clear_scope_and_annex(&mut self) { // Drop the scope stack. ScopeData::drop_root(self); @@ -1069,7 +1089,11 @@ impl Isolate { // Subtract one from the Arc reference count. Arc::from_raw(annex); self.set_data(0, null_mut()); + } + /// Disposes the isolate. The isolate must not be entered by any + /// thread to be disposable. + unsafe fn dispose(&mut self) { // No test case in rusty_v8 show this, but there have been situations in // deno where dropping Annex before the states causes a segfault. v8__Isolate__Dispose(self) @@ -1102,12 +1126,76 @@ impl Isolate { let arg = &mut callback as *mut F as *mut c_void; unsafe { v8__HeapProfiler__TakeHeapSnapshot(self, trampoline::, arg) } } + + /// Set the default context to be included in the snapshot blob. + /// The snapshot will not contain the global proxy, and we expect one or a + /// global object template to create one, to be provided upon deserialization. + /// + /// # Panics + /// + /// Panics if the isolate was not created using [`Isolate::snapshot_creator`] + #[inline(always)] + pub fn set_default_context(&mut self, context: Local) { + let snapshot_creator = self + .get_annex_mut() + .maybe_snapshot_creator + .as_mut() + .unwrap(); + snapshot_creator.set_default_context(context); + } + + /// Attach arbitrary `v8::Data` to the isolate snapshot, which can be + /// retrieved via `HandleScope::get_context_data_from_snapshot_once()` after + /// deserialization. This data does not survive when a new snapshot is created + /// from an existing snapshot. + /// + /// # Panics + /// + /// Panics if the isolate was not created using [`Isolate::snapshot_creator`] + #[inline(always)] + pub fn add_isolate_data(&mut self, data: Local) -> usize + where + for<'l> Local<'l, T>: Into>, + { + let snapshot_creator = self + .get_annex_mut() + .maybe_snapshot_creator + .as_mut() + .unwrap(); + snapshot_creator.add_isolate_data(data) + } + + /// Attach arbitrary `v8::Data` to the context snapshot, which can be + /// retrieved via `HandleScope::get_context_data_from_snapshot_once()` after + /// deserialization. This data does not survive when a new snapshot is + /// created from an existing snapshot. + /// + /// # Panics + /// + /// Panics if the isolate was not created using [`Isolate::snapshot_creator`] + #[inline(always)] + pub fn add_context_data( + &mut self, + context: Local, + data: Local, + ) -> usize + where + for<'l> Local<'l, T>: Into>, + { + let snapshot_creator = self + .get_annex_mut() + .maybe_snapshot_creator + .as_mut() + .unwrap(); + snapshot_creator.add_context_data(context, data) + } } pub(crate) struct IsolateAnnex { create_param_allocations: Box, slots: HashMap, finalizer_map: FinalizerMap, + maybe_snapshot_creator: Option, // The `isolate` and `isolate_mutex` fields are there so an `IsolateHandle` // (which may outlive the isolate itself) can determine whether the isolate // is still alive, and if so, get a reference to it. Safety rules: @@ -1128,6 +1216,7 @@ impl IsolateAnnex { create_param_allocations, slots: HashMap::default(), finalizer_map: FinalizerMap::default(), + maybe_snapshot_creator: None, isolate, isolate_mutex: Mutex::new(()), } @@ -1272,8 +1361,14 @@ impl OwnedIsolate { impl Drop for OwnedIsolate { fn drop(&mut self) { unsafe { + let snapshot_creator = self.get_annex_mut().maybe_snapshot_creator.take(); + assert!( + snapshot_creator.is_none(), + "If isolate was created using v8::Isolate::snapshot_creator, you should use v8::OwnedIsolate::create_blob before dropping an isolate." + ); self.exit(); - self.cxx_isolate.as_mut().dispose() + self.cxx_isolate.as_mut().clear_scope_and_annex(); + self.cxx_isolate.as_mut().dispose(); } } } @@ -1303,6 +1398,29 @@ impl AsMut for Isolate { } } +impl OwnedIsolate { + /// Creates a snapshot data blob. + /// This must not be called from within a handle scope. + /// + /// # Panics + /// + /// Panics if the isolate was not created using [`Isolate::snapshot_creator`] + #[inline(always)] + pub fn create_blob( + mut self, + function_code_handling: FunctionCodeHandling, + ) -> Option { + let mut snapshot_creator = + self.get_annex_mut().maybe_snapshot_creator.take().unwrap(); + ScopeData::get_root_mut(&mut self); + unsafe { self.cxx_isolate.as_mut().clear_scope_and_annex() }; + // The isolate is owned by the snapshot creator; we need to forget it + // here as the snapshot creator will drop it when running the destructor. + std::mem::forget(self); + snapshot_creator.create_blob(function_code_handling) + } +} + impl HeapStatistics { #[inline(always)] pub fn total_heap_size(&self) -> usize { diff --git a/src/lib.rs b/src/lib.rs index faaaf8ce..2477cd70 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -131,7 +131,6 @@ pub use scope::TryCatch; pub use script::ScriptOrigin; pub use script_compiler::CachedData; pub use snapshot::FunctionCodeHandling; -pub use snapshot::SnapshotCreator; pub use snapshot::StartupData; pub use string::NewStringType; pub use string::WriteOptions; diff --git a/src/snapshot.rs b/src/snapshot.rs index 800879b6..007125bd 100644 --- a/src/snapshot.rs +++ b/src/snapshot.rs @@ -89,26 +89,37 @@ pub enum FunctionCodeHandling { /// Helper class to create a snapshot data blob. #[repr(C)] #[derive(Debug)] -pub struct SnapshotCreator([usize; 1]); +pub(crate) struct SnapshotCreator([usize; 1]); impl SnapshotCreator { /// Create and enter an isolate, and set it up for serialization. /// The isolate is created from scratch. #[inline(always)] - pub fn new(external_references: Option<&'static ExternalReferences>) -> Self { + #[allow(clippy::new_ret_no_self)] + pub(crate) fn new( + external_references: Option<&'static ExternalReferences>, + ) -> OwnedIsolate { let mut snapshot_creator: MaybeUninit = MaybeUninit::uninit(); let external_references_ptr = if let Some(er) = external_references { er.as_ptr() } else { std::ptr::null() }; - unsafe { + let snapshot_creator = unsafe { v8__SnapshotCreator__CONSTRUCT( &mut snapshot_creator, external_references_ptr, ); snapshot_creator.assume_init() - } + }; + + let isolate_ptr = + unsafe { v8__SnapshotCreator__GetIsolate(&snapshot_creator) }; + let mut owned_isolate = OwnedIsolate::new(isolate_ptr); + ScopeData::new_root(&mut owned_isolate); + owned_isolate.create_annex(Box::new(())); + owned_isolate.set_snapshot_creator(snapshot_creator); + owned_isolate } } @@ -123,7 +134,7 @@ impl SnapshotCreator { /// The snapshot will not contain the global proxy, and we expect one or a /// global object template to create one, to be provided upon deserialization. #[inline(always)] - pub fn set_default_context(&mut self, context: Local) { + pub(crate) fn set_default_context(&mut self, context: Local) { unsafe { v8__SnapshotCreator__SetDefaultContext(self, &*context) }; } @@ -132,7 +143,7 @@ impl SnapshotCreator { /// deserialization. This data does not survive when a new snapshot is created /// from an existing snapshot. #[inline(always)] - pub fn add_isolate_data(&mut self, data: Local) -> usize + pub(crate) fn add_isolate_data(&mut self, data: Local) -> usize where for<'l> Local<'l, T>: Into>, { @@ -144,7 +155,7 @@ impl SnapshotCreator { /// deserialization. This data does not survive when a new snapshot is /// created from an existing snapshot. #[inline(always)] - pub fn add_context_data( + pub(crate) fn add_context_data( &mut self, context: Local, data: Local, @@ -160,14 +171,10 @@ impl SnapshotCreator { /// Creates a snapshot data blob. /// This must not be called from within a handle scope. #[inline(always)] - pub fn create_blob( + pub(crate) fn create_blob( &mut self, function_code_handling: FunctionCodeHandling, ) -> Option { - { - let isolate = unsafe { &mut *v8__SnapshotCreator__GetIsolate(self) }; - ScopeData::get_root_mut(isolate); - } let blob = unsafe { v8__SnapshotCreator__CreateBlob(self, function_code_handling) }; if blob.data.is_null() { @@ -178,18 +185,4 @@ impl SnapshotCreator { Some(blob) } } - - /// This is marked unsafe because it should be called at most once per - /// snapshot creator. - // TODO Because the SnapshotCreator creates its own isolate, we need a way to - // get an owned handle to it. This is a questionable design which ought to be - // revisited after the libdeno integration is complete. - #[inline(always)] - pub unsafe fn get_owned_isolate(&mut self) -> OwnedIsolate { - let isolate_ptr = v8__SnapshotCreator__GetIsolate(self); - let mut owned_isolate = OwnedIsolate::new(isolate_ptr); - ScopeData::new_root(&mut owned_isolate); - owned_isolate.create_annex(Box::new(())); - owned_isolate - } } diff --git a/tests/slots.rs b/tests/slots.rs index 9ebf56d4..5e204c5b 100644 --- a/tests/slots.rs +++ b/tests/slots.rs @@ -326,23 +326,19 @@ fn dropped_context_slots_on_kept_context() { fn clear_all_context_slots() { setup(); - let mut snapshot_creator = v8::SnapshotCreator::new(None); - let mut isolate = unsafe { snapshot_creator.get_owned_isolate() }; + let mut snapshot_creator = v8::Isolate::snapshot_creator(None); { - let scope = &mut v8::HandleScope::new(&mut isolate); + let scope = &mut v8::HandleScope::new(&mut snapshot_creator); let context = v8::Context::new(scope); let scope = &mut v8::ContextScope::new(scope, context); context.set_slot(scope, TestState(0)); context.clear_all_slots(scope); assert!(context.get_slot::(scope).is_none()); - - snapshot_creator.set_default_context(context); + scope.set_default_context(context); } - std::mem::forget(isolate); - snapshot_creator .create_blob(v8::FunctionCodeHandling::Keep) .unwrap(); diff --git a/tests/test_api.rs b/tests/test_api.rs index 65d26ac8..8a6078c0 100644 --- a/tests/test_api.rs +++ b/tests/test_api.rs @@ -3786,15 +3786,12 @@ fn snapshot_creator() { let context_data_index; let context_data_index_2; let startup_data = { - let mut snapshot_creator = v8::SnapshotCreator::new(None); - // TODO(ry) this shouldn't be necessary. workaround unfinished business in - // the scope type system. - let mut isolate = unsafe { snapshot_creator.get_owned_isolate() }; + let mut snapshot_creator = v8::Isolate::snapshot_creator(None); { // Check that the SnapshotCreator isolate has been set up correctly. - let _ = isolate.thread_safe_handle(); + let _ = snapshot_creator.thread_safe_handle(); - let scope = &mut v8::HandleScope::new(&mut isolate); + let scope = &mut v8::HandleScope::new(&mut snapshot_creator); let context = v8::Context::new(scope); let scope = &mut v8::ContextScope::new(scope, context); @@ -3802,16 +3799,15 @@ fn snapshot_creator() { let script = v8::Script::compile(scope, source, None).unwrap(); script.run(scope).unwrap(); - snapshot_creator.set_default_context(context); + scope.set_default_context(context); - isolate_data_index = - snapshot_creator.add_isolate_data(v8::Number::new(scope, 1.0)); - context_data_index = - snapshot_creator.add_context_data(context, v8::Number::new(scope, 2.0)); - context_data_index_2 = - snapshot_creator.add_context_data(context, v8::Number::new(scope, 3.0)); + let n1 = v8::Number::new(scope, 1.0); + let n2 = v8::Number::new(scope, 2.0); + let n3 = v8::Number::new(scope, 3.0); + isolate_data_index = scope.add_isolate_data(n1); + context_data_index = scope.add_context_data(context, n2); + context_data_index_2 = scope.add_context_data(context, n3); } - std::mem::forget(isolate); // TODO(ry) this shouldn't be necessary. snapshot_creator .create_blob(v8::FunctionCodeHandling::Clear) .unwrap() @@ -3881,12 +3877,9 @@ fn external_references() { // First we create the snapshot, there is a single global variable 'a' set to // the value 3. let startup_data = { - let mut snapshot_creator = v8::SnapshotCreator::new(Some(refs)); - // TODO(ry) this shouldn't be necessary. workaround unfinished business in - // the scope type system. - let mut isolate = unsafe { snapshot_creator.get_owned_isolate() }; + let mut snapshot_creator = v8::Isolate::snapshot_creator(Some(refs)); { - let scope = &mut v8::HandleScope::new(&mut isolate); + let scope = &mut v8::HandleScope::new(&mut snapshot_creator); let context = v8::Context::new(scope); let scope = &mut v8::ContextScope::new(scope, context); @@ -3903,9 +3896,8 @@ fn external_references() { let key = v8::String::new(scope, "F").unwrap(); global.set(scope, key.into(), function.into()); - snapshot_creator.set_default_context(context); + scope.set_default_context(context); } - std::mem::forget(isolate); // TODO(ry) this shouldn't be necessary. snapshot_creator .create_blob(v8::FunctionCodeHandling::Clear) .unwrap() @@ -5307,12 +5299,9 @@ fn module_snapshot() { let _setup_guard = setup(); let startup_data = { - let mut snapshot_creator = v8::SnapshotCreator::new(None); - // TODO(ry) this shouldn't be necessary. workaround unfinished business in - // the scope type system. - let mut isolate = unsafe { snapshot_creator.get_owned_isolate() }; + let mut snapshot_creator = v8::Isolate::snapshot_creator(None); { - let scope = &mut v8::HandleScope::new(&mut isolate); + let scope = &mut v8::HandleScope::new(&mut snapshot_creator); let context = v8::Context::new(scope); let scope = &mut v8::ContextScope::new(scope, context); @@ -5344,9 +5333,8 @@ fn module_snapshot() { assert_eq!(v8::ModuleStatus::Evaluated, module.get_status()); assert_eq!(script_id, module.script_id()); - snapshot_creator.set_default_context(context); + scope.set_default_context(context); } - std::mem::forget(isolate); // TODO(ry) this shouldn't be necessary. snapshot_creator .create_blob(v8::FunctionCodeHandling::Keep) .unwrap()