FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

lib: defer AbortSignal.any() following · nodejs/node@ada3ce8 · GitHub

/ node Public

Commit ada3ce8

Browse files
Han5991
authored and
aduh95
committed
lib: defer AbortSignal.any() following
Avoid registering AbortSignal.any() composites as dependants until they are actually observed. This fixes the long-lived source retention pattern from #62363 while preserving abort semantics through lazy refresh and follow paths. Also unregister fired timeout signals from the timeout finalization registry so timeout churn releases memory more promptly. PR-URL: #62367 Fixes: #62363 Refs: #54614 Reviewed-By: Edy Silva <edigleyssonsilva@gmail.com> Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
1 parent 332088f commit ada3ce8

2 files changed

Lines changed: 116 additions & 28 deletions

File tree

‎lib/internal/abort_controller.js‎

Lines changed: 86 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -85,17 +85,16 @@ function lazyMessageChannel() {
8585
}
8686

8787
const clearTimeoutRegistry = new SafeFinalizationRegistry(clearTimeout);
88-
const dependantSignalsCleanupRegistry = new SafeFinalizationRegistry((signalWeakRef) => {
89-
const signal = signalWeakRef.deref();
90-
if (signal === undefined) {
91-
return;
92-
}
93-
signal[kDependantSignals].forEach((ref) => {
94-
if (ref.deref() === undefined) {
95-
signal[kDependantSignals].delete(ref);
88+
const dependantSignalsCleanupRegistry = new SafeFinalizationRegistry(
89+
({ sourceSignalRef, dependantSignalRef, sourceSignalsCleanupToken }) => {
90+
sourceSignalsCleanupRegistry.unregister(sourceSignalsCleanupToken);
91+
92+
const sourceSignal = sourceSignalRef.deref();
93+
if (sourceSignal === undefined) {
94+
return;
9695
}
96+
sourceSignal[kDependantSignals].delete(dependantSignalRef);
9797
});
98-
});
9998

10099
const gcPersistentSignals = new SafeSet();
101100

@@ -117,6 +116,8 @@ const kCloneData = Symbol('kCloneData');
117116
const kTimeout = Symbol('kTimeout');
118117
const kMakeTransferable = Symbol('kMakeTransferable');
119118
const kComposite = Symbol('kComposite');
119+
const kFollowing = Symbol('kFollowing');
120+
const kResultSignalWeakRef = Symbol('kResultSignalWeakRef');
120121
const kSourceSignals = Symbol('kSourceSignals');
121122
const kDependantSignals = Symbol('kDependantSignals');
122123

@@ -136,6 +137,60 @@ function validateThisAbortSignal(obj) {
136137
throw new ERR_INVALID_THIS('AbortSignal');
137138
}
138139

140+
function refreshCompositeSignal(signal) {
141+
if (!signal[kComposite] || signal[kAborted] || !signal[kSourceSignals]?.size) {
142+
return;
143+
}
144+
145+
for (const sourceSignalWeakRef of signal[kSourceSignals]) {
146+
const sourceSignal = sourceSignalWeakRef.deref();
147+
if (sourceSignal === undefined) {
148+
signal[kSourceSignals].delete(sourceSignalWeakRef);
149+
continue;
150+
}
151+
152+
if (sourceSignal.aborted) {
153+
abortSignal(signal, sourceSignal.reason);
154+
return;
155+
}
156+
}
157+
}
158+
159+
function followCompositeSignal(signal) {
160+
if (signal[kFollowing] || signal[kAborted] || !signal[kSourceSignals]?.size) {
161+
return;
162+
}
163+
164+
const resultSignalWeakRef = signal[kResultSignalWeakRef] ??= new SafeWeakRef(signal);
165+
166+
for (const sourceSignalWeakRef of signal[kSourceSignals]) {
167+
const sourceSignal = sourceSignalWeakRef.deref();
168+
if (sourceSignal === undefined) {
169+
signal[kSourceSignals].delete(sourceSignalWeakRef);
170+
continue;
171+
}
172+
173+
if (sourceSignal.aborted) {
174+
abortSignal(signal, sourceSignal.reason);
175+
return;
176+
}
177+
178+
sourceSignal[kDependantSignals] ??= new SafeSet();
179+
sourceSignal[kDependantSignals].add(resultSignalWeakRef);
180+
dependantSignalsCleanupRegistry.register(signal, {
181+
sourceSignalRef: sourceSignalWeakRef,
182+
dependantSignalRef: resultSignalWeakRef,
183+
sourceSignalsCleanupToken: sourceSignalWeakRef,
184+
});
185+
sourceSignalsCleanupRegistry.register(sourceSignal, {
186+
sourceSignalRef: sourceSignalWeakRef,
187+
composedSignalRef: resultSignalWeakRef,
188+
}, sourceSignalWeakRef);
189+
}
190+
191+
signal[kFollowing] = true;
192+
}
193+
139194
// Because the AbortSignal timeout cannot be canceled, we don't want the
140195
// presence of the timer alone to keep the AbortSignal from being garbage
141196
// collected if it otherwise no longer accessible. We also don't want the
@@ -148,6 +203,7 @@ function setWeakAbortSignalTimeout(weakRef, delay) {
148203
const timeout = setTimeout(() => {
149204
const signal = weakRef.deref();
150205
if (signal !== undefined) {
206+
clearTimeoutRegistry.unregister(signal);
151207
gcPersistentSignals.delete(signal);
152208
abortSignal(
153209
signal,
@@ -198,6 +254,7 @@ class AbortSignal extends EventTarget {
198254
*/
199255
get aborted() {
200256
validateThisAbortSignal(this);
257+
refreshCompositeSignal(this);
201258
return !!this[kAborted];
202259
}
203260

@@ -206,11 +263,13 @@ class AbortSignal extends EventTarget {
206263
*/
207264
get reason() {
208265
validateThisAbortSignal(this);
266+
refreshCompositeSignal(this);
209267
return this[kReason];
210268
}
211269

212270
throwIfAborted() {
213271
validateThisAbortSignal(this);
272+
refreshCompositeSignal(this);
214273
if (this[kAborted]) {
215274
throw this[kReason];
216275
}
@@ -241,7 +300,8 @@ class AbortSignal extends EventTarget {
241300
signal[kTimeout] = true;
242301
clearTimeoutRegistry.register(
243302
signal,
244-
setWeakAbortSignalTimeout(new SafeWeakRef(signal), delay));
303+
setWeakAbortSignalTimeout(new SafeWeakRef(signal), delay),
304+
signal);
245305
return signal;
246306
}
247307

@@ -260,7 +320,6 @@ class AbortSignal extends EventTarget {
260320
return resultSignal;
261321
}
262322

263-
const resultSignalWeakRef = new SafeWeakRef(resultSignal);
264323
resultSignal[kSourceSignals] = new SafeSet();
265324

266325
// Track if we have any timeout signals
@@ -283,51 +342,51 @@ class AbortSignal extends EventTarget {
283342
return resultSignal;
284343
}
285344

286-
signal[kDependantSignals] ??= new SafeSet();
287345
if (!signal[kComposite]) {
288346
const signalWeakRef = new SafeWeakRef(signal);
289347
resultSignal[kSourceSignals].add(signalWeakRef);
290-
signal[kDependantSignals].add(resultSignalWeakRef);
291-
dependantSignalsCleanupRegistry.register(resultSignal, signalWeakRef);
292-
sourceSignalsCleanupRegistry.register(signal, {
293-
sourceSignalRef: signalWeakRef,
294-
composedSignalRef: resultSignalWeakRef,
295-
});
296348
} else if (!signal[kSourceSignals]) {
297349
continue;
298350
} else {
351+
refreshCompositeSignal(signal);
352+
if (signal.aborted) {
353+
abortSignal(resultSignal, signal.reason);
354+
return resultSignal;
355+
}
299356
for (const sourceSignalWeakRef of signal[kSourceSignals]) {
300357
const sourceSignal = sourceSignalWeakRef.deref();
301358
if (!sourceSignal) {
302359
continue;
303360
}
304-
assert(!sourceSignal.aborted);
305361
assert(!sourceSignal[kComposite]);
306362

363+
if (sourceSignal.aborted) {
364+
abortSignal(resultSignal, sourceSignal.reason);
365+
return resultSignal;
366+
}
367+
307368
if (resultSignal[kSourceSignals].has(sourceSignalWeakRef)) {
308369
continue;
309370
}
310371
resultSignal[kSourceSignals].add(sourceSignalWeakRef);
311-
sourceSignal[kDependantSignals].add(resultSignalWeakRef);
312-
dependantSignalsCleanupRegistry.register(resultSignal, sourceSignalWeakRef);
313-
sourceSignalsCleanupRegistry.register(signal, {
314-
sourceSignalRef: sourceSignalWeakRef,
315-
composedSignalRef: resultSignalWeakRef,
316-
});
317372
}
318373
}
319374
}
320375

321-
// If we have any timeout signals, add the composite signal to gcPersistentSignals
322376
if (hasTimeoutSignals && resultSignal[kSourceSignals].size > 0) {
323-
gcPersistentSignals.add(resultSignal);
377+
resultSignal[kTimeout] = true;
324378
}
325379

326380
return resultSignal;
327381
}
328382

329383
[kNewListener](size, type, listener, once, capture, passive, weak) {
330384
super[kNewListener](size, type, listener, once, capture, passive, weak);
385+
386+
if (this[kComposite] && type === 'abort' && !this.aborted && size === 1) {
387+
followCompositeSignal(this);
388+
}
389+
331390
const isTimeoutOrNonEmptyCompositeSignal = this[kTimeout] || (this[kComposite] && this[kSourceSignals]?.size);
332391
if (isTimeoutOrNonEmptyCompositeSignal &&
333392
type === 'abort' &&

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

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,9 @@ function makeSubsequentCalls(limit, done, holdReferences = false) {
2323
}
2424

2525
if (holdReferences) {
26-
retainedSignals.push(AbortSignal.any([ac.signal]));
26+
const signal = AbortSignal.any([ac.signal]);
27+
signal.addEventListener('abort', handler);
28+
retainedSignals.push(signal);
2729
} else {
2830
// Using a WeakRef to avoid retaining information that will interfere with the test
2931
signalRef = new WeakRef(AbortSignal.any([ac.signal]));
@@ -119,6 +121,27 @@ describe('when there is a long-lived signal', () => {
119121
done();
120122
}, true);
121123
});
124+
125+
it('does not keep retained dependent signals without listeners', (t, done) => {
126+
const ac = new AbortController();
127+
const retainedSignals = [];
128+
const kDependantSignals = Object.getOwnPropertySymbols(ac.signal).find(
129+
(s) => s.toString() === 'Symbol(kDependantSignals)'
130+
);
131+
132+
function run(iteration) {
133+
if (iteration > limit) {
134+
t.assert.strictEqual(ac.signal[kDependantSignals]?.size ?? 0, 0);
135+
done();
136+
return;
137+
}
138+
139+
retainedSignals.push(AbortSignal.any([ac.signal]));
140+
setImmediate(() => run(iteration + 1));
141+
}
142+
143+
run(1);
144+
});
122145
});
123146

124147
it('does not prevent source signal from being GCed if it is short-lived', (t, done) => {
@@ -134,10 +157,13 @@ it('does not prevent source signal from being GCed if it is short-lived', (t, do
134157

135158
it('drops settled dependent signals when signal is composite', (t, done) => {
136159
const controllers = Array.from({ length: 2 }, () => new AbortController());
160+
const handler = () => {};
137161

138162
// Using WeakRefs to avoid this test to retain information that will make the test fail
139163
const composedSignal1 = new WeakRef(AbortSignal.any([controllers[0].signal]));
140164
const composedSignalRef = new WeakRef(AbortSignal.any([composedSignal1.deref(), controllers[1].signal]));
165+
composedSignal1.deref().addEventListener('abort', handler);
166+
composedSignalRef.deref().addEventListener('abort', handler);
141167

142168
const kDependantSignals = Object.getOwnPropertySymbols(controllers[0].signal).find(
143169
(s) => s.toString() === 'Symbol(kDependantSignals)'
@@ -147,6 +173,9 @@ it('drops settled dependent signals when signal is composite', (t, done) => {
147173
t.assert.strictEqual(controllers[1].signal[kDependantSignals].size, 1);
148174

149175
setImmediate(mustCall(() => {
176+
composedSignal1.deref()?.removeEventListener('abort', handler);
177+
composedSignalRef.deref()?.removeEventListener('abort', handler);
178+
150179
globalThis.gc({ execution: 'async' }).then(async () => {
151180
await gcUntil('all signals are GCed', () => {
152181
const totalDependantSignals = Math.max(

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL