From 867c9bc98e5b94ab719e33d1905552c5fb3a3391 Mon Sep 17 00:00:00 2001 From: Attila Szegedi Date: Wed, 27 May 2026 14:24:57 +0200 Subject: [PATCH] track live context pointers via an intrusive list MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PersistentContextPtr instances were registered in an unordered_set on the WallProfiler so that ~WallProfiler could delete any PCPs that V8 had not GC'd yet — otherwise they show up as leaks under LSAN at process exit, since LSAN's check runs before V8's late-shutdown finalizers. The hash-set was heavier than that single use case deserved: every live PCP cost a back-pointer into the set (8 bytes) plus a separate hash node + bucket overhead (~40 bytes) external to the object. Replace it with an intrusive doubly-linked list threaded through the PCPs themselves. Each PCP carries: WallProfiler* const profiler_ // 8 bytes PersistentContextPtr** pprev_ // 8 bytes PersistentContextPtr* next_ // 8 bytes ~PCP unlinks itself when pprev_ != nullptr — the "still on a live profiler's list" predicate. ~WallProfiler walks the list and nulls each pprev_ before delete, so the destructor that fires as part of the walk becomes a no-op rather than poking at the dying profiler's head pointer. Outside of teardown, pprev_ != nullptr is also the proof that the back-pointer is still valid, so ~PCP can dereference profiler_ to decrement its counter without any thread-local or ID lookup. PCP-tracking memory drops from ~48 bytes/PCP (8-byte back-pointer into set + ~40 bytes of hash node / bucket) to 24 bytes/PCP (back- pointer + two list pointers), a saving of ~24 bytes per live async context. Replace GetMetrics's liveContextPtrs_.size() read with an atomic counter incremented/decremented alongside the list link/unlink so the metric path doesn't depend on the container. usedAsyncContextCount / totalAsyncContextCount keep their existing semantics (both report the live count, as before). --- bindings/profilers/wall.cc | 81 +++++++++++++++++++++++++++----------- bindings/profilers/wall.hh | 32 +++++++++++++-- 2 files changed, 88 insertions(+), 25 deletions(-) diff --git a/bindings/profilers/wall.cc b/bindings/profilers/wall.cc index b6e836c9..8d4202c6 100644 --- a/bindings/profilers/wall.cc +++ b/bindings/profilers/wall.cc @@ -108,22 +108,25 @@ void SetContextPtr(ContextPtr& contextPtr, class PersistentContextPtr : public node::ObjectWrap { ContextPtr context; - std::unordered_set* live; + // Back-pointer to the WallProfiler that created this PCP. Guaranteed to be + // a valid pointer whenever pprev_ != nullptr — ~WallProfiler nulls pprev_ + // on every live PCP before any of them can outlive the profiler. + WallProfiler* const profiler_; + // Intrusive doubly-linked list, threaded through the WallProfiler's + // liveContextPtrHead_. pprev_ points at the slot that currently holds *this + // (either the head field or another PCP's next_), enabling O(1) unlink in + // ~PCP. pprev_ == nullptr is the "detached" sentinel — set by + // ~WallProfiler before deleting us, so our unlink becomes a no-op and we + // don't poke at the dying profiler's memory. + PersistentContextPtr** pprev_ = nullptr; + PersistentContextPtr* next_ = nullptr; + + friend class WallProfiler; public: - PersistentContextPtr(std::unordered_set* live, - Local wrap) - : live(live) { - Wrap(wrap); - } + PersistentContextPtr(WallProfiler* profiler, Local wrap); - void Detach() { live = nullptr; } - - ~PersistentContextPtr() { - if (live) { - live->erase(this); - } - } + ~PersistentContextPtr(); void Set(Isolate* isolate, const Local& value) { SetContextPtr(context, isolate, value); @@ -136,6 +139,32 @@ class PersistentContextPtr : public node::ObjectWrap { } }; +PersistentContextPtr::PersistentContextPtr(WallProfiler* profiler, + Local wrap) + : profiler_(profiler) { + Wrap(wrap); + // Splice ourselves at the head of profiler's live list. + auto** headSlot = profiler->liveContextPtrHeadSlot(); + next_ = *headSlot; + pprev_ = headSlot; + if (next_ != nullptr) next_->pprev_ = &next_; + *headSlot = this; + profiler->recordContextCreate(); +} + +PersistentContextPtr::~PersistentContextPtr() { + // pprev_ != nullptr means we're still on profiler_'s live list, which means + // ~WallProfiler hasn't run yet and the back-pointer is still valid. Unlink + // and release the slot in one step. If pprev_ is null, ~WallProfiler is + // taking care of us so don't do anything (don't unlink us, and leave its + // counter alone as it's about to go away). + if (pprev_ != nullptr) { + *pprev_ = next_; + if (next_ != nullptr) next_->pprev_ = pprev_; + profiler_->recordContextRelease(); + } +} + inline void* GetAlignedPointerFromInternalField(Object* object, int index) { #if NODE_MAJOR_VERSION >= 26 return object->GetAlignedPointerFromInternalField( @@ -617,12 +646,21 @@ WallProfiler::~WallProfiler() { } g_profilers.Remove(this); - // Delete all live contexts - for (auto ptr : liveContextPtrs_) { - ptr->Detach(); // so it doesn't invalidate our iterator - delete ptr; - } - liveContextPtrs_.clear(); + // Delete every PCP still live in the CPED map. ~PCP would normally unlink + // itself via pprev_/next_, but we're tearing down the list we point into — + // so null pprev_ first to signal "already detached" and let ~PCP skip the + // unlink. (~ObjectWrap will still clear V8's weak callback during delete, + // so the dangling internal-field pointer in the wrap object stays inert + // even if V8 later GCs the wrap.) + auto* p = liveContextPtrHead_; + while (p != nullptr) { + auto* next = p->next_; + p->pprev_ = nullptr; + p->next_ = nullptr; + delete p; + p = next; + } + liveContextPtrHead_ = nullptr; } void WallProfiler::Dispose(Isolate* isolate) { @@ -1207,8 +1245,7 @@ Local WallProfiler::CreateContextHolder(Isolate* isolate, // 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(this, wrap); contextPtr->Set(isolate, value); return wrap; } @@ -1273,7 +1310,7 @@ ContextPtr WallProfiler::GetContextPtr(Isolate* isolate) { } Local WallProfiler::GetMetrics(Isolate* isolate) { - auto usedAsyncContextCount = liveContextPtrs_.size(); + auto usedAsyncContextCount = liveContextPtrCount(); auto context = isolate->GetCurrentContext(); auto metrics = Object::New(isolate); metrics diff --git a/bindings/profilers/wall.hh b/bindings/profilers/wall.hh index f40ccdbd..d55d0b1d 100644 --- a/bindings/profilers/wall.hh +++ b/bindings/profilers/wall.hh @@ -57,9 +57,19 @@ 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_; + // The list lets ~WallProfiler delete any PCPs V8 hasn't GC'd yet, so they + // don't show up as leaks under LSAN at exit. + // PCPs link themselves in at construction and unlink in their destructor. + // This used to be an unordered set, but storing two pointers in the PCP is + // much cheaper memorywise than the hash set's overhead of buckets and nodes. + PersistentContextPtr* liveContextPtrHead_ = nullptr; + + // Number of PersistentContextPtr instances created by this profiler that + // are still alive. Incremented in PersistentContextPtr's constructor; + // decremented in its destructor when it unlinks from the list (i.e., while + // the profiler is still alive). PCPs cleaned up by ~WallProfiler's walk + // skip the decrement — the counter is going away with the profiler anyway. + std::atomic liveContextPtrCount_{0}; std::atomic gcCount = 0; std::atomic setInProgress_ = false; @@ -175,6 +185,22 @@ class WallProfiler : public Nan::ObjectWrap { bool interceptSignal() const { return withContexts_ || workaroundV8Bug_; } + // Returns the address of the live-PCP list head so a newly constructed PCP + // can splice itself in. Only used by PersistentContextPtr's constructor. + PersistentContextPtr** liveContextPtrHeadSlot() { + return &liveContextPtrHead_; + } + + void recordContextCreate() { + liveContextPtrCount_.fetch_add(1, std::memory_order_relaxed); + } + void recordContextRelease() { + liveContextPtrCount_.fetch_sub(1, std::memory_order_relaxed); + } + size_t liveContextPtrCount() const { + return liveContextPtrCount_.load(std::memory_order_relaxed); + } + int v8ProfilerStuckEventLoopDetected() const { return v8ProfilerStuckEventLoopDetected_; }