From b21e7c7bcf23a2715951e4cd96180e4dbf1dcd4d Mon Sep 17 00:00:00 2001
From: Anna Henningsen <anna@addaleax.net>
Date: Fri, 8 Mar 2019 16:55:40 +0100
Subject: [PATCH] embedding: make `NewIsolate()` API more flexible

Split the API up into its essential parts, namely setting up
the creation parameters for the Isolate, creating it, and performing
Node.js-specific customization afterwards.

PR-URL: https://github.com/nodejs/node/pull/26525
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
---
 src/api/environment.cc | 43 +++++++++++++++++++++++++++++-------------
 src/env.cc             |  4 ----
 src/node.h             | 15 +++++++++++++++
 3 files changed, 45 insertions(+), 17 deletions(-)

diff --git a/src/api/environment.cc b/src/api/environment.cc
index 421735a82a0c1a..9b8e4686852811 100644
--- a/src/api/environment.cc
+++ b/src/api/environment.cc
@@ -161,30 +161,25 @@ 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 |
@@ -193,7 +188,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;
 }
diff --git a/src/env.cc b/src/env.cc
index a1870785a816b3..2448754db991a5 100644
--- a/src/env.cc
+++ b/src/env.cc
@@ -253,10 +253,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)
diff --git a/src/node.h b/src/node.h
index 3aaaf43668b079..17eae72e9c5ceb 100644
--- a/src/node.h
+++ b/src/node.h
@@ -270,9 +270,24 @@ class NODE_EXTERN MultiIsolatePlatform : public v8::Platform {
                                           void* data) = 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(