9 files changed, 116 insertions, 194 deletions

diff --git a/src/allocated_buffer-inl.h b/src/allocated_buffer-inl.h
index 22956b2f2c..e10aa08e7f 100644
--- a/src/allocated_buffer-inl.h
+++ b/src/allocated_buffer-inl.h
@@ -12,97 +12,97 @@
 
 namespace node {
 
+// It's a bit awkward to define this Buffer::New() overload here, but it
+// avoids a circular dependency with node_internals.h.
+namespace Buffer {
+v8::MaybeLocal<v8::Uint8Array> New(Environment* env,
+                                   v8::Local<v8::ArrayBuffer> ab,
+                                   size_t byte_offset,
+                                   size_t length);
+}
+
+NoArrayBufferZeroFillScope::NoArrayBufferZeroFillScope(
+    IsolateData* isolate_data)
+  : node_allocator_(isolate_data->node_allocator()) {
+  if (node_allocator_ != nullptr) node_allocator_->zero_fill_field()[0] = 0;
+}
+
+NoArrayBufferZeroFillScope::~NoArrayBufferZeroFillScope() {
+  if (node_allocator_ != nullptr) node_allocator_->zero_fill_field()[0] = 1;
+}
+
 AllocatedBuffer AllocatedBuffer::AllocateManaged(
     Environment* env,
-    size_t size,
-    int flags) {
-  char* data = flags & ALLOCATE_MANAGED_UNCHECKED ?
-      env->AllocateUnchecked(size) :
-      env->Allocate(size);
-  if (data == nullptr) size = 0;
-  return AllocatedBuffer(env, uv_buf_init(data, size));
+    size_t size) {
+  NoArrayBufferZeroFillScope no_zero_fill_scope(env->isolate_data());
+  std::unique_ptr<v8::BackingStore> bs =
+      v8::ArrayBuffer::NewBackingStore(env->isolate(), size);
+  return AllocatedBuffer(env, std::move(bs));
 }
 
-inline AllocatedBuffer::AllocatedBuffer(Environment* env, uv_buf_t buf)
-    : env_(env), buffer_(buf) {}
+AllocatedBuffer::AllocatedBuffer(
+    Environment* env, std::unique_ptr<v8::BackingStore> bs)
+    : env_(env), backing_store_(std::move(bs)) {}
+
+AllocatedBuffer::AllocatedBuffer(
+    Environment* env, uv_buf_t buffer)
+    : env_(env) {
+  if (buffer.base == nullptr) return;
+  auto map = env->released_allocated_buffers();
+  auto it = map->find(buffer.base);
+  CHECK_NE(it, map->end());
+  backing_store_ = std::move(it->second);
+  map->erase(it);
+}
 
-inline void AllocatedBuffer::Resize(size_t len) {
-  // The `len` check is to make sure we don't end up with `nullptr` as our base.
-  char* new_data = env_->Reallocate(buffer_.base, buffer_.len,
-                                    len > 0 ? len : 1);
-  CHECK_NOT_NULL(new_data);
-  buffer_ = uv_buf_init(new_data, len);
+void AllocatedBuffer::Resize(size_t len) {
+  if (len == 0) {
+    backing_store_ = v8::ArrayBuffer::NewBackingStore(env_->isolate(), 0);
+    return;
+  }
+  NoArrayBufferZeroFillScope no_zero_fill_scope(env_->isolate_data());
+  backing_store_ = v8::BackingStore::Reallocate(
+      env_->isolate(), std::move(backing_store_), len);
 }
 
 inline uv_buf_t AllocatedBuffer::release() {
-  uv_buf_t ret = buffer_;
-  buffer_ = uv_buf_init(nullptr, 0);
+  if (data() == nullptr) return uv_buf_init(nullptr, 0);
+
+  CHECK_NOT_NULL(env_);
+  uv_buf_t ret = uv_buf_init(data(), size());
+  env_->released_allocated_buffers()->emplace(
+      ret.base, std::move(backing_store_));
   return ret;
 }
 
 inline char* AllocatedBuffer::data() {
-  return buffer_.base;
+  if (!backing_store_) return nullptr;
+  return static_cast<char*>(backing_store_->Data());
 }
 
 inline const char* AllocatedBuffer::data() const {
-  return buffer_.base;
+  if (!backing_store_) return nullptr;
+  return static_cast<char*>(backing_store_->Data());
 }
 
-inline size_t AllocatedBuffer::size() const {
-  return buffer_.len;
-}
-
-inline AllocatedBuffer::AllocatedBuffer(Environment* env)
-    : env_(env), buffer_(uv_buf_init(nullptr, 0)) {}
-
-inline AllocatedBuffer::AllocatedBuffer(AllocatedBuffer&& other)
-    : AllocatedBuffer() {
-  *this = std::move(other);
-}
-
-inline AllocatedBuffer& AllocatedBuffer::operator=(AllocatedBuffer&& other) {
-  clear();
-  env_ = other.env_;
-  buffer_ = other.release();
-  return *this;
-}
 
-inline AllocatedBuffer::~AllocatedBuffer() {
-  clear();
+inline size_t AllocatedBuffer::size() const {
+  if (!backing_store_) return 0;
+  return backing_store_->ByteLength();
 }
 
 inline void AllocatedBuffer::clear() {
-  uv_buf_t buf = release();
-  if (buf.base != nullptr) {
-    CHECK_NOT_NULL(env_);
-    env_->Free(buf.base, buf.len);
-  }
+  backing_store_.reset();
 }
 
 inline v8::MaybeLocal<v8::Object> AllocatedBuffer::ToBuffer() {
-  CHECK_NOT_NULL(env_);
-  v8::MaybeLocal<v8::Object> obj = Buffer::New(env_, data(), size(), false);
-  if (!obj.IsEmpty()) release();
-  return obj;
+  v8::Local<v8::ArrayBuffer> ab = ToArrayBuffer();
+  return Buffer::New(env_, ab, 0, ab->ByteLength())
+      .FromMaybe(v8::Local<v8::Uint8Array>());
 }
 
 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(),
-                              std::move(backing));
+  return v8::ArrayBuffer::New(env_->isolate(), std::move(backing_store_));
 }
 
 }  // namespace node
diff --git a/src/allocated_buffer.h b/src/allocated_buffer.h
index b54f74b2d2..c984a342a8 100644
--- a/src/allocated_buffer.h
+++ b/src/allocated_buffer.h
@@ -6,31 +6,43 @@
 #include "base_object.h"
 #include "uv.h"
 #include "v8.h"
+#include "env.h"
 
 namespace node {
 
 class Environment;
 
+// Disables zero-filling for ArrayBuffer allocations in this scope. This is
+// similar to how we implement Buffer.allocUnsafe() in JS land.
+class NoArrayBufferZeroFillScope{
+ public:
+  inline explicit NoArrayBufferZeroFillScope(IsolateData* isolate_data);
+  inline ~NoArrayBufferZeroFillScope();
+
+ private:
+  NodeArrayBufferAllocator* node_allocator_;
+
+  friend class Environment;
+};
+
 // A unique-pointer-ish object that is compatible with the JS engine's
 // ArrayBuffer::Allocator.
+// TODO(addaleax): We may want to start phasing this out as it's only a
+// thin wrapper around v8::BackingStore at this point
 struct AllocatedBuffer {
  public:
-  enum AllocateManagedFlags {
-    ALLOCATE_MANAGED_FLAG_NONE,
-    ALLOCATE_MANAGED_UNCHECKED
-  };
-
   // Utilities that allocate memory using the Isolate's ArrayBuffer::Allocator.
   // In particular, using AllocateManaged() will provide a RAII-style object
   // with easy conversion to `Buffer` and `ArrayBuffer` objects.
-  inline static AllocatedBuffer AllocateManaged(
-      Environment* env,
-      size_t size,
-      int flags = ALLOCATE_MANAGED_FLAG_NONE);
-
-  explicit inline AllocatedBuffer(Environment* env = nullptr);
-  inline AllocatedBuffer(Environment* env, uv_buf_t buf);
-  inline ~AllocatedBuffer();
+  inline static AllocatedBuffer AllocateManaged(Environment* env, size_t size);
+
+  AllocatedBuffer() = default;
+  inline AllocatedBuffer(
+      Environment* env, std::unique_ptr<v8::BackingStore> bs);
+  // For this constructor variant, `buffer` *must* come from an earlier call
+  // to .release
+  inline AllocatedBuffer(Environment* env, uv_buf_t buffer);
+
   inline void Resize(size_t len);
 
   inline uv_buf_t release();
@@ -42,16 +54,14 @@ struct AllocatedBuffer {
   inline v8::MaybeLocal<v8::Object> ToBuffer();
   inline v8::Local<v8::ArrayBuffer> ToArrayBuffer();
 
-  inline AllocatedBuffer(AllocatedBuffer&& other);
-  inline AllocatedBuffer& operator=(AllocatedBuffer&& other);
+  AllocatedBuffer(AllocatedBuffer&& other) = default;
+  AllocatedBuffer& operator=(AllocatedBuffer&& other) = default;
   AllocatedBuffer(const AllocatedBuffer& other) = delete;
   AllocatedBuffer& operator=(const AllocatedBuffer& other) = delete;
 
  private:
-  Environment* env_;
-  // We do not pass this to libuv directly, but uv_buf_t is a convenient way
-  // to represent a chunk of memory, and plays nicely with other parts of core.
-  uv_buf_t buffer_;
+  Environment* env_ = nullptr;
+  std::unique_ptr<v8::BackingStore> backing_store_;
 
   friend class Environment;
 };
diff --git a/src/env-inl.h b/src/env-inl.h
index 3afda388bc..fe92fcb3dd 100644
--- a/src/env-inl.h
+++ b/src/env-inl.h
@@ -867,29 +867,9 @@ inline IsolateData* Environment::isolate_data() const {
   return isolate_data_;
 }
 
-inline char* Environment::AllocateUnchecked(size_t size) {
-  return static_cast<char*>(
-      isolate_data()->allocator()->AllocateUninitialized(size));
-}
-
-inline char* Environment::Allocate(size_t size) {
-  char* ret = AllocateUnchecked(size);
-  CHECK_NE(ret, nullptr);
-  return ret;
-}
-
-inline void Environment::Free(char* data, size_t size) {
-  if (data != nullptr)
-    isolate_data()->allocator()->Free(data, size);
-}
-
-// It's a bit awkward to define this Buffer::New() overload here, but it
-// avoids a circular dependency with node_internals.h.
-namespace Buffer {
-v8::MaybeLocal<v8::Object> New(Environment* env,
-                               char* data,
-                               size_t length,
-                               bool uses_malloc);
+std::unordered_map<char*, std::unique_ptr<v8::BackingStore>>*
+    Environment::released_allocated_buffers() {
+  return &released_allocated_buffers_;
 }
 
 inline void Environment::ThrowError(const char* errmsg) {
diff --git a/src/env.cc b/src/env.cc
index a28679728e..7591ed1c43 100644
--- a/src/env.cc
+++ b/src/env.cc
@@ -1104,25 +1104,6 @@ void Environment::MemoryInfo(MemoryTracker* tracker) const {
   // node, we shift its sizeof() size out of the Environment node.
 }
 
-char* Environment::Reallocate(char* data, size_t old_size, size_t size) {
-  if (old_size == size) return data;
-  // If we know that the allocator is our ArrayBufferAllocator, we can let
-  // if reallocate directly.
-  if (isolate_data()->uses_node_allocator()) {
-    return static_cast<char*>(
-        isolate_data()->node_allocator()->Reallocate(data, old_size, size));
-  }
-  // Generic allocators do not provide a reallocation method; we need to
-  // allocate a new chunk of memory and copy the data over.
-  char* new_data = AllocateUnchecked(size);
-  if (new_data == nullptr) return nullptr;
-  memcpy(new_data, data, std::min(size, old_size));
-  if (size > old_size)
-    memset(new_data + old_size, 0, size - old_size);
-  Free(data, old_size);
-  return new_data;
-}
-
 void Environment::RunWeakRefCleanup() {
   isolate()->ClearKeptObjects();
 }
diff --git a/src/env.h b/src/env.h
index eb6e63dc3f..dafb59c046 100644
--- a/src/env.h
+++ b/src/env.h
@@ -926,11 +926,6 @@ class Environment : public MemoryRetainer {
 
   inline IsolateData* isolate_data() const;
 
-  inline char* Allocate(size_t size);
-  inline char* AllocateUnchecked(size_t size);
-  char* Reallocate(char* data, size_t old_size, size_t size);
-  inline void Free(char* data, size_t size);
-
   inline bool printed_error() const;
   inline void set_printed_error(bool value);
 
@@ -1218,6 +1213,9 @@ class Environment : public MemoryRetainer {
   void RunAndClearNativeImmediates(bool only_refed = false);
   void RunAndClearInterrupts();
 
+  inline std::unordered_map<char*, std::unique_ptr<v8::BackingStore>>*
+      released_allocated_buffers();
+
  private:
   inline void ThrowError(v8::Local<v8::Value> (*fun)(v8::Local<v8::String>),
                          const char* errmsg);
@@ -1378,6 +1376,11 @@ class Environment : public MemoryRetainer {
   // We should probably find a way to just use plain `v8::String`s created from
   // the source passed to LoadEnvironment() directly instead.
   std::unique_ptr<v8::String::Value> main_utf16_;
+
+  // Used by AllocatedBuffer::release() to keep track of the BackingStore for
+  // a given pointer.
+  std::unordered_map<char*, std::unique_ptr<v8::BackingStore>>
+      released_allocated_buffers_;
 };
 
 }  // namespace node
diff --git a/src/node_buffer.cc b/src/node_buffer.cc
index 398d586084..75d3828954 100644
--- a/src/node_buffer.cc
+++ b/src/node_buffer.cc
@@ -349,19 +349,8 @@ MaybeLocal<Object> New(Environment* env, size_t length) {
     return Local<Object>();
   }
 
-  AllocatedBuffer ret(env);
-  if (length > 0) {
-    ret = AllocatedBuffer::AllocateManaged(
-        env,
-        length,
-        AllocatedBuffer::ALLOCATE_MANAGED_UNCHECKED);
-    if (ret.data() == nullptr) {
-      THROW_ERR_MEMORY_ALLOCATION_FAILED(env);
-      return Local<Object>();
-    }
-  }
-
-  return scope.EscapeMaybe(ret.ToBuffer());
+  return scope.EscapeMaybe(
+      AllocatedBuffer::AllocateManaged(env, length).ToBuffer());
 }
 
 
@@ -388,17 +377,8 @@ MaybeLocal<Object> Copy(Environment* env, const char* data, size_t length) {
     return Local<Object>();
   }
 
-  AllocatedBuffer ret(env);
+  AllocatedBuffer ret = AllocatedBuffer::AllocateManaged(env, length);
   if (length > 0) {
-    CHECK_NOT_NULL(data);
-    ret = AllocatedBuffer::AllocateManaged(
-        env,
-        length,
-        AllocatedBuffer::ALLOCATE_MANAGED_UNCHECKED);
-    if (ret.data() == nullptr) {
-      THROW_ERR_MEMORY_ALLOCATION_FAILED(env);
-      return Local<Object>();
-    }
     memcpy(ret.data(), data, length);
   }
 
@@ -462,53 +442,23 @@ MaybeLocal<Object> New(Isolate* isolate, char* data, size_t length) {
     return MaybeLocal<Object>();
   }
   Local<Object> obj;
-  if (Buffer::New(env, data, length, true).ToLocal(&obj))
+  if (Buffer::New(env, data, length).ToLocal(&obj))
     return handle_scope.Escape(obj);
   return Local<Object>();
 }
 
-// Warning: If this call comes through the public node_buffer.h API,
-// the contract for this function is that `data` is allocated with malloc()
+// 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) {
+                       size_t length) {
   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);
-    }
-  }
-
-  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>());
+  auto free_callback = [](char* data, void* hint) { free(data); };
+  return New(env, data, length, free_callback, nullptr);
 }
 
 namespace {
diff --git a/src/node_crypto.cc b/src/node_crypto.cc
index fe9ac2d0cb..360732e37f 100644
--- a/src/node_crypto.cc
+++ b/src/node_crypto.cc
@@ -4028,7 +4028,7 @@ bool CipherBase::Final(AllocatedBuffer* out) {
   bool ok;
   if (kind_ == kDecipher && mode == EVP_CIPH_CCM_MODE) {
     ok = !pending_auth_failed_;
-    *out = AllocatedBuffer(env());  // Empty buffer.
+    *out = AllocatedBuffer::AllocateManaged(env(), 0);  // Empty buffer.
   } else {
     int out_len = out->size();
     ok = EVP_CipherFinal_ex(ctx_.get(),
diff --git a/src/node_internals.h b/src/node_internals.h
index 0ae17f71ec..65a4edcbea 100644
--- a/src/node_internals.h
+++ b/src/node_internals.h
@@ -160,8 +160,7 @@ v8::MaybeLocal<v8::Object> New(Environment* env,
 // ArrayBuffer::Allocator().
 v8::MaybeLocal<v8::Object> New(Environment* env,
                                char* data,
-                               size_t length,
-                               bool uses_malloc);
+                               size_t length);
 // Creates a Buffer instance over an existing ArrayBuffer.
 v8::MaybeLocal<v8::Uint8Array> New(Environment* env,
                                    v8::Local<v8::ArrayBuffer> ab,
@@ -183,7 +182,7 @@ static v8::MaybeLocal<v8::Object> New(Environment* env,
   const size_t len_in_bytes = buf->length() * sizeof(buf->out()[0]);
 
   if (buf->IsAllocated())
-    ret = New(env, src, len_in_bytes, true);
+    ret = New(env, src, len_in_bytes);
   else if (!buf->IsInvalidated())
     ret = Copy(env, src, len_in_bytes);
 
diff --git a/src/node_serdes.cc b/src/node_serdes.cc
index 86fb822dd5..c787721591 100644
--- a/src/node_serdes.cc
+++ b/src/node_serdes.cc
@@ -206,8 +206,7 @@ void SerializerContext::ReleaseBuffer(const FunctionCallbackInfo<Value>& args) {
   std::pair<uint8_t*, size_t> ret = ctx->serializer_.Release();
   auto buf = Buffer::New(ctx->env(),
                          reinterpret_cast<char*>(ret.first),
-                         ret.second,
-                         true /* uses_malloc */);
+                         ret.second);
 
   if (!buf.IsEmpty()) {
     args.GetReturnValue().Set(buf.ToLocalChecked());