src: migrate to new V8 ArrayBuffer API

ArrayBuffer without BackingStore will soon be deprecated.

Fixes:https://github.com/nodejs/node/issues/30529

PR-URL: https://github.com/nodejs/node/pull/30782
Fixes: https://github.com/nodejs/node/issues/30529
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>

7 files changed, 128 insertions, 27 deletions

diff --git a/src/env-inl.h b/src/env-inl.h
index d75b4ea743c..91a69ddd8f3 100644
--- a/src/env-inl.h
+++ b/src/env-inl.h
@@ -983,10 +983,20 @@ inline v8::MaybeLocal<v8::Object> AllocatedBuffer::ToBuffer() {
 inline v8::Local<v8::ArrayBuffer> AllocatedBuffer::ToArrayBuffer() {
   CHECK_NOT_NULL(env_);
   uv_buf_t buf = release();
+  auto callback = [](void* data, size_t length, void* deleter_data){
+    CHECK_NOT_NULL(deleter_data);
+
+    static_cast<v8::ArrayBuffer::Allocator*>(deleter_data)
+        ->Free(data, length);
+  };
+  std::unique_ptr<v8::BackingStore> backing =
+      v8::ArrayBuffer::NewBackingStore(buf.base,
+                                       buf.len,
+                                       callback,
+                                       env_->isolate()
+                                          ->GetArrayBufferAllocator());
   return v8::ArrayBuffer::New(env_->isolate(),
-                              buf.base,
-                              buf.len,
-                              v8::ArrayBufferCreationMode::kInternalized);
+                              std::move(backing));
 }
 
 inline void Environment::ThrowError(const char* errmsg) {
diff --git a/src/js_native_api_v8.cc b/src/js_native_api_v8.cc
index 5506b2b4c62..52e2d7f582e 100644
--- a/src/js_native_api_v8.cc
+++ b/src/js_native_api_v8.cc
@@ -2612,8 +2612,18 @@ napi_status napi_create_external_arraybuffer(napi_env env,
   CHECK_ARG(env, result);
 
   v8::Isolate* isolate = env->isolate;
+  // The buffer will be freed with v8impl::ArrayBufferReference::New()
+  // below, hence this BackingStore does not need to free the buffer.
+  std::unique_ptr<v8::BackingStore> backing =
+      v8::ArrayBuffer::NewBackingStore(external_data,
+                                       byte_length,
+                                       [](void*, size_t, void*){},
+                                       nullptr);
   v8::Local<v8::ArrayBuffer> buffer =
-      v8::ArrayBuffer::New(isolate, external_data, byte_length);
+      v8::ArrayBuffer::New(isolate, std::move(backing));
+  // TODO(thangktran): drop this check when V8 is pumped to 8.0 .
+  if (!buffer->IsExternal())
+    buffer->Externalize(buffer->GetBackingStore());
   v8::Maybe<bool> marked = env->mark_arraybuffer_as_untransferable(buffer);
   CHECK_MAYBE_NOTHING(env, marked, napi_generic_failure);
 
diff --git a/src/node_buffer.cc b/src/node_buffer.cc
index e5c4655b4cc..c04be68d1b0 100644
--- a/src/node_buffer.cc
+++ b/src/node_buffer.cc
@@ -47,8 +47,8 @@ namespace node {
 namespace Buffer {
 
 using v8::ArrayBuffer;
-using v8::ArrayBufferCreationMode;
 using v8::ArrayBufferView;
+using v8::BackingStore;
 using v8::Context;
 using v8::EscapableHandleScope;
 using v8::FunctionCallbackInfo;
@@ -127,8 +127,8 @@ CallbackInfo::CallbackInfo(Environment* env,
       data_(data),
       hint_(hint),
       env_(env) {
-  ArrayBuffer::Contents obj_c = object->GetContents();
-  CHECK_EQ(data_, static_cast<char*>(obj_c.Data()));
+  std::shared_ptr<BackingStore> obj_backing = object->GetBackingStore();
+  CHECK_EQ(data_, static_cast<char*>(obj_backing->Data()));
   if (object->ByteLength() != 0)
     CHECK_NOT_NULL(data_);
 
@@ -406,7 +406,19 @@ MaybeLocal<Object> New(Environment* env,
     return Local<Object>();
   }
 
-  Local<ArrayBuffer> ab = ArrayBuffer::New(env->isolate(), data, length);
+
+  // The buffer will be released by a CallbackInfo::New() below,
+  // hence this BackingStore callback is empty.
+  std::unique_ptr<BackingStore> backing =
+      ArrayBuffer::NewBackingStore(data,
+                                   length,
+                                   [](void*, size_t, void*){},
+                                   nullptr);
+  Local<ArrayBuffer> ab = ArrayBuffer::New(env->isolate(),
+                                           std::move(backing));
+  // TODO(thangktran): drop this check when V8 is pumped to 8.0 .
+  if (!ab->IsExternal())
+    ab->Externalize(ab->GetBackingStore());
   if (ab->SetPrivate(env->context(),
                      env->arraybuffer_untransferable_private_symbol(),
                      True(env->isolate())).IsNothing()) {
@@ -465,11 +477,21 @@ MaybeLocal<Object> New(Environment* env,
     }
   }
 
-  Local<ArrayBuffer> ab =
-      ArrayBuffer::New(env->isolate(),
-                       data,
-                       length,
-                       ArrayBufferCreationMode::kInternalized);
+  auto callback = [](void* data, size_t length, void* deleter_data){
+    CHECK_NOT_NULL(deleter_data);
+
+    static_cast<v8::ArrayBuffer::Allocator*>(deleter_data)
+        ->Free(data, length);
+  };
+  std::unique_ptr<v8::BackingStore> backing =
+      v8::ArrayBuffer::NewBackingStore(data,
+                                       length,
+                                       callback,
+                                       env->isolate()
+                                          ->GetArrayBufferAllocator());
+  Local<ArrayBuffer> ab = ArrayBuffer::New(env->isolate(),
+                                           std::move(backing));
+
   return Buffer::New(env, ab, 0, length).FromMaybe(Local<Object>());
 }
 
@@ -1181,8 +1203,16 @@ void Initialize(Local<Object> target,
   if (NodeArrayBufferAllocator* allocator =
           env->isolate_data()->node_allocator()) {
     uint32_t* zero_fill_field = allocator->zero_fill_field();
-    Local<ArrayBuffer> array_buffer = ArrayBuffer::New(
-        env->isolate(), zero_fill_field, sizeof(*zero_fill_field));
+    std::unique_ptr<BackingStore> backing =
+      ArrayBuffer::NewBackingStore(zero_fill_field,
+                                   sizeof(*zero_fill_field),
+                                   [](void*, size_t, void*){},
+                                   nullptr);
+    Local<ArrayBuffer> array_buffer =
+        ArrayBuffer::New(env->isolate(), std::move(backing));
+    // TODO(thangktran): drop this check when V8 is pumped to 8.0 .
+    if (!array_buffer->IsExternal())
+      array_buffer->Externalize(array_buffer->GetBackingStore());
     CHECK(target
               ->Set(env->context(),
                     FIXED_ONE_BYTE_STRING(env->isolate(), "zeroFill"),
diff --git a/src/node_http2.cc b/src/node_http2.cc
index 760533b6054..db3bd035b34 100644
--- a/src/node_http2.cc
+++ b/src/node_http2.cc
@@ -16,6 +16,7 @@ namespace node {
 
 using v8::ArrayBuffer;
 using v8::ArrayBufferView;
+using v8::BackingStore;
 using v8::Boolean;
 using v8::Context;
 using v8::Float64Array;
@@ -566,10 +567,18 @@ Http2Session::Http2Session(Environment* env,
 
   {
     // Make the js_fields_ property accessible to JS land.
+    std::unique_ptr<BackingStore> backing =
+        ArrayBuffer::NewBackingStore(
+            reinterpret_cast<uint8_t*>(&js_fields_),
+            kSessionUint8FieldCount,
+            [](void*, size_t, void*){},
+            nullptr);
     Local<ArrayBuffer> ab =
-        ArrayBuffer::New(env->isolate(),
-                         reinterpret_cast<uint8_t*>(&js_fields_),
-                         kSessionUint8FieldCount);
+        ArrayBuffer::New(env->isolate(), std::move(backing));
+    // TODO(thangktran): drop this check when V8 is pumped to 8.0 .
+    if (!ab->IsExternal())
+      ab->Externalize(ab->GetBackingStore());
+    js_fields_ab_.Reset(env->isolate(), ab);
     Local<Uint8Array> uint8_arr =
         Uint8Array::New(ab, 0, kSessionUint8FieldCount);
     USE(wrap->Set(env->context(), env->fields_string(), uint8_arr));
@@ -581,6 +590,14 @@ Http2Session::~Http2Session() {
   Debug(this, "freeing nghttp2 session");
   nghttp2_session_del(session_);
   CHECK_EQ(current_nghttp2_memory_, 0);
+  HandleScope handle_scope(env()->isolate());
+  // Detach js_fields_ab_ to avoid having problem when new Http2Session
+  // instances are created on the same location of previous
+  // instances. This in turn will call ArrayBuffer::NewBackingStore()
+  // multiple times with the same buffer address and causing error.
+  // Ref: https://github.com/nodejs/node/pull/30782
+  Local<ArrayBuffer> ab = js_fields_ab_.Get(env()->isolate());
+  ab->Detach();
 }
 
 std::string Http2Session::diagnostic_name() const {
diff --git a/src/node_http2.h b/src/node_http2.h
index 07febd7da40..264b112fec7 100644
--- a/src/node_http2.h
+++ b/src/node_http2.h
@@ -989,6 +989,7 @@ class Http2Session : public AsyncWrap,
 
   // JS-accessible numeric fields, as indexed by SessionUint8Fields.
   SessionJSFields js_fields_ = {};
+  v8::Global<v8::ArrayBuffer> js_fields_ab_;
 
   // The session type: client or server
   nghttp2_session_type session_type_;
diff --git a/src/node_v8.cc b/src/node_v8.cc
index ed2e71de106..1f4ef0e35f5 100644
--- a/src/node_v8.cc
+++ b/src/node_v8.cc
@@ -28,6 +28,7 @@ namespace node {
 
 using v8::Array;
 using v8::ArrayBuffer;
+using v8::BackingStore;
 using v8::Context;
 using v8::FunctionCallbackInfo;
 using v8::HeapCodeStatistics;
@@ -162,12 +163,22 @@ void Initialize(Local<Object> target,
   const size_t heap_statistics_buffer_byte_length =
       sizeof(*env->heap_statistics_buffer()) * kHeapStatisticsPropertiesCount;
 
+  std::unique_ptr<BackingStore> heap_statistics_backing =
+      ArrayBuffer::NewBackingStore(env->heap_statistics_buffer(),
+                                   heap_statistics_buffer_byte_length,
+                                   [](void*, size_t, void*){},
+                                   nullptr);
+  Local<ArrayBuffer> heap_statistics_ab =
+      ArrayBuffer::New(env->isolate(),
+                       std::move(heap_statistics_backing));
+  // TODO(thangktran): drop this check when V8 is pumped to 8.0 .
+  if (!heap_statistics_ab->IsExternal())
+    heap_statistics_ab->Externalize(
+        heap_statistics_ab->GetBackingStore());
   target->Set(env->context(),
               FIXED_ONE_BYTE_STRING(env->isolate(),
                                     "heapStatisticsArrayBuffer"),
-              ArrayBuffer::New(env->isolate(),
-                               env->heap_statistics_buffer(),
-                               heap_statistics_buffer_byte_length)).Check();
+              heap_statistics_ab).Check();
 
 #define V(i, _, name)                                                         \
   target->Set(env->context(),                                                 \
@@ -189,12 +200,22 @@ void Initialize(Local<Object> target,
       sizeof(*env->heap_code_statistics_buffer())
       * kHeapCodeStatisticsPropertiesCount;
 
+  std::unique_ptr<BackingStore> heap_code_statistics_backing =
+      ArrayBuffer::NewBackingStore(env->heap_code_statistics_buffer(),
+                                   heap_code_statistics_buffer_byte_length,
+                                   [](void*, size_t, void*){},
+                                   nullptr);
+  Local<ArrayBuffer> heap_code_statistics_ab =
+      ArrayBuffer::New(env->isolate(),
+                       std::move(heap_code_statistics_backing));
+  // TODO(thangktran): drop this check when V8 is pumped to 8.0 .
+  if (!heap_code_statistics_ab->IsExternal())
+    heap_code_statistics_ab->Externalize(
+        heap_code_statistics_ab->GetBackingStore());
   target->Set(env->context(),
               FIXED_ONE_BYTE_STRING(env->isolate(),
                                     "heapCodeStatisticsArrayBuffer"),
-              ArrayBuffer::New(env->isolate(),
-                               env->heap_code_statistics_buffer(),
-                               heap_code_statistics_buffer_byte_length))
+              heap_code_statistics_ab)
   .Check();
 
 #define V(i, _, name)                                                         \
@@ -244,12 +265,22 @@ void Initialize(Local<Object> target,
       kHeapSpaceStatisticsPropertiesCount *
       number_of_heap_spaces;
 
+  std::unique_ptr<BackingStore> heap_space_statistics_backing =
+      ArrayBuffer::NewBackingStore(env->heap_space_statistics_buffer(),
+                                   heap_space_statistics_buffer_byte_length,
+                                   [](void*, size_t, void*){},
+                                   nullptr);
+  Local<ArrayBuffer> heap_space_statistics_ab =
+      ArrayBuffer::New(env->isolate(),
+                               std::move(heap_space_statistics_backing));
+  // TODO(thangktran): drop this check when V8 is pumped to 8.0 .
+  if (!heap_space_statistics_ab->IsExternal())
+    heap_space_statistics_ab->Externalize(
+        heap_space_statistics_ab->GetBackingStore());
   target->Set(env->context(),
               FIXED_ONE_BYTE_STRING(env->isolate(),
                                     "heapSpaceStatisticsArrayBuffer"),
-              ArrayBuffer::New(env->isolate(),
-                               env->heap_space_statistics_buffer(),
-                               heap_space_statistics_buffer_byte_length))
+              heap_space_statistics_ab)
               .Check();
 
 #define V(i, _, name)                                                         \
diff --git a/src/node_worker.cc b/src/node_worker.cc
index 9976ec3f0ac..a7449e44118 100644
--- a/src/node_worker.cc
+++ b/src/node_worker.cc
@@ -19,6 +19,7 @@
 using node::kDisallowedInEnvironment;
 using v8::Array;
 using v8::ArrayBuffer;
+using v8::BackingStore;
 using v8::Boolean;
 using v8::Context;
 using v8::Float64Array;
@@ -622,6 +623,7 @@ void Worker::GetResourceLimits(const FunctionCallbackInfo<Value>& args) {
 
 Local<Float64Array> Worker::GetResourceLimits(Isolate* isolate) const {
   Local<ArrayBuffer> ab = ArrayBuffer::New(isolate, sizeof(resource_limits_));
+
   memcpy(ab->GetBackingStore()->Data(),
          resource_limits_,
          sizeof(resource_limits_));