From d14d9e8b1b8e6d114b89b2eb112c582c968105f8 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Sat, 2 Nov 2019 19:06:47 +0100 Subject: [PATCH] src: make EndStartedProfilers an exit hook Run `EndStartedProfilers` 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_profiler.cc | 27 +++++++++++++++++++++------ src/node.cc | 1 - src/node_errors.cc | 4 +--- src/node_internals.h | 1 - src/node_process_methods.cc | 13 +++++++++---- src/node_worker.cc | 3 --- 6 files changed, 31 insertions(+), 18 deletions(-) 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 ae53d0c31c3568..51df2ee58bf058 100644 --- a/src/node.cc +++ b/src/node.cc @@ -157,7 +157,6 @@ static const unsigned kMaxSignal = 32; 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 diff --git a/src/node_errors.cc b/src/node_errors.cc index d3a409b1ab455a..3dad079bb97f52 100644 --- a/src/node_errors.cc +++ b/src/node_errors.cc @@ -980,9 +980,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 050e9a5c46ff90..3c1a004942aecc 100644 --- a/src/node_internals.h +++ b/src/node_internals.h @@ -310,7 +310,6 @@ void SetIsolateCreateParamsForNode(v8::Isolate::CreateParams* params); #if HAVE_INSPECTOR namespace profiler { void StartProfilers(Environment* env); -void EndStartedProfilers(Environment* env); } #endif // HAVE_INSPECTOR diff --git a/src/node_process_methods.cc b/src/node_process_methods.cc index 8237826a824fd3..0d44ecc5ec6365 100644 --- a/src/node_process_methods.cc +++ b/src/node_process_methods.cc @@ -165,11 +165,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); @@ -421,6 +425,7 @@ 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 af79540631f153..d9e5891fddff20 100644 --- a/src/node_worker.cc +++ b/src/node_worker.cc @@ -323,9 +323,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_); }