Add custom deleter for externalized arraybuffers

Currently, we use an AllocationKind to signal to the embedder how v8
has allocated the backingstore of an array buffer. The embedder then has
to implement matching logic to free an exernalized buffer. By Using
custom deleters v8 can communicate how to free an externalized array
buffer without requiring all embedders to implement corresponding
freeing logic.

Bug: v8:8073
Cq-Include-Trybots: luci.chromium.try:linux_chromium_rel_ng
Change-Id: I553dec31ba167d6a7b342ded50d685be7dffd1c5
Reviewed-on: https://chromium-review.googlesource.com/1183484
Commit-Queue: Stephan Herhut <herhut@chromium.org>
Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#55347}

include/v8.h

@@ -4597,17 +4597,22 @@ class V8_EXPORT ArrayBuffer : public Object {
    * returns an instance of this class, populated, with a pointer to data
    * and byte length.
    *
-   * The Data pointer of ArrayBuffer::Contents is always allocated with
-   * Allocator::Allocate that is set via Isolate::CreateParams.
+   * The Data pointer of ArrayBuffer::Contents must be freed using the provided
+   * deleter, which will call ArrayBuffer::Allocator::Free if the buffer
+   * was allocated with ArraryBuffer::Allocator::Allocate.
    */
   class V8_EXPORT Contents { // NOLINT
    public:
+    using DeleterCallback = void (*)(void* buffer, size_t length, void* info);
+
     Contents()
         : data_(nullptr),
           byte_length_(0),
           allocation_base_(nullptr),
           allocation_length_(0),
-          allocation_mode_(Allocator::AllocationMode::kNormal) {}
+          allocation_mode_(Allocator::AllocationMode::kNormal),
+          deleter_(nullptr),
+          deleter_data_(nullptr) {}
 
     void* AllocationBase() const { return allocation_base_; }
     size_t AllocationLength() const { return allocation_length_; }
@@ -4617,13 +4622,22 @@ class V8_EXPORT ArrayBuffer : public Object {
 
     void* Data() const { return data_; }
     size_t ByteLength() const { return byte_length_; }
+    DeleterCallback Deleter() const { return deleter_; }
+    void* DeleterData() const { return deleter_data_; }
 
    private:
+    Contents(void* data, size_t byte_length, void* allocation_base,
+             size_t allocation_length,
+             Allocator::AllocationMode allocation_mode, DeleterCallback deleter,
+             void* deleter_data);
+
     void* data_;
     size_t byte_length_;
     void* allocation_base_;
     size_t allocation_length_;
     Allocator::AllocationMode allocation_mode_;
+    DeleterCallback deleter_;
+    void* deleter_data_;
 
     friend class ArrayBuffer;
   };
@@ -4680,8 +4694,9 @@ class V8_EXPORT ArrayBuffer : public Object {
    * had been externalized, it does no longer own the memory block. The caller
    * should take steps to free memory when it is no longer needed.
    *
-   * The memory block is guaranteed to be allocated with |Allocator::Allocate|
-   * that has been set via Isolate::CreateParams.
+   * The Data pointer of ArrayBuffer::Contents must be freed using the provided
+   * deleter, which will call ArrayBuffer::Allocator::Free if the buffer
+   * was allocated with ArraryBuffer::Allocator::Allocate.
    */
   Contents Externalize();
 
@@ -4692,8 +4707,6 @@ class V8_EXPORT ArrayBuffer : public Object {
    *
    * The embedder should make sure to hold a strong reference to the
    * ArrayBuffer while accessing this pointer.
-   *
-   * The memory block is guaranteed to be allocated with |Allocator::Allocate|.
    */
   Contents GetContents();
 
@@ -5000,41 +5013,54 @@ class V8_EXPORT SharedArrayBuffer : public Object {
    * |SharedArrayBuffer| returns an instance of this class, populated, with a
    * pointer to data and byte length.
    *
-   * The Data pointer of SharedArrayBuffer::Contents is always allocated with
-   * |ArrayBuffer::Allocator::Allocate| by the allocator specified in
-   * v8::Isolate::CreateParams::array_buffer_allocator.
+   * The Data pointer of ArrayBuffer::Contents must be freed using the provided
+   * deleter, which will call ArrayBuffer::Allocator::Free if the buffer
+   * was allocated with ArraryBuffer::Allocator::Allocate.
    *
    * This API is experimental and may change significantly.
    */
   class V8_EXPORT Contents {  // NOLINT
    public:
+    using Allocator = v8::ArrayBuffer::Allocator;
+    using DeleterCallback = void (*)(void* buffer, size_t length, void* info);
+
     Contents()
         : data_(nullptr),
           byte_length_(0),
           allocation_base_(nullptr),
           allocation_length_(0),
-          allocation_mode_(ArrayBuffer::Allocator::AllocationMode::kNormal) {}
+          allocation_mode_(Allocator::AllocationMode::kNormal),
+          deleter_(nullptr),
+          deleter_data_(nullptr) {}
 
     void* AllocationBase() const { return allocation_base_; }
     size_t AllocationLength() const { return allocation_length_; }
-    ArrayBuffer::Allocator::AllocationMode AllocationMode() const {
+    Allocator::AllocationMode AllocationMode() const {
       return allocation_mode_;
     }
 
     void* Data() const { return data_; }
     size_t ByteLength() const { return byte_length_; }
+    DeleterCallback Deleter() const { return deleter_; }
+    void* DeleterData() const { return deleter_data_; }
 
    private:
+    Contents(void* data, size_t byte_length, void* allocation_base,
+             size_t allocation_length,
+             Allocator::AllocationMode allocation_mode, DeleterCallback deleter,
+             void* deleter_data);
+
     void* data_;
     size_t byte_length_;
     void* allocation_base_;
     size_t allocation_length_;
-    ArrayBuffer::Allocator::AllocationMode allocation_mode_;
+    Allocator::AllocationMode allocation_mode_;
+    DeleterCallback deleter_;
+    void* deleter_data_;
 
     friend class SharedArrayBuffer;
   };
 
-
   /**
    * Data length in bytes.
    */

src/api.cc

@@ -7489,18 +7489,46 @@ v8::ArrayBuffer::Contents v8::ArrayBuffer::Externalize() {
   return contents;
 }
 
+v8::ArrayBuffer::Contents::Contents(void* data, size_t byte_length,
+                                    void* allocation_base,
+                                    size_t allocation_length,
+                                    Allocator::AllocationMode allocation_mode,
+                                    DeleterCallback deleter, void* deleter_data)
+    : data_(data),
+      byte_length_(byte_length),
+      allocation_base_(allocation_base),
+      allocation_length_(allocation_length),
+      allocation_mode_(allocation_mode),
+      deleter_(deleter),
+      deleter_data_(deleter_data) {
+  DCHECK_LE(allocation_base_, data_);
+  DCHECK_LE(byte_length_, allocation_length_);
+}
+
+void WasmMemoryDeleter(void* buffer, size_t lenght,
+                       internal::wasm::WasmEngine* engine) {
+  CHECK(engine->memory_tracker()->FreeMemoryIfIsWasmMemory(nullptr, buffer));
+}
+
+void ArrayBufferDeleter(void* buffer, size_t length,
+                        ArrayBufferAllocator* allocator) {
+  allocator->Free(buffer, length);
+}
 
 v8::ArrayBuffer::Contents v8::ArrayBuffer::GetContents() {
   i::Handle<i::JSArrayBuffer> self = Utils::OpenHandle(this);
   size_t byte_length = static_cast<size_t>(self->byte_length()->Number());
-  Contents contents;
-  contents.allocation_base_ = self->allocation_base();
-  contents.allocation_length_ = self->allocation_length();
-  contents.allocation_mode_ = self->is_wasm_memory()
-                                  ? Allocator::AllocationMode::kReservation
-                                  : Allocator::AllocationMode::kNormal;
-  contents.data_ = self->backing_store();
-  contents.byte_length_ = byte_length;
+  Contents contents(
+      self->backing_store(), byte_length, self->allocation_base(),
+      self->allocation_length(),
+      self->is_wasm_memory() ? Allocator::AllocationMode::kReservation
+                             : Allocator::AllocationMode::kNormal,
+      self->is_wasm_memory()
+          ? reinterpret_cast<Contents::DeleterCallback>(WasmMemoryDeleter)
+          : reinterpret_cast<Contents::DeleterCallback>(ArrayBufferDeleter),
+      self->is_wasm_memory()
+          ? static_cast<void*>(self->GetIsolate()->wasm_engine())
+          : static_cast<void*>(self->GetIsolate()->array_buffer_allocator()));
   return contents;
 }
 
@@ -7698,7 +7726,6 @@ bool v8::SharedArrayBuffer::IsExternal() const {
   return Utils::OpenHandle(this)->is_external();
 }
 
-
 v8::SharedArrayBuffer::Contents v8::SharedArrayBuffer::Externalize() {
   i::Handle<i::JSArrayBuffer> self = Utils::OpenHandle(this);
   i::Isolate* isolate = self->GetIsolate();
@@ -7723,29 +7750,44 @@ v8::SharedArrayBuffer::Contents v8::SharedArrayBuffer::Externalize() {
   return contents;
 }
 
+v8::SharedArrayBuffer::Contents::Contents(
+    void* data, size_t byte_length, void* allocation_base,
+    size_t allocation_length, Allocator::AllocationMode allocation_mode,
+    DeleterCallback deleter, void* deleter_data)
+    : data_(data),
+      byte_length_(byte_length),
+      allocation_base_(allocation_base),
+      allocation_length_(allocation_length),
+      allocation_mode_(allocation_mode),
+      deleter_(deleter),
+      deleter_data_(deleter_data) {
+  DCHECK_LE(allocation_base_, data_);
+  DCHECK_LE(byte_length_, allocation_length_);
+}
 
 v8::SharedArrayBuffer::Contents v8::SharedArrayBuffer::GetContents() {
   i::Handle<i::JSArrayBuffer> self = Utils::OpenHandle(this);
   size_t byte_length = static_cast<size_t>(self->byte_length()->Number());
-  Contents contents;
-  contents.allocation_base_ = self->allocation_base();
-  contents.allocation_length_ = self->allocation_length();
-  contents.allocation_mode_ =
+  Contents contents(
+      self->backing_store(), byte_length, self->allocation_base(),
+      self->allocation_length(),
+      self->is_wasm_memory()
+          ? ArrayBuffer::Allocator::AllocationMode::kReservation
+          : ArrayBuffer::Allocator::AllocationMode::kNormal,
       self->is_wasm_memory()
-          ? ArrayBufferAllocator::Allocator::AllocationMode::kReservation
-          : ArrayBufferAllocator::Allocator::AllocationMode::kNormal;
-  contents.data_ = self->backing_store();
-  contents.byte_length_ = byte_length;
+          ? reinterpret_cast<Contents::DeleterCallback>(WasmMemoryDeleter)
+          : reinterpret_cast<Contents::DeleterCallback>(ArrayBufferDeleter),
+      self->is_wasm_memory()
+          ? static_cast<void*>(self->GetIsolate()->wasm_engine())
+          : static_cast<void*>(self->GetIsolate()->array_buffer_allocator()));
   return contents;
 }
 
-
 size_t v8::SharedArrayBuffer::ByteLength() const {
   i::Handle<i::JSArrayBuffer> obj = Utils::OpenHandle(this);
   return static_cast<size_t>(obj->byte_length()->Number());
 }
 
-
 Local<SharedArrayBuffer> v8::SharedArrayBuffer::New(Isolate* isolate,
                                                     size_t byte_length) {
   CHECK(i::FLAG_harmony_sharedarraybuffer);

src/wasm/wasm-memory.cc

@@ -179,7 +179,10 @@ WasmMemoryTracker::AllocationData WasmMemoryTracker::ReleaseAllocation(
     DCHECK_LE(num_bytes, allocated_address_space_);
     reserved_address_space_ -= num_bytes;
     allocated_address_space_ -= num_bytes;
-    AddAddressSpaceSample(isolate);
+    // ReleaseAllocation might be called with a nullptr as isolate if the
+    // embedder is releasing the allocation and not a specific isolate. This
+    // happens if the allocation was shared between multiple isolates (threads).
+    if (isolate) AddAddressSpaceSample(isolate);
 
     AllocationData allocation_data = find_result->second;
     allocations_.erase(find_result);