src: expose granular SetIsolateUpForNode

This PR exposes a new embedder-focused API: SetIsolateUpForNode. It maintains previous behavior for the single-param version of SetIsolateUpForNode and changes no defaults, but was designed to be flexible by allowing for embedders to conditionally override all callbacks and flags set by the previous two-param version of SetIsolateUpForNode. PR-URL: #30150 Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Franziska Hinkelmann <[email protected]> Reviewed-By: David Carlier <[email protected]>
nodejs · Jan 12, 2020 · bf4c39d · bf4c39d
1 parent c2757d1
commit bf4c39d
Show file tree

Hide file tree

Showing 4 changed files with 87 additions and 33 deletions.
diff --git a/src/api/environment.cc b/src/api/environment.cc
@@ -196,36 +196,56 @@ void SetIsolateCreateParamsForNode(Isolate::CreateParams* params) {
   }
 }
 
-void SetIsolateUpForNode(v8::Isolate* isolate, IsolateSettingCategories cat) {
-  switch (cat) {
-    case IsolateSettingCategories::kErrorHandlers:
-      isolate->AddMessageListenerWithErrorLevel(
-          errors::PerIsolateMessageListener,
-          Isolate::MessageErrorLevel::kMessageError |
-              Isolate::MessageErrorLevel::kMessageWarning);
-      isolate->SetAbortOnUncaughtExceptionCallback(
-          ShouldAbortOnUncaughtException);
-      isolate->SetFatalErrorHandler(OnFatalError);
-      isolate->SetPrepareStackTraceCallback(PrepareStackTraceCallback);
-      break;
-    case IsolateSettingCategories::kMisc:
-      isolate->SetMicrotasksPolicy(MicrotasksPolicy::kExplicit);
-      isolate->SetAllowWasmCodeGenerationCallback(
-          AllowWasmCodeGenerationCallback);
-      isolate->SetPromiseRejectCallback(task_queue::PromiseRejectCallback);
-      isolate->SetHostCleanupFinalizationGroupCallback(
-          HostCleanupFinalizationGroupCallback);
-      v8::CpuProfiler::UseDetailedSourcePositionsForProfiling(isolate);
-      break;
-    default:
-      UNREACHABLE();
-      break;
-  }
+void SetIsolateErrorHandlers(v8::Isolate* isolate, const IsolateSettings& s) {
+  if (s.flags & MESSAGE_LISTENER_WITH_ERROR_LEVEL)
+    isolate->AddMessageListenerWithErrorLevel(
+            errors::PerIsolateMessageListener,
+            Isolate::MessageErrorLevel::kMessageError |
+                Isolate::MessageErrorLevel::kMessageWarning);
+
+  auto* abort_callback = s.should_abort_on_uncaught_exception_callback ?
+      s.should_abort_on_uncaught_exception_callback :
+      ShouldAbortOnUncaughtException;
+  isolate->SetAbortOnUncaughtExceptionCallback(abort_callback);
+
+  auto* fatal_error_cb = s.fatal_error_callback ?
+      s.fatal_error_callback : OnFatalError;
+  isolate->SetFatalErrorHandler(fatal_error_cb);
+
+  auto* prepare_stack_trace_cb = s.prepare_stack_trace_callback ?
+      s.prepare_stack_trace_callback : PrepareStackTraceCallback;
+  isolate->SetPrepareStackTraceCallback(prepare_stack_trace_cb);
+}
+
+void SetIsolateMiscHandlers(v8::Isolate* isolate, const IsolateSettings& s) {
+  isolate->SetMicrotasksPolicy(s.policy);
+
+  auto* allow_wasm_codegen_cb = s.allow_wasm_code_generation_callback ?
+    s.allow_wasm_code_generation_callback : AllowWasmCodeGenerationCallback;
+  isolate->SetAllowWasmCodeGenerationCallback(allow_wasm_codegen_cb);
+
+  auto* promise_reject_cb = s.promise_reject_callback ?
+    s.promise_reject_callback : task_queue::PromiseRejectCallback;
+  isolate->SetPromiseRejectCallback(promise_reject_cb);
+
+  auto* host_cleanup_cb = s.host_cleanup_finalization_group_callback ?
+    s.host_cleanup_finalization_group_callback :
+    HostCleanupFinalizationGroupCallback;
+  isolate->SetHostCleanupFinalizationGroupCallback(host_cleanup_cb);
+
+  if (s.flags & DETAILED_SOURCE_POSITIONS_FOR_PROFILING)
+    v8::CpuProfiler::UseDetailedSourcePositionsForProfiling(isolate);
+}
+
+void SetIsolateUpForNode(v8::Isolate* isolate,
+                         const IsolateSettings& settings) {
+  SetIsolateErrorHandlers(isolate, settings);
+  SetIsolateMiscHandlers(isolate, settings);
 }
 
 void SetIsolateUpForNode(v8::Isolate* isolate) {
-  SetIsolateUpForNode(isolate, IsolateSettingCategories::kErrorHandlers);
-  SetIsolateUpForNode(isolate, IsolateSettingCategories::kMisc);
+  IsolateSettings settings;
+  SetIsolateUpForNode(isolate, settings);
 }
 
 Isolate* NewIsolate(ArrayBufferAllocator* allocator, uv_loop_t* event_loop) {

diff --git a/src/node.h b/src/node.h
@@ -280,9 +280,39 @@ class NODE_EXTERN MultiIsolatePlatform : public v8::Platform {
                                           void* data) = 0;
 };
 
+enum IsolateSettingsFlags {
+  MESSAGE_LISTENER_WITH_ERROR_LEVEL = 1 << 0,
+  DETAILED_SOURCE_POSITIONS_FOR_PROFILING = 1 << 1
+};
+
+struct IsolateSettings {
+  uint64_t flags = MESSAGE_LISTENER_WITH_ERROR_LEVEL |
+      DETAILED_SOURCE_POSITIONS_FOR_PROFILING;
+  v8::MicrotasksPolicy policy = v8::MicrotasksPolicy::kExplicit;
+
+  // Error handling callbacks
+  v8::Isolate::AbortOnUncaughtExceptionCallback
+      should_abort_on_uncaught_exception_callback = nullptr;
+  v8::FatalErrorCallback fatal_error_callback = nullptr;
+  v8::PrepareStackTraceCallback prepare_stack_trace_callback = nullptr;
+
+  // Miscellaneous callbacks
+  v8::PromiseRejectCallback promise_reject_callback = nullptr;
+  v8::AllowWasmCodeGenerationCallback
+      allow_wasm_code_generation_callback = nullptr;
+  v8::HostCleanupFinalizationGroupCallback
+      host_cleanup_finalization_group_callback = nullptr;
+};
+
+// Overriding IsolateSettings may produce unexpected behavior
+// in Node.js core functionality, so proceed at your own risk.
+NODE_EXTERN void SetIsolateUpForNode(v8::Isolate* isolate,
+                                     const IsolateSettings& settings);
+
 // 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(),

diff --git a/src/node_internals.h b/src/node_internals.h
@@ -313,8 +313,8 @@ struct InitializationResult {
 };
 InitializationResult InitializeOncePerProcess(int argc, char** argv);
 void TearDownOncePerProcess();
-enum class IsolateSettingCategories { kErrorHandlers, kMisc };
-void SetIsolateUpForNode(v8::Isolate* isolate, IsolateSettingCategories cat);
+void SetIsolateErrorHandlers(v8::Isolate* isolate, const IsolateSettings& s);
+void SetIsolateMiscHandlers(v8::Isolate* isolate, const IsolateSettings& s);
 void SetIsolateCreateParamsForNode(v8::Isolate::CreateParams* params);
 
 #if HAVE_INSPECTOR

diff --git a/src/node_main_instance.cc b/src/node_main_instance.cc
@@ -35,7 +35,9 @@ NodeMainInstance::NodeMainInstance(Isolate* isolate,
       owns_isolate_(false),
       deserialize_mode_(false) {
   isolate_data_.reset(new IsolateData(isolate_, event_loop, platform, nullptr));
-  SetIsolateUpForNode(isolate_, IsolateSettingCategories::kMisc);
+
+  IsolateSettings misc;
+  SetIsolateMiscHandlers(isolate_, misc);
 }
 
 std::unique_ptr<NodeMainInstance> NodeMainInstance::Create(
@@ -79,11 +81,12 @@ NodeMainInstance::NodeMainInstance(
                                       platform,
                                       array_buffer_allocator_.get(),
                                       per_isolate_data_indexes));
-  SetIsolateUpForNode(isolate_, IsolateSettingCategories::kMisc);
+  IsolateSettings s;
+  SetIsolateMiscHandlers(isolate_, s);
   if (!deserialize_mode_) {
     // If in deserialize mode, delay until after the deserialization is
     // complete.
-    SetIsolateUpForNode(isolate_, IsolateSettingCategories::kErrorHandlers);
+    SetIsolateErrorHandlers(isolate_, s);
   }
 }
 
@@ -201,7 +204,8 @@ std::unique_ptr<Environment> NodeMainInstance::CreateMainEnvironment(
     context =
         Context::FromSnapshot(isolate_, kNodeContextIndex).ToLocalChecked();
     InitializeContextRuntime(context);
-    SetIsolateUpForNode(isolate_, IsolateSettingCategories::kErrorHandlers);
+    IsolateSettings s;
+    SetIsolateErrorHandlers(isolate_, s);
   } else {
     context = NewContext(isolate_);
   }