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; + } +});