From d77e818a54a72dc6eef752c215471913573a4874 Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Fri, 21 Oct 2016 13:47:49 +0200 Subject: [PATCH 1/2] src: simplify code, remove NodeInstanceData NodeInstanceData is not used meaningfully and makes the initialization logic harder to follow. Let's remove it and delete 100 lines of code in one fell swoop. PR-URL: https://github.com/nodejs/node/pull/9224 Reviewed-By: James M Snell --- src/node.cc | 51 +++++++++------------------- src/node_internals.h | 81 -------------------------------------------- 2 files changed, 16 insertions(+), 116 deletions(-) diff --git a/src/node.cc b/src/node.cc index 84c627b32d50c0..ae9d6a8d73b1c1 100644 --- a/src/node.cc +++ b/src/node.cc @@ -4374,10 +4374,9 @@ void FreeEnvironment(Environment* env) { } -// Entry point for new node instances, also called directly for the main -// node instance. -static void StartNodeInstance(void* arg) { - NodeInstanceData* instance_data = static_cast(arg); +inline int Start(uv_loop_t* event_loop, + int argc, const char* const* argv, + int exec_argc, const char* const* exec_argv) { Isolate::CreateParams params; ArrayBufferAllocator array_buffer_allocator; params.array_buffer_allocator = &array_buffer_allocator; @@ -4388,39 +4387,32 @@ static void StartNodeInstance(void* arg) { { Mutex::ScopedLock scoped_lock(node_isolate_mutex); - if (instance_data->is_main()) { - CHECK_EQ(node_isolate, nullptr); - node_isolate = isolate; - } + CHECK_EQ(node_isolate, nullptr); + node_isolate = isolate; } if (track_heap_objects) { isolate->GetHeapProfiler()->StartTrackingHeapObjects(true); } + int exit_code; { Locker locker(isolate); Isolate::Scope isolate_scope(isolate); HandleScope handle_scope(isolate); - IsolateData isolate_data(isolate, instance_data->event_loop(), + IsolateData isolate_data(isolate, event_loop, array_buffer_allocator.zero_fill_field()); Local context = Context::New(isolate); Context::Scope context_scope(context); Environment env(&isolate_data, context); - env.Start(instance_data->argc(), - instance_data->argv(), - instance_data->exec_argc(), - instance_data->exec_argv(), - v8_is_profiling); + env.Start(argc, argv, exec_argc, exec_argv, v8_is_profiling); isolate->SetAbortOnUncaughtExceptionCallback( ShouldAbortOnUncaughtException); // Start debug agent when argv has --debug - if (instance_data->use_debug_agent()) { - const char* path = instance_data->argc() > 1 - ? instance_data->argv()[1] - : nullptr; + if (use_debug_agent) { + const char* path = argc > 1 ? argv[1] : nullptr; StartDebug(&env, path, debug_wait_connect); if (use_inspector && !debugger_running) { exit(12); @@ -4435,7 +4427,7 @@ static void StartNodeInstance(void* arg) { env.set_trace_sync_io(trace_sync_io); // Enable debugger - if (instance_data->use_debug_agent()) + if (use_debug_agent) EnableDebug(&env); { @@ -4460,9 +4452,7 @@ static void StartNodeInstance(void* arg) { env.set_trace_sync_io(false); - int exit_code = EmitExit(&env); - if (instance_data->is_main()) - instance_data->set_exit_code(exit_code); + exit_code = EmitExit(&env); RunAtExit(&env); WaitForInspectorDisconnect(&env); @@ -4480,6 +4470,8 @@ static void StartNodeInstance(void* arg) { CHECK_NE(isolate, nullptr); isolate->Dispose(); isolate = nullptr; + + return exit_code; } int Start(int argc, char** argv) { @@ -4510,19 +4502,8 @@ int Start(int argc, char** argv) { v8_platform.Initialize(v8_thread_pool_size); V8::Initialize(); v8_initialized = true; - - int exit_code = 1; - { - NodeInstanceData instance_data(NodeInstanceType::MAIN, - uv_default_loop(), - argc, - const_cast(argv), - exec_argc, - exec_argv, - use_debug_agent); - StartNodeInstance(&instance_data); - exit_code = instance_data.exit_code(); - } + const int exit_code = + Start(uv_default_loop(), argc, argv, exec_argc, exec_argv); v8_initialized = false; V8::Dispose(); diff --git a/src/node_internals.h b/src/node_internals.h index 99b0cec2df2178..ae284660782cfe 100644 --- a/src/node_internals.h +++ b/src/node_internals.h @@ -179,87 +179,6 @@ class ArrayBufferAllocator : public v8::ArrayBuffer::Allocator { // by clearing all callbacks that could handle the error. void ClearFatalExceptionHandlers(Environment* env); -enum NodeInstanceType { MAIN, WORKER, REMOTE_DEBUG_SERVER }; - -class NodeInstanceData { - public: - NodeInstanceData(NodeInstanceType node_instance_type, - uv_loop_t* event_loop, - int argc, - const char** argv, - int exec_argc, - const char** exec_argv, - bool use_debug_agent_flag) - : node_instance_type_(node_instance_type), - exit_code_(1), - event_loop_(event_loop), - argc_(argc), - argv_(argv), - exec_argc_(exec_argc), - exec_argv_(exec_argv), - use_debug_agent_flag_(use_debug_agent_flag) { - CHECK_NE(event_loop_, nullptr); - } - - uv_loop_t* event_loop() const { - return event_loop_; - } - - int exit_code() { - CHECK(is_main()); - return exit_code_; - } - - void set_exit_code(int exit_code) { - CHECK(is_main()); - exit_code_ = exit_code; - } - - bool is_main() { - return node_instance_type_ == MAIN; - } - - bool is_worker() { - return node_instance_type_ == WORKER; - } - - bool is_remote_debug_server() { - return node_instance_type_ == REMOTE_DEBUG_SERVER; - } - - int argc() { - return argc_; - } - - const char** argv() { - return argv_; - } - - int exec_argc() { - return exec_argc_; - } - - const char** exec_argv() { - return exec_argv_; - } - - bool use_debug_agent() { - return is_main() && use_debug_agent_flag_; - } - - private: - const NodeInstanceType node_instance_type_; - int exit_code_; - uv_loop_t* const event_loop_; - const int argc_; - const char** argv_; - const int exec_argc_; - const char** exec_argv_; - const bool use_debug_agent_flag_; - - DISALLOW_COPY_AND_ASSIGN(NodeInstanceData); -}; - namespace Buffer { v8::MaybeLocal Copy(Environment* env, const char* data, size_t len); v8::MaybeLocal New(Environment* env, size_t size); From ceb602326066674b894c9fc4d53166d06a918c02 Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Fri, 21 Oct 2016 14:24:48 +0200 Subject: [PATCH 2/2] src: clean up program/isolate/env init logic Reorder the initialization logic so that program-wide, per-isolate and per-environment initialization is more cleanly separated. PR-URL: https://github.com/nodejs/node/pull/9224 Reviewed-By: James M Snell --- src/env.cc | 2 - src/node.cc | 161 ++++++++++++++++++++++++++-------------------------- 2 files changed, 81 insertions(+), 82 deletions(-) diff --git a/src/env.cc b/src/env.cc index 8efe13816c0ee9..efa2d53f0435b2 100644 --- a/src/env.cc +++ b/src/env.cc @@ -30,8 +30,6 @@ void Environment::Start(int argc, HandleScope handle_scope(isolate()); Context::Scope context_scope(context()); - isolate()->SetAutorunMicrotasks(false); - uv_check_init(event_loop(), immediate_check_handle()); uv_unref(reinterpret_cast(immediate_check_handle())); diff --git a/src/node.cc b/src/node.cc index ae9d6a8d73b1c1..111b83bf70c016 100644 --- a/src/node.cc +++ b/src/node.cc @@ -3341,11 +3341,6 @@ void SetupProcessObject(Environment* env, #undef READONLY_PROPERTY -static void AtProcessExit() { - uv_tty_reset_mode(); -} - - void SignalExit(int signo) { uv_tty_reset_mode(); #ifdef __FreeBSD__ @@ -3375,11 +3370,6 @@ static void RawDebug(const FunctionCallbackInfo& args) { void LoadEnvironment(Environment* env) { HandleScope handle_scope(env->isolate()); - env->isolate()->SetFatalErrorHandler(node::OnFatalError); - env->isolate()->AddMessageListener(OnMessage); - - atexit(AtProcessExit); - TryCatch try_catch(env->isolate()); // Disable verbose mode to stop FatalException() handler from trying @@ -4374,16 +4364,89 @@ void FreeEnvironment(Environment* env) { } +inline int Start(Isolate* isolate, IsolateData* isolate_data, + int argc, const char* const* argv, + int exec_argc, const char* const* exec_argv) { + HandleScope handle_scope(isolate); + Local context = Context::New(isolate); + Context::Scope context_scope(context); + Environment env(isolate_data, context); + env.Start(argc, argv, exec_argc, exec_argv, v8_is_profiling); + + // Start debug agent when argv has --debug + if (use_debug_agent) { + const char* path = argc > 1 ? argv[1] : nullptr; + StartDebug(&env, path, debug_wait_connect); + if (use_inspector && !debugger_running) + return 12; // Signal internal error. + } + + { + Environment::AsyncCallbackScope callback_scope(&env); + LoadEnvironment(&env); + } + + env.set_trace_sync_io(trace_sync_io); + + // Enable debugger + if (use_debug_agent) + EnableDebug(&env); + + { + SealHandleScope seal(isolate); + bool more; + do { + v8_platform.PumpMessageLoop(isolate); + more = uv_run(env.event_loop(), UV_RUN_ONCE); + + if (more == false) { + v8_platform.PumpMessageLoop(isolate); + EmitBeforeExit(&env); + + // Emit `beforeExit` if the loop became alive either after emitting + // event, or after running some callbacks. + more = uv_loop_alive(env.event_loop()); + if (uv_run(env.event_loop(), UV_RUN_NOWAIT) != 0) + more = true; + } + } while (more == true); + } + + env.set_trace_sync_io(false); + + const int exit_code = EmitExit(&env); + RunAtExit(&env); + + WaitForInspectorDisconnect(&env); +#if defined(LEAK_SANITIZER) + __lsan_do_leak_check(); +#endif + + return exit_code; +} + inline int Start(uv_loop_t* event_loop, int argc, const char* const* argv, int exec_argc, const char* const* exec_argv) { Isolate::CreateParams params; - ArrayBufferAllocator array_buffer_allocator; - params.array_buffer_allocator = &array_buffer_allocator; + ArrayBufferAllocator allocator; + params.array_buffer_allocator = &allocator; #ifdef NODE_ENABLE_VTUNE_PROFILING params.code_event_handler = vTune::GetVtuneCodeEventHandler(); #endif - Isolate* isolate = Isolate::New(params); + + Isolate* const isolate = Isolate::New(params); + if (isolate == nullptr) + return 12; // Signal internal error. + + isolate->AddMessageListener(OnMessage); + isolate->SetAbortOnUncaughtExceptionCallback(ShouldAbortOnUncaughtException); + isolate->SetAutorunMicrotasks(false); + isolate->SetFatalErrorHandler(OnFatalError); + + if (track_heap_objects) { + isolate->GetHeapProfiler()->StartTrackingHeapObjects(true); + } { Mutex::ScopedLock scoped_lock(node_isolate_mutex); @@ -4391,90 +4454,28 @@ inline int Start(uv_loop_t* event_loop, node_isolate = isolate; } - if (track_heap_objects) { - isolate->GetHeapProfiler()->StartTrackingHeapObjects(true); - } - int exit_code; { Locker locker(isolate); Isolate::Scope isolate_scope(isolate); HandleScope handle_scope(isolate); - IsolateData isolate_data(isolate, event_loop, - array_buffer_allocator.zero_fill_field()); - Local context = Context::New(isolate); - Context::Scope context_scope(context); - Environment env(&isolate_data, context); - env.Start(argc, argv, exec_argc, exec_argv, v8_is_profiling); - - isolate->SetAbortOnUncaughtExceptionCallback( - ShouldAbortOnUncaughtException); - - // Start debug agent when argv has --debug - if (use_debug_agent) { - const char* path = argc > 1 ? argv[1] : nullptr; - StartDebug(&env, path, debug_wait_connect); - if (use_inspector && !debugger_running) { - exit(12); - } - } - - { - Environment::AsyncCallbackScope callback_scope(&env); - LoadEnvironment(&env); - } - - env.set_trace_sync_io(trace_sync_io); - - // Enable debugger - if (use_debug_agent) - EnableDebug(&env); - - { - SealHandleScope seal(isolate); - bool more; - do { - v8_platform.PumpMessageLoop(isolate); - more = uv_run(env.event_loop(), UV_RUN_ONCE); - - if (more == false) { - v8_platform.PumpMessageLoop(isolate); - EmitBeforeExit(&env); - - // Emit `beforeExit` if the loop became alive either after emitting - // event, or after running some callbacks. - more = uv_loop_alive(env.event_loop()); - if (uv_run(env.event_loop(), UV_RUN_NOWAIT) != 0) - more = true; - } - } while (more == true); - } - - env.set_trace_sync_io(false); - - exit_code = EmitExit(&env); - RunAtExit(&env); - - WaitForInspectorDisconnect(&env); -#if defined(LEAK_SANITIZER) - __lsan_do_leak_check(); -#endif + IsolateData isolate_data(isolate, event_loop, allocator.zero_fill_field()); + exit_code = Start(isolate, &isolate_data, argc, argv, exec_argc, exec_argv); } { Mutex::ScopedLock scoped_lock(node_isolate_mutex); - if (node_isolate == isolate) - node_isolate = nullptr; + CHECK_EQ(node_isolate, isolate); + node_isolate = nullptr; } - CHECK_NE(isolate, nullptr); isolate->Dispose(); - isolate = nullptr; return exit_code; } int Start(int argc, char** argv) { + atexit([] () { uv_tty_reset_mode(); }); PlatformInit(); CHECK_GT(argc, 0);