diff --git a/bindings/profilers/wall.cc b/bindings/profilers/wall.cc index 0fb382f0..95600d3b 100644 --- a/bindings/profilers/wall.cc +++ b/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); + } } } diff --git a/bindings/profilers/wall.hh b/bindings/profilers/wall.hh index 74a03c2b..5fdf0d22 100644 --- a/bindings/profilers/wall.hh +++ b/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. diff --git a/suppressions/lsan_suppr.txt b/suppressions/lsan_suppr.txt index 2f08beb7..083464a2 100644 --- a/suppressions/lsan_suppr.txt +++ b/suppressions/lsan_suppr.txt @@ -1 +1,2 @@ -leak:CRYPTO_zalloc \ No newline at end of file +leak:CRYPTO_zalloc +leak:ossl_load_builtin_compressions \ No newline at end of file