src: allocate Buffer memory using ArrayBuffer allocator

Always use the right allocator for memory that is turned into an `ArrayBuffer` at a later point. This enables embedders to use their own `ArrayBuffer::Allocator`s, and is inspired by Electron’s electron/node@f61bae3440e. It should render their downstream patch unnecessary. Refs: electron/node@f61bae3 PR-URL: #26207 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
nodejs · addaleax · Feb 19, 2019 · Feb 18, 2019 · Feb 19, 2019 · Feb 18, 2019
commit 3a08a7a6139926a71cf5a3865e91300ee3aa975a
diff --git a/src/node_buffer.cc b/src/node_buffer.cc
@@ -54,17 +54,6 @@
   size_t length = end - start;
 
 namespace node {
-
-namespace {
-
-inline void* BufferMalloc(size_t length) {
-  return per_process::cli_options->zero_fill_all_buffers ?
-             node::UncheckedCalloc(length) :
-             node::UncheckedMalloc(length);
-}
-
-}  // namespace
-
 namespace Buffer {
 
 using v8::ArrayBuffer;
@@ -260,7 +249,7 @@ MaybeLocal<Object> New(Isolate* isolate,
   char* data = nullptr;
 
   if (length > 0) {
-    data = static_cast<char*>(BufferMalloc(length));
+    data = UncheckedMalloc(length);
 
     if (data == nullptr)
       return Local<Object>();
@@ -276,13 +265,7 @@ MaybeLocal<Object> New(Isolate* isolate,
     }
   }
 
-  Local<Object> buf;
-  if (New(isolate, data, actual).ToLocal(&buf))
-    return scope.Escape(buf);
-
-  // Object failed to be created. Clean up resources.
-  free(data);
-  return Local<Object>();
+  return scope.EscapeMaybe(New(isolate, data, actual));
 }
 
 
@@ -308,28 +291,15 @@ MaybeLocal<Object> New(Environment* env, size_t length) {
     return Local<Object>();
   }
 
-  void* data;
+  AllocatedBuffer ret(env);
   if (length > 0) {
-    data = BufferMalloc(length);
-    if (data == nullptr)
+    ret = env->AllocateManaged(length, false);
+    if (ret.data() == nullptr) {
       return Local<Object>();
-  } else {
-    data = nullptr;
-  }
-
-  Local<ArrayBuffer> ab =
-    ArrayBuffer::New(env->isolate(),
-        data,
-        length,
-        ArrayBufferCreationMode::kInternalized);
-  MaybeLocal<Uint8Array> ui = Buffer::New(env, ab, 0, length);
-
-  if (ui.IsEmpty()) {
-    // Object failed to be created. Clean up resources.
-    free(data);
+    }
   }
 
-  return scope.Escape(ui.FromMaybe(Local<Uint8Array>()));
+  return scope.EscapeMaybe(ret.ToBuffer());
 }
 
 
@@ -355,30 +325,17 @@ MaybeLocal<Object> Copy(Environment* env, const char* data, size_t length) {
     return Local<Object>();
   }
 
-  void* new_data;
+  AllocatedBuffer ret(env);
   if (length > 0) {
     CHECK_NOT_NULL(data);
-    new_data = node::UncheckedMalloc(length);
-    if (new_data == nullptr)
+    ret = env->AllocateManaged(length, false);
+    if (ret.data() == nullptr) {
       return Local<Object>();
-    memcpy(new_data, data, length);
-  } else {
-    new_data = nullptr;
-  }
-
-  Local<ArrayBuffer> ab =
-    ArrayBuffer::New(env->isolate(),
-        new_data,
-        length,
-        ArrayBufferCreationMode::kInternalized);
-  MaybeLocal<Uint8Array> ui = Buffer::New(env, ab, 0, length);
-
-  if (ui.IsEmpty()) {
-    // Object failed to be created. Clean up resources.
-    free(new_data);
+    }
+    memcpy(ret.data(), data, length);
   }
 
-  return scope.Escape(ui.FromMaybe(Local<Uint8Array>()));
+  return scope.EscapeMaybe(ret.ToBuffer());
 }
 
 
@@ -423,7 +380,8 @@ MaybeLocal<Object> New(Environment* env,
   return scope.Escape(ui.ToLocalChecked());
 }
 
-
+// Warning: This function needs `data` to be allocated with malloc() and not
+// necessarily isolate's ArrayBuffer::Allocator.
 MaybeLocal<Object> New(Isolate* isolate, char* data, size_t length) {
   EscapableHandleScope handle_scope(isolate);
   Environment* env = Environment::GetCurrent(isolate);
@@ -433,18 +391,37 @@ MaybeLocal<Object> New(Isolate* isolate, char* data, size_t length) {
     return MaybeLocal<Object>();
   }
   Local<Object> obj;
-  if (Buffer::New(env, data, length).ToLocal(&obj))
+  if (Buffer::New(env, data, length, true).ToLocal(&obj))
     return handle_scope.Escape(obj);
   return Local<Object>();
 }
 
-
-MaybeLocal<Object> New(Environment* env, char* data, size_t length) {
+// Warning: If this call comes through the public node_buffer.h API,
+// the contract for this function is that `data` is allocated with malloc()
+// and not necessarily isolate's ArrayBuffer::Allocator.
+MaybeLocal<Object> New(Environment* env,
+                       char* data,
+                       size_t length,
+                       bool uses_malloc) {
   if (length > 0) {
     CHECK_NOT_NULL(data);
     CHECK(length <= kMaxLength);
   }
 
+  if (uses_malloc) {
+    if (env->isolate_data()->uses_node_allocator()) {
+      // We don't know for sure that the allocator is malloc()-based, so we need
+      // to fall back to the FreeCallback variant.
+      auto free_callback = [](char* data, void* hint) { free(data); };
+      return New(env, data, length, free_callback, nullptr);
+    } else {
+      // This is malloc()-based, so we can acquire it into our own
+      // ArrayBufferAllocator.
+      CHECK_NOT_NULL(env->isolate_data()->node_allocator());
+      env->isolate_data()->node_allocator()->RegisterPointer(data, length);
+    }
+  }
+
   Local<ArrayBuffer> ab =
       ArrayBuffer::New(env->isolate(),
                        data,
@@ -1051,15 +1028,13 @@ static void EncodeUtf8String(const FunctionCallbackInfo<Value>& args) {
 
   Local<String> str = args[0].As<String>();
   size_t length = str->Utf8Length(isolate);
-  char* data = node::UncheckedMalloc(length);
+  AllocatedBuffer buf = env->AllocateManaged(length);
   str->WriteUtf8(isolate,
-                 data,
+                 buf.data(),
                  -1,  // We are certain that `data` is sufficiently large
                  nullptr,
                  String::NO_NULL_TERMINATION | String::REPLACE_INVALID_UTF8);
-  auto array_buf = ArrayBuffer::New(
-      isolate, data, length, ArrayBufferCreationMode::kInternalized);
-  auto array = Uint8Array::New(array_buf, 0, length);
+  auto array = Uint8Array::New(buf.ToArrayBuffer(), 0, length);
   args.GetReturnValue().Set(array);
 }
 
@@ -1121,7 +1096,8 @@ void Initialize(Local<Object> target,
 
   // It can be a nullptr when running inside an isolate where we
   // do not own the ArrayBuffer allocator.
-  if (uint32_t* zero_fill_field = env->isolate_data()->zero_fill_field()) {
+  if (ArrayBufferAllocator* 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));
     CHECK(target