diff --git a/src/binding.cc b/src/binding.cc index 7ba97b7b..216c252e 100644 --- a/src/binding.cc +++ b/src/binding.cc @@ -2345,6 +2345,8 @@ void v8__SnapshotCreator__DESTRUCT(v8::SnapshotCreator* self) { self->~SnapshotCreator(); } +void v8__StartupData__DESTRUCT(v8::StartupData* self) { delete[] self->data; } + v8::Isolate* v8__SnapshotCreator__GetIsolate(const v8::SnapshotCreator& self) { // `v8::SnapshotCreator::GetIsolate()` is not declared as a const method, but // this appears to be a mistake. @@ -3159,7 +3161,4 @@ const char* v8__CompiledWasmModule__SourceUrl(v8::CompiledWasmModule* self, void v8__CompiledWasmModule__DELETE(v8::CompiledWasmModule* self) { delete self; } - -void char__DELETE_ARRAY(const char* ptr[]) { delete[] ptr; } - } // extern "C" diff --git a/src/isolate_create_params.rs b/src/isolate_create_params.rs index c2408382..818b8cba 100644 --- a/src/isolate_create_params.rs +++ b/src/isolate_create_params.rs @@ -237,7 +237,7 @@ pub(crate) mod raw { } impl StartupData { - pub(super) fn boxed_header(data: &Allocation<[u8]>) -> Box { + pub fn boxed_header(data: &Allocation<[u8]>) -> Box { Box::new(Self { data: &data[0] as *const _ as *const char, raw_size: int::try_from(data.len()).unwrap(), diff --git a/src/snapshot.rs b/src/snapshot.rs index 9772d0c2..6cd24275 100644 --- a/src/snapshot.rs +++ b/src/snapshot.rs @@ -1,27 +1,28 @@ use crate::external_references::ExternalReferences; +use crate::isolate_create_params::raw; use crate::scope::data::ScopeData; use crate::support::char; use crate::support::int; use crate::support::intptr_t; use crate::support::Allocated; -use crate::support::CharArray; +use crate::support::Allocation; use crate::Context; use crate::Data; use crate::Isolate; use crate::Local; use crate::OwnedIsolate; -use std::any::Any; use std::borrow::Borrow; use std::convert::TryFrom; use std::mem::MaybeUninit; use std::ops::Deref; +use std::ptr::null; extern "C" { fn v8__SnapshotCreator__CONSTRUCT( buf: *mut MaybeUninit, external_references: *const intptr_t, - existing_blob: *const StartupData, + existing_blob: *const raw::StartupData, ); fn v8__SnapshotCreator__DESTRUCT(this: *mut SnapshotCreator); fn v8__SnapshotCreator__GetIsolate( @@ -48,6 +49,7 @@ extern "C" { context: *const Context, data: *const Data, ) -> usize; + fn v8__StartupData__DESTRUCT(this: *mut StartupData); } // TODO(piscisaureus): merge this struct with @@ -59,18 +61,9 @@ pub struct StartupData { raw_size: int, } -impl StartupData { - pub fn from_static(data: &'static [u8]) -> Self { - Self { - data: data.as_ptr() as *const char, - raw_size: i32::try_from(data.len()).unwrap(), - } - } -} - -impl From<&'static [u8]> for StartupData { - fn from(data: &'static [u8]) -> Self { - Self::from_static(data) +impl Drop for StartupData { + fn drop(&mut self) { + unsafe { v8__StartupData__DESTRUCT(self) } } } @@ -95,54 +88,6 @@ impl Borrow<[u8]> for StartupData { } } -/// A `StartupData` object that owns the data it references, which is kept in a -/// char array allocated in C++. This is what V8 returns when calling -/// `SnapshotCreator::CreateBlob()`. -pub struct OwnedStartupData { - inner: StartupData, - _owner: Box, -} - -impl OwnedStartupData { - pub fn new(owned_data: T) -> Self - where - T: Deref + 'static, - { - let inner = { - let slice = owned_data.deref(); - let data = slice.as_ptr() as *const char; - let raw_size = int::try_from(slice.len()).unwrap(); - StartupData { data, raw_size } - }; - Self { - inner, - _owner: Box::new(owned_data), - } - } - - fn from_raw_char_array(raw: StartupData) -> Self { - let char_array = unsafe { CharArray::from_ptr(raw.data) }; - Self { - inner: raw, - _owner: Box::new(char_array), - } - } -} - -impl Deref for OwnedStartupData { - type Target = StartupData; - - fn deref(&self) -> &Self::Target { - &self.inner - } -} - -impl Borrow<[u8]> for OwnedStartupData { - fn borrow(&self) -> &[u8] { - &**self - } -} - #[repr(C)] #[derive(Debug)] pub enum FunctionCodeHandling { @@ -159,9 +104,26 @@ impl SnapshotCreator { /// Create and enter an isolate, and set it up for serialization. /// The isolate is created from scratch. #[inline(always)] - pub fn new( + pub fn new(external_references: Option<&'static ExternalReferences>) -> Self { + Self::new_impl(external_references, None::<&[u8]>) + } + + /// Create and enter an isolate, and set it up for serialization. + /// The isolate is created from scratch. + #[inline(always)] + pub fn from_existing_snapshot( + existing_snapshot_blob: impl Allocated<[u8]>, external_references: Option<&'static ExternalReferences>, - existing_blob: Option>, + ) -> Self { + Self::new_impl(external_references, Some(existing_snapshot_blob)) + } + + /// Create and enter an isolate, and set it up for serialization. + /// The isolate is created from scratch. + #[inline(always)] + fn new_impl( + external_references: Option<&'static ExternalReferences>, + existing_snapshot_blob: Option>, ) -> Self { let mut snapshot_creator: MaybeUninit = MaybeUninit::uninit(); let external_references_ptr = if let Some(er) = external_references { @@ -169,19 +131,39 @@ impl SnapshotCreator { } else { std::ptr::null() }; - let existing_blob_ptr = if let Some(startup_data) = existing_blob { - startup_data + + let snapshot_blob_ptr; + let snapshot_allocations; + if let Some(snapshot_blob) = existing_snapshot_blob { + let data = Allocation::of(snapshot_blob); + let header = Allocation::of(raw::StartupData::boxed_header(&data)); + snapshot_blob_ptr = &*header as *const _; + snapshot_allocations = Some((header, data)); } else { - std::ptr::null() - }; - unsafe { + snapshot_blob_ptr = null(); + snapshot_allocations = None; + } + + let snapshot_creator = unsafe { v8__SnapshotCreator__CONSTRUCT( &mut snapshot_creator, external_references_ptr, - existing_blob_ptr, + snapshot_blob_ptr, ); snapshot_creator.assume_init() + }; + + // Store any owned buffers that need to live as long as the SnapshotCreator + // itself in the Isolate's annex (this is where we would put the + // IsolateCreateParams if this was a normal isolate.) + { + let isolate = + unsafe { &mut *(v8__SnapshotCreator__GetIsolate(&snapshot_creator)) }; + ScopeData::new_root(isolate); + isolate.create_annex(Box::new(snapshot_allocations)); } + + snapshot_creator } } @@ -244,7 +226,7 @@ impl SnapshotCreator { pub fn create_blob( &mut self, function_code_handling: FunctionCodeHandling, - ) -> Option { + ) -> Option { { let isolate = unsafe { &mut *v8__SnapshotCreator__GetIsolate(self) }; ScopeData::get_root_mut(isolate); @@ -256,10 +238,7 @@ impl SnapshotCreator { None } else { debug_assert!(blob.raw_size > 0); - // Wrap the returned blob in a `OwnedStartupData`, which has a drop - // handler that will release the heap allocated `char[]` buffer that holds - // the snapshot blob returned by `v8::SnapshotCreator::CreateBlob()`. - Some(OwnedStartupData::from_raw_char_array(blob)) + Some(blob) } } @@ -271,9 +250,7 @@ impl SnapshotCreator { #[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(())); + let owned_isolate = OwnedIsolate::new(isolate_ptr); owned_isolate } } diff --git a/src/support.rs b/src/support.rs index 4352266f..ae45c13e 100644 --- a/src/support.rs +++ b/src/support.rs @@ -365,29 +365,6 @@ fn assert_shared_ptr_use_count_eq( ); } -extern "C" { - fn char__DELETE_ARRAY(ptr: *const char); -} - -/// Wrapper around a C++ heap allocated `const char*` array. This wrapper calls -/// `delete[]` in C++ when it is dropped. -#[repr(transparent)] -pub struct CharArray(*const char); - -impl CharArray { - pub unsafe fn from_ptr(ptr: *const char) -> Self { - Self(ptr) - } -} - -impl Drop for CharArray { - fn drop(&mut self) { - if !self.0.is_null() { - unsafe { char__DELETE_ARRAY(self.0) }; - } - } -} - /// A trait for values with static lifetimes that are allocated at a fixed /// address in memory. Practically speaking, that means they're either a /// `&'static` reference, or they're heap-allocated in a `Arc`, `Box`, `Rc`, diff --git a/tests/slots.rs b/tests/slots.rs index bb9972d8..30489eb8 100644 --- a/tests/slots.rs +++ b/tests/slots.rs @@ -298,7 +298,7 @@ fn dropped_context_slots() { fn clear_all_context_slots() { setup(); - let mut snapshot_creator = v8::SnapshotCreator::new(None, None); + let mut snapshot_creator = v8::SnapshotCreator::new(None); let mut isolate = unsafe { snapshot_creator.get_owned_isolate() }; { diff --git a/tests/test_api.rs b/tests/test_api.rs index 9a482658..c910b368 100644 --- a/tests/test_api.rs +++ b/tests/test_api.rs @@ -3480,7 +3480,7 @@ fn snapshot_creator() { let context_data_index; let context_data_index_2; let startup_data = { - let mut snapshot_creator = v8::SnapshotCreator::new(None, None); + let mut snapshot_creator = v8::SnapshotCreator::new(None); let mut isolate = unsafe { snapshot_creator.get_owned_isolate() }; { let scope = &mut v8::HandleScope::new(&mut isolate); @@ -3498,7 +3498,7 @@ fn snapshot_creator() { let startup_data = { let mut snapshot_creator = - v8::SnapshotCreator::new(None, Some(&startup_data)); + v8::SnapshotCreator::from_existing_snapshot(startup_data, 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() }; @@ -3570,7 +3570,7 @@ fn snapshot_creator() { fn snapshot_creator_multiple_contexts() { let _setup_guard = setup(); let startup_data = { - let mut snapshot_creator = v8::SnapshotCreator::new(None, None); + let mut snapshot_creator = v8::SnapshotCreator::new(None); let mut isolate = unsafe { snapshot_creator.get_owned_isolate() }; { let mut scope = v8::HandleScope::new(&mut isolate); @@ -3607,7 +3607,7 @@ fn snapshot_creator_multiple_contexts() { let startup_data = { let mut snapshot_creator = - v8::SnapshotCreator::new(None, Some(&startup_data)); + v8::SnapshotCreator::from_existing_snapshot(startup_data, None); let mut isolate = unsafe { snapshot_creator.get_owned_isolate() }; { let scope = &mut v8::HandleScope::new(&mut isolate); @@ -3744,7 +3744,7 @@ 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), None); + 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() }; @@ -5167,7 +5167,7 @@ fn module_snapshot() { let _setup_guard = setup(); let startup_data = { - let mut snapshot_creator = v8::SnapshotCreator::new(None, None); + 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() };