V5.15.1 proposal

bindings/profilers/wall.cc

@@ -467,7 +467,10 @@ void WallProfiler::Cleanup(Isolate* isolate) {
     if (interceptSignal()) {
       SignalHandler::DecreaseUseCount();
     }
-    Dispose(isolate);
+    // We're running inside the environment cleanup hook itself; Node removes
+    // the hook (and frees its internal record) after we return, so we must not
+    // remove it here — doing so would be a use-after-free. See Dispose().
+    Dispose(isolate, /*removeCleanupHook=*/false);
   }
 }
 
@@ -689,7 +692,7 @@ WallProfiler::~WallProfiler() {
   liveContextPtrHead_ = nullptr;
 }
 
-void WallProfiler::Dispose(Isolate* isolate) {
+void WallProfiler::Dispose(Isolate* isolate, bool removeCleanupHook) {
   if (cpuProfiler_ != nullptr) {
     cpuProfiler_->Dispose();
     cpuProfiler_ = nullptr;
@@ -704,8 +707,10 @@ void WallProfiler::Dispose(Isolate* isolate) {
       isolate->RemoveGCEpilogueCallback(&GCEpilogueCallback, this);
     }
 
-    node::RemoveEnvironmentCleanupHook(
-        isolate, &WallProfiler::CleanupHook, isolate);
+    if (removeCleanupHook) {
+      node::RemoveEnvironmentCleanupHook(
+          isolate, &WallProfiler::CleanupHook, isolate);
+    }
   }
 }
 

bindings/profilers/wall.hh

@@ -113,7 +113,14 @@ class WallProfiler : public Nan::ObjectWrap {
   ContextBuffer contexts_;
 
   ~WallProfiler();
-  void Dispose(v8::Isolate* isolate);
+  // removeCleanupHook must be false when Dispose is called from within the
+  // environment cleanup hook (CleanupHook). Node removes the hook itself after
+  // invoking it and dereferences its internal hook record again afterwards, so
+  // calling node::RemoveEnvironmentCleanupHook from inside the hook frees that
+  // record early and leaves Node reading freed memory (use-after-free crash on
+  // worker termination). In all other (still-running) call sites the hook is
+  // live and must be removed so it doesn't fire later against a dead profiler.
+  void Dispose(v8::Isolate* isolate, bool removeCleanupHook = true);
 
   // A new CPU profiler object will be created each time profiling is started
   // to work around https://bugs.chromium.org/p/v8/issues/detail?id=11051.