diff --git a/lib/diagnostics_channel.js b/lib/diagnostics_channel.js index 312bd258f5844a..3deb301e7f3cd2 100644 --- a/lib/diagnostics_channel.js +++ b/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(); } diff --git a/test/fixtures/source-map/output/source_map_assert_source_line.snapshot b/test/fixtures/source-map/output/source_map_assert_source_line.snapshot index fe11794e9c032d..9def6eb4d7bedb 100644 --- a/test/fixtures/source-map/output/source_map_assert_source_line.snapshot +++ b/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) * * * diff --git a/test/parallel/test-diagnostics-channel-gc-maintains-subcriptions.js b/test/parallel/test-diagnostics-channel-gc-maintains-subcriptions.js new file mode 100644 index 00000000000000..3de30a63d4b33b --- /dev/null +++ b/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(); diff --git a/test/parallel/test-diagnostics-channel-gc-race-condition.js b/test/parallel/test-diagnostics-channel-gc-race-condition.js new file mode 100644 index 00000000000000..6a46325d4dc312 --- /dev/null +++ b/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);