From b03f41d1688b8ae89ffa8eb4b9a884aa1eb70bc7 Mon Sep 17 00:00:00 2001 From: Kuldeep Yadav Date: Fri, 25 Sep 2026 02:16:36 +0530 Subject: [PATCH] lib: format default abort reason stack eagerly The default AbortError created by AbortController.prototype.abort() and AbortSignal.abort() keeps its stack unformatted. Until it is formatted, V8 holds on to the call sites, which retain the receivers and functions of the calling frames. A signal that outlives the work it cancelled can then keep that work alive, including large buffers. Read the stack once when the default reason is created, as streams already do. Reasons passed by the caller are left untouched. Signed-off-by: Kuldeep Yadav --- lib/internal/abort_controller.js | 19 ++++++-- ...test-abortcontroller-default-reason-gc.mjs | 47 +++++++++++++++++++ 2 files changed, 63 insertions(+), 3 deletions(-) create mode 100644 test/parallel/test-abortcontroller-default-reason-gc.mjs diff --git a/lib/internal/abort_controller.js b/lib/internal/abort_controller.js index 55be0b6f3669..0e9033341d29 100644 --- a/lib/internal/abort_controller.js +++ b/lib/internal/abort_controller.js @@ -216,6 +216,20 @@ function setWeakAbortSignalTimeout(weakRef, delay) { return timeout; } +function createDefaultAbortReason() { + const reason = new DOMException('This operation was aborted', 'AbortError'); + // Format the stack eagerly. Until it is formatted, V8 keeps the call sites, + // which retain the receivers and functions of the caller's frames, so a + // retained signal would keep the aborted work alive. + // Refs: https://github.com/nodejs/node/pull/34103#issuecomment-652002364 + try { + reason.stack; // eslint-disable-line no-unused-expressions + } catch { + // A throwing Error.prepareStackTrace must not make abort() throw. + } + return reason; +} + class AbortSignal extends EventTarget { #brand; @@ -293,8 +307,7 @@ class AbortSignal extends EventTarget { * @param {any} [reason] * @returns {AbortSignal} */ - static abort( - reason = new DOMException('This operation was aborted', 'AbortError')) { + static abort(reason = createDefaultAbortReason()) { return new AbortSignal(kDontThrowSymbol, { aborted: true, reason }); } @@ -576,7 +589,7 @@ class AbortController { /** * @param {any} [reason] */ - abort(reason = new DOMException('This operation was aborted', 'AbortError')) { + abort(reason = createDefaultAbortReason()) { abortSignal(this.#signal ??= new AbortSignal(kDontThrowSymbol), reason); } diff --git a/test/parallel/test-abortcontroller-default-reason-gc.mjs b/test/parallel/test-abortcontroller-default-reason-gc.mjs new file mode 100644 index 000000000000..11fccb2bc5e3 --- /dev/null +++ b/test/parallel/test-abortcontroller-default-reason-gc.mjs @@ -0,0 +1,47 @@ +// Flags: --expose-gc + +import '../common/index.mjs'; +import { gcUntil } from '../common/gc.js'; +import assert from 'node:assert/strict'; +import { it } from 'node:test'; + +// The default abort reason must not keep the objects on the caller's stack +// alive. Refs: https://github.com/nodejs/node/issues/66192 + +class Job { + controller = new AbortController(); + + cancel() { + this.controller.abort(); + return this.controller.signal; + } + + cancelStatic() { + return AbortSignal.abort(); + } +} + +for (const method of ['cancel', 'cancelStatic']) { + it(`does not retain the caller through the default reason (${method})`, async () => { + let job = new Job(); + const jobRef = new WeakRef(job); + const signal = job[method](); + job = null; + + await gcUntil('job is collected', () => jobRef.deref() === undefined); + assert.strictEqual(signal.aborted, true); + assert.strictEqual(signal.reason.name, 'AbortError'); + assert.match(signal.reason.stack, new RegExp(`at Job\\.${method} `)); + }); +} + +it('does not throw when Error.prepareStackTrace throws', () => { + const { prepareStackTrace } = Error; + Error.prepareStackTrace = () => { throw new Error('boom'); }; + try { + assert.strictEqual(new Job().cancel().reason.name, 'AbortError'); + assert.strictEqual(new Job().cancelStatic().reason.name, 'AbortError'); + } finally { + Error.prepareStackTrace = prepareStackTrace; + } +});