From bf4c39dfa4561a48b0dd17da5d73431542ca5016 Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Fri, 25 Oct 2019 21:14:36 -0700 Subject: [PATCH] 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: https://github.com/nodejs/node/pull/30150 Reviewed-By: Gus Caplan Reviewed-By: Anna Henningsen Reviewed-By: Franziska Hinkelmann Reviewed-By: David Carlier --- src/api/environment.cc | 74 +++++++++++++++++++++++++-------------- src/node.h | 30 ++++++++++++++++ src/node_internals.h | 4 +-- src/node_main_instance.cc | 12 ++++--- 4 files changed, 87 insertions(+), 33 deletions(-) diff --git a/src/api/environment.cc b/src/api/environment.cc index e6a87d5a93a3b6..846e4a873da51c 100644 --- 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 index 025c6b949dc438..6e7f88389d7fb5 100644 --- 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 index ddf784e957ada4..2c69ea6d22c9e0 100644 --- 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 index 212b7f33e190dd..eea847725300f0 100644 --- 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::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 NodeMainInstance::CreateMainEnvironment( context = Context::FromSnapshot(isolate_, kNodeContextIndex).ToLocalChecked(); InitializeContextRuntime(context); - SetIsolateUpForNode(isolate_, IsolateSettingCategories::kErrorHandlers); + IsolateSettings s; + SetIsolateErrorHandlers(isolate_, s); } else { context = NewContext(isolate_); }