Skip to content

Commit c749402

Browse files
committed
chore: address pr's comments
1 parent c1a1c2c commit c749402

File tree

2 files changed

+20
-28
lines changed

2 files changed

+20
-28
lines changed

lib/internal/abort_controller.js

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -84,13 +84,13 @@ function lazyMessageChannel() {
8484
}
8585

8686
const clearTimeoutRegistry = new SafeFinalizationRegistry(clearTimeout);
87-
const finalizers = new SafeFinalizationRegistry((signalWeakRef) => {
87+
const dependantSignalsCleanupRegistry = new SafeFinalizationRegistry((signalWeakRef) => {
8888
const signal = signalWeakRef.deref();
89-
if (!signal) {
89+
if (signal === undefined) {
9090
return;
9191
}
9292
signal[kDependantSignals].forEach((ref) => {
93-
if (!ref.deref()) {
93+
if (ref.deref() === undefined) {
9494
signal[kDependantSignals].delete(ref);
9595
}
9696
});
@@ -255,9 +255,9 @@ class AbortSignal extends EventTarget {
255255
signal[kDependantSignals] ??= new SafeSet();
256256
if (!signal[kComposite]) {
257257
const signalWeakRef = new SafeWeakRef(signal);
258-
finalizers.register(resultSignal, signalWeakRef);
259258
resultSignal[kSourceSignals].add(signalWeakRef);
260259
signal[kDependantSignals].add(resultSignalWeakRef);
260+
dependantSignalsCleanupRegistry.register(resultSignal, signalWeakRef);
261261
} else if (!signal[kSourceSignals]) {
262262
continue;
263263
} else {
@@ -274,7 +274,7 @@ class AbortSignal extends EventTarget {
274274
}
275275
resultSignal[kSourceSignals].add(sourceSignalWeakRef);
276276
sourceSignal[kDependantSignals].add(resultSignalWeakRef);
277-
finalizers.register(resultSignal, sourceSignalWeakRef);
277+
dependantSignalsCleanupRegistry.register(resultSignal, sourceSignalWeakRef);
278278
}
279279
}
280280
}

test/parallel/test-abortsignal-drop-settled-signals.mjs

Lines changed: 15 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,9 @@
11
// Flags: --expose_gc
2-
3-
import * as common from '../common/index.mjs';
4-
5-
if (common.isASan) {
6-
common.skip('ASan messes with memory measurements');
7-
}
8-
2+
//
3+
import '../common/index.mjs';
94
import { describe, it } from 'node:test';
105

11-
const makeSubsequentCalls = (limit, done, holdReferences = false) => {
6+
function makeSubsequentCalls(limit, done, holdReferences = false) {
127
let dependantSymbol;
138
let signalRef;
149
const ac = new AbortController();
@@ -34,12 +29,9 @@ const makeSubsequentCalls = (limit, done, holdReferences = false) => {
3429
signalRef.deref().addEventListener('abort', handler);
3530
}
3631

37-
if (!dependantSymbol) {
38-
const kDependantSignals = Object.getOwnPropertySymbols(ac.signal).find(
39-
(s) => s.toString() === 'Symbol(kDependantSignals)'
40-
);
41-
dependantSymbol = kDependantSignals;
42-
}
32+
dependantSymbol ??= Object.getOwnPropertySymbols(ac.signal).find(
33+
(s) => s.toString() === 'Symbol(kDependantSignals)'
34+
);
4335

4436
setImmediate(() => {
4537
// Removing the event listener at some moment in the future
@@ -52,7 +44,7 @@ const makeSubsequentCalls = (limit, done, holdReferences = false) => {
5244
run(1);
5345
};
5446

55-
const runShortLivedSourceSignal = (limit, done) => {
47+
function runShortLivedSourceSignal(limit, done) {
5648
const signalRefs = new Set();
5749

5850
function run(iteration) {
@@ -78,15 +70,15 @@ describe('when there is a long-lived signal', () => {
7870
it('drops settled dependant signals', (t, done) => {
7971
makeSubsequentCalls(limit, (signal, depandantSignalsKey) => {
8072
setImmediate(() => {
81-
t.assert.equal(signal[depandantSignalsKey].size, 0);
73+
t.assert.strictEqual(signal[depandantSignalsKey].size, 0);
8274
done();
8375
});
8476
});
8577
});
8678

8779
it('keeps all active dependant signals', (t, done) => {
8880
makeSubsequentCalls(limit, (signal, depandantSignalsKey) => {
89-
t.assert.equal(signal[depandantSignalsKey].size, limit);
81+
t.assert.strictEqual(signal[depandantSignalsKey].size, limit);
9082

9183
done();
9284
}, true);
@@ -98,7 +90,7 @@ it('does not prevent source signal from being GCed if it is short-lived', (t, do
9890
setImmediate(() => {
9991
const unGCedSignals = [...signalRefs].filter((ref) => ref.deref());
10092

101-
t.assert.equal(unGCedSignals, 0);
93+
t.assert.strictEqual(unGCedSignals.length, 0);
10294
done();
10395
});
10496
});
@@ -116,13 +108,13 @@ it('drops settled dependant signals when signal is composite', (t, done) => {
116108
setImmediate(() => {
117109
global.gc();
118110

119-
t.assert.equal(composedSignalRef.deref(), undefined);
120-
t.assert.equal(controllers[0].signal[kDependantSignals].size, 2);
121-
t.assert.equal(controllers[1].signal[kDependantSignals].size, 1);
111+
t.assert.strictEqual(composedSignalRef.deref(), undefined);
112+
t.assert.strictEqual(controllers[0].signal[kDependantSignals].size, 2);
113+
t.assert.strictEqual(controllers[1].signal[kDependantSignals].size, 1);
122114

123115
setImmediate(() => {
124-
t.assert.equal(controllers[0].signal[kDependantSignals].size, 0);
125-
t.assert.equal(controllers[1].signal[kDependantSignals].size, 0);
116+
t.assert.strictEqual(controllers[0].signal[kDependantSignals].size, 0);
117+
t.assert.strictEqual(controllers[1].signal[kDependantSignals].size, 0);
126118

127119
done();
128120
});

0 commit comments

Comments
 (0)