diagnostics_channel: fix race condition with diagnostics_channel and GC

diagnostics_channel: fix race condition with diagnostics_channel and GC · nodejs/node@b0c1e67

File tree

lines changed

lines changed

Original file line number	Diff line number	Diff line change
`@@ -37,7 +37,9 @@ const { WeakReference } = require('internal/util');`
`37`	`37`	`// Only GC can be used as a valid time to clean up the channels map.`
`38`	`38`	`class WeakRefMap extends SafeMap {`
`39`	`39`	`#finalizers = new SafeFinalizationRegistry((key) => {`
`40`		`-this.delete(key);`
	`40`	`+// Check that the key doesn't have any value before deleting, as the WeakRef for the key`
	`41`	`+// may have been replaced since finalization callbacks aren't synchronous with GC.`
	`42`	`+if (!this.has(key)) this.delete(key);`
`41`	`43`	`});`
`42`	`44`
`43`	`45`	`set(key, value) {`
`@@ -49,6 +51,10 @@ class WeakRefMap extends SafeMap {`
`49`	`51`	`return super.get(key)?.get();`
`50`	`52`	`}`
`51`	`53`
	`54`	`+has(key) {`
	`55`	`+return !!this.get(key);`
	`56`	`+}`
	`57`	`+`
`52`	`58`	`incRef(key) {`
`53`	`59`	`return super.get(key)?.incRef();`
`54`	`60`	`}`

Original file line number	Diff line number	Diff line change
`@@ -7,7 +7,7 @@ AssertionError [ERR_ASSERTION]: The expression evaluated to a falsy value:`
`7`	`7`	`*`
`8`	`8`	`*`
`9`	`9`	`*`
`10`		`- at TracingChannel.traceSync (node:diagnostics_channel:322:14)`
	`10`	`+ at TracingChannel.traceSync (node:diagnostics_channel:328:14)`
`11`	`11`	`*`
`12`	`12`	`*`
`13`	`13`	`*`

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,21 @@`
	`1`	`+// Flags: --expose-gc`
	`2`	`+'use strict';`
	`3`	`+`
	`4`	`+require('../common');`
	`5`	`+const assert = require('assert');`
	`6`	`+const { channel } = require('diagnostics_channel');`
	`7`	`+`
	`8`	`+function test() {`
	`9`	`+function subscribe() {`
	`10`	`+channel('test-gc').subscribe(function noop() {});`
	`11`	`+}`
	`12`	`+`
	`13`	`+subscribe();`
	`14`	`+`
	`15`	`+setTimeout(() => {`
	`16`	`+global.gc();`
	`17`	`+assert.ok(channel('test-gc').hasSubscribers, 'Channel must have subscribers');`
	`18`	`+});`
	`19`	`+}`
	`20`	`+`
	`21`	`+test();`

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,23 @@`
	`1`	`+// Flags: --expose-gc`
	`2`	`+'use strict';`
	`3`	`+`
	`4`	`+require('../common');`
	`5`	`+const assert = require('assert');`
	`6`	`+const { channel } = require('diagnostics_channel');`
	`7`	`+`
	`8`	`+function test() {`
	`9`	`+const testChannel = channel('test-gc');`
	`10`	`+`
	`11`	`+setTimeout(() => {`
	`12`	`+const testChannel2 = channel('test-gc');`
	`13`	`+`
	`14`	`+assert.ok(testChannel === testChannel2, 'Channel instances must be the same');`
	`15`	`+});`
	`16`	`+}`
	`17`	`+`
	`18`	`+test();`
	`19`	`+`
	`20`	`+setTimeout(() => {`
	`21`	`+global.gc();`
	`22`	`+test();`
	`23`	`+}, 10);`