From cab1dc5bb346b6c4d0e02e6785715af6ff6fb090 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Sun, 14 Apr 2019 14:41:04 +0800 Subject: [PATCH] src: use RAII to manage the main isolate data This patch encapsulates the main isolate management into a NodeMainInstance class that manages the resources with RAII and controls the Isolate::CreateParams (which is necessary for deserializing snapshots with external references) PR-URL: https://github.com/nodejs/node/pull/27220 Reviewed-By: Anna Henningsen Reviewed-By: James M Snell Reviewed-By: Daniel Bevenius Reviewed-By: Ben Noordhuis --- node.gyp | 2 + src/api/environment.cc | 24 ++--- src/env.cc | 2 + src/node.cc | 181 ++------------------------------------ src/node_internals.h | 4 +- src/node_main_instance.cc | 162 ++++++++++++++++++++++++++++++++++ src/node_main_instance.h | 45 ++++++++++ 7 files changed, 236 insertions(+), 184 deletions(-) create mode 100644 src/node_main_instance.cc create mode 100644 src/node_main_instance.h diff --git a/node.gyp b/node.gyp index 6e792c907b07cf..ead1d67ff2fb46 100644 --- a/node.gyp +++ b/node.gyp @@ -460,6 +460,7 @@ 'src/node_http_parser_traditional.cc', 'src/node_http2.cc', 'src/node_i18n.cc', + 'src/node_main_instance.cc', 'src/node_messaging.cc', 'src/node_metadata.cc', 'src/node_native_module.cc', @@ -540,6 +541,7 @@ 'src/node_http2_state.h', 'src/node_i18n.h', 'src/node_internals.h', + 'src/node_main_instance.h', 'src/node_messaging.h', 'src/node_metadata.h', 'src/node_mutex.h', diff --git a/src/api/environment.cc b/src/api/environment.cc index 70228b77d8860b..0543e0323d8ee2 100644 --- a/src/api/environment.cc +++ b/src/api/environment.cc @@ -169,11 +169,7 @@ void FreeArrayBufferAllocator(ArrayBufferAllocator* allocator) { delete allocator; } -void SetIsolateCreateParams(Isolate::CreateParams* params, - ArrayBufferAllocator* allocator) { - if (allocator != nullptr) - params->array_buffer_allocator = allocator; - +void SetIsolateCreateParamsForNode(Isolate::CreateParams* params) { const uint64_t total_memory = uv_get_total_memory(); if (total_memory > 0) { // V8 defaults to 700MB or 1.4GB on 32 and 64 bit platforms respectively. @@ -204,25 +200,33 @@ Isolate* NewIsolate(ArrayBufferAllocator* allocator, uv_loop_t* event_loop) { return NewIsolate(allocator, event_loop, GetMainThreadMultiIsolatePlatform()); } -Isolate* NewIsolate(ArrayBufferAllocator* allocator, +// TODO(joyeecheung): we may want to expose this, but then we need to be +// careful about what we override in the params. +Isolate* NewIsolate(Isolate::CreateParams* params, uv_loop_t* event_loop, MultiIsolatePlatform* platform) { - Isolate::CreateParams params; - SetIsolateCreateParams(¶ms, 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); + SetIsolateCreateParamsForNode(params); + Isolate::Initialize(isolate, *params); SetIsolateUpForNode(isolate); return isolate; } +Isolate* NewIsolate(ArrayBufferAllocator* allocator, + uv_loop_t* event_loop, + MultiIsolatePlatform* platform) { + Isolate::CreateParams params; + if (allocator != nullptr) params.array_buffer_allocator = allocator; + return NewIsolate(¶ms, event_loop, platform); +} + IsolateData* CreateIsolateData(Isolate* isolate, uv_loop_t* loop, MultiIsolatePlatform* platform, diff --git a/src/env.cc b/src/env.cc index 88017ab54524e7..ccb3c404f91ba2 100644 --- a/src/env.cc +++ b/src/env.cc @@ -76,6 +76,8 @@ IsolateData::IsolateData(Isolate* isolate, // One byte because our strings are ASCII and we can safely skip V8's UTF-8 // decoding step. + HandleScope handle_scope(isolate); + #define V(PropertyName, StringValue) \ PropertyName ## _.Set( \ isolate, \ diff --git a/src/node.cc b/src/node.cc index 0e4e3618240303..53a9ae397bf20b 100644 --- a/src/node.cc +++ b/src/node.cc @@ -25,16 +25,12 @@ #include "debug_utils.h" #include "node_binding.h" -#include "node_buffer.h" -#include "node_constants.h" -#include "node_context_data.h" -#include "node_errors.h" #include "node_internals.h" +#include "node_main_instance.h" #include "node_metadata.h" #include "node_native_module_env.h" #include "node_options-inl.h" #include "node_perf.h" -#include "node_platform.h" #include "node_process.h" #include "node_revert.h" #include "node_v8_platform-inl.h" @@ -56,13 +52,6 @@ #include "node_dtrace.h" #endif -#include "async_wrap-inl.h" -#include "env-inl.h" -#include "handle_wrap.h" -#include "req_wrap-inl.h" -#include "string_bytes.h" -#include "util.h" -#include "uv.h" #if NODE_USE_V8_PLATFORM #include "libplatform/libplatform.h" #endif // NODE_USE_V8_PLATFORM @@ -122,10 +111,8 @@ using native_module::NativeModuleEnv; using options_parser::kAllowedInEnvironment; using options_parser::kDisallowedInEnvironment; -using v8::Array; using v8::Boolean; using v8::Context; -using v8::DEFAULT; using v8::EscapableHandleScope; using v8::Exception; using v8::Function; @@ -133,12 +120,10 @@ using v8::FunctionCallbackInfo; using v8::HandleScope; using v8::Isolate; using v8::Local; -using v8::Locker; using v8::Maybe; using v8::MaybeLocal; using v8::Object; using v8::Script; -using v8::SealHandleScope; using v8::String; using v8::Undefined; using v8::V8; @@ -767,161 +752,6 @@ void Init(int* argc, argv[i] = strdup(argv_[i].c_str()); } -void RunBeforeExit(Environment* env) { - env->RunBeforeExitCallbacks(); - - if (!uv_loop_alive(env->event_loop())) - EmitBeforeExit(env); -} - -// TODO(joyeecheung): align this with the CreateEnvironment exposed in node.h -// and the environment creation routine in workers somehow. -inline std::unique_ptr CreateMainEnvironment( - IsolateData* isolate_data, - const std::vector& args, - const std::vector& exec_args, - int* exit_code) { - Isolate* isolate = isolate_data->isolate(); - HandleScope handle_scope(isolate); - - // TODO(addaleax): This should load a real per-Isolate option, currently - // this is still effectively per-process. - if (isolate_data->options()->track_heap_objects) { - isolate->GetHeapProfiler()->StartTrackingHeapObjects(true); - } - - Local context = NewContext(isolate); - Context::Scope context_scope(context); - - std::unique_ptr env = std::make_unique( - isolate_data, - context, - static_cast(Environment::kIsMainThread | - Environment::kOwnsProcessState | - Environment::kOwnsInspector)); - env->InitializeLibuv(per_process::v8_is_profiling); - env->ProcessCliArgs(args, exec_args); - -#if HAVE_INSPECTOR && NODE_USE_V8_PLATFORM - CHECK(!env->inspector_agent()->IsListening()); - // Inspector agent can't fail to start, but if it was configured to listen - // right away on the websocket port and fails to bind/etc, this will return - // false. - env->inspector_agent()->Start(args.size() > 1 ? args[1].c_str() : "", - env->options()->debug_options(), - env->inspector_host_port(), - true); - if (env->options()->debug_options().inspector_enabled && - !env->inspector_agent()->IsListening()) { - *exit_code = 12; // Signal internal error. - return env; - } -#else - // inspector_enabled can't be true if !HAVE_INSPECTOR or !NODE_USE_V8_PLATFORM - // - the option parser should not allow that. - CHECK(!env->options()->debug_options().inspector_enabled); -#endif // HAVE_INSPECTOR && NODE_USE_V8_PLATFORM - - if (RunBootstrapping(env.get()).IsEmpty()) { - *exit_code = 1; - } - - return env; -} - -inline int StartNodeWithIsolate(Isolate* isolate, - IsolateData* isolate_data, - const std::vector& args, - const std::vector& exec_args) { - int exit_code = 0; - std::unique_ptr env = - CreateMainEnvironment(isolate_data, args, exec_args, &exit_code); - CHECK_NOT_NULL(env); - HandleScope handle_scope(env->isolate()); - Context::Scope context_scope(env->context()); - - if (exit_code == 0) { - { - AsyncCallbackScope callback_scope(env.get()); - env->async_hooks()->push_async_ids(1, 0); - LoadEnvironment(env.get()); - env->async_hooks()->pop_async_id(1); - } - - { - SealHandleScope seal(isolate); - bool more; - env->performance_state()->Mark( - node::performance::NODE_PERFORMANCE_MILESTONE_LOOP_START); - do { - uv_run(env->event_loop(), UV_RUN_DEFAULT); - - per_process::v8_platform.DrainVMTasks(isolate); - - more = uv_loop_alive(env->event_loop()); - if (more && !env->is_stopping()) continue; - - RunBeforeExit(env.get()); - - // Emit `beforeExit` if the loop became alive either after emitting - // event, or after running some callbacks. - more = uv_loop_alive(env->event_loop()); - } while (more == true && !env->is_stopping()); - env->performance_state()->Mark( - node::performance::NODE_PERFORMANCE_MILESTONE_LOOP_EXIT); - } - - env->set_trace_sync_io(false); - exit_code = EmitExit(env.get()); - WaitForInspectorDisconnect(env.get()); - } - - env->set_can_call_into_js(false); - env->stop_sub_worker_contexts(); - uv_tty_reset_mode(); - env->RunCleanup(); - RunAtExit(env.get()); - - per_process::v8_platform.DrainVMTasks(isolate); - per_process::v8_platform.CancelVMTasks(isolate); - -#if defined(LEAK_SANITIZER) - __lsan_do_leak_check(); -#endif - - return exit_code; -} - -inline int StartNodeWithLoopAndArgs(uv_loop_t* event_loop, - const std::vector& args, - const std::vector& exec_args) { - std::unique_ptr - allocator(CreateArrayBufferAllocator(), &FreeArrayBufferAllocator); - Isolate* const isolate = NewIsolate(allocator.get(), event_loop); - if (isolate == nullptr) - return 12; // Signal internal error. - - int exit_code; - { - Locker locker(isolate); - Isolate::Scope isolate_scope(isolate); - HandleScope handle_scope(isolate); - std::unique_ptr isolate_data( - CreateIsolateData(isolate, - event_loop, - per_process::v8_platform.Platform(), - allocator.get()), - &FreeIsolateData); - exit_code = - StartNodeWithIsolate(isolate, isolate_data.get(), args, exec_args); - } - - isolate->Dispose(); - per_process::v8_platform.Platform()->UnregisterIsolate(isolate); - - return exit_code; -} - int Start(int argc, char** argv) { atexit([] () { uv_tty_reset_mode(); }); PlatformInit(); @@ -981,8 +811,13 @@ int Start(int argc, char** argv) { V8::Initialize(); performance::performance_v8_start = PERFORMANCE_NOW(); per_process::v8_initialized = true; - const int exit_code = - StartNodeWithLoopAndArgs(uv_default_loop(), args, exec_args); + + int exit_code = 0; + { + NodeMainInstance main_instance(uv_default_loop(), args, exec_args); + exit_code = main_instance.Run(); + } + per_process::v8_initialized = false; V8::Dispose(); diff --git a/src/node_internals.h b/src/node_internals.h index bfc3a519f6f500..53f7d3cdbad237 100644 --- a/src/node_internals.h +++ b/src/node_internals.h @@ -297,7 +297,9 @@ bool SafeGetenv(const char* key, std::string* text, Environment* env = nullptr); } // namespace credentials void DefineZlibConstants(v8::Local target); - +v8::Isolate* NewIsolate(v8::Isolate::CreateParams* params, + uv_loop_t* event_loop, + MultiIsolatePlatform* platform); v8::MaybeLocal RunBootstrapping(Environment* env); v8::MaybeLocal StartExecution(Environment* env, const char* main_script_id); diff --git a/src/node_main_instance.cc b/src/node_main_instance.cc new file mode 100644 index 00000000000000..6ef992d006f79c --- /dev/null +++ b/src/node_main_instance.cc @@ -0,0 +1,162 @@ +#include "node_main_instance.h" +#include "node_internals.h" +#include "node_options-inl.h" +#include "node_v8_platform-inl.h" + +namespace node { + +using v8::Context; +using v8::HandleScope; +using v8::Isolate; +using v8::Local; +using v8::Locker; +using v8::SealHandleScope; + +NodeMainInstance::NodeMainInstance(uv_loop_t* event_loop, + 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()); + CHECK_NOT_NULL(isolate_); + isolate_data_.reset(CreateIsolateData(isolate_, + event_loop, + per_process::v8_platform.Platform(), + array_buffer_allocator_.get())); +} + +NodeMainInstance::~NodeMainInstance() { + isolate_->Dispose(); + per_process::v8_platform.Platform()->UnregisterIsolate(isolate_); +} + +int NodeMainInstance::Run() { + Locker locker(isolate_); + Isolate::Scope isolate_scope(isolate_); + HandleScope handle_scope(isolate_); + + int exit_code = 0; + std::unique_ptr env = CreateMainEnvironment(&exit_code); + + CHECK_NOT_NULL(env); + Context::Scope context_scope(env->context()); + + if (exit_code == 0) { + { + AsyncCallbackScope callback_scope(env.get()); + env->async_hooks()->push_async_ids(1, 0); + LoadEnvironment(env.get()); + env->async_hooks()->pop_async_id(1); + } + + { + SealHandleScope seal(isolate_); + bool more; + env->performance_state()->Mark( + node::performance::NODE_PERFORMANCE_MILESTONE_LOOP_START); + do { + uv_run(env->event_loop(), UV_RUN_DEFAULT); + + per_process::v8_platform.DrainVMTasks(isolate_); + + more = uv_loop_alive(env->event_loop()); + if (more && !env->is_stopping()) continue; + + env->RunBeforeExitCallbacks(); + + if (!uv_loop_alive(env->event_loop())) { + EmitBeforeExit(env.get()); + } + + // Emit `beforeExit` if the loop became alive either after emitting + // event, or after running some callbacks. + more = uv_loop_alive(env->event_loop()); + } while (more == true && !env->is_stopping()); + env->performance_state()->Mark( + node::performance::NODE_PERFORMANCE_MILESTONE_LOOP_EXIT); + } + + env->set_trace_sync_io(false); + exit_code = EmitExit(env.get()); + WaitForInspectorDisconnect(env.get()); + } + + env->set_can_call_into_js(false); + env->stop_sub_worker_contexts(); + uv_tty_reset_mode(); + env->RunCleanup(); + RunAtExit(env.get()); + + per_process::v8_platform.DrainVMTasks(isolate_); + per_process::v8_platform.CancelVMTasks(isolate_); + +#if defined(LEAK_SANITIZER) + __lsan_do_leak_check(); +#endif + + return exit_code; +} + +// TODO(joyeecheung): align this with the CreateEnvironment exposed in node.h +// and the environment creation routine in workers somehow. +std::unique_ptr NodeMainInstance::CreateMainEnvironment( + int* exit_code) { + *exit_code = 0; // Reset the exit code to 0 + + HandleScope handle_scope(isolate_); + + // TODO(addaleax): This should load a real per-Isolate option, currently + // this is still effectively per-process. + if (isolate_data_->options()->track_heap_objects) { + isolate_->GetHeapProfiler()->StartTrackingHeapObjects(true); + } + + Local context = NewContext(isolate_); + Context::Scope context_scope(context); + + std::unique_ptr env = std::make_unique( + isolate_data_.get(), + context, + static_cast(Environment::kIsMainThread | + Environment::kOwnsProcessState | + Environment::kOwnsInspector)); + env->InitializeLibuv(per_process::v8_is_profiling); + env->ProcessCliArgs(args_, exec_args_); + +#if HAVE_INSPECTOR && NODE_USE_V8_PLATFORM + CHECK(!env->inspector_agent()->IsListening()); + // Inspector agent can't fail to start, but if it was configured to listen + // right away on the websocket port and fails to bind/etc, this will return + // false. + env->inspector_agent()->Start(args_.size() > 1 ? args_[1].c_str() : "", + env->options()->debug_options(), + env->inspector_host_port(), + true); + if (env->options()->debug_options().inspector_enabled && + !env->inspector_agent()->IsListening()) { + *exit_code = 12; // Signal internal error. + return env; + } +#else + // inspector_enabled can't be true if !HAVE_INSPECTOR or + // !NODE_USE_V8_PLATFORM + // - the option parser should not allow that. + CHECK(!env->options()->debug_options().inspector_enabled); +#endif // HAVE_INSPECTOR && NODE_USE_V8_PLATFORM + + if (RunBootstrapping(env.get()).IsEmpty()) { + *exit_code = 1; + } + + return env; +} + +} // namespace node diff --git a/src/node_main_instance.h b/src/node_main_instance.h new file mode 100644 index 00000000000000..f140edcfd72277 --- /dev/null +++ b/src/node_main_instance.h @@ -0,0 +1,45 @@ +#ifndef SRC_NODE_MAIN_INSTANCE_H_ +#define SRC_NODE_MAIN_INSTANCE_H_ + +#if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS + +#include "node.h" +#include "util.h" +#include "uv.h" +#include "v8.h" + +namespace node { + +// TODO(joyeecheung): align this with the Worker/WorkerThreadData class. +// 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; + + NodeMainInstance(uv_loop_t* event_loop, + const std::vector& args, + const std::vector& exec_args); + ~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); + + std::vector args_; + std::vector exec_args_; + std::unique_ptr array_buffer_allocator_; + v8::Isolate* isolate_; + std::unique_ptr isolate_data_; +}; + +} // namespace node + +#endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS +#endif // SRC_NODE_MAIN_INSTANCE_H_