diagnostics_channel: fix ref counting bug when reaching zero subscribers

Author: Stephen Belanger

lib/diagnostics_channel.js

@@ -5,6 +5,7 @@ const {
   ArrayPrototypeIndexOf,
   ArrayPrototypePush,
   ArrayPrototypeSplice,
+  SafeFinalizationRegistry,
   ObjectGetPrototypeOf,
   ObjectSetPrototypeOf,
   Promise,
@@ -29,13 +30,17 @@ const { triggerUncaughtException } = internalBinding('errors');
 
 const { WeakReference } = internalBinding('util');
 
-function decRef(channel) {
-  if (channels.get(channel.name).decRef() === 0) {
-    channels.delete(channel.name);
-  }
+// Can't delete when weakref count reaches 0 as it could increment again.
+// Only GC can be used as a valid time to clean up the channels map.
+const finalizers = new SafeFinalizationRegistry((name) => {
+  channels.delete(name);
+});
+
+function decRef (channel) {
+  channels.get(channel.name).decRef();
 }
 
-function incRef(channel) {
+function incRef (channel) {
   channels.get(channel.name).incRef();
 }
 
@@ -155,6 +160,7 @@ class Channel {
     this.name = name;
 
     channels.set(name, new WeakReference(this));
+    finalizers.register(this, name);
   }
 
   static [SymbolHasInstance](instance) {

test/parallel/test-diagnostics-channel-pub-sub.js

@@ -42,3 +42,10 @@ assert.ok(!dc.unsubscribe(name, subscriber));
 assert.throws(() => {
   dc.subscribe(name, null);
 }, { code: 'ERR_INVALID_ARG_TYPE' });
+
+// Reaching zero subscribers should not delete from the channels map as there
+// will be no more weakref to incRef if another subscribe happens while the
+// channel object itself exists.
+channel.subscribe(subscriber);
+channel.unsubscribe(subscriber);
+channel.subscribe(subscriber);