diff --git a/src/api/hooks.cc b/src/api/hooks.cc index cec58cee00847c..2dd0cc994ea7f2 100644 --- a/src/api/hooks.cc +++ b/src/api/hooks.cc @@ -30,6 +30,8 @@ void AtExit(Environment* env, void (*cb)(void* arg), void* arg) { } void EmitBeforeExit(Environment* env) { + env->RunBeforeExitCallbacks(); + HandleScope handle_scope(env->isolate()); Context::Scope context_scope(env->context()); Local exit_code = env->process_object() diff --git a/src/env.h b/src/env.h index a649675c7cc902..15832edcda1df5 100644 --- a/src/env.h +++ b/src/env.h @@ -874,7 +874,8 @@ class Environment : public MemoryRetainer { #if HAVE_INSPECTOR // If the environment is created for a worker, pass parent_handle and // the ownership if transferred into the Environment. - int InitializeInspector(inspector::ParentInspectorHandle* parent_handle); + int InitializeInspector( + std::unique_ptr parent_handle); #endif v8::MaybeLocal BootstrapInternalLoaders(); diff --git a/src/inspector_agent.cc b/src/inspector_agent.cc index 469e0b4f8f335a..f13e68c067529e 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/inspector_profiler.cc b/src/inspector_profiler.cc index b5f63b2b41389d..e0d02d6952a3f9 100644 --- a/src/inspector_profiler.cc +++ b/src/inspector_profiler.cc @@ -4,6 +4,7 @@ #include "diagnosticfilename-inl.h" #include "memory_tracker-inl.h" #include "node_file.h" +#include "node_errors.h" #include "node_internals.h" #include "util-inl.h" #include "v8-inspector.h" @@ -13,6 +14,7 @@ namespace node { namespace profiler { +using errors::TryCatchScope; using v8::Context; using v8::Function; using v8::FunctionCallbackInfo; @@ -219,12 +221,21 @@ void V8CoverageConnection::WriteProfile(Local message) { } // append source-map cache information to coverage object: - Local source_map_cache_getter = env_->source_map_cache_getter(); Local source_map_cache_v; - if (!source_map_cache_getter->Call(env()->context(), - Undefined(isolate), 0, nullptr) - .ToLocal(&source_map_cache_v)) { - return; + { + TryCatchScope try_catch(env()); + { + Isolate::AllowJavascriptExecutionScope allow_js_here(isolate); + Local source_map_cache_getter = env_->source_map_cache_getter(); + if (!source_map_cache_getter->Call( + context, Undefined(isolate), 0, nullptr) + .ToLocal(&source_map_cache_v)) { + return; + } + } + if (try_catch.HasCaught() && !try_catch.HasTerminated()) { + PrintCaughtException(isolate, context, try_catch); + } } // Avoid writing to disk if no source-map data: if (!source_map_cache_v->IsUndefined()) { @@ -351,7 +362,7 @@ void V8HeapProfilerConnection::End() { // For now, we only support coverage profiling, but we may add more // in the future. -void EndStartedProfilers(Environment* env) { +static void EndStartedProfilers(Environment* env) { Debug(env, DebugCategory::INSPECTOR_PROFILER, "EndStartedProfilers\n"); V8ProfilerConnection* connection = env->cpu_profiler_connection(); if (connection != nullptr && !connection->ending()) { @@ -390,6 +401,10 @@ std::string GetCwd(Environment* env) { } void StartProfilers(Environment* env) { + AtExit(env, [](void* env) { + EndStartedProfilers(static_cast(env)); + }, env); + Isolate* isolate = env->isolate(); Local coverage_str = env->env_vars()->Get( isolate, FIXED_ONE_BYTE_STRING(isolate, "NODE_V8_COVERAGE")) diff --git a/src/node.cc b/src/node.cc index f437ea4be96450..1246f20026ff3a 100644 --- a/src/node.cc +++ b/src/node.cc @@ -162,32 +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 - profiler::EndStartedProfilers(env); - - 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(); @@ -228,13 +202,12 @@ MaybeLocal ExecuteBootstrapper(Environment* env, #if HAVE_INSPECTOR int Environment::InitializeInspector( - inspector::ParentInspectorHandle* parent_handle) { + std::unique_ptr parent_handle) { std::string inspector_path; - if (parent_handle != nullptr) { + if (parent_handle) { DCHECK(!is_main_thread()); inspector_path = parent_handle->url(); - inspector_agent_->SetParentHandle( - std::unique_ptr(parent_handle)); + inspector_agent_->SetParentHandle(std::move(parent_handle)); } else { inspector_path = argv_.size() > 1 ? argv_[1].c_str() : ""; } @@ -559,6 +532,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_errors.cc b/src/node_errors.cc index ec14746514e420..e094fe4681fc56 100644 --- a/src/node_errors.cc +++ b/src/node_errors.cc @@ -993,9 +993,7 @@ void TriggerUncaughtException(Isolate* isolate, // Now we are certain that the exception is fatal. ReportFatalException(env, error, message, EnhanceFatalException::kEnhance); -#if HAVE_INSPECTOR - profiler::EndStartedProfilers(env); -#endif + RunAtExit(env); // If the global uncaught exception handler sets process.exitCode, // exit with that code. Otherwise, exit with 1. diff --git a/src/node_internals.h b/src/node_internals.h index 2ec230d8b54bfd..9a80d998d5d24e 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); @@ -307,10 +306,15 @@ void SetIsolateCreateParamsForNode(v8::Isolate::CreateParams* params); #if HAVE_INSPECTOR namespace profiler { void StartProfilers(Environment* env); -void EndStartedProfilers(Environment* env); } #endif // HAVE_INSPECTOR +#ifdef __POSIX__ +static constexpr unsigned kMaxSignal = 32; +#endif + +bool HasSignalJSHandler(int signum); + #ifdef _WIN32 typedef SYSTEMTIME TIME_TYPE; #else // UNIX, OSX diff --git a/src/node_main_instance.cc b/src/node_main_instance.cc index 1c1a09280b70d8..b7ca367ad89a55 100644 --- a/src/node_main_instance.cc +++ b/src/node_main_instance.cc @@ -7,6 +7,10 @@ #include #endif +#if HAVE_INSPECTOR +#include "inspector/worker_inspector.h" // ParentInspectorHandle +#endif + namespace node { using v8::Context; @@ -132,8 +136,6 @@ int NodeMainInstance::Run() { 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()); } @@ -148,13 +150,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_); @@ -208,7 +223,7 @@ std::unique_ptr NodeMainInstance::CreateMainEnvironment( // TODO(joyeecheung): when we snapshot the bootstrapped context, // the inspector and diagnostics setup should after after deserialization. #if HAVE_INSPECTOR - *exit_code = env->InitializeInspector(nullptr); + *exit_code = env->InitializeInspector({}); #endif if (*exit_code != 0) { return env; diff --git a/src/node_process_methods.cc b/src/node_process_methods.cc index 3a2c1efd812fca..f2fb5cf38cf211 100644 --- a/src/node_process_methods.cc +++ b/src/node_process_methods.cc @@ -172,11 +172,15 @@ static void Kill(const FunctionCallbackInfo& args) { if (!args[0]->Int32Value(context).To(&pid)) return; int sig; if (!args[1]->Int32Value(context).To(&sig)) return; - // TODO(joyeecheung): white list the signals? -#if HAVE_INSPECTOR - profiler::EndStartedProfilers(env); -#endif + uv_pid_t own_pid = uv_os_getpid(); + if (sig > 0 && + (pid == 0 || pid == -1 || pid == own_pid || pid == -own_pid) && + !HasSignalJSHandler(sig)) { + // This is most likely going to terminate this process. + // It's not an exact method but it might be close enough. + RunAtExit(env); + } int err = uv_kill(pid, sig); args.GetReturnValue().Set(err); @@ -428,7 +432,7 @@ static void DebugEnd(const FunctionCallbackInfo& args) { static void ReallyExit(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); - WaitForInspectorDisconnect(env); + RunAtExit(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 c79968bad90c68..cc6c2813a07881 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 @@ -343,8 +325,7 @@ void Worker::Run() { { env_->InitializeDiagnostics(); #if HAVE_INSPECTOR - env_->InitializeInspector(inspector_parent_handle_.release()); - inspector_started = true; + env_->InitializeInspector(std::move(inspector_parent_handle_)); #endif HandleScope handle_scope(isolate_); AsyncCallbackScope callback_scope(env_.get()); @@ -397,9 +378,6 @@ void Worker::Run() { if (exit_code_ == 0 && !stopped) exit_code_ = exit_code; -#if HAVE_INSPECTOR - profiler::EndStartedProfilers(env_.get()); -#endif Debug(this, "Exiting thread for worker %llu with exit code %d", thread_id_, exit_code_); } diff --git a/src/signal_wrap.cc b/src/signal_wrap.cc index 90b91f35a86c8b..cf67dc590f6d51 100644 --- a/src/signal_wrap.cc +++ b/src/signal_wrap.cc @@ -38,8 +38,13 @@ using v8::Object; using v8::String; using v8::Value; +void DecreaseSignalHandlerCount(int signum); + namespace { +static Mutex handled_signals_mutex; +static std::map handled_signals; // Signal -> number of handlers + class SignalWrap : public HandleWrap { public: static void Initialize(Local target, @@ -85,6 +90,11 @@ class SignalWrap : public HandleWrap { CHECK_EQ(r, 0); } + void Close(v8::Local close_callback) override { + if (active_) DecreaseSignalHandlerCount(handle_.signum); + HandleWrap::Close(close_callback); + } + static void Start(const FunctionCallbackInfo& args) { SignalWrap* wrap; ASSIGN_OR_RETURN_UNWRAP(&wrap, args.Holder()); @@ -112,21 +122,49 @@ class SignalWrap : public HandleWrap { wrap->MakeCallback(env->onsignal_string(), 1, &arg); }, signum); + + if (err == 0) { + CHECK(!wrap->active_); + wrap->active_ = true; + Mutex::ScopedLock lock(handled_signals_mutex); + handled_signals[signum]++; + } + args.GetReturnValue().Set(err); } static void Stop(const FunctionCallbackInfo& args) { SignalWrap* wrap; ASSIGN_OR_RETURN_UNWRAP(&wrap, args.Holder()); + + if (wrap->active_) { + wrap->active_ = false; + DecreaseSignalHandlerCount(wrap->handle_.signum); + } + int err = uv_signal_stop(&wrap->handle_); args.GetReturnValue().Set(err); } uv_signal_t handle_; + bool active_ = false; }; } // anonymous namespace + +void DecreaseSignalHandlerCount(int signum) { + Mutex::ScopedLock lock(handled_signals_mutex); + int new_handler_count = --handled_signals[signum]; + CHECK_GE(new_handler_count, 0); + if (new_handler_count == 0) + handled_signals.erase(signum); +} + +bool HasSignalJSHandler(int signum) { + Mutex::ScopedLock lock(handled_signals_mutex); + return handled_signals.find(signum) != handled_signals.end(); +} } // namespace node