From 401b26c7622c2b46647f905a6bc9113447b045bf Mon Sep 17 00:00:00 2001 From: Attila Szegedi Date: Thu, 19 Feb 2026 15:29:50 +0100 Subject: [PATCH] Don't track live pointers anymore --- bindings/profilers/wall.cc | 38 ++++++++++---------------------------- bindings/profilers/wall.hh | 8 +------- 2 files changed, 11 insertions(+), 35 deletions(-) diff --git a/bindings/profilers/wall.cc b/bindings/profilers/wall.cc index 98a8d2dd..d19518c7 100644 --- a/bindings/profilers/wall.cc +++ b/bindings/profilers/wall.cc @@ -107,22 +107,9 @@ void SetContextPtr(ContextPtr& contextPtr, class PersistentContextPtr : public node::ObjectWrap { ContextPtr context; - std::unordered_set* live; public: - PersistentContextPtr(std::unordered_set* live, - Local wrap) - : live(live) { - Wrap(wrap); - } - - void Detach() { live = nullptr; } - - ~PersistentContextPtr() { - if (live) { - live->erase(this); - } - } + PersistentContextPtr(Local wrap) { Wrap(wrap); } void Set(Isolate* isolate, const Local& value) { SetContextPtr(context, isolate, value); @@ -668,15 +655,6 @@ WallProfiler::WallProfiler(std::chrono::microseconds samplingPeriod, } } -WallProfiler::~WallProfiler() { - // Delete all live contexts - for (auto ptr : liveContextPtrs_) { - ptr->Detach(); // so it doesn't invalidate our iterator - delete ptr; - } - liveContextPtrs_.clear(); -} - void WallProfiler::Dispose(Isolate* isolate, bool removeFromMap) { if (cpuProfiler_ != nullptr) { cpuProfiler_->Dispose(); @@ -1181,8 +1159,7 @@ void WallProfiler::SetContext(Isolate* isolate, Local value) { // for easy access from JS when cpedKey is an ALS, it can do // als.getStore()?.[0]; wrap->Set(v8Ctx, 0, value).Check(); - auto contextPtr = new PersistentContextPtr(&liveContextPtrs_, wrap); - liveContextPtrs_.insert(contextPtr); + auto contextPtr = new PersistentContextPtr(wrap); contextPtr->Set(isolate, value); SignalGuard m(setInProgress_); @@ -1253,18 +1230,23 @@ ContextPtr WallProfiler::GetContextPtr(Isolate* isolate) { } Local WallProfiler::GetMetrics(Isolate* isolate) { - auto usedAsyncContextCount = liveContextPtrs_.size(); + // TODO: maybe rewrite this so that we use a counter field, and + // PersistenContextPtr destructor uses GetProfiler(Isolate*) to find the + // profiler and decrement the field. Obvious problem being that GetProfiler() + // can spuriously return nullptr if the map is updated from a different thread + // and thus can cause an imbalance in the counter that looks like a leak. auto context = isolate->GetCurrentContext(); auto metrics = Object::New(isolate); + auto zero = Number::New(isolate, 0); metrics ->Set(context, String::NewFromUtf8Literal(isolate, "usedAsyncContextCount"), - Number::New(isolate, usedAsyncContextCount)) + zero) .ToChecked(); metrics ->Set(context, String::NewFromUtf8Literal(isolate, "totalAsyncContextCount"), - Number::New(isolate, usedAsyncContextCount)) + zero) .ToChecked(); return metrics; } diff --git a/bindings/profilers/wall.hh b/bindings/profilers/wall.hh index 9b4ed58a..69790ab1 100644 --- a/bindings/profilers/wall.hh +++ b/bindings/profilers/wall.hh @@ -38,8 +38,6 @@ struct Result { using ContextPtr = std::shared_ptr>; -class PersistentContextPtr; - class WallProfiler : public Nan::ObjectWrap { public: enum class CollectionMode { kNoCollect, kPassThrough, kCollectContexts }; @@ -57,10 +55,6 @@ class WallProfiler : public Nan::ObjectWrap { int cpedKeyHash_ = 0; v8::Global wrapObjectTemplate_; - // We track live context pointers in a set to avoid memory leaks. They will - // be deleted when the profiler is disposed. - std::unordered_set liveContextPtrs_; - std::atomic gcCount = 0; std::atomic setInProgress_ = false; double gcAsyncId; @@ -98,7 +92,7 @@ class WallProfiler : public Nan::ObjectWrap { using ContextBuffer = std::vector; ContextBuffer contexts_; - ~WallProfiler(); + ~WallProfiler() = default; void Dispose(v8::Isolate* isolate, bool removeFromMap); // A new CPU profiler object will be created each time profiling is started