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: #30229 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Shelley Vohr <[email protected]>
nodejs · Nov 17, 2019 · cd233e3 · cd233e3
1 parent 8234d04
commit cd233e3
Show file tree

Hide file tree

Showing 6 changed files with 31 additions and 18 deletions.
diff --git 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<String> message) {
   }
 
   // append source-map cache information to coverage object:
-  Local<Function> source_map_cache_getter = env_->source_map_cache_getter();
   Local<Value> 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<Function> 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<Environment*>(env));
+  }, env);
+
   Isolate* isolate = env->isolate();
   Local<String> 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
@@ -168,7 +168,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
@@ -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
@@ -318,7 +318,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
@@ -172,11 +172,15 @@ static void Kill(const FunctionCallbackInfo<Value>& 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,6 +432,7 @@ static void DebugEnd(const FunctionCallbackInfo<Value>& args) {
 
 static void ReallyExit(const FunctionCallbackInfo<Value>& 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
@@ -400,9 +400,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_);
     }