diagnostics_channel: fix race condition with diagnostics_channel and GC

PR-URL: #59910
Reviewed-By: Stephen Belanger <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Gerhard Stöbich <[email protected]>

Author: uurien

lib/diagnostics_channel.js

@@ -37,7 +37,9 @@ const { WeakReference } = require('internal/util');
 // Only GC can be used as a valid time to clean up the channels map.
 class WeakRefMap extends SafeMap {
   #finalizers = new SafeFinalizationRegistry((key) => {
-    this.delete(key);
+    // Check that the key doesn't have any value before deleting, as the WeakRef for the key
+    // may have been replaced since finalization callbacks aren't synchronous with GC.
+    if (!this.has(key)) this.delete(key);
   });
 
   set(key, value) {
@@ -49,6 +51,10 @@ class WeakRefMap extends SafeMap {
     return super.get(key)?.get();
   }
 
+  has(key) {
+    return !!this.get(key);
+  }
+
   incRef(key) {
     return super.get(key)?.incRef();
   }

test/fixtures/source-map/output/source_map_assert_source_line.snapshot

@@ -7,7 +7,7 @@ AssertionError [ERR_ASSERTION]: The expression evaluated to a falsy value:
     *
     *
     *
-    at TracingChannel.traceSync (node:diagnostics_channel:322:14)
+    at TracingChannel.traceSync (node:diagnostics_channel:328:14)
     *
     *
     *

test/parallel/test-diagnostics-channel-gc-maintains-subcriptions.js

@@ -0,0 +1,21 @@
+// Flags: --expose-gc
+'use strict';
+
+require('../common');
+const assert = require('assert');
+const { channel } = require('diagnostics_channel');
+
+function test() {
+  function subscribe() {
+    channel('test-gc').subscribe(function noop() {});
+  }
+
+  subscribe();
+
+  setTimeout(() => {
+    global.gc();
+    assert.ok(channel('test-gc').hasSubscribers, 'Channel must have subscribers');
+  });
+}
+
+test();

test/parallel/test-diagnostics-channel-gc-race-condition.js

@@ -0,0 +1,23 @@
+// Flags: --expose-gc
+'use strict';
+
+require('../common');
+const assert = require('assert');
+const { channel } = require('diagnostics_channel');
+
+function test() {
+  const testChannel = channel('test-gc');
+
+  setTimeout(() => {
+    const testChannel2 = channel('test-gc');
+
+    assert.ok(testChannel === testChannel2, 'Channel instances must be the same');
+  });
+}
+
+test();
+
+setTimeout(() => {
+  global.gc();
+  test();
+}, 10);