deps: V8: backport 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 <ulan@chromium.org>
    Commit-Queue: Ulan Degenbaev <ulan@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#67420}

Refs: https://github.com/v8/v8/commit/e29c62b7485462c1ce8f4129b26e7f7a4d4b193c

PR-URL: https://github.com/nodejs/node/pull/33125
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>

3 files changed, 45 insertions, 2 deletions

diff --git a/common.gypi b/common.gypi
index 2ebabf8f80..5e4f9b6dba 100644
--- a/common.gypi
+++ b/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.32',
+    'v8_embedder_string': '-node.33',
 
     ##### V8 defaults for Node.js #####
 
diff --git a/deps/v8/src/objects/backing-store.cc b/deps/v8/src/objects/backing-store.cc
index 8d604ba35d..c9a5c2e4c8 100644
--- a/deps/v8/src/objects/backing-store.cc
+++ b/deps/v8/src/objects/backing-store.cc
@@ -164,7 +164,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_);
diff --git a/deps/v8/test/cctest/test-api-array-buffer.cc b/deps/v8/test/cctest/test-api-array-buffer.cc
index e8e026f156..eeddbcca10 100644
--- a/deps/v8/test/cctest/test-api-array-buffer.cc
+++ b/deps/v8/test/cctest/test-api-array-buffer.cc
@@ -729,6 +729,46 @@ TEST(BackingStore_HoldAllocatorAlive_UntilIsolateShutdown) {
   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_HoldAllocatorAlive_AfterIsolateShutdown) {
   std::shared_ptr<DummyAllocator> allocator =
       std::make_shared<DummyAllocator>();