From 7e2a182d03c40faa567daebee5ce064675190bff Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Sat, 2 Nov 2019 19:06:59 +0100 Subject: src: make WaitForInspectorDisconnect an exit hook Run inspector cleanup code on Environment teardown. This is part of a series of changes to make embedding easier, by requiring fewer internal methods to build a fully functioning Node.js instance. PR-URL: https://github.com/nodejs/node/pull/30229 Reviewed-By: Colin Ihrig Reviewed-By: Gireesh Punathil Reviewed-By: Joyee Cheung Reviewed-By: Shelley Vohr --- src/inspector_agent.cc | 7 +++++++ src/node.cc | 26 +------------------------- src/node_internals.h | 5 ++++- src/node_main_instance.cc | 15 ++++++++++++++- src/node_process_methods.cc | 1 - src/node_worker.cc | 19 ------------------- 6 files changed, 26 insertions(+), 47 deletions(-) (limited to 'src') diff --git a/src/inspector_agent.cc b/src/inspector_agent.cc index 469e0b4f8f..f13e68c067 100644 --- a/src/inspector_agent.cc +++ b/src/inspector_agent.cc @@ -778,6 +778,13 @@ bool Agent::Start(const std::string& path, StartDebugSignalHandler(); } + AtExit(parent_env_, [](void* env) { + Agent* agent = static_cast(env)->inspector_agent(); + if (agent->IsActive()) { + agent->WaitForDisconnect(); + } + }, parent_env_); + bool wait_for_connect = options.wait_for_connect(); if (parent_handle_) { wait_for_connect = parent_handle_->WaitForConnect(); diff --git a/src/node.cc b/src/node.cc index 18c4665b49..94c85cae74 100644 --- a/src/node.cc +++ b/src/node.cc @@ -162,31 +162,6 @@ bool v8_is_profiling = false; struct V8Platform v8_platform; } // namespace per_process -#ifdef __POSIX__ -static const unsigned kMaxSignal = 32; -#endif - -void WaitForInspectorDisconnect(Environment* env) { -#if HAVE_INSPECTOR - - if (env->inspector_agent()->IsActive()) { - // Restore signal dispositions, the app is done and is no longer - // capable of handling signals. -#if defined(__POSIX__) && !defined(NODE_SHARED_MODE) - struct sigaction act; - memset(&act, 0, sizeof(act)); - for (unsigned nr = 1; nr < kMaxSignal; nr += 1) { - if (nr == SIGKILL || nr == SIGSTOP || nr == SIGPROF) - continue; - act.sa_handler = (nr == SIGPIPE) ? SIG_IGN : SIG_DFL; - CHECK_EQ(0, sigaction(nr, &act, nullptr)); - } -#endif - env->inspector_agent()->WaitForDisconnect(); - } -#endif -} - #ifdef __POSIX__ void SignalExit(int signo, siginfo_t* info, void* ucontext) { ResetStdio(); @@ -553,6 +528,7 @@ inline void PlatformInit() { CHECK_EQ(err, 0); #endif // HAVE_INSPECTOR + // TODO(addaleax): NODE_SHARED_MODE does not really make sense here. #ifndef NODE_SHARED_MODE // Restore signal dispositions, the parent process may have changed them. struct sigaction act; diff --git a/src/node_internals.h b/src/node_internals.h index ecd217086f..10ef3bf5ed 100644 --- a/src/node_internals.h +++ b/src/node_internals.h @@ -90,7 +90,6 @@ void PrintCaughtException(v8::Isolate* isolate, v8::Local context, const v8::TryCatch& try_catch); -void WaitForInspectorDisconnect(Environment* env); void ResetStdio(); // Safe to call more than once and from signal handlers. #ifdef __POSIX__ void SignalExit(int signal, siginfo_t* info, void* ucontext); @@ -321,6 +320,10 @@ void StartProfilers(Environment* env); } #endif // HAVE_INSPECTOR +#ifdef __POSIX__ +static constexpr unsigned kMaxSignal = 32; +#endif + bool HasSignalJSHandler(int signum); #ifdef _WIN32 diff --git a/src/node_main_instance.cc b/src/node_main_instance.cc index b1e20c2b14..64b3ceabaf 100644 --- a/src/node_main_instance.cc +++ b/src/node_main_instance.cc @@ -152,13 +152,26 @@ int NodeMainInstance::Run() { 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(); ResetStdio(); env->RunCleanup(); + + // TODO(addaleax): Neither NODE_SHARED_MODE nor HAVE_INSPECTOR really + // make sense here. +#if HAVE_INSPECTOR && defined(__POSIX__) && !defined(NODE_SHARED_MODE) + struct sigaction act; + memset(&act, 0, sizeof(act)); + for (unsigned nr = 1; nr < kMaxSignal; nr += 1) { + if (nr == SIGKILL || nr == SIGSTOP || nr == SIGPROF) + continue; + act.sa_handler = (nr == SIGPIPE) ? SIG_IGN : SIG_DFL; + CHECK_EQ(0, sigaction(nr, &act, nullptr)); + } +#endif + RunAtExit(env.get()); per_process::v8_platform.DrainVMTasks(isolate_); diff --git a/src/node_process_methods.cc b/src/node_process_methods.cc index 3f0590aef7..f2fb5cf38c 100644 --- a/src/node_process_methods.cc +++ b/src/node_process_methods.cc @@ -433,7 +433,6 @@ static void DebugEnd(const FunctionCallbackInfo& args) { static void ReallyExit(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); RunAtExit(env); - WaitForInspectorDisconnect(env); int code = args[0]->Int32Value(env->context()).FromMaybe(0); env->Exit(code); } diff --git a/src/node_worker.cc b/src/node_worker.cc index e09bb4e7aa..8730e8b02b 100644 --- a/src/node_worker.cc +++ b/src/node_worker.cc @@ -42,17 +42,6 @@ using v8::Value; namespace node { namespace worker { -namespace { - -#if HAVE_INSPECTOR -void WaitForWorkerInspectorToStop(Environment* child) { - child->inspector_agent()->WaitForDisconnect(); - child->inspector_agent()->Stop(); -} -#endif - -} // anonymous namespace - Worker::Worker(Environment* env, Local wrap, const std::string& url, @@ -251,9 +240,6 @@ void Worker::Run() { Locker locker(isolate_); Isolate::Scope isolate_scope(isolate_); SealHandleScope outer_seal(isolate_); -#if HAVE_INSPECTOR - bool inspector_started = false; -#endif DeleteFnPtr env_; OnScopeLeave cleanup_env([&]() { @@ -283,10 +269,6 @@ void Worker::Run() { env_->stop_sub_worker_contexts(); env_->RunCleanup(); RunAtExit(env_.get()); -#if HAVE_INSPECTOR - if (inspector_started) - WaitForWorkerInspectorToStop(env_.get()); -#endif // This call needs to be made while the `Environment` is still alive // because we assume that it is available for async tracking in the @@ -344,7 +326,6 @@ void Worker::Run() { env_->InitializeDiagnostics(); #if HAVE_INSPECTOR env_->InitializeInspector(inspector_parent_handle_.release()); - inspector_started = true; #endif HandleScope handle_scope(isolate_); InternalCallbackScope callback_scope( -- cgit v1.2.3