Revert "lib,src: clean up ArrayBufferAllocator"

This reverts commit 27e84dd.

nodejs#7082 was landed too fast and did
not have sufficient review time.

That PR also broke some things (testcases will follow separately).

lib/buffer.js

@@ -59,16 +59,18 @@ Buffer.prototype.swap32 = function swap32() {
   return swap32n.apply(this);
 };
 
-// |binding.zeroFill| can be undefined when running inside an isolate where we
-// do not own the ArrayBuffer allocator.  Zero fill is always on in that case.
-const zeroFill = bindingObj.zeroFill || [0];
+const flags = bindingObj.flags;
+const kNoZeroFill = 0;
 
 function createBuffer(size, noZeroFill) {
-  if (noZeroFill)
-    zeroFill[0] = 0;  // Reset by the runtime.
-  const ui8 = new Uint8Array(size);
-  Object.setPrototypeOf(ui8, Buffer.prototype);
-  return ui8;
+  flags[kNoZeroFill] = noZeroFill ? 1 : 0;
+  try {
+    const ui8 = new Uint8Array(size);
+    Object.setPrototypeOf(ui8, Buffer.prototype);
+    return ui8;
+  } finally {
+    flags[kNoZeroFill] = 0;
+  }
 }
 
 function createPool() {

src/debug-agent.cc

@@ -169,8 +169,7 @@ void Agent::WorkerRun() {
     Isolate::Scope isolate_scope(isolate);
 
     HandleScope handle_scope(isolate);
-    IsolateData isolate_data(isolate, &child_loop_,
-                             array_buffer_allocator.zero_fill_field());
+    IsolateData isolate_data(isolate, &child_loop_);
     Local<Context> context = Context::New(isolate);
 
     Context::Scope context_scope(context);

src/env-inl.h

@@ -24,8 +24,7 @@ namespace node {
 //
 // One byte because our strings are ASCII and we can safely skip V8's UTF-8
 // decoding step.  It's a one-time cost, but why pay it when you don't have to?
-inline IsolateData::IsolateData(v8::Isolate* isolate, uv_loop_t* event_loop,
-                                uint32_t* zero_fill_field)
+inline IsolateData::IsolateData(v8::Isolate* isolate, uv_loop_t* event_loop)
     :
 #define V(PropertyName, StringValue)                                          \
     PropertyName ## _(                                                        \
@@ -49,17 +48,12 @@ inline IsolateData::IsolateData(v8::Isolate* isolate, uv_loop_t* event_loop,
             sizeof(StringValue) - 1).ToLocalChecked()),
     PER_ISOLATE_STRING_PROPERTIES(V)
 #undef V
-    isolate_(isolate), event_loop_(event_loop),
-    zero_fill_field_(zero_fill_field) {}
+    isolate_(isolate), event_loop_(event_loop) {}
 
 inline uv_loop_t* IsolateData::event_loop() const {
   return event_loop_;
 }
 
-inline uint32_t* IsolateData::zero_fill_field() const {
-  return zero_fill_field_;
-}
-
 inline Environment::AsyncHooks::AsyncHooks() {
   for (int i = 0; i < kFieldsCount; i++) fields_[i] = 0;
 }
@@ -134,6 +128,27 @@ inline void Environment::TickInfo::set_index(uint32_t value) {
   fields_[kIndex] = value;
 }
 
+inline Environment::ArrayBufferAllocatorInfo::ArrayBufferAllocatorInfo() {
+  for (int i = 0; i < kFieldsCount; ++i)
+    fields_[i] = 0;
+}
+
+inline uint32_t* Environment::ArrayBufferAllocatorInfo::fields() {
+  return fields_;
+}
+
+inline int Environment::ArrayBufferAllocatorInfo::fields_count() const {
+  return kFieldsCount;
+}
+
+inline bool Environment::ArrayBufferAllocatorInfo::no_zero_fill() const {
+  return fields_[kNoZeroFill] != 0;
+}
+
+inline void Environment::ArrayBufferAllocatorInfo::reset_fill_flag() {
+  fields_[kNoZeroFill] = 0;
+}
+
 inline Environment* Environment::New(IsolateData* isolate_data,
                                      v8::Local<v8::Context> context) {
   Environment* env = new Environment(isolate_data, context);
@@ -303,6 +318,11 @@ inline Environment::TickInfo* Environment::tick_info() {
   return &tick_info_;
 }
 
+inline Environment::ArrayBufferAllocatorInfo*
+    Environment::array_buffer_allocator_info() {
+  return &array_buffer_allocator_info_;
+}
+
 inline uint64_t Environment::timer_base() const {
   return timer_base_;
 }

src/env.h

@@ -304,10 +304,8 @@ RB_HEAD(ares_task_list, ares_task_t);
 
 class IsolateData {
  public:
-  inline IsolateData(v8::Isolate* isolate, uv_loop_t* event_loop,
-                     uint32_t* zero_fill_field = nullptr);
+  inline IsolateData(v8::Isolate* isolate, uv_loop_t* event_loop);
   inline uv_loop_t* event_loop() const;
-  inline uint32_t* zero_fill_field() const;
 
 #define VP(PropertyName, StringValue) V(v8::Private, PropertyName, StringValue)
 #define VS(PropertyName, StringValue) V(v8::String, PropertyName, StringValue)
@@ -332,7 +330,6 @@ class IsolateData {
 
   v8::Isolate* const isolate_;
   uv_loop_t* const event_loop_;
-  uint32_t* const zero_fill_field_;
 
   DISALLOW_COPY_AND_ASSIGN(IsolateData);
 };
@@ -417,6 +414,27 @@ class Environment {
     DISALLOW_COPY_AND_ASSIGN(TickInfo);
   };
 
+  class ArrayBufferAllocatorInfo {
+   public:
+    inline uint32_t* fields();
+    inline int fields_count() const;
+    inline bool no_zero_fill() const;
+    inline void reset_fill_flag();
+
+   private:
+    friend class Environment;  // So we can call the constructor.
+    inline ArrayBufferAllocatorInfo();
+
+    enum Fields {
+      kNoZeroFill,
+      kFieldsCount
+    };
+
+    uint32_t fields_[kFieldsCount];
+
+    DISALLOW_COPY_AND_ASSIGN(ArrayBufferAllocatorInfo);
+  };
+
   typedef void (*HandleCleanupCb)(Environment* env,
                                   uv_handle_t* handle,
                                   void* arg);
@@ -479,14 +497,14 @@ class Environment {
   inline AsyncHooks* async_hooks();
   inline DomainFlag* domain_flag();
   inline TickInfo* tick_info();
+  inline ArrayBufferAllocatorInfo* array_buffer_allocator_info();
   inline uint64_t timer_base() const;
 
   static inline Environment* from_cares_timer_handle(uv_timer_t* handle);
   inline uv_timer_t* cares_timer_handle();
   inline ares_channel cares_channel();
   inline ares_channel* cares_channel_ptr();
   inline ares_task_list* cares_task_list();
-  inline IsolateData* isolate_data() const;
 
   inline bool using_domains() const;
   inline void set_using_domains(bool value);
@@ -584,6 +602,7 @@ class Environment {
  private:
   inline Environment(IsolateData* isolate_data, v8::Local<v8::Context> context);
   inline ~Environment();
+  inline IsolateData* isolate_data() const;
 
   v8::Isolate* const isolate_;
   IsolateData* const isolate_data_;
@@ -594,6 +613,7 @@ class Environment {
   AsyncHooks async_hooks_;
   DomainFlag domain_flag_;
   TickInfo tick_info_;
+  ArrayBufferAllocatorInfo array_buffer_allocator_info_;
   const uint64_t timer_base_;
   uv_timer_t cares_timer_handle_;
   ares_channel cares_channel_;

src/node.cc

@@ -971,9 +971,11 @@ Local<Value> WinapiErrnoException(Isolate* isolate,
 
 
 void* ArrayBufferAllocator::Allocate(size_t size) {
-  if (zero_fill_field_ || zero_fill_all_buffers)
+  if (env_ == nullptr ||
+      !env_->array_buffer_allocator_info()->no_zero_fill() ||
+      zero_fill_all_buffers)
     return calloc(size, 1);
-  zero_fill_field_ = 1;
+  env_->array_buffer_allocator_info()->reset_fill_flag();
   return malloc(size);
 }
 
@@ -4361,8 +4363,8 @@ Environment* CreateEnvironment(IsolateData* isolate_data,
 static void StartNodeInstance(void* arg) {
   NodeInstanceData* instance_data = static_cast<NodeInstanceData*>(arg);
   Isolate::CreateParams params;
-  ArrayBufferAllocator array_buffer_allocator;
-  params.array_buffer_allocator = &array_buffer_allocator;
+  ArrayBufferAllocator* array_buffer_allocator = new ArrayBufferAllocator();
+  params.array_buffer_allocator = array_buffer_allocator;
 #ifdef NODE_ENABLE_VTUNE_PROFILING
   params.code_event_handler = vTune::GetVtuneCodeEventHandler();
 #endif
@@ -4383,8 +4385,7 @@ static void StartNodeInstance(void* arg) {
     Locker locker(isolate);
     Isolate::Scope isolate_scope(isolate);
     HandleScope handle_scope(isolate);
-    IsolateData isolate_data(isolate, instance_data->event_loop(),
-                             array_buffer_allocator.zero_fill_field());
+    IsolateData isolate_data(isolate, instance_data->event_loop());
     Local<Context> context = Context::New(isolate);
     Context::Scope context_scope(context);
     Environment* env = CreateEnvironment(&isolate_data,
@@ -4394,6 +4395,8 @@ static void StartNodeInstance(void* arg) {
                                          instance_data->exec_argc(),
                                          instance_data->exec_argv());
 
+    array_buffer_allocator->set_env(env);
+
     isolate->SetAbortOnUncaughtExceptionCallback(
         ShouldAbortOnUncaughtException);
 
@@ -4461,6 +4464,7 @@ static void StartNodeInstance(void* arg) {
     __lsan_do_leak_check();
 #endif
 
+    array_buffer_allocator->set_env(nullptr);
     env->Dispose();
     env = nullptr;
   }
@@ -4473,6 +4477,7 @@ static void StartNodeInstance(void* arg) {
   CHECK_NE(isolate, nullptr);
   isolate->Dispose();
   isolate = nullptr;
+  delete array_buffer_allocator;
 }
 
 int Start(int argc, char** argv) {

src/node_buffer.cc

@@ -1224,14 +1224,18 @@ void SetupBufferJS(const FunctionCallbackInfo<Value>& args) {
 
   env->SetMethod(proto, "copy", Copy);
 
-  if (auto zero_fill_field = env->isolate_data()->zero_fill_field()) {
-    CHECK(args[1]->IsObject());
-    auto binding_object = args[1].As<Object>();
-    auto array_buffer = ArrayBuffer::New(env->isolate(), zero_fill_field, 1);
-    auto name = FIXED_ONE_BYTE_STRING(env->isolate(), "zeroFill");
-    auto value = Uint32Array::New(array_buffer, 0, 1);
-    CHECK(binding_object->Set(env->context(), name, value).FromJust());
-  }
+  CHECK(args[1]->IsObject());
+  Local<Object> bObj = args[1].As<Object>();
+
+  uint32_t* const fields = env->array_buffer_allocator_info()->fields();
+  uint32_t const fields_count =
+      env->array_buffer_allocator_info()->fields_count();
+
+  Local<ArrayBuffer> array_buffer =
+      ArrayBuffer::New(env->isolate(), fields, sizeof(*fields) * fields_count);
+
+  bObj->Set(String::NewFromUtf8(env->isolate(), "flags"),
+            Uint32Array::New(array_buffer, 0, fields_count));
 }
 
 

src/node_internals.h

@@ -204,14 +204,16 @@ void ThrowUVException(v8::Isolate* isolate,
 
 class ArrayBufferAllocator : public v8::ArrayBuffer::Allocator {
  public:
-  inline uint32_t* zero_fill_field() { return &zero_fill_field_; }
+  ArrayBufferAllocator() : env_(nullptr) { }
+
+  inline void set_env(Environment* env) { env_ = env; }
 
   virtual void* Allocate(size_t size);  // Defined in src/node.cc
   virtual void* AllocateUninitialized(size_t size) { return malloc(size); }
   virtual void Free(void* data, size_t) { free(data); }
 
  private:
-  uint32_t zero_fill_field_ = 1;  // Boolean but exposed as uint32 to JS land.
+  Environment* env_;
 };
 
 // Clear any domain and/or uncaughtException handlers to force the error's