diff --git a/lib/internal/abort_controller.js b/lib/internal/abort_controller.js index 55be0b6f3669..2320c7baf84a 100644 --- a/lib/internal/abort_controller.js +++ b/lib/internal/abort_controller.js @@ -33,6 +33,7 @@ const { customInspectSymbol, kEmptyObject, kEnumerableProperty, + lazyDOMException, } = require('internal/util'); const { inspect } = require('internal/util/inspect'); const { @@ -216,6 +217,16 @@ function setWeakAbortSignalTimeout(weakRef, delay) { return timeout; } +// The default reason captures no stack. A captured stack keeps the receivers +// and functions of the aborting caller's frames alive until it is formatted, +// so a retained signal would keep the aborted work alive, and formatting it +// eagerly costs several microseconds per abort. Callers that want the stack +// can pass their own reason. +// Refs: https://github.com/nodejs/node/issues/66192 +function createDefaultAbortReason() { + return lazyDOMException('This operation was aborted', 'AbortError', false); +} + class AbortSignal extends EventTarget { #brand; @@ -293,8 +304,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 +586,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/lib/internal/util.js b/lib/internal/util.js index 2f72e636ab90..16f5075466fc 100644 --- a/lib/internal/util.js +++ b/lib/internal/util.js @@ -681,14 +681,14 @@ const lazyDOMExceptionClass = () => { return _DOMException; }; -const lazyDOMException = (message, name) => { +const lazyDOMException = (message, name, captureStack = true) => { _DOMException ??= internalBinding('messaging').DOMException; if (isErrorStackTraceLimitWritable()) { const limit = Error.stackTraceLimit; Error.stackTraceLimit = 0; const ex = new _DOMException(message, name); Error.stackTraceLimit = limit; - ErrorCaptureStackTrace(ex, lazyDOMException); + if (captureStack) ErrorCaptureStackTrace(ex, lazyDOMException); return ex; } return new _DOMException(message, name); 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..4f7777f576f2 --- /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.doesNotMatch(signal.reason.stack, /\n\s+at /); + }); +} + +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; + } +});