From a6363389456e19448b9507271b68ccf3f40a3f98 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Fri, 19 Apr 2019 12:50:38 +0800 Subject: [PATCH] src: allow creating NodeMainInstance that does not own the isolate Allows instantiating a NodeMainInstance with an isolate whose initialization and disposal are controlled by the caller. PR-URL: https://github.com/nodejs/node/pull/27321 Refs: https://github.com/nodejs/node/issues/17058 Reviewed-By: Anna Henningsen Reviewed-By: Refael Ackermann Reviewed-By: James M Snell Reviewed-By: Ben Noordhuis --- src/api/environment.cc | 36 +++++++++++++++------ src/node.cc | 8 +++-- src/node_internals.h | 3 ++ src/node_main_instance.cc | 67 +++++++++++++++++++++++++++++++-------- src/node_main_instance.h | 48 ++++++++++++++++++++++++---- 5 files changed, 131 insertions(+), 31 deletions(-) diff --git a/src/api/environment.cc b/src/api/environment.cc index 0543e0323d8ee2..9a29ad1e5e1ff0 100644 --- a/src/api/environment.cc +++ b/src/api/environment.cc @@ -183,17 +183,33 @@ void SetIsolateCreateParamsForNode(Isolate::CreateParams* params) { #endif } +void SetIsolateUpForNode(v8::Isolate* isolate, IsolateSettingCategories cat) { + switch (cat) { + case IsolateSettingCategories::kErrorHandlers: + isolate->AddMessageListenerWithErrorLevel( + OnMessage, + Isolate::MessageErrorLevel::kMessageError | + Isolate::MessageErrorLevel::kMessageWarning); + isolate->SetAbortOnUncaughtExceptionCallback( + ShouldAbortOnUncaughtException); + isolate->SetFatalErrorHandler(OnFatalError); + break; + case IsolateSettingCategories::kMisc: + isolate->SetMicrotasksPolicy(MicrotasksPolicy::kExplicit); + isolate->SetAllowWasmCodeGenerationCallback( + AllowWasmCodeGenerationCallback); + isolate->SetPromiseRejectCallback(task_queue::PromiseRejectCallback); + v8::CpuProfiler::UseDetailedSourcePositionsForProfiling(isolate); + break; + default: + UNREACHABLE(); + break; + } +} + void SetIsolateUpForNode(v8::Isolate* isolate) { - isolate->AddMessageListenerWithErrorLevel( - OnMessage, - Isolate::MessageErrorLevel::kMessageError | - Isolate::MessageErrorLevel::kMessageWarning); - isolate->SetAbortOnUncaughtExceptionCallback(ShouldAbortOnUncaughtException); - isolate->SetMicrotasksPolicy(MicrotasksPolicy::kExplicit); - isolate->SetFatalErrorHandler(OnFatalError); - isolate->SetAllowWasmCodeGenerationCallback(AllowWasmCodeGenerationCallback); - isolate->SetPromiseRejectCallback(task_queue::PromiseRejectCallback); - v8::CpuProfiler::UseDetailedSourcePositionsForProfiling(isolate); + SetIsolateUpForNode(isolate, IsolateSettingCategories::kErrorHandlers); + SetIsolateUpForNode(isolate, IsolateSettingCategories::kMisc); } Isolate* NewIsolate(ArrayBufferAllocator* allocator, uv_loop_t* event_loop) { diff --git a/src/node.cc b/src/node.cc index 061d6bb424431f..9e969d33dc1e9f 100644 --- a/src/node.cc +++ b/src/node.cc @@ -886,8 +886,12 @@ int Start(int argc, char** argv) { } { - NodeMainInstance main_instance( - uv_default_loop(), result.args, result.exec_args); + Isolate::CreateParams params; + NodeMainInstance main_instance(¶ms, + uv_default_loop(), + per_process::v8_platform.Platform(), + result.args, + result.exec_args); result.exit_code = main_instance.Run(); } diff --git a/src/node_internals.h b/src/node_internals.h index 3fc6ea07361a27..fc924e3e1637ce 100644 --- a/src/node_internals.h +++ b/src/node_internals.h @@ -319,6 +319,9 @@ struct InitializationResult { }; InitializationResult InitializeOncePerProcess(int argc, char** argv); void TearDownOncePerProcess(); +enum class IsolateSettingCategories { kErrorHandlers, kMisc }; +void SetIsolateUpForNode(v8::Isolate* isolate, IsolateSettingCategories cat); +void SetIsolateCreateParamsForNode(v8::Isolate::CreateParams* params); #if HAVE_INSPECTOR namespace profiler { diff --git a/src/node_main_instance.cc b/src/node_main_instance.cc index 6ef992d006f79c..4b05149b23c163 100644 --- a/src/node_main_instance.cc +++ b/src/node_main_instance.cc @@ -12,30 +12,70 @@ using v8::Local; using v8::Locker; using v8::SealHandleScope; -NodeMainInstance::NodeMainInstance(uv_loop_t* event_loop, +NodeMainInstance::NodeMainInstance(Isolate* isolate, + uv_loop_t* event_loop, + MultiIsolatePlatform* platform, + const std::vector& args, + const std::vector& exec_args) + : args_(args), + exec_args_(exec_args), + array_buffer_allocator_(nullptr), + isolate_(isolate), + platform_(platform), + isolate_data_(nullptr), + owns_isolate_(false) { + isolate_data_.reset(new IsolateData(isolate_, event_loop, platform, nullptr)); + SetIsolateUpForNode(isolate_, IsolateSettingCategories::kMisc); +} + +NodeMainInstance* NodeMainInstance::Create( + Isolate* isolate, + uv_loop_t* event_loop, + MultiIsolatePlatform* platform, + const std::vector& args, + const std::vector& exec_args) { + return new NodeMainInstance(isolate, event_loop, platform, args, exec_args); +} + +NodeMainInstance::NodeMainInstance(Isolate::CreateParams* params, + uv_loop_t* event_loop, + MultiIsolatePlatform* platform, const std::vector& args, const std::vector& exec_args) : args_(args), exec_args_(exec_args), array_buffer_allocator_(ArrayBufferAllocator::Create()), isolate_(nullptr), - isolate_data_(nullptr) { - // TODO(joyeecheung): when we implement snapshot integration this needs to - // set params.external_references. - Isolate::CreateParams params; - params.array_buffer_allocator = array_buffer_allocator_.get(); - isolate_ = - NewIsolate(¶ms, event_loop, per_process::v8_platform.Platform()); + platform_(platform), + isolate_data_(nullptr), + owns_isolate_(true) { + params->array_buffer_allocator = array_buffer_allocator_.get(); + isolate_ = Isolate::Allocate(); CHECK_NOT_NULL(isolate_); - isolate_data_.reset(CreateIsolateData(isolate_, - event_loop, - per_process::v8_platform.Platform(), - array_buffer_allocator_.get())); + // 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); + SetIsolateCreateParamsForNode(params); + Isolate::Initialize(isolate_, *params); + + isolate_data_.reset(new IsolateData( + isolate_, event_loop, platform, array_buffer_allocator_.get())); + SetIsolateUpForNode(isolate_, IsolateSettingCategories::kMisc); + SetIsolateUpForNode(isolate_, IsolateSettingCategories::kErrorHandlers); +} + +void NodeMainInstance::Dispose() { + CHECK(!owns_isolate_); + platform_->DrainTasks(isolate_); + delete this; } NodeMainInstance::~NodeMainInstance() { + if (!owns_isolate_) { + return; + } isolate_->Dispose(); - per_process::v8_platform.Platform()->UnregisterIsolate(isolate_); + platform_->UnregisterIsolate(isolate_); } int NodeMainInstance::Run() { @@ -120,6 +160,7 @@ std::unique_ptr NodeMainInstance::CreateMainEnvironment( } Local context = NewContext(isolate_); + CHECK(!context.IsEmpty()); Context::Scope context_scope(context); std::unique_ptr env = std::make_unique( diff --git a/src/node_main_instance.h b/src/node_main_instance.h index f140edcfd72277..0a8be137a0d3ee 100644 --- a/src/node_main_instance.h +++ b/src/node_main_instance.h @@ -3,6 +3,7 @@ #if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS +#include #include "node.h" #include "util.h" #include "uv.h" @@ -14,12 +15,35 @@ namespace node { // We may be able to create an abstract class to reuse some of the routines. class NodeMainInstance { public: - NodeMainInstance(const NodeMainInstance&) = delete; - NodeMainInstance& operator=(const NodeMainInstance&) = delete; - NodeMainInstance(NodeMainInstance&&) = delete; - NodeMainInstance& operator=(NodeMainInstance&&) = delete; + // To create a main instance that does not own the isoalte, + // The caller needs to do: + // + // Isolate* isolate = Isolate::Allocate(); + // platform->RegisterIsolate(isolate, loop); + // isolate->Initialize(...); + // isolate->Enter(); + // NodeMainInstance* main_instance = + // NodeMainInstance::Create(isolate, loop, args, exec_args); + // + // When tearing it down: + // + // main_instance->Cleanup(); // While the isolate is entered + // isolate->Exit(); + // isolate->Dispose(); + // platform->UnregisterIsolate(isolate); + // + // After calling Dispose() the main_instance is no longer accessible. + static NodeMainInstance* Create(v8::Isolate* isolate, + uv_loop_t* event_loop, + MultiIsolatePlatform* platform, + const std::vector& args, + const std::vector& exec_args); + void Dispose(); - NodeMainInstance(uv_loop_t* event_loop, + // Create a main instance that owns the isolate + NodeMainInstance(v8::Isolate::CreateParams* params, + uv_loop_t* event_loop, + MultiIsolatePlatform* platform, const std::vector& args, const std::vector& exec_args); ~NodeMainInstance(); @@ -27,16 +51,28 @@ class NodeMainInstance { // Start running the Node.js instances, return the exit code when finished. int Run(); - private: // TODO(joyeecheung): align this with the CreateEnvironment exposed in node.h // and the environment creation routine in workers somehow. std::unique_ptr CreateMainEnvironment(int* exit_code); + private: + NodeMainInstance(const NodeMainInstance&) = delete; + NodeMainInstance& operator=(const NodeMainInstance&) = delete; + NodeMainInstance(NodeMainInstance&&) = delete; + NodeMainInstance& operator=(NodeMainInstance&&) = delete; + + NodeMainInstance(v8::Isolate* isolate, + uv_loop_t* event_loop, + MultiIsolatePlatform* platform, + const std::vector& args, + const std::vector& exec_args); std::vector args_; std::vector exec_args_; std::unique_ptr array_buffer_allocator_; v8::Isolate* isolate_; + MultiIsolatePlatform* platform_; std::unique_ptr isolate_data_; + bool owns_isolate_ = false; }; } // namespace node