diff --git a/bindings/otel-thread-ctx.cc b/bindings/otel-thread-ctx.cc index 69d7aa43..8ef326c3 100644 --- a/bindings/otel-thread-ctx.cc +++ b/bindings/otel-thread-ctx.cc @@ -292,29 +292,34 @@ static_assert(offsetof(CtxWrap, record_) == 0, // wall profiler uses for its active-profiler pointer. // `otel_thread_ctx_nodejs_v1` above is thread-local for the same reason. thread_local CtxWrap* g_live_ctx_wraps = nullptr; -// Whether DrainLiveCtxWraps is registered for the current isolate. Cleared by -// the drain itself so an isolate torn down and re-created on the same thread -// re-registers, matching how `undefined_addr` gates ResetDiscoveryStruct. -thread_local bool g_drain_hook_registered = false; - -// Delete every CtxWrap V8 has not collected yet. This is the teardown -// deletion that node::ObjectWrap's per-instance cleanup hook used to provide; -// without it the records would simply leak at exit. Registered once per -// isolate from Wrap(), which runs inside a JS constructor call where a -// context is entered, so AddEnvironmentCleanupHook's own CHECK is satisfied, -// and never removed — it fires exactly once, at teardown, while the -// Environment is still alive. -void DrainLiveCtxWraps(void* /*arg*/) { + +// Delete every CtxWrap V8 has not collected yet. This is the teardown deletion +// that node::ObjectWrap's per-instance cleanup hook used to provide; without it +// the records would simply leak at exit. Registered once per isolate from +// Init(), which runs at module initialisation with a context entered, so +// AddEnvironmentCleanupHook's own CHECK is satisfied, and never removed — it +// fires exactly once, at teardown, while the Environment is still alive. +void DrainLiveCtxWraps(void* arg) { + auto* isolate = static_cast(arg); + v8::HandleScope scope(isolate); CtxWrap* p = g_live_ctx_wraps; while (p != nullptr) { CtxWrap* next = p->next_; p->pprev_ = nullptr; p->next_ = nullptr; + // Clear the holder's internal field (containing p as pointer value), so + // nothing can reach a dangling CtxWrap through it including the + // out-of-process reader, which walks this slot. Being on the live list + // means V8 has not collected the holder, so the handle is safe to read + // here; the WeakCallback path cannot do this and does not need to, + // since there the holder is the thing being collected. + if (!p->handle_.IsEmpty()) { + SetAlignedPointerInInternalField(p->handle_.Get(isolate), 0, nullptr); + } delete p; p = next; } g_live_ctx_wraps = nullptr; - g_drain_hook_registered = false; } CtxWrap::~CtxWrap() { @@ -334,10 +339,6 @@ void CtxWrap::WeakCallback(const v8::WeakCallbackInfo& data) { void CtxWrap::Wrap(Local holder) { Isolate* isolate = Isolate::GetCurrent(); - if (!g_drain_hook_registered) { - node::AddEnvironmentCleanupHook(isolate, DrainLiveCtxWraps, nullptr); - g_drain_hook_registered = true; - } SetAlignedPointerInInternalField(holder, 0, this); handle_.Reset(isolate, holder); handle_.SetWeak(this, &WeakCallback, v8::WeakCallbackType::kParameter); @@ -690,6 +691,7 @@ void CtxWrap::DebugBytes(const FunctionCallbackInfo& args) { void CtxWrap::Init(Local exports) { Isolate* isolate = Isolate::GetCurrent(); + node::AddEnvironmentCleanupHook(isolate, DrainLiveCtxWraps, isolate); Local context = isolate->GetCurrentContext(); Local tpl = FunctionTemplate::New(isolate, New); diff --git a/bindings/profilers/wall.cc b/bindings/profilers/wall.cc index 6f33179b..fe85c876 100644 --- a/bindings/profilers/wall.cc +++ b/bindings/profilers/wall.cc @@ -27,6 +27,7 @@ #include #include +#include "internal-field.hh" #include "map-get.hh" #include "per-isolate-data.hh" #include "translate-time-profile.hh" @@ -106,26 +107,6 @@ void SetContextPtr(ContextPtr& contextPtr, } } -inline void* GetAlignedPointerFromInternalField(Object* object, int index) { -#if NODE_MAJOR_VERSION >= 26 - return object->GetAlignedPointerFromInternalField( - index, kEmbedderDataTypeTagDefault); -#else - return object->GetAlignedPointerFromInternalField(index); -#endif -} - -inline void SetAlignedPointerInInternalField(Local object, - int index, - void* value) { -#if NODE_MAJOR_VERSION >= 26 - object->SetAlignedPointerInInternalField( - index, value, kEmbedderDataTypeTagDefault); -#else - object->SetAlignedPointerInInternalField(index, value); -#endif -} - // Deliberately not a node::ObjectWrap. That base registers a per-instance // environment cleanup hook in its constructor and calls // RemoveEnvironmentCleanupHook from its destructor, which CHECKs that the @@ -720,10 +701,14 @@ WallProfiler::~WallProfiler() { // internal-field pointer in the wrap object stays inert even if V8 later // GCs the wrap.) auto* p = liveContextPtrHead_; + auto isolate = Isolate::GetCurrent(); while (p != nullptr) { auto* next = p->next_; p->pprev_ = nullptr; p->next_ = nullptr; + if (isolate != nullptr && !p->handle_.IsEmpty()) { + SetAlignedPointerInInternalField(p->handle_.Get(isolate), 0, nullptr); + } delete p; p = next; }