Fix profiler shutdown when isolate is terminated

When an isolate is terminated abruptly (eg. by Worker.terminate()), the
profiler is not stopped and not removed from the isolate->profiler map.
This can lead to a crash.

This commit adds a cleanup hook with `node::AddEnvironmentCleanupHook`
that is called when the isolate is destroyed.
The cleanup hook stops the profiler and removes it from the profiler map.

Author: nsavoire

bindings/profilers/wall.cc

@@ -366,6 +366,27 @@ static int64_t GetV8ToEpochOffset() {
   return V8toEpochOffset;
 }
 
+void WallProfiler::CleanupHook(void* data) {
+  auto isolate = static_cast<Isolate*>(data);
+  auto prof = g_profilers.GetProfiler(isolate);
+  if (prof) {
+    prof->Cleanup(isolate);
+    delete prof;
+  }
+}
+
+// This is only called when isolate is terminated without `beforeExit`
+// notification.
+void WallProfiler::Cleanup(Isolate* isolate) {
+  if (started_) {
+    cpuProfiler_->Stop(profileId_);
+    if (interceptSignal()) {
+      SignalHandler::DecreaseUseCount();
+    }
+    Dispose(isolate);
+  }
+}
+
 ContextsByNode WallProfiler::GetContextsByNode(CpuProfile* profile,
                                                ContextBuffer& contexts,
                                                int64_t startCpuTime) {
@@ -562,6 +583,9 @@ void WallProfiler::Dispose(Isolate* isolate) {
       isolate->RemoveGCPrologueCallback(&GCPrologueCallback, this);
       isolate->RemoveGCEpilogueCallback(&GCEpilogueCallback, this);
     }
+
+    node::RemoveEnvironmentCleanupHook(
+        isolate, &WallProfiler::CleanupHook, isolate);
   }
 }
 
@@ -702,17 +726,19 @@ Result WallProfiler::StartImpl() {
                                                 : CollectionMode::kNoCollect);
   collectionMode_.store(collectionMode, std::memory_order_relaxed);
   started_ = true;
+  auto isolate = Isolate::GetCurrent();
+  node::AddEnvironmentCleanupHook(isolate, &WallProfiler::CleanupHook, isolate);
   return {};
 }
 
-std::string WallProfiler::StartInternal() {
+v8::ProfilerId WallProfiler::StartInternal() {
   // Reuse the same names for the profiles because strings used for profile
   // names are not released until v8::CpuProfiler object is destroyed.
   // https://github.com/nodejs/node/blob/b53c51995380b1f8d642297d848cab6010d2909c/deps/v8/src/profiler/profile-generator.h#L516
   char buf[128];
   snprintf(buf, sizeof(buf), "pprof-%" PRId64, (profileIdx_++) % 2);
   v8::Local<v8::String> title = Nan::New<String>(buf).ToLocalChecked();
-  cpuProfiler_->StartProfiling(
+  auto result = cpuProfiler_->Start(
       title,
       includeLines_ ? CpuProfilingMode::kCallerLineNumbers
                     : CpuProfilingMode::kLeafNodeLineNumbers,
@@ -752,7 +778,7 @@ std::string WallProfiler::StartInternal() {
     cpuProfiler_->CollectSample(v8::Isolate::GetCurrent());
   }
 
-  return buf;
+  return result.id;
 }
 
 NAN_METHOD(WallProfiler::Stop) {
@@ -837,12 +863,11 @@ Result WallProfiler::StopImpl(bool restart, v8::Local<v8::Value>& profile) {
     std::atomic_signal_fence(std::memory_order_acquire);
   }
 
-  if (withContexts_ || workaroundV8Bug_) {
+  if (interceptSignal()) {
     SignalHandler::DecreaseUseCount();
   }
 
-  auto v8_profile = cpuProfiler_->StopProfiling(
-      Nan::New<String>(oldProfileId).ToLocalChecked());
+  auto v8_profile = cpuProfiler_->Stop(oldProfileId);
 
   ContextBuffer contexts;
   if (withContexts_) {
@@ -1016,6 +1041,7 @@ NAN_METHOD(WallProfiler::V8ProfilerStuckEventLoopDetected) {
 }
 
 NAN_METHOD(WallProfiler::Dispose) {
+  // Profiler should already be stopped when this is called.
   auto profiler = Nan::ObjectWrap::Unwrap<WallProfiler>(info.This());
   delete profiler;
 }

bindings/profilers/wall.hh

@@ -62,7 +62,7 @@ class WallProfiler : public Nan::ObjectWrap {
 
   std::atomic<CollectionMode> collectionMode_;
   std::atomic<uint64_t> noCollectCallCount_;
-  std::string profileId_;
+  v8::ProfilerId profileId_;
   uint64_t profileIdx_ = 0;
   bool includeLines_ = false;
   bool withContexts_ = false;
@@ -104,6 +104,8 @@ class WallProfiler : public Nan::ObjectWrap {
                                    int64_t startCpuTime);
 
   bool waitForSignal(uint64_t targetCallCount = 0);
+  static void CleanupHook(void* data);
+  void Cleanup(v8::Isolate* isolate);
 
  public:
   /**
@@ -129,7 +131,7 @@ class WallProfiler : public Nan::ObjectWrap {
                    int64_t cpu_time,
                    double async_id);
   Result StartImpl();
-  std::string StartInternal();
+  v8::ProfilerId StartInternal();
   Result StopImpl(bool restart, v8::Local<v8::Value>& profile);
 
   CollectionMode collectionMode() {
@@ -143,6 +145,8 @@ class WallProfiler : public Nan::ObjectWrap {
 
   bool collectCpuTime() const { return collectCpuTime_; }
 
+  bool interceptSignal() const { return withContexts_ || workaroundV8Bug_; }
+
   int v8ProfilerStuckEventLoopDetected() const {
     return v8ProfilerStuckEventLoopDetected_;
   }