diagnostics_channel: fix diagnostics channel memory leak

PR-URL: #45633 Reviewed-By: Stephen Belanger <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Minwoo Jung <[email protected]> Reviewed-By: Darshan Sen <[email protected]>
nodejs · Jan 4, 2023 · 6b5f767 · 6b5f767
1 parent 7755b1f
commit 6b5f767
Show file tree

Hide file tree

Showing 4 changed files with 37 additions and 0 deletions.
diff --git a/lib/diagnostics_channel.js b/lib/diagnostics_channel.js
@@ -122,6 +122,9 @@ function unsubscribe(name, subscription) {
   }
 
   channels[name].decRef();
+  if (channels[name].getRef() === 0) {
+    delete channels[name];
+  }
   return true;
 }
 

diff --git a/src/node_util.cc b/src/node_util.cc
@@ -294,6 +294,13 @@ void WeakReference::Get(const FunctionCallbackInfo<Value>& args) {
     args.GetReturnValue().Set(weak_ref->target_.Get(isolate));
 }
 
+void WeakReference::GetRef(const FunctionCallbackInfo<Value>& args) {
+  WeakReference* weak_ref = Unwrap<WeakReference>(args.Holder());
+  Isolate* isolate = args.GetIsolate();
+  args.GetReturnValue().Set(
+      v8::Number::New(isolate, weak_ref->reference_count_));
+}
+
 void WeakReference::IncRef(const FunctionCallbackInfo<Value>& args) {
   WeakReference* weak_ref = Unwrap<WeakReference>(args.Holder());
   weak_ref->reference_count_++;
@@ -391,6 +398,7 @@ void RegisterExternalReferences(ExternalReferenceRegistry* registry) {
   registry->Register(ArrayBufferViewHasBuffer);
   registry->Register(WeakReference::New);
   registry->Register(WeakReference::Get);
+  registry->Register(WeakReference::GetRef);
   registry->Register(WeakReference::IncRef);
   registry->Register(WeakReference::DecRef);
   registry->Register(GuessHandleType);
@@ -464,6 +472,7 @@ void Initialize(Local<Object> target,
       WeakReference::kInternalFieldCount);
   weak_ref->Inherit(BaseObject::GetConstructorTemplate(env));
   SetProtoMethod(isolate, weak_ref, "get", WeakReference::Get);
+  SetProtoMethod(isolate, weak_ref, "getRef", WeakReference::GetRef);
   SetProtoMethod(isolate, weak_ref, "incRef", WeakReference::IncRef);
   SetProtoMethod(isolate, weak_ref, "decRef", WeakReference::DecRef);
   SetConstructorFunction(context, target, "WeakReference", weak_ref);

diff --git a/src/node_util.h b/src/node_util.h
@@ -23,6 +23,7 @@ class WeakReference : public SnapshotableObject {
                 v8::Local<v8::Object> target);
   static void New(const v8::FunctionCallbackInfo<v8::Value>& args);
   static void Get(const v8::FunctionCallbackInfo<v8::Value>& args);
+  static void GetRef(const v8::FunctionCallbackInfo<v8::Value>& args);
   static void IncRef(const v8::FunctionCallbackInfo<v8::Value>& args);
   static void DecRef(const v8::FunctionCallbackInfo<v8::Value>& args);
 

diff --git a/test/parallel/test-diagnostics-channel-memory-leak.js b/test/parallel/test-diagnostics-channel-memory-leak.js
@@ -0,0 +1,24 @@
+// Flags: --expose-gc
+'use strict';
+
+// This test ensures that diagnostic channel references aren't leaked.
+
+require('../common');
+const { ok } = require('assert');
+
+const { subscribe, unsubscribe } = require('diagnostics_channel');
+
+function noop() {}
+
+const heapUsedBefore = process.memoryUsage().heapUsed;
+
+for (let i = 0; i < 1000; i++) {
+  subscribe(String(i), noop);
+  unsubscribe(String(i), noop);
+}
+
+global.gc();
+
+const heapUsedAfter = process.memoryUsage().heapUsed;
+
+ok(heapUsedBefore >= heapUsedAfter);