src: various improvements toward a complete embedder API

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<Value> exit_code = env->process_object()

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<inspector::ParentInspectorHandle> parent_handle);
 #endif
 
   v8::MaybeLocal<v8::Value> BootstrapInternalLoaders();

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<Environment*>(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();

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"))

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<Value> ExecuteBootstrapper(Environment* env,
 
 #if HAVE_INSPECTOR
 int Environment::InitializeInspector(
-    inspector::ParentInspectorHandle* parent_handle) {
+    std::unique_ptr<inspector::ParentInspectorHandle> 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<inspector::ParentInspectorHandle>(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;

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.

src/node_internals.h

@@ -90,7 +90,6 @@ void PrintCaughtException(v8::Isolate* isolate,
                           v8::Local<v8::Context> 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

src/node_main_instance.cc

@@ -7,6 +7,10 @@
 #include <sanitizer/lsan_interface.h>
 #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<Environment> 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;

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,7 +432,7 @@ static void DebugEnd(const FunctionCallbackInfo<Value>& args) {
 
 static void ReallyExit(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
-  WaitForInspectorDisconnect(env);
+  RunAtExit(env);
   int code = args[0]->Int32Value(env->context()).FromMaybe(0);
   env->Exit(code);
 }

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<Object> 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<Environment, FreeEnvironment> 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_);
     }