From 98b86895e38e271e063e9ef092b08a5d5f88593f Mon Sep 17 00:00:00 2001 From: Daniel Bevenius Date: Thu, 23 Jun 2016 14:07:53 +0200 Subject: [PATCH 1/6] src: moving dispatch_debug_messages_async to Environment This commit attempts to fix one of the items in https://github.com/nodejs/node/issues/4641, which was to move dispatch_debug_messages_async to the Environment class. --- src/env-inl.h | 4 ++++ src/env.h | 2 ++ src/node.cc | 47 ++++++++++++++++++++++++++--------------------- 3 files changed, 32 insertions(+), 21 deletions(-) diff --git a/src/env-inl.h b/src/env-inl.h index ea2d01aa6b3a35..301b358a88b42b 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -247,6 +247,10 @@ inline uv_idle_t* Environment::immediate_idle_handle() { return &immediate_idle_handle_; } +inline uv_async_t* Environment::dispatch_debug_messages_async() { + return &dispatch_debug_messages_async_; +} + inline void Environment::RegisterHandleCleanup(uv_handle_t* handle, HandleCleanupCb cb, void *arg) { diff --git a/src/env.h b/src/env.h index a426f60a9b6316..ed75f4d7598d0d 100644 --- a/src/env.h +++ b/src/env.h @@ -467,6 +467,7 @@ class Environment { static inline Environment* from_immediate_check_handle(uv_check_t* handle); inline uv_check_t* immediate_check_handle(); inline uv_idle_t* immediate_idle_handle(); + inline uv_async_t* dispatch_debug_messages_async(); // Register clean-up cb to be called on environment destruction. inline void RegisterHandleCleanup(uv_handle_t* handle, @@ -586,6 +587,7 @@ class Environment { uv_idle_t immediate_idle_handle_; uv_prepare_t idle_prepare_handle_; uv_check_t idle_check_handle_; + uv_async_t dispatch_debug_messages_async_; AsyncHooks async_hooks_; DomainFlag domain_flag_; TickInfo tick_info_; diff --git a/src/node.cc b/src/node.cc index b6fa4606f5fae0..7bb00e8a5a0cb6 100644 --- a/src/node.cc +++ b/src/node.cc @@ -179,7 +179,6 @@ bool config_preserve_symlinks = false; // process-relative uptime base, initialized at start-up static double prog_start_time; static bool debugger_running; -static uv_async_t dispatch_debug_messages_async; static Mutex node_isolate_mutex; static v8::Isolate* node_isolate; @@ -3643,8 +3642,7 @@ static void ParseArgs(int* argc, // Called from V8 Debug Agent TCP thread. static void DispatchMessagesDebugAgentCallback(Environment* env) { - // TODO(indutny): move async handle to environment - uv_async_send(&dispatch_debug_messages_async); + uv_async_send(env->dispatch_debug_messages_async()); } @@ -3697,11 +3695,11 @@ static void EnableDebug(Environment* env) { // Called from an arbitrary thread. -static void TryStartDebugger() { +static void TryStartDebugger(uv_async_t* dispatch_debug_messages_async) { Mutex::ScopedLock scoped_lock(node_isolate_mutex); if (auto isolate = node_isolate) { v8::Debug::DebugBreak(isolate); - uv_async_send(&dispatch_debug_messages_async); + uv_async_send(dispatch_debug_messages_async); } } @@ -3768,16 +3766,20 @@ void DebugProcess(const FunctionCallbackInfo& args) { } -inline void* DebugSignalThreadMain(void* unused) { +inline void* DebugSignalThreadMain(void* async_handler) { + uv_async_t* dispatch_debug_messages_async = + reinterpret_cast(async_handler); + CHECK_NE(dispatch_debug_messages_async, nullptr); for (;;) { uv_sem_wait(&debug_semaphore); - TryStartDebugger(); + TryStartDebugger(dispatch_debug_messages_async); } return nullptr; } -static int RegisterDebugSignalHandler() { +static int RegisterDebugSignalHandler( + uv_async_t* dispatch_debug_messages_async) { // Start a watchdog thread for calling v8::Debug::DebugBreak() because // it's not safe to call directly from the signal handler, it can // deadlock with the thread it interrupts. @@ -3795,8 +3797,10 @@ static int RegisterDebugSignalHandler() { sigfillset(&sigmask); CHECK_EQ(0, pthread_sigmask(SIG_SETMASK, &sigmask, &sigmask)); pthread_t thread; - const int err = - pthread_create(&thread, &attr, DebugSignalThreadMain, nullptr); + const int err = pthread_create(&thread, + &attr, + DebugSignalThreadMain, + dispatch_debug_messages_async); CHECK_EQ(0, pthread_sigmask(SIG_SETMASK, &sigmask, nullptr)); CHECK_EQ(0, pthread_attr_destroy(&attr)); if (err != 0) { @@ -4057,13 +4061,6 @@ void Init(int* argc, // Make inherited handles noninheritable. uv_disable_stdio_inheritance(); - // init async debug messages dispatching - // Main thread uses uv_default_loop - uv_async_init(uv_default_loop(), - &dispatch_debug_messages_async, - DispatchDebugMessagesAsyncCallback); - uv_unref(reinterpret_cast(&dispatch_debug_messages_async)); - #if defined(NODE_V8_OPTIONS) // Should come before the call to V8::SetFlagsFromCommandLine() // so the user can disable a flag --foo at run-time by passing @@ -4130,10 +4127,6 @@ void Init(int* argc, const char no_typed_array_heap[] = "--typed_array_max_size_in_heap=0"; V8::SetFlagsFromString(no_typed_array_heap, sizeof(no_typed_array_heap) - 1); - if (!use_debug_agent) { - RegisterDebugSignalHandler(); - } - // We should set node_is_initialized here instead of in node::Start, // otherwise embedders using node::Init to initialize everything will not be // able to set it and native modules will not load for them. @@ -4270,7 +4263,19 @@ static void StartNodeInstance(void* arg) { array_buffer_allocator.zero_fill_field()); Local context = Context::New(isolate); Context::Scope context_scope(context); + Environment env(&isolate_data, context); + + uv_async_init(uv_default_loop(), + env.dispatch_debug_messages_async(), + DispatchDebugMessagesAsyncCallback); + uv_unref(reinterpret_cast( + env.dispatch_debug_messages_async())); + + if (!use_debug_agent) { + RegisterDebugSignalHandler(env.dispatch_debug_messages_async()); + } + env.Start(instance_data->argc(), instance_data->argv(), instance_data->exec_argc(), From cc7279f4532fabf6357245d7df2730da60816325 Mon Sep 17 00:00:00 2001 From: Daniel Bevenius Date: Thu, 23 Jun 2016 19:54:45 +0200 Subject: [PATCH 2/6] Changing reinterpret_cast to static_cast --- src/node.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/node.cc b/src/node.cc index 7bb00e8a5a0cb6..6efeeb09d1b820 100644 --- a/src/node.cc +++ b/src/node.cc @@ -3768,7 +3768,7 @@ void DebugProcess(const FunctionCallbackInfo& args) { inline void* DebugSignalThreadMain(void* async_handler) { uv_async_t* dispatch_debug_messages_async = - reinterpret_cast(async_handler); + static_cast(async_handler); CHECK_NE(dispatch_debug_messages_async, nullptr); for (;;) { uv_sem_wait(&debug_semaphore); From 0870bcbfb13214dd617735614c022934a1e276dd Mon Sep 17 00:00:00 2001 From: Daniel Bevenius Date: Thu, 23 Jun 2016 19:56:09 +0200 Subject: [PATCH 3/6] Fixing indentation for RegisterDebugSignalHandler --- src/node.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/node.cc b/src/node.cc index 6efeeb09d1b820..c78a738bf148bb 100644 --- a/src/node.cc +++ b/src/node.cc @@ -3779,7 +3779,7 @@ inline void* DebugSignalThreadMain(void* async_handler) { static int RegisterDebugSignalHandler( - uv_async_t* dispatch_debug_messages_async) { + uv_async_t* dispatch_debug_messages_async) { // Start a watchdog thread for calling v8::Debug::DebugBreak() because // it's not safe to call directly from the signal handler, it can // deadlock with the thread it interrupts. From 5b430fab5e446fe243668644460356d7af4081aa Mon Sep 17 00:00:00 2001 From: Daniel Bevenius Date: Thu, 23 Jun 2016 20:00:01 +0200 Subject: [PATCH 4/6] Changing to use env.event_loop instead uv_default_loop --- src/node.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/node.cc b/src/node.cc index c78a738bf148bb..416f63cdde342f 100644 --- a/src/node.cc +++ b/src/node.cc @@ -4266,7 +4266,7 @@ static void StartNodeInstance(void* arg) { Environment env(&isolate_data, context); - uv_async_init(uv_default_loop(), + uv_async_init(env.event_loop(), env.dispatch_debug_messages_async(), DispatchDebugMessagesAsyncCallback); uv_unref(reinterpret_cast( From 11cc99f8081b66752c1432e4bc1ad26836361b27 Mon Sep 17 00:00:00 2001 From: Daniel Bevenius Date: Fri, 24 Jun 2016 10:27:26 +0200 Subject: [PATCH 5/6] Removing extra empty line in StartNodeInstance. --- src/node.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/src/node.cc b/src/node.cc index 416f63cdde342f..25b164b04fe815 100644 --- a/src/node.cc +++ b/src/node.cc @@ -4263,7 +4263,6 @@ static void StartNodeInstance(void* arg) { array_buffer_allocator.zero_fill_field()); Local context = Context::New(isolate); Context::Scope context_scope(context); - Environment env(&isolate_data, context); uv_async_init(env.event_loop(), From c8b9c3a4f4f359a7907bcf1c20282d7e7b0f993b Mon Sep 17 00:00:00 2001 From: Daniel Bevenius Date: Fri, 24 Jun 2016 10:30:41 +0200 Subject: [PATCH 6/6] Changing to use instance_data->use_debug_agent method. --- src/node.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/node.cc b/src/node.cc index 25b164b04fe815..43d38aa039e5b7 100644 --- a/src/node.cc +++ b/src/node.cc @@ -4271,7 +4271,7 @@ static void StartNodeInstance(void* arg) { uv_unref(reinterpret_cast( env.dispatch_debug_messages_async())); - if (!use_debug_agent) { + if (!instance_data->use_debug_agent()) { RegisterDebugSignalHandler(env.dispatch_debug_messages_async()); }