embedding: refactor NewIsolate()

src/api/environment.cc

@@ -71,7 +71,7 @@ static void OnMessage(Local<Message> message, Local<Value> error) {
   }
 }
 
-void* ArrayBufferAllocator::Allocate(size_t size) {
+void* NodeArrayBufferAllocator::Allocate(size_t size) {
   if (zero_fill_field_ || per_process::cli_options->zero_fill_all_buffers)
     return UncheckedCalloc(size);
   else
@@ -84,29 +84,29 @@ DebuggingArrayBufferAllocator::~DebuggingArrayBufferAllocator() {
 
 void* DebuggingArrayBufferAllocator::Allocate(size_t size) {
   Mutex::ScopedLock lock(mutex_);
-  void* data = ArrayBufferAllocator::Allocate(size);
+  void* data = NodeArrayBufferAllocator::Allocate(size);
   RegisterPointerInternal(data, size);
   return data;
 }
 
 void* DebuggingArrayBufferAllocator::AllocateUninitialized(size_t size) {
   Mutex::ScopedLock lock(mutex_);
-  void* data = ArrayBufferAllocator::AllocateUninitialized(size);
+  void* data = NodeArrayBufferAllocator::AllocateUninitialized(size);
   RegisterPointerInternal(data, size);
   return data;
 }
 
 void DebuggingArrayBufferAllocator::Free(void* data, size_t size) {
   Mutex::ScopedLock lock(mutex_);
   UnregisterPointerInternal(data, size);
-  ArrayBufferAllocator::Free(data, size);
+  NodeArrayBufferAllocator::Free(data, size);
 }
 
 void* DebuggingArrayBufferAllocator::Reallocate(void* data,
                                                 size_t old_size,
                                                 size_t size) {
   Mutex::ScopedLock lock(mutex_);
-  void* ret = ArrayBufferAllocator::Reallocate(data, old_size, size);
+  void* ret = NodeArrayBufferAllocator::Reallocate(data, old_size, size);
   if (ret == nullptr) {
     if (size == 0)  // i.e. equivalent to free().
       UnregisterPointerInternal(data, old_size);
@@ -149,41 +149,40 @@ void DebuggingArrayBufferAllocator::RegisterPointerInternal(void* data,
   allocations_[data] = size;
 }
 
-ArrayBufferAllocator* CreateArrayBufferAllocator() {
-  if (per_process::cli_options->debug_arraybuffer_allocations)
-    return new DebuggingArrayBufferAllocator();
+std::unique_ptr<ArrayBufferAllocator> ArrayBufferAllocator::Create(bool debug) {
+  if (debug || per_process::cli_options->debug_arraybuffer_allocations)
+    return std::make_unique<DebuggingArrayBufferAllocator>();
   else
-    return new ArrayBufferAllocator();
+    return std::make_unique<NodeArrayBufferAllocator>();
+}
+
+ArrayBufferAllocator* CreateArrayBufferAllocator() {
+  return ArrayBufferAllocator::Create().release();
 }
 
 void FreeArrayBufferAllocator(ArrayBufferAllocator* allocator) {
   delete allocator;
 }
 
-Isolate* NewIsolate(ArrayBufferAllocator* allocator, uv_loop_t* event_loop) {
-  Isolate::CreateParams params;
-  params.array_buffer_allocator = allocator;
+void SetIsolateCreateParams(Isolate::CreateParams* params,
+                            ArrayBufferAllocator* allocator) {
+  if (allocator != nullptr)
+    params->array_buffer_allocator = allocator;
 
   double total_memory = uv_get_total_memory();
   if (total_memory > 0) {
     // V8 defaults to 700MB or 1.4GB on 32 and 64 bit platforms respectively.
     // This default is based on browser use-cases. Tell V8 to configure the
     // heap based on the actual physical memory.
-    params.constraints.ConfigureDefaults(total_memory, 0);
+    params->constraints.ConfigureDefaults(total_memory, 0);
   }
 
 #ifdef NODE_ENABLE_VTUNE_PROFILING
-  params.code_event_handler = vTune::GetVtuneCodeEventHandler();
+  params->code_event_handler = vTune::GetVtuneCodeEventHandler();
 #endif
+}
 
-  Isolate* isolate = Isolate::Allocate();
-  if (isolate == nullptr) return nullptr;
-
-  // Register the isolate on the platform before the isolate gets initialized,
-  // so that the isolate can access the platform during initialization.
-  per_process::v8_platform.Platform()->RegisterIsolate(isolate, event_loop);
-  Isolate::Initialize(isolate, params);
-
+void SetIsolateUpForNode(v8::Isolate* isolate) {
   isolate->AddMessageListenerWithErrorLevel(
       OnMessage,
       Isolate::MessageErrorLevel::kMessageError |
@@ -192,7 +191,29 @@ Isolate* NewIsolate(ArrayBufferAllocator* allocator, uv_loop_t* event_loop) {
   isolate->SetMicrotasksPolicy(MicrotasksPolicy::kExplicit);
   isolate->SetFatalErrorHandler(OnFatalError);
   isolate->SetAllowWasmCodeGenerationCallback(AllowWasmCodeGenerationCallback);
+  isolate->SetPromiseRejectCallback(task_queue::PromiseRejectCallback);
   v8::CpuProfiler::UseDetailedSourcePositionsForProfiling(isolate);
+}
+
+Isolate* NewIsolate(ArrayBufferAllocator* allocator, uv_loop_t* event_loop) {
+  return NewIsolate(allocator, event_loop, GetMainThreadMultiIsolatePlatform());
+}
+
+Isolate* NewIsolate(ArrayBufferAllocator* allocator,
+                    uv_loop_t* event_loop,
+                    MultiIsolatePlatform* platform) {
+  Isolate::CreateParams params;
+  SetIsolateCreateParams(&params, allocator);
+
+  Isolate* isolate = Isolate::Allocate();
+  if (isolate == nullptr) return nullptr;
+
+  // Register the isolate on the platform before the isolate gets initialized,
+  // so that the isolate can access the platform during initialization.
+  platform->RegisterIsolate(isolate, event_loop);
+  Isolate::Initialize(isolate, params);
+
+  SetIsolateUpForNode(isolate);
 
   return isolate;
 }

src/env-inl.h

@@ -57,7 +57,7 @@ inline v8::ArrayBuffer::Allocator* IsolateData::allocator() const {
   return allocator_;
 }
 
-inline ArrayBufferAllocator* IsolateData::node_allocator() const {
+inline NodeArrayBufferAllocator* IsolateData::node_allocator() const {
   return node_allocator_;
 }
 

src/env.cc

@@ -77,7 +77,8 @@ IsolateData::IsolateData(Isolate* isolate,
     : isolate_(isolate),
       event_loop_(event_loop),
       allocator_(isolate->GetArrayBufferAllocator()),
-      node_allocator_(node_allocator),
+      node_allocator_(node_allocator == nullptr ?
+          nullptr : node_allocator->GetImpl()),
       uses_node_allocator_(allocator_ == node_allocator_),
       platform_(platform) {
   CHECK_NOT_NULL(allocator_);
@@ -257,10 +258,6 @@ Environment::Environment(IsolateData* isolate_data,
   if (options_->no_force_async_hooks_checks) {
     async_hooks_.no_force_checks();
   }
-
-  // TODO(addaleax): the per-isolate state should not be controlled by
-  // a single Environment.
-  isolate()->SetPromiseRejectCallback(task_queue::PromiseRejectCallback);
 }
 
 CompileFnEntry::CompileFnEntry(Environment* env, uint32_t id)

src/env.h

@@ -407,7 +407,7 @@ class IsolateData {
 
   inline bool uses_node_allocator() const;
   inline v8::ArrayBuffer::Allocator* allocator() const;
-  inline ArrayBufferAllocator* node_allocator() const;
+  inline NodeArrayBufferAllocator* node_allocator() const;
 
 #define VP(PropertyName, StringValue) V(v8::Private, PropertyName)
 #define VY(PropertyName, StringValue) V(v8::Symbol, PropertyName)
@@ -442,7 +442,7 @@ class IsolateData {
   v8::Isolate* const isolate_;
   uv_loop_t* const event_loop_;
   v8::ArrayBuffer::Allocator* const allocator_;
-  ArrayBufferAllocator* const node_allocator_;
+  NodeArrayBufferAllocator* const node_allocator_;
   const bool uses_node_allocator_;
   MultiIsolatePlatform* platform_;
   std::shared_ptr<PerIsolateOptions> options_;

src/node.h

@@ -64,6 +64,8 @@
 #include "v8-platform.h"  // NOLINT(build/include_order)
 #include "node_version.h"  // NODE_MODULE_VERSION
 
+#include <memory>
+
 #define NODE_MAKE_VERSION(major, minor, patch)                                \
   ((major) * 0x1000 + (minor) * 0x100 + (patch))
 
@@ -210,8 +212,29 @@ NODE_EXTERN void Init(int* argc,
                       int* exec_argc,
                       const char*** exec_argv);
 
-class ArrayBufferAllocator;
+class NodeArrayBufferAllocator;
+
+// An ArrayBuffer::Allocator class with some Node.js-specific tweaks. If you do
+// not have to use another allocator, using this class is recommended:
+// - It supports Buffer.allocUnsafe() and Buffer.allocUnsafeSlow() with
+//   uninitialized memory.
+// - It supports transferring, rather than copying, ArrayBuffers when using
+//   MessagePorts.
+class NODE_EXTERN ArrayBufferAllocator : public v8::ArrayBuffer::Allocator {
+ public:
+  // If `always_debug` is true, create an ArrayBuffer::Allocator instance
+  // that performs additional integrity checks (e.g. make sure that only memory
+  // that was allocated by the it is also freed by it).
+  // This can also be set using the --debug-arraybuffer-allocations flag.
+  static std::unique_ptr<ArrayBufferAllocator> Create(bool always_debug = true);
+
+ private:
+  virtual NodeArrayBufferAllocator* GetImpl() = 0;
+
+  friend class IsolateData;
+};
 
+// Legacy equivalents for ArrayBufferAllocator::Create().
 NODE_EXTERN ArrayBufferAllocator* CreateArrayBufferAllocator();
 NODE_EXTERN void FreeArrayBufferAllocator(ArrayBufferAllocator* allocator);
 
@@ -234,9 +257,24 @@ class NODE_EXTERN MultiIsolatePlatform : public v8::Platform {
   virtual void UnregisterIsolate(v8::Isolate* isolate) = 0;
 };
 
+// Set up some Node.js-specific defaults for `params`, in particular
+// the ArrayBuffer::Allocator if it is provided, memory limits, and
+// possibly a code event handler.
+NODE_EXTERN void SetIsolateCreateParams(v8::Isolate::CreateParams* params,
+                                        ArrayBufferAllocator* allocator
+                                            = nullptr);
+// Set a number of callbacks for the `isolate`, in particular the Node.js
+// uncaught exception listener.
+NODE_EXTERN void SetIsolateUpForNode(v8::Isolate* isolate);
 // Creates a new isolate with Node.js-specific settings.
+// This is a convenience method equivalent to using SetIsolateCreateParams(),
+// Isolate::Allocate(), MultiIsolatePlatform::RegisterIsolate(),
+// Isolate::Initialize(), and SetIsolateUpForNode().
 NODE_EXTERN v8::Isolate* NewIsolate(ArrayBufferAllocator* allocator,
                                     struct uv_loop_s* event_loop);
+NODE_EXTERN v8::Isolate* NewIsolate(ArrayBufferAllocator* allocator,
+                                    struct uv_loop_s* event_loop,
+                                    MultiIsolatePlatform* platform);
 
 // Creates a new context with Node.js-specific tweaks.
 NODE_EXTERN v8::Local<v8::Context> NewContext(

src/node_buffer.cc

@@ -1104,7 +1104,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 (ArrayBufferAllocator* allocator = env->isolate_data()->node_allocator()) {
+  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));

src/node_internals.h

@@ -101,7 +101,7 @@ namespace task_queue {
 void PromiseRejectCallback(v8::PromiseRejectMessage message);
 }  // namespace task_queue
 
-class ArrayBufferAllocator : public v8::ArrayBuffer::Allocator {
+class NodeArrayBufferAllocator : public ArrayBufferAllocator {
  public:
   inline uint32_t* zero_fill_field() { return &zero_fill_field_; }
 
@@ -116,11 +116,13 @@ class ArrayBufferAllocator : public v8::ArrayBuffer::Allocator {
   virtual void RegisterPointer(void* data, size_t size) {}
   virtual void UnregisterPointer(void* data, size_t size) {}
 
+  NodeArrayBufferAllocator* GetImpl() final { return this; }
+
  private:
   uint32_t zero_fill_field_ = 1;  // Boolean but exposed as uint32 to JS land.
 };
 
-class DebuggingArrayBufferAllocator final : public ArrayBufferAllocator {
+class DebuggingArrayBufferAllocator final : public NodeArrayBufferAllocator {
  public:
   ~DebuggingArrayBufferAllocator() override;
   void* Allocate(size_t size) override;