diff --git a/bindings/profilers/wall.cc b/bindings/profilers/wall.cc index b6e836c9..d7d0d106 100644 --- a/bindings/profilers/wall.cc +++ b/bindings/profilers/wall.cc @@ -108,22 +108,29 @@ 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_ is the address of the pointer that currently + // references this PCP — &profiler_->liveContextPtrHead_ if we're the head, + // &predecessor->next_ otherwise — so by construction *pprev_ == this. + // Storing the address-of (rather than the predecessor itself) lets ~PCP + // unlink without a head/non-head branch: *pprev_ = next_ followed by + // next_->pprev_ = pprev_ does both ends in one shape. 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 +143,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 +650,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 +1249,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 +1314,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..2196c521 100644 --- a/bindings/profilers/wall.hh +++ b/bindings/profilers/wall.hh @@ -57,9 +57,23 @@ 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. + // + // Not atomic: all accesses happen on the isolate's thread (construction via + // SetContext from JS, destruction via V8 weak callbacks on the same thread, + // metric reads from JS). Signal handler does not touch this field. + size_t liveContextPtrCount_ = 0; std::atomic gcCount = 0; std::atomic setInProgress_ = false; @@ -175,6 +189,16 @@ 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_; } + void recordContextRelease() { --liveContextPtrCount_; } + size_t liveContextPtrCount() const { return liveContextPtrCount_; } + int v8ProfilerStuckEventLoopDetected() const { return v8ProfilerStuckEventLoopDetected_; }