diff --git a/src/array_buffer.rs b/src/array_buffer.rs index 873acb6b..9d894748 100644 --- a/src/array_buffer.rs +++ b/src/array_buffer.rs @@ -64,6 +64,22 @@ extern "C" { fn std__shared_ptr__v8__BackingStore__use_count( ptr: *const SharedRef, ) -> long; + + fn std__shared_ptr__v8__ArrayBuffer__Allocator__COPY( + ptr: *const SharedRef, + ) -> SharedRef; + fn std__shared_ptr__v8__ArrayBuffer__Allocator__CONVERT__std__unique_ptr( + unique: UniqueRef, + ) -> SharedRef; + fn std__shared_ptr__v8__ArrayBuffer__Allocator__get( + ptr: *const SharedRef, + ) -> *mut Allocator; + fn std__shared_ptr__v8__ArrayBuffer__Allocator__reset( + ptr: *mut SharedRef, + ); + fn std__shared_ptr__v8__ArrayBuffer__Allocator__use_count( + ptr: *const SharedRef, + ) -> long; } /// A thread-safe allocator that V8 uses to allocate |ArrayBuffer|'s memory. @@ -86,14 +102,34 @@ extern "C" { #[repr(C)] pub struct Allocator(Opaque); +impl Shared for Allocator { + fn clone(ptr: *const SharedRef) -> SharedRef { + unsafe { std__shared_ptr__v8__ArrayBuffer__Allocator__COPY(ptr) } + } + fn from_unique(unique: UniqueRef) -> SharedRef { + unsafe { + std__shared_ptr__v8__ArrayBuffer__Allocator__CONVERT__std__unique_ptr( + unique, + ) + } + } + fn deref(ptr: *const SharedRef) -> *mut Self { + unsafe { std__shared_ptr__v8__ArrayBuffer__Allocator__get(ptr) } + } + fn reset(ptr: *mut SharedRef) { + unsafe { std__shared_ptr__v8__ArrayBuffer__Allocator__reset(ptr) } + } + fn use_count(ptr: *const SharedRef) -> long { + unsafe { std__shared_ptr__v8__ArrayBuffer__Allocator__use_count(ptr) } + } +} + /// malloc/free based convenience allocator. -/// -/// Caller takes ownership, i.e. the returned object needs to be freed using -/// |delete allocator| once it is no longer in use. -pub fn new_default_allocator() -> UniqueRef { +pub fn new_default_allocator() -> SharedRef { unsafe { UniqueRef::from_raw(v8__ArrayBuffer__Allocator__NewDefaultAllocator()) } + .make_shared() } #[test] diff --git a/src/binding.cc b/src/binding.cc index d37b660b..881085fa 100644 --- a/src/binding.cc +++ b/src/binding.cc @@ -100,11 +100,7 @@ v8::Isolate* v8__Isolate__New(v8::Isolate::CreateParams& params) { return isolate; } -void v8__Isolate__Dispose(v8::Isolate* isolate) { - auto allocator = isolate->GetArrayBufferAllocator(); - isolate->Dispose(); - delete allocator; -} +void v8__Isolate__Dispose(v8::Isolate* isolate) { isolate->Dispose(); } void v8__Isolate__Enter(v8::Isolate* isolate) { isolate->Enter(); } @@ -196,15 +192,16 @@ v8::Isolate::CreateParams* v8__Isolate__CreateParams__NEW() { // This function is only called if the Isolate::CreateParams object is *not* // consumed by Isolate::New(). void v8__Isolate__CreateParams__DELETE(v8::Isolate::CreateParams& self) { - delete self.array_buffer_allocator; + assert(self.array_buffer_allocator == + nullptr); // We only used the shared version. delete &self; } // This function takes ownership of the ArrayBuffer::Allocator. void v8__Isolate__CreateParams__SET__array_buffer_allocator( - v8::Isolate::CreateParams& self, v8::ArrayBuffer::Allocator* value) { - delete self.array_buffer_allocator; - self.array_buffer_allocator = value; + v8::Isolate::CreateParams& self, + std::shared_ptr& allocator) { + self.array_buffer_allocator_shared = allocator; } // external_references should probably have static lifetime. @@ -628,6 +625,33 @@ long std__shared_ptr__v8__BackingStore__use_count( return ptr.use_count(); } +two_pointers_t std__shared_ptr__v8__ArrayBuffer__Allocator__COPY( + const std::shared_ptr& ptr) { + return make_pod(ptr); +} + +two_pointers_t +std__shared_ptr__v8__ArrayBuffer__Allocator__CONVERT__std__unique_ptr( + v8::ArrayBuffer::Allocator* ptr) { + return make_pod( + std::shared_ptr(ptr)); +} + +v8::ArrayBuffer::Allocator* std__shared_ptr__v8__ArrayBuffer__Allocator__get( + const std::shared_ptr& ptr) { + return ptr.get(); +} + +void std__shared_ptr__v8__ArrayBuffer__Allocator__reset( + std::shared_ptr& ptr) { + ptr.reset(); +} + +long std__shared_ptr__v8__ArrayBuffer__Allocator__use_count( + const std::shared_ptr& ptr) { + return ptr.use_count(); +} + v8::String* v8__String__Empty(v8::Isolate* isolate) { return local_to_ptr(v8::String::Empty(isolate)); } @@ -742,9 +766,7 @@ v8::Array* v8__Array__New_with_elements(v8::Isolate* isolate, return local_to_ptr(v8::Array::New(isolate, elements, length)); } -uint32_t v8__Array__Length(const v8::Array& self) { - return self.Length(); -} +uint32_t v8__Array__Length(const v8::Array& self) { return self.Length(); } v8::Number* v8__Number__New(v8::Isolate* isolate, double value) { return *v8::Number::New(isolate, value); diff --git a/src/isolate.rs b/src/isolate.rs index d338489c..16179de8 100644 --- a/src/isolate.rs +++ b/src/isolate.rs @@ -5,6 +5,7 @@ use crate::promise::PromiseRejectMessage; use crate::support::intptr_t; use crate::support::Delete; use crate::support::Opaque; +use crate::support::SharedRef; use crate::support::UniqueRef; use crate::Context; use crate::Function; @@ -121,7 +122,7 @@ extern "C" { fn v8__Isolate__CreateParams__DELETE(this: &mut CreateParams); fn v8__Isolate__CreateParams__SET__array_buffer_allocator( this: &mut CreateParams, - value: *mut Allocator, + allocator: &SharedRef, ); fn v8__Isolate__CreateParams__SET__external_references( this: &mut CreateParams, @@ -493,16 +494,12 @@ impl CreateParams { /// The ArrayBuffer::Allocator to use for allocating and freeing the backing /// store of ArrayBuffers. /// - /// If the shared_ptr version is used, the Isolate instance and every - /// |BackingStore| allocated using this allocator hold a std::shared_ptr - /// to the allocator, in order to facilitate lifetime - /// management for the allocator instance. - pub fn set_array_buffer_allocator(&mut self, value: UniqueRef) { + /// The Isolate instance and every |BackingStore| allocated using this + /// allocator hold a SharedRef to the allocator, in order to facilitate + /// lifetime management for the allocator instance. + pub fn set_array_buffer_allocator(&mut self, value: SharedRef) { unsafe { - v8__Isolate__CreateParams__SET__array_buffer_allocator( - self, - value.into_raw(), - ) + v8__Isolate__CreateParams__SET__array_buffer_allocator(self, &value) }; } diff --git a/tests/test_api.rs b/tests/test_api.rs index 1467ff04..3d93f0cf 100644 --- a/tests/test_api.rs +++ b/tests/test_api.rs @@ -334,6 +334,33 @@ fn array_buffer() { } } +#[test] +fn backing_store_segfault() { + let _setup_guard = setup(); + let array_buffer_allocator = v8::new_default_allocator(); + let shared_bs = { + assert_eq!(1, v8::SharedRef::use_count(&array_buffer_allocator)); + let mut params = v8::Isolate::create_params(); + params.set_array_buffer_allocator(array_buffer_allocator.clone()); + assert_eq!(2, v8::SharedRef::use_count(&array_buffer_allocator)); + let mut isolate = v8::Isolate::new(params); + assert_eq!(2, v8::SharedRef::use_count(&array_buffer_allocator)); + let mut hs = v8::HandleScope::new(&mut isolate); + let scope = hs.enter(); + let context = v8::Context::new(scope); + let mut cs = v8::ContextScope::new(scope, context); + let scope = cs.enter(); + let ab = v8::ArrayBuffer::new(scope, 10); + let shared_bs = ab.get_backing_store(); + assert_eq!(3, v8::SharedRef::use_count(&array_buffer_allocator)); + shared_bs + }; + assert_eq!(1, v8::SharedRef::use_count(&shared_bs)); + assert_eq!(2, v8::SharedRef::use_count(&array_buffer_allocator)); + drop(array_buffer_allocator); + drop(shared_bs); // Error occurred here. +} + #[test] fn array_buffer_with_shared_backing_store() { let _setup_guard = setup();