Fix BackingStore segfault (#294)

src/array_buffer.rs

@@ -64,6 +64,22 @@ extern "C" {
   fn std__shared_ptr__v8__BackingStore__use_count(
     ptr: *const SharedRef<BackingStore>,
   ) -> long;
+
+  fn std__shared_ptr__v8__ArrayBuffer__Allocator__COPY(
+    ptr: *const SharedRef<Allocator>,
+  ) -> SharedRef<Allocator>;
+  fn std__shared_ptr__v8__ArrayBuffer__Allocator__CONVERT__std__unique_ptr(
+    unique: UniqueRef<Allocator>,
+  ) -> SharedRef<Allocator>;
+  fn std__shared_ptr__v8__ArrayBuffer__Allocator__get(
+    ptr: *const SharedRef<Allocator>,
+  ) -> *mut Allocator;
+  fn std__shared_ptr__v8__ArrayBuffer__Allocator__reset(
+    ptr: *mut SharedRef<Allocator>,
+  );
+  fn std__shared_ptr__v8__ArrayBuffer__Allocator__use_count(
+    ptr: *const SharedRef<Allocator>,
+  ) -> 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<Self>) -> SharedRef<Self> {
+    unsafe { std__shared_ptr__v8__ArrayBuffer__Allocator__COPY(ptr) }
+  }
+  fn from_unique(unique: UniqueRef<Self>) -> SharedRef<Self> {
+    unsafe {
+      std__shared_ptr__v8__ArrayBuffer__Allocator__CONVERT__std__unique_ptr(
+        unique,
+      )
+    }
+  }
+  fn deref(ptr: *const SharedRef<Self>) -> *mut Self {
+    unsafe { std__shared_ptr__v8__ArrayBuffer__Allocator__get(ptr) }
+  }
+  fn reset(ptr: *mut SharedRef<Self>) {
+    unsafe { std__shared_ptr__v8__ArrayBuffer__Allocator__reset(ptr) }
+  }
+  fn use_count(ptr: *const SharedRef<Self>) -> 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<Allocator> {
+pub fn new_default_allocator() -> SharedRef<Allocator> {
   unsafe {
     UniqueRef::from_raw(v8__ArrayBuffer__Allocator__NewDefaultAllocator())
   }
+  .make_shared()
 }
 
 #[test]

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<v8::ArrayBuffer::Allocator>& 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<v8::ArrayBuffer::Allocator>& ptr) {
+  return make_pod<two_pointers_t>(ptr);
+}
+
+two_pointers_t
+std__shared_ptr__v8__ArrayBuffer__Allocator__CONVERT__std__unique_ptr(
+    v8::ArrayBuffer::Allocator* ptr) {
+  return make_pod<two_pointers_t>(
+      std::shared_ptr<v8::ArrayBuffer::Allocator>(ptr));
+}
+
+v8::ArrayBuffer::Allocator* std__shared_ptr__v8__ArrayBuffer__Allocator__get(
+    const std::shared_ptr<v8::ArrayBuffer::Allocator>& ptr) {
+  return ptr.get();
+}
+
+void std__shared_ptr__v8__ArrayBuffer__Allocator__reset(
+    std::shared_ptr<v8::ArrayBuffer::Allocator>& ptr) {
+  ptr.reset();
+}
+
+long std__shared_ptr__v8__ArrayBuffer__Allocator__use_count(
+    const std::shared_ptr<v8::ArrayBuffer::Allocator>& 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);

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<Allocator>,
   );
   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<Allocator>) {
+  /// 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<Allocator>) {
     unsafe {
-      v8__Isolate__CreateParams__SET__array_buffer_allocator(
-        self,
-        value.into_raw(),
-      )
+      v8__Isolate__CreateParams__SET__array_buffer_allocator(self, &value)
     };
   }
 

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();