From 4f6e60226ffda76640d6e113f02675bd1caf9bd4 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Thu, 2 Apr 2026 15:44:42 +0200 Subject: [PATCH 1/3] tools: do not run `commit-lint` on release proposals MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR-URL: https://github.com/nodejs/node/pull/62523 Reviewed-By: René Reviewed-By: Luigi Pinca Reviewed-By: Michaël Zasso --- .github/workflows/commit-lint.yml | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/.github/workflows/commit-lint.yml b/.github/workflows/commit-lint.yml index cb40dd731ccdee..d7d43732ec8e8e 100644 --- a/.github/workflows/commit-lint.yml +++ b/.github/workflows/commit-lint.yml @@ -1,6 +1,10 @@ name: First commit message adheres to guidelines -on: [pull_request] +on: + pull_request: + branches: + - main + - v[0-9]+.x-staging env: NODE_VERSION: lts/* From a9ac9b12eec0f777b99e80e8672c3f429c7329db Mon Sep 17 00:00:00 2001 From: sangwook Date: Thu, 2 Apr 2026 23:15:43 +0900 Subject: [PATCH 2/3] 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: https://github.com/nodejs/node/pull/62367 Fixes: https://github.com/nodejs/node/issues/62363 Refs: https://github.com/nodejs/node/issues/54614 Reviewed-By: Edy Silva Reviewed-By: Chemi Atlow --- lib/internal/abort_controller.js | 113 +++++++++++++----- .../test-abortsignal-drop-settled-signals.mjs | 31 ++++- 2 files changed, 116 insertions(+), 28 deletions(-) diff --git a/lib/internal/abort_controller.js b/lib/internal/abort_controller.js index ef77e0af30e40a..a24b5b556e1a5e 100644 --- a/lib/internal/abort_controller.js +++ b/lib/internal/abort_controller.js @@ -85,17 +85,16 @@ function lazyMessageChannel() { } const clearTimeoutRegistry = new SafeFinalizationRegistry(clearTimeout); -const dependantSignalsCleanupRegistry = new SafeFinalizationRegistry((signalWeakRef) => { - const signal = signalWeakRef.deref(); - if (signal === undefined) { - return; - } - signal[kDependantSignals].forEach((ref) => { - if (ref.deref() === undefined) { - signal[kDependantSignals].delete(ref); +const dependantSignalsCleanupRegistry = new SafeFinalizationRegistry( + ({ sourceSignalRef, dependantSignalRef, sourceSignalsCleanupToken }) => { + sourceSignalsCleanupRegistry.unregister(sourceSignalsCleanupToken); + + const sourceSignal = sourceSignalRef.deref(); + if (sourceSignal === undefined) { + return; } + sourceSignal[kDependantSignals].delete(dependantSignalRef); }); -}); const gcPersistentSignals = new SafeSet(); @@ -117,6 +116,8 @@ const kCloneData = Symbol('kCloneData'); const kTimeout = Symbol('kTimeout'); const kMakeTransferable = Symbol('kMakeTransferable'); const kComposite = Symbol('kComposite'); +const kFollowing = Symbol('kFollowing'); +const kResultSignalWeakRef = Symbol('kResultSignalWeakRef'); const kSourceSignals = Symbol('kSourceSignals'); const kDependantSignals = Symbol('kDependantSignals'); @@ -136,6 +137,60 @@ function validateThisAbortSignal(obj) { throw new ERR_INVALID_THIS('AbortSignal'); } +function refreshCompositeSignal(signal) { + if (!signal[kComposite] || signal[kAborted] || !signal[kSourceSignals]?.size) { + return; + } + + for (const sourceSignalWeakRef of signal[kSourceSignals]) { + const sourceSignal = sourceSignalWeakRef.deref(); + if (sourceSignal === undefined) { + signal[kSourceSignals].delete(sourceSignalWeakRef); + continue; + } + + if (sourceSignal.aborted) { + abortSignal(signal, sourceSignal.reason); + return; + } + } +} + +function followCompositeSignal(signal) { + if (signal[kFollowing] || signal[kAborted] || !signal[kSourceSignals]?.size) { + return; + } + + const resultSignalWeakRef = signal[kResultSignalWeakRef] ??= new SafeWeakRef(signal); + + for (const sourceSignalWeakRef of signal[kSourceSignals]) { + const sourceSignal = sourceSignalWeakRef.deref(); + if (sourceSignal === undefined) { + signal[kSourceSignals].delete(sourceSignalWeakRef); + continue; + } + + if (sourceSignal.aborted) { + abortSignal(signal, sourceSignal.reason); + return; + } + + sourceSignal[kDependantSignals] ??= new SafeSet(); + sourceSignal[kDependantSignals].add(resultSignalWeakRef); + dependantSignalsCleanupRegistry.register(signal, { + sourceSignalRef: sourceSignalWeakRef, + dependantSignalRef: resultSignalWeakRef, + sourceSignalsCleanupToken: sourceSignalWeakRef, + }); + sourceSignalsCleanupRegistry.register(sourceSignal, { + sourceSignalRef: sourceSignalWeakRef, + composedSignalRef: resultSignalWeakRef, + }, sourceSignalWeakRef); + } + + signal[kFollowing] = true; +} + // Because the AbortSignal timeout cannot be canceled, we don't want the // presence of the timer alone to keep the AbortSignal from being garbage // collected if it otherwise no longer accessible. We also don't want the @@ -148,6 +203,7 @@ function setWeakAbortSignalTimeout(weakRef, delay) { const timeout = setTimeout(() => { const signal = weakRef.deref(); if (signal !== undefined) { + clearTimeoutRegistry.unregister(signal); gcPersistentSignals.delete(signal); abortSignal( signal, @@ -198,6 +254,7 @@ class AbortSignal extends EventTarget { */ get aborted() { validateThisAbortSignal(this); + refreshCompositeSignal(this); return !!this[kAborted]; } @@ -206,11 +263,13 @@ class AbortSignal extends EventTarget { */ get reason() { validateThisAbortSignal(this); + refreshCompositeSignal(this); return this[kReason]; } throwIfAborted() { validateThisAbortSignal(this); + refreshCompositeSignal(this); if (this[kAborted]) { throw this[kReason]; } @@ -241,7 +300,8 @@ class AbortSignal extends EventTarget { signal[kTimeout] = true; clearTimeoutRegistry.register( signal, - setWeakAbortSignalTimeout(new SafeWeakRef(signal), delay)); + setWeakAbortSignalTimeout(new SafeWeakRef(signal), delay), + signal); return signal; } @@ -260,7 +320,6 @@ class AbortSignal extends EventTarget { return resultSignal; } - const resultSignalWeakRef = new SafeWeakRef(resultSignal); resultSignal[kSourceSignals] = new SafeSet(); // Track if we have any timeout signals @@ -283,44 +342,39 @@ class AbortSignal extends EventTarget { return resultSignal; } - signal[kDependantSignals] ??= new SafeSet(); if (!signal[kComposite]) { const signalWeakRef = new SafeWeakRef(signal); resultSignal[kSourceSignals].add(signalWeakRef); - signal[kDependantSignals].add(resultSignalWeakRef); - dependantSignalsCleanupRegistry.register(resultSignal, signalWeakRef); - sourceSignalsCleanupRegistry.register(signal, { - sourceSignalRef: signalWeakRef, - composedSignalRef: resultSignalWeakRef, - }); } else if (!signal[kSourceSignals]) { continue; } else { + refreshCompositeSignal(signal); + if (signal.aborted) { + abortSignal(resultSignal, signal.reason); + return resultSignal; + } for (const sourceSignalWeakRef of signal[kSourceSignals]) { const sourceSignal = sourceSignalWeakRef.deref(); if (!sourceSignal) { continue; } - assert(!sourceSignal.aborted); assert(!sourceSignal[kComposite]); + if (sourceSignal.aborted) { + abortSignal(resultSignal, sourceSignal.reason); + return resultSignal; + } + if (resultSignal[kSourceSignals].has(sourceSignalWeakRef)) { continue; } resultSignal[kSourceSignals].add(sourceSignalWeakRef); - sourceSignal[kDependantSignals].add(resultSignalWeakRef); - dependantSignalsCleanupRegistry.register(resultSignal, sourceSignalWeakRef); - sourceSignalsCleanupRegistry.register(signal, { - sourceSignalRef: sourceSignalWeakRef, - composedSignalRef: resultSignalWeakRef, - }); } } } - // If we have any timeout signals, add the composite signal to gcPersistentSignals if (hasTimeoutSignals && resultSignal[kSourceSignals].size > 0) { - gcPersistentSignals.add(resultSignal); + resultSignal[kTimeout] = true; } return resultSignal; @@ -328,6 +382,11 @@ class AbortSignal extends EventTarget { [kNewListener](size, type, listener, once, capture, passive, weak) { super[kNewListener](size, type, listener, once, capture, passive, weak); + + if (this[kComposite] && type === 'abort' && !this.aborted && size === 1) { + followCompositeSignal(this); + } + const isTimeoutOrNonEmptyCompositeSignal = this[kTimeout] || (this[kComposite] && this[kSourceSignals]?.size); if (isTimeoutOrNonEmptyCompositeSignal && type === 'abort' && diff --git a/test/parallel/test-abortsignal-drop-settled-signals.mjs b/test/parallel/test-abortsignal-drop-settled-signals.mjs index 29a0dcf8b58638..c481db069d6f13 100644 --- a/test/parallel/test-abortsignal-drop-settled-signals.mjs +++ b/test/parallel/test-abortsignal-drop-settled-signals.mjs @@ -23,7 +23,9 @@ function makeSubsequentCalls(limit, done, holdReferences = false) { } if (holdReferences) { - retainedSignals.push(AbortSignal.any([ac.signal])); + const signal = AbortSignal.any([ac.signal]); + signal.addEventListener('abort', handler); + retainedSignals.push(signal); } else { // Using a WeakRef to avoid retaining information that will interfere with the test signalRef = new WeakRef(AbortSignal.any([ac.signal])); @@ -119,6 +121,27 @@ describe('when there is a long-lived signal', () => { done(); }, true); }); + + it('does not keep retained dependent signals without listeners', (t, done) => { + const ac = new AbortController(); + const retainedSignals = []; + const kDependantSignals = Object.getOwnPropertySymbols(ac.signal).find( + (s) => s.toString() === 'Symbol(kDependantSignals)' + ); + + function run(iteration) { + if (iteration > limit) { + t.assert.strictEqual(ac.signal[kDependantSignals]?.size ?? 0, 0); + done(); + return; + } + + retainedSignals.push(AbortSignal.any([ac.signal])); + setImmediate(() => run(iteration + 1)); + } + + run(1); + }); }); 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 it('drops settled dependent signals when signal is composite', (t, done) => { const controllers = Array.from({ length: 2 }, () => new AbortController()); + const handler = () => {}; // Using WeakRefs to avoid this test to retain information that will make the test fail const composedSignal1 = new WeakRef(AbortSignal.any([controllers[0].signal])); const composedSignalRef = new WeakRef(AbortSignal.any([composedSignal1.deref(), controllers[1].signal])); + composedSignal1.deref().addEventListener('abort', handler); + composedSignalRef.deref().addEventListener('abort', handler); const kDependantSignals = Object.getOwnPropertySymbols(controllers[0].signal).find( (s) => s.toString() === 'Symbol(kDependantSignals)' @@ -147,6 +173,9 @@ it('drops settled dependent signals when signal is composite', (t, done) => { t.assert.strictEqual(controllers[1].signal[kDependantSignals].size, 1); setImmediate(mustCall(() => { + composedSignal1.deref()?.removeEventListener('abort', handler); + composedSignalRef.deref()?.removeEventListener('abort', handler); + globalThis.gc({ execution: 'async' }).then(async () => { await gcUntil('all signals are GCed', () => { const totalDependantSignals = Math.max( From 25443db104d39a3dcd410fa882884580734cfaa4 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Tue, 31 Mar 2026 16:35:57 +0200 Subject: [PATCH 3/3] tools: allow triagers to queue a PR for CI until it's reviewed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR-URL: https://github.com/nodejs/node/pull/62524 Fixes: https://github.com/nodejs/node/issues/62496 Reviewed-By: Filip Skokan Reviewed-By: Michaël Zasso --- .github/workflows/auto-start-ci.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/auto-start-ci.yml b/.github/workflows/auto-start-ci.yml index 747b957ee33c85..72f93539ec7f70 100644 --- a/.github/workflows/auto-start-ci.yml +++ b/.github/workflows/auto-start-ci.yml @@ -28,13 +28,13 @@ jobs: - name: Get Pull Requests id: get_prs_for_ci run: > - numbers=$(gh pr list \ - --repo ${{ github.repository }} \ + echo "numbers=$(gh pr list \ + --repo "$GITHUB_REPOSITORY" \ --label 'request-ci' \ --json 'number' \ + --search 'review:approved' \ -t '{{ range . }}{{ .number }} {{ end }}' \ - --limit 5) - echo "numbers=$numbers" >> $GITHUB_OUTPUT + --limit 5)" >> "$GITHUB_OUTPUT" env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} start-ci: