deps: V8: cherry-pick e29c62b74854

Original commit message:

    [arraybuffer] Clean up BackingStore even if it pointer to nullptr

    For a zero-length BackingStore allocation, it is valid for the
    underlying memory to be a null pointer. However, some cleanup
    is still necessary, since the BackingStore may hold a reference
    to the allocator itself, which needs to be released when destroying
    the `BackingStore` instance.

    Change-Id: I1f168079d39e4592d2fde31fbe5f705586690e85
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2169646
    Reviewed-by: Ulan Degenbaev <[email protected]>
    Commit-Queue: Ulan Degenbaev <[email protected]>
    Cr-Commit-Position: refs/heads/master@{#67420}

Refs: v8/v8@e29c62b

Backport-PR-URL: #33376
PR-URL: #32831
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>

Author: targos

common.gypi

@@ -36,7 +36,7 @@
 
     # Reset this number to 0 on major V8 upgrades.
     # Increment by one for each non-official patch applied to deps/v8.
-    'v8_embedder_string': '-node.11',
+    'v8_embedder_string': '-node.12',
 
     ##### V8 defaults for Node.js #####
 

deps/v8/src/objects/backing-store.cc

@@ -165,7 +165,10 @@ void BackingStore::Clear() {
 BackingStore::~BackingStore() {
   GlobalBackingStoreRegistry::Unregister(this);
 
-  if (buffer_start_ == nullptr) return;  // nothing to deallocate
+  if (buffer_start_ == nullptr) {
+    Clear();
+    return;
+  }
 
   if (is_wasm_memory_) {
     DCHECK(free_on_destruct_);

deps/v8/test/cctest/test-api-array-buffer.cc

@@ -799,6 +799,46 @@ TEST(BackingStore_HoldAllocatorAlive_AfterIsolateShutdown) {
   CHECK(allocator_weak.expired());
 }
 
+class NullptrAllocator final : public v8::ArrayBuffer::Allocator {
+ public:
+  void* Allocate(size_t length) override {
+    CHECK_EQ(length, 0);
+    return nullptr;
+  }
+  void* AllocateUninitialized(size_t length) override {
+    CHECK_EQ(length, 0);
+    return nullptr;
+  }
+  void Free(void* data, size_t length) override { CHECK_EQ(data, nullptr); }
+};
+
+TEST(BackingStore_ReleaseAllocator_NullptrBackingStore) {
+  std::shared_ptr<NullptrAllocator> allocator =
+      std::make_shared<NullptrAllocator>();
+  std::weak_ptr<NullptrAllocator> allocator_weak(allocator);
+
+  v8::Isolate::CreateParams create_params;
+  create_params.array_buffer_allocator_shared = allocator;
+  v8::Isolate* isolate = v8::Isolate::New(create_params);
+  isolate->Enter();
+
+  allocator.reset();
+  create_params.array_buffer_allocator_shared.reset();
+  CHECK(!allocator_weak.expired());
+
+  {
+    std::shared_ptr<v8::BackingStore> backing_store =
+        v8::ArrayBuffer::NewBackingStore(isolate, 0);
+    // This should release a reference to the allocator, even though the
+    // buffer is empty/nullptr.
+    backing_store.reset();
+  }
+
+  isolate->Exit();
+  isolate->Dispose();
+  CHECK(allocator_weak.expired());
+}
+
 TEST(BackingStore_ReallocateExpand) {
   LocalContext env;
   v8::Isolate* isolate = env->GetIsolate();