diff --git a/benchmark/abort_controller/abort-controller-abort.js b/benchmark/abort_controller/abort-controller-abort.js new file mode 100644 index 000000000000..c1c97718f82e --- /dev/null +++ b/benchmark/abort_controller/abort-controller-abort.js @@ -0,0 +1,21 @@ +'use strict'; +const common = require('../common.js'); + +const bench = common.createBenchmark(main, { + n: [1e6], + listeners: [0, 1], +}); + +function main({ n, listeners }) { + const reason = new Error('reason'); + const noop = () => {}; + + bench.start(); + for (let i = 0; i < n; i++) { + const ac = new AbortController(); + if (listeners > 0) + ac.signal.addEventListener('abort', noop); + ac.abort(reason); + } + bench.end(n); +} diff --git a/benchmark/abort_controller/abort-signal-add-remove.js b/benchmark/abort_controller/abort-signal-add-remove.js new file mode 100644 index 000000000000..77a5216f2388 --- /dev/null +++ b/benchmark/abort_controller/abort-signal-add-remove.js @@ -0,0 +1,43 @@ +'use strict'; +const common = require('../common.js'); +const { addAbortListener } = require('events'); + +const bench = common.createBenchmark(main, { + n: [1e6], + kind: ['addEventListener', 'addEventListener-once', 'addAbortListener'], +}); + +function main({ n, kind }) { + const { signal } = new AbortController(); + const noop = () => {}; + + switch (kind) { + case 'addEventListener': + bench.start(); + for (let i = 0; i < n; i++) { + signal.addEventListener('abort', noop); + signal.removeEventListener('abort', noop); + } + bench.end(n); + break; + case 'addEventListener-once': { + const options = { once: true }; + bench.start(); + for (let i = 0; i < n; i++) { + signal.addEventListener('abort', noop, options); + signal.removeEventListener('abort', noop); + } + bench.end(n); + break; + } + case 'addAbortListener': + bench.start(); + for (let i = 0; i < n; i++) { + addAbortListener(signal, noop)[Symbol.dispose](); + } + bench.end(n); + break; + default: + throw new Error('Invalid kind'); + } +} diff --git a/benchmark/abort_controller/abort-signal-any.js b/benchmark/abort_controller/abort-signal-any.js new file mode 100644 index 000000000000..96981067e50d --- /dev/null +++ b/benchmark/abort_controller/abort-signal-any.js @@ -0,0 +1,46 @@ +'use strict'; +const common = require('../common.js'); + +const bench = common.createBenchmark(main, { + n: [2e5], + kind: ['create', 'listen', 'abort'], +}); + +// Combines a per-iteration signal with a long-lived one, like a request +// signal combined with a server wide one. The work is split into batches +// that run in separate macrotasks because WeakRef targets are kept alive +// until the end of the current job, as they would be between requests. +function main({ n, kind }) { + const reason = new Error('reason'); + const noop = () => {}; + const longLived = new AbortController().signal; + const batch = 1000; + + function run(count) { + for (let i = 0; i < count; i++) { + const ac = new AbortController(); + const signal = AbortSignal.any([ac.signal, longLived]); + if (kind === 'create') + continue; + signal.addEventListener('abort', noop, { once: true }); + if (kind === 'listen') + signal.removeEventListener('abort', noop); + else + ac.abort(reason); + } + } + + let left = n; + function step() { + const count = Math.min(batch, left); + run(count); + left -= count; + if (left > 0) + setImmediate(step); + else + bench.end(n); + } + + bench.start(); + step(); +} diff --git a/lib/internal/abort_controller.js b/lib/internal/abort_controller.js index 9722484a4226..7daa63e1f91d 100644 --- a/lib/internal/abort_controller.js +++ b/lib/internal/abort_controller.js @@ -5,9 +5,11 @@ const { ArrayPrototypePush, + FunctionPrototypeCall, ObjectAssign, ObjectDefineProperties, ObjectDefineProperty, + ObjectFreeze, ObjectSetPrototypeOf, PromiseResolve, PromiseWithResolvers, @@ -22,6 +24,7 @@ const { defineEventHandler, EventTarget, Event, + kEvents, kTrustEvent, kNewListener, kRemoveListener, @@ -31,7 +34,6 @@ const { const { kMaxEventTargetListeners } = require('events'); const { customInspectSymbol, - kEmptyObject, kEnumerableProperty, } = require('internal/util'); const { inspect } = require('internal/util/inspect'); @@ -41,6 +43,7 @@ const { ERR_INVALID_ARG_TYPE, ERR_INVALID_THIS, }, + isErrorStackTraceLimitWritable, } = require('internal/errors'); const { converters, @@ -75,6 +78,15 @@ let _MessageChannel; const kDontThrowSymbol = Symbol('kDontThrowSymbol'); +const kTrustedEventInit = ObjectFreeze({ [kTrustEvent]: true }); + +// V8 doesn't optimize keyed super property access such as +// super[kNewListener](), so call the EventTarget implementations directly. +const { + [kNewListener]: eventTargetNewListener, + [kRemoveListener]: eventTargetRemoveListener, +} = EventTarget.prototype; + // Loading the MessageChannel and markTransferable have to be done lazily // because otherwise we'll end up with a require cycle that ends up with // an incomplete initialization of abort_controller. @@ -84,28 +96,37 @@ function lazyMessageChannel() { return new _MessageChannel(); } -const clearTimeoutRegistry = new SafeFinalizationRegistry(clearTimeout); -const dependantSignalsCleanupRegistry = new SafeFinalizationRegistry( - ({ sourceSignalRef, dependantSignalRef }) => { - sourceSignalsCleanupRegistry.unregister(dependantSignalRef); +const clearTimeoutRegistry = new SafeFinalizationRegistry((holder) => { + if (holder.timeout !== undefined) { + clearTimeout(holder.timeout); + } +}); - const sourceSignal = sourceSignalRef.deref(); - if (sourceSignal === undefined) { - return; +// Each followed composite signal is registered once. When it is collected, +// remove it from the dependant signals of its remaining sources. +const dependantSignalsCleanupRegistry = new SafeFinalizationRegistry( + ({ sourceSignals, dependantSignalRef }) => { + for (const sourceSignalRef of sourceSignals) { + sourceSignalRef.deref()?.[kDependantSignals]?.delete(dependantSignalRef); } - sourceSignal[kDependantSignals].delete(dependantSignalRef); }); const gcPersistentSignals = new SafeSet(); -const sourceSignalsCleanupRegistry = new SafeFinalizationRegistry(({ sourceSignalRef, composedSignalRef }) => { - const composedSignal = composedSignalRef.deref(); - if (composedSignal !== undefined) { - composedSignal[kSourceSignals].delete(sourceSignalRef); - - if (composedSignal[kSourceSignals].size === 0) { - // This signal will no longer abort. There's no need to keep it in the gcPersistentSignals set. - gcPersistentSignals.delete(composedSignal); +// Each source signal is registered once, when it gets its first dependant. +// When it is collected, remove it from the source signals of its remaining +// dependants. Registering per source rather than per (source, dependant) pair +// avoids unregister tokens, which are expensive to register. +const sourceSignalsCleanupRegistry = new SafeFinalizationRegistry(({ sourceSignalRef, dependantSignals }) => { + for (const composedSignalRef of dependantSignals) { + const composedSignal = composedSignalRef.deref(); + if (composedSignal !== undefined) { + composedSignal[kSourceSignals].delete(sourceSignalRef); + + if (composedSignal[kSourceSignals].size === 0) { + // This signal will no longer abort. There's no need to keep it in the gcPersistentSignals set. + gcPersistentSignals.delete(composedSignal); + } } } }); @@ -117,10 +138,16 @@ const kTimeout = Symbol('kTimeout'); const kMakeTransferable = Symbol('kMakeTransferable'); const kComposite = Symbol('kComposite'); const kFollowing = Symbol('kFollowing'); -const kResultSignalWeakRef = Symbol('kResultSignalWeakRef'); +const kWeakRef = Symbol('kWeakRef'); const kSourceSignals = Symbol('kSourceSignals'); const kDependantSignals = Symbol('kDependantSignals'); +// A signal's own WeakRef, shared by every composite that follows it so that +// a long-lived source doesn't allocate a new WeakRef per AbortSignal.any(). +function getWeakRef(signal) { + return signal[kWeakRef] ??= new SafeWeakRef(signal); +} + function customInspect(self, obj, depth, options) { if (depth < 0) return self; @@ -161,12 +188,18 @@ function followCompositeSignal(signal) { return; } - const resultSignalWeakRef = signal[kResultSignalWeakRef] ??= new SafeWeakRef(signal); + const resultSignalWeakRef = getWeakRef(signal); + const sourceSignals = signal[kSourceSignals]; - for (const sourceSignalWeakRef of signal[kSourceSignals]) { + dependantSignalsCleanupRegistry.register(signal, { + sourceSignals, + dependantSignalRef: resultSignalWeakRef, + }); + + for (const sourceSignalWeakRef of sourceSignals) { const sourceSignal = sourceSignalWeakRef.deref(); if (sourceSignal === undefined) { - signal[kSourceSignals].delete(sourceSignalWeakRef); + sourceSignals.delete(sourceSignalWeakRef); continue; } @@ -175,19 +208,15 @@ function followCompositeSignal(signal) { return; } - sourceSignal[kDependantSignals] ??= new SafeSet(); - sourceSignal[kDependantSignals].add(resultSignalWeakRef); - dependantSignalsCleanupRegistry.register(signal, { - sourceSignalRef: sourceSignalWeakRef, - dependantSignalRef: resultSignalWeakRef, - }); - // Nested composites share their sources' WeakRefs, so use this signal's - // own WeakRef as the unregister token. Otherwise collecting an inner - // composite would also unregister the outer composite from its sources. - sourceSignalsCleanupRegistry.register(sourceSignal, { - sourceSignalRef: sourceSignalWeakRef, - composedSignalRef: resultSignalWeakRef, - }, resultSignalWeakRef); + let dependantSignals = sourceSignal[kDependantSignals]; + if (dependantSignals === undefined) { + dependantSignals = sourceSignal[kDependantSignals] = new SafeSet(); + sourceSignalsCleanupRegistry.register(sourceSignal, { + sourceSignalRef: sourceSignalWeakRef, + dependantSignals, + }); + } + dependantSignals.add(resultSignalWeakRef); } signal[kFollowing] = true; @@ -201,11 +230,16 @@ function followCompositeSignal(signal) { // over the WeakRef rather than directly over the AbortSignal, and we unref // the created timer object. Separately, we add the signal to a // FinalizerRegistry that will clear the timeout when the signal is gc'd. +// The registry holds the timeout through a holder that is emptied once the +// timer fires, which releases the fired timer without having to unregister +// the signal. Registering with an unregister token is several times more +// expensive than registering without one. function setWeakAbortSignalTimeout(weakRef, delay) { - const timeout = setTimeout(() => { + const holder = { timeout: undefined }; + holder.timeout = setTimeout(() => { + holder.timeout = undefined; const signal = weakRef.deref(); if (signal !== undefined) { - clearTimeoutRegistry.unregister(signal); gcPersistentSignals.delete(signal); abortSignal( signal, @@ -214,8 +248,25 @@ function setWeakAbortSignalTimeout(weakRef, delay) { 'TimeoutError')); } }, delay); - timeout.unref(); - return timeout; + holder.timeout.unref(); + return holder; +} + +// 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() { + if (!isErrorStackTraceLimitWritable()) { + return new DOMException('This operation was aborted', 'AbortError'); + } + const limit = Error.stackTraceLimit; + Error.stackTraceLimit = 0; + const reason = new DOMException('This operation was aborted', 'AbortError'); + Error.stackTraceLimit = limit; + return reason; } class AbortSignal extends EventTarget { @@ -229,31 +280,37 @@ class AbortSignal extends EventTarget { } /** + * Takes positional arguments rather than an options object: destructuring + * the default kEmptyObject, which V8 keeps in dictionary mode, cost more + * than the rest of the constructor. * @param {symbol | undefined} dontThrowSymbol - * @param {{ - * aborted? : boolean, - * reason? : any, - * transferable? : boolean, - * composite? : boolean, - * }} [init] + * @param {boolean} [aborted] + * @param {any} [reason] + * @param {boolean} [transferable] + * @param {boolean} [composite] * @private */ - constructor(dontThrowSymbol = undefined, init = kEmptyObject) { + constructor(dontThrowSymbol = undefined, aborted = false, reason = undefined, + transferable = false, composite = false) { if (dontThrowSymbol !== kDontThrowSymbol) { throw new ERR_ILLEGAL_CONSTRUCTOR(); } super(); this[kMaxEventTargetListeners] = 0; - const { - aborted = false, - reason = undefined, - transferable = false, - composite = false, - } = init; this[kAborted] = aborted; this[kReason] = reason; this[kComposite] = composite; + // Signals that become sources of AbortSignal.any() get these slots later. + // Initialize them up front so that adding them doesn't change the shape + // of live signals and make the call sites that use them polymorphic. + this[kWeakRef] = undefined; + this[kDependantSignals] = undefined; + if (composite) { + this[kTimeout] = false; + this[kFollowing] = false; + this[kSourceSignals] = undefined; + } if (transferable) { markTransferMode(this, false, true); } @@ -295,9 +352,8 @@ class AbortSignal extends EventTarget { * @param {any} [reason] * @returns {AbortSignal} */ - static abort( - reason = new DOMException('This operation was aborted', 'AbortError')) { - return new AbortSignal(kDontThrowSymbol, { aborted: true, reason }); + static abort(reason = createDefaultAbortReason()) { + return new AbortSignal(kDontThrowSymbol, true, reason); } /** @@ -310,8 +366,7 @@ class AbortSignal extends EventTarget { signal[kTimeout] = true; clearTimeoutRegistry.register( signal, - setWeakAbortSignalTimeout(new SafeWeakRef(signal), delay), - signal); + setWeakAbortSignalTimeout(getWeakRef(signal), delay)); return signal; } @@ -325,7 +380,7 @@ class AbortSignal extends EventTarget { { __proto__: null, context: 'signals' }, ); - const resultSignal = new AbortSignal(kDontThrowSymbol, { composite: true }); + const resultSignal = new AbortSignal(kDontThrowSymbol, false, undefined, false, true); if (!signalsArray.length) { return resultSignal; } @@ -354,8 +409,7 @@ class AbortSignal extends EventTarget { } if (!signal[kComposite]) { - const signalWeakRef = new SafeWeakRef(signal); - resultSignal[kSourceSignals].add(signalWeakRef); + resultSignal[kSourceSignals].add(getWeakRef(signal)); } else if (!signal[kSourceSignals]) { continue; } else { @@ -391,7 +445,7 @@ class AbortSignal extends EventTarget { } [kNewListener](size, type, listener, once, capture, passive, weak) { - super[kNewListener](size, type, listener, once, capture, passive, weak); + FunctionPrototypeCall(eventTargetNewListener, this, size, type, listener, once, capture, passive, weak); if (this[kComposite] && type === 'abort' && !this.aborted && size === 1) { followCompositeSignal(this); @@ -412,7 +466,7 @@ class AbortSignal extends EventTarget { } [kRemoveListener](size, type, listener, capture) { - super[kRemoveListener](size, type, listener, capture); + FunctionPrototypeCall(eventTargetRemoveListener, this, size, type, listener, capture); const isTimeoutOrNonEmptyCompositeSignal = this[kTimeout] || (this[kComposite] && this[kSourceSignals]?.size); if (isTimeoutOrNonEmptyCompositeSignal && type === 'abort' && size === 0) { gcPersistentSignals.delete(this); @@ -479,7 +533,7 @@ class AbortSignal extends EventTarget { converters['sequence'] = createSequenceConverter(converters.AbortSignal); function ClonedAbortSignal() { - return new AbortSignal(kDontThrowSymbol, { transferable: true }); + return new AbortSignal(kDontThrowSymbol, false, undefined, true); } ClonedAbortSignal.prototype[kDeserialize] = () => {}; @@ -508,36 +562,43 @@ function abortSignal(signal, reason) { signal[kReason] = reason; // 3. Let dependentSignalsToAbort be a new list. - const dependentSignalsToAbort = ObjectSetPrototypeOf([], null); + // The list is only allocated when there are dependent signals. + let dependentSignalsToAbort; // 4. For each dependentSignal of signal's dependent signals: - signal[kDependantSignals]?.forEach((s) => { - const dependentSignal = s.deref(); - // 1. If dependentSignal is not aborted, then: - if (dependentSignal && !dependentSignal[kAborted]) { - // 1. Set dependentSignal's abort reason to signal's abort reason. - dependentSignal[kReason] = reason; - dependentSignal[kAborted] = true; - // 2. Append dependentSignal to dependentSignalsToAbort. - ArrayPrototypePush(dependentSignalsToAbort, dependentSignal); - } - }); + const dependantSignals = signal[kDependantSignals]; + if (dependantSignals !== undefined && dependantSignals.size > 0) { + dependentSignalsToAbort = ObjectSetPrototypeOf([], null); + dependantSignals.forEach((s) => { + const dependentSignal = s.deref(); + // 1. If dependentSignal is not aborted, then: + if (dependentSignal && !dependentSignal[kAborted]) { + // 1. Set dependentSignal's abort reason to signal's abort reason. + dependentSignal[kReason] = reason; + dependentSignal[kAborted] = true; + // 2. Append dependentSignal to dependentSignalsToAbort. + ArrayPrototypePush(dependentSignalsToAbort, dependentSignal); + } + }); + } // 5. Run the abort steps for signal runAbort(signal); // 6. For each dependentSignal of dependentSignalsToAbort, // run the abort steps for dependentSignal. - for (let i = 0; i < dependentSignalsToAbort.length; i++) { - const dependentSignal = dependentSignalsToAbort[i]; - runAbort(dependentSignal); - // A transitively-aborted dependent signal can never abort again, so there - // is no longer any reason to keep it alive. Dropping it from - // gcPersistentSignals lets it be collected, which in turn prunes its - // WeakRef from its sources' kDependantSignals sets. Otherwise, an observed - // composite that follows a long-lived source stays retained forever and - // its entry accumulates in that source's kDependantSignals. - gcPersistentSignals.delete(dependentSignal); + if (dependentSignalsToAbort !== undefined) { + for (let i = 0; i < dependentSignalsToAbort.length; i++) { + const dependentSignal = dependentSignalsToAbort[i]; + runAbort(dependentSignal); + // A transitively-aborted dependent signal can never abort again, so there + // is no longer any reason to keep it alive. Dropping it from + // gcPersistentSignals lets it be collected, which in turn prunes its + // WeakRef from its sources' kDependantSignals sets. Otherwise, an observed + // composite that follows a long-lived source stays retained forever and + // its entry accumulates in that source's kDependantSignals. + gcPersistentSignals.delete(dependentSignal); + } } // Clean up the signal from gcPersistentSignals @@ -557,9 +618,13 @@ function abortSignal(signal, reason) { // To run the abort steps for an AbortSignal signal function runAbort(signal) { - const event = new Event('abort', { - [kTrustEvent]: true, - }); + // Firing an event at a signal without abort listeners has no observable + // effect, so skip allocating and dispatching it. + const root = signal[kEvents]?.get('abort'); + if (root === undefined || root.next === undefined) { + return; + } + const event = new Event('abort', kTrustedEventInit); signal.dispatchEvent(event); } @@ -578,7 +643,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); } @@ -590,7 +655,7 @@ class AbortController { static [kMakeTransferable]() { const controller = new AbortController(); - controller.#signal = new AbortSignal(kDontThrowSymbol, { transferable: true }); + controller.#signal = new AbortSignal(kDontThrowSymbol, false, undefined, true); return controller; } } diff --git a/lib/internal/event_target.js b/lib/internal/event_target.js index 69cdc4aac5b9..c5f104085132 100644 --- a/lib/internal/event_target.js +++ b/lib/internal/event_target.js @@ -17,7 +17,6 @@ const { SafeSet, SafeWeakMap, SafeWeakRef, - SafeWeakSet, String, Symbol, SymbolFor, @@ -79,19 +78,7 @@ const kType = Symbol('type'); const kDetail = Symbol('detail'); const kInPassiveListener = Symbol('kInPassiveListener'); -const isTrustedSet = new SafeWeakSet(); -const isTrusted = ObjectGetOwnPropertyDescriptor({ - get isTrusted() { - return isTrustedSet.has(this); - }, -}, 'isTrusted').get; - -const isTrustedDescriptor = { - __proto__: null, - configurable: false, - enumerable: true, - get: isTrusted, -}; +let isTrusted; function isEvent(value) { return typeof value?.[kType] === 'string'; @@ -104,6 +91,16 @@ class Event { #defaultPrevented = false; #timestamp = now(); #propagationStopped = false; + #isTrusted = false; + + static { + isTrusted = ObjectGetOwnPropertyDescriptor({ + get isTrusted() { + return typeof this === 'object' && this !== null && + #isTrusted in this && this.#isTrusted; + }, + }, 'isTrusted').get; + } /** * @param {string} type @@ -124,7 +121,7 @@ class Event { this[kType] = `${type}`; if (options?.[kTrustEvent]) { - isTrustedSet.add(this); + this.#isTrusted = true; } this[kTarget] = null; @@ -367,7 +364,12 @@ ObjectDefineProperties( // LegacyUnforgeable but defining it in the constructor has a big // performance impact and the property doesn't seem to be useful outside of // browsers. - isTrusted: isTrustedDescriptor, + isTrusted: { + __proto__: null, + configurable: false, + enumerable: true, + get: isTrusted, + }, }); const staticProps = ['NONE', 'CAPTURING_PHASE', 'AT_TARGET', 'BUBBLING_PHASE']; @@ -481,9 +483,7 @@ class Listener { flags |= kFlagResistStopPropagation; this.flags = flags; - this.removed = false; - - if (this.weak) { + if (weak) { this.callback = new SafeWeakRef(listener); weakListeners().registry.register(listener, { __proto__: null, @@ -545,8 +545,9 @@ class Listener { } same(listener, capture) { - const myListener = this.weak ? this.listener.deref() : this.listener; - return myListener === listener && this.capture === capture; + const flags = this.flags; + const myListener = (flags & kFlagWeak) !== 0 ? this.listener.deref() : this.listener; + return myListener === listener && ((flags & kFlagCapture) !== 0) === capture; } remove() { @@ -554,8 +555,8 @@ class Listener { this.previous.next = this.next; if (this.next !== undefined) this.next.previous = this.previous; - this.removed = true; - if (this.weak) { + this.flags |= kFlagRemoved; + if ((this.flags & kFlagWeak) !== 0) { weakListeners().registry.unregister(this); const weakKey = this.weakKeyRef?.deref(); const listener = this.callback?.deref(); @@ -646,28 +647,31 @@ class EventTarget { let weak = false; let resistStopPropagation = false; - if (options !== kEmptyObject) { - // We validateOptions before the validateListener check because the spec - // requires us to hit getters. - options = validateEventListenerOptions(options); - - once = options.once; - capture = options.capture; - passive = options.passive; - isNodeStyleListener = options.isNodeStyleListener; - weak = options.weak; - resistStopPropagation = options.resistStopPropagation; - + // We flatten the options before the validateListener check because the + // spec requires us to hit getters. + if (typeof options === 'boolean') { + capture = options; + } else if (options !== kEmptyObject && options !== null) { + validateObject(options, 'options', kValidateObjectAllowObjects); + once = Boolean(options.once); + capture = Boolean(options.capture); + passive = Boolean(options.passive); const signal = options.signal; + weak = options[kWeakHandler]; + resistStopPropagation = options[kResistStopPropagation] ?? false; + isNodeStyleListener = Boolean(options[kIsNodeStyleListener]); - validateAbortSignal(signal, 'options.signal'); + if (signal !== undefined) { + validateAbortSignal(signal, 'options.signal'); - if (signal) { if (signal.aborted) { return; } + // Capture a copy so that the closure doesn't move `capture` into the + // function context, which slows down the listener lookup below. + const removeCapture = capture; signal.addEventListener('abort', () => { - this.removeEventListener(type, listener, options); + this.removeEventListener(type, listener, removeCapture); }, { __proto__: null, once: true, [kWeakHandler]: this, [kResistStopPropagation]: true }); } } @@ -1106,24 +1110,6 @@ function validateEventListener(listener) { throw new ERR_INVALID_ARG_TYPE('listener', 'EventListener', listener); } -function validateEventListenerOptions(options) { - if (typeof options === 'boolean') - return { capture: options }; - - if (options === null) - return kEmptyObject; - validateObject(options, 'options', kValidateObjectAllowObjects); - return { - once: Boolean(options.once), - capture: Boolean(options.capture), - passive: Boolean(options.passive), - signal: options.signal, - weak: options[kWeakHandler], - resistStopPropagation: options[kResistStopPropagation] ?? false, - isNodeStyleListener: Boolean(options[kIsNodeStyleListener]), - }; -} - // Test whether the argument is an event object. This is far from a fool-proof // test, for example this input will result in a false positive: // > isEventTarget({ constructor: EventTarget }) diff --git a/test/fixtures/test-runner/output/abort.snapshot b/test/fixtures/test-runner/output/abort.snapshot index 72504832704c..2798e6e3a5a0 100644 --- a/test/fixtures/test-runner/output/abort.snapshot +++ b/test/fixtures/test-runner/output/abort.snapshot @@ -54,8 +54,6 @@ TAP version 13 error: 'This operation was aborted' code: 20 name: 'AbortError' - stack: |- - ... # Subtest: not ok 4 not ok 8 - not ok 4 @@ -67,8 +65,6 @@ TAP version 13 error: 'This operation was aborted' code: 20 name: 'AbortError' - stack: |- - ... # Subtest: not ok 5 not ok 9 - not ok 5 @@ -80,8 +76,6 @@ TAP version 13 error: 'This operation was aborted' code: 20 name: 'AbortError' - stack: |- - ... 1..9 not ok 1 - promise timeout signal @@ -106,10 +100,6 @@ not ok 2 - promise abort signal error: 'This operation was aborted' code: 20 name: 'AbortError' - stack: |- - - Object. (/test/fixtures/test-runner/output/abort.js:21:52) - ... # Subtest: callback timeout signal # Subtest: ok 1 @@ -166,8 +156,6 @@ not ok 2 - promise abort signal error: 'This operation was aborted' code: 20 name: 'AbortError' - stack: |- - ... # Subtest: not ok 4 not ok 8 - not ok 4 @@ -179,8 +167,6 @@ not ok 2 - promise abort signal error: 'This operation was aborted' code: 20 name: 'AbortError' - stack: |- - ... # Subtest: not ok 5 not ok 9 - not ok 5 @@ -192,8 +178,6 @@ not ok 2 - promise abort signal error: 'This operation was aborted' code: 20 name: 'AbortError' - stack: |- - ... 1..9 not ok 3 - callback timeout signal @@ -218,10 +202,6 @@ not ok 4 - callback abort signal error: 'This operation was aborted' code: 20 name: 'AbortError' - stack: |- - - Object. (/test/fixtures/test-runner/output/abort.js:39:53) - ... 1..4 # tests 22 diff --git a/test/fixtures/test-runner/output/abort_hooks.snapshot b/test/fixtures/test-runner/output/abort_hooks.snapshot index 10a9c189e788..15e4fa6bddc5 100644 --- a/test/fixtures/test-runner/output/abort_hooks.snapshot +++ b/test/fixtures/test-runner/output/abort_hooks.snapshot @@ -38,10 +38,6 @@ not ok 1 - 1 before describe error: 'This operation was aborted' code: 20 name: 'AbortError' - stack: |- - - before.signal (/test/fixtures/test-runner/output/abort_hooks.js:8:8) - ... # Subtest: 2 after describe # Subtest: test 1 @@ -66,10 +62,6 @@ not ok 2 - 2 after describe error: 'This operation was aborted' code: 20 name: 'AbortError' - stack: |- - - after.signal (/test/fixtures/test-runner/output/abort_hooks.js:23:8) - ... # Subtest: 3 beforeEach describe # Subtest: test 1 @@ -82,10 +74,6 @@ not ok 2 - 2 after describe error: 'This operation was aborted' code: 20 name: 'AbortError' - stack: |- - - beforeEach.signal (/test/fixtures/test-runner/output/abort_hooks.js:38:8) - ... # Subtest: test 2 not ok 2 - test 2 @@ -97,10 +85,6 @@ not ok 2 - 2 after describe error: 'This operation was aborted' code: 20 name: 'AbortError' - stack: |- - - beforeEach.signal (/test/fixtures/test-runner/output/abort_hooks.js:38:8) - ... 1..2 not ok 3 - 3 beforeEach describe @@ -123,10 +107,6 @@ not ok 3 - 3 beforeEach describe error: 'This operation was aborted' code: 20 name: 'AbortError' - stack: |- - - afterEach.signal (/test/fixtures/test-runner/output/abort_hooks.js:53:8) - ... # Subtest: test 2 not ok 2 - test 2 @@ -138,10 +118,6 @@ not ok 3 - 3 beforeEach describe error: 'This operation was aborted' code: 20 name: 'AbortError' - stack: |- - - afterEach.signal (/test/fixtures/test-runner/output/abort_hooks.js:53:8) - ... 1..2 not ok 4 - 4 afterEach describe diff --git a/test/fixtures/test-runner/output/abort_suite.snapshot b/test/fixtures/test-runner/output/abort_suite.snapshot index 0f1a1710c77d..acbc7fab73ea 100644 --- a/test/fixtures/test-runner/output/abort_suite.snapshot +++ b/test/fixtures/test-runner/output/abort_suite.snapshot @@ -54,8 +54,6 @@ TAP version 13 error: 'This operation was aborted' code: 20 name: 'AbortError' - stack: |- - ... # Subtest: not ok 4 not ok 8 - not ok 4 @@ -67,8 +65,6 @@ TAP version 13 error: 'This operation was aborted' code: 20 name: 'AbortError' - stack: |- - ... # Subtest: not ok 5 not ok 9 - not ok 5 @@ -80,8 +76,6 @@ TAP version 13 error: 'This operation was aborted' code: 20 name: 'AbortError' - stack: |- - ... 1..9 not ok 1 - describe timeout signal @@ -106,10 +100,6 @@ not ok 2 - describe abort signal error: 'This operation was aborted' code: 20 name: 'AbortError' - stack: |- - - Object. (/test/fixtures/test-runner/output/abort_suite.js:19:57) - ... 1..2 # tests 9 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..7e004dd80651 --- /dev/null +++ b/test/parallel/test-abortcontroller-default-reason-gc.mjs @@ -0,0 +1,66 @@ +// 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. It captures no stack at all. 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.ok(signal.reason instanceof DOMException); + assert.strictEqual(signal.reason.name, 'AbortError'); + assert.strictEqual(signal.reason.message, 'This operation was aborted'); + assert.doesNotMatch(signal.reason.stack, /\n\s+at /); + }); +} + +it('creates a new default reason for every abort', () => { + assert.notStrictEqual(new Job().cancel().reason, new Job().cancel().reason); + assert.notStrictEqual(AbortSignal.abort().reason, AbortSignal.abort().reason); +}); + +it('restores Error.stackTraceLimit', () => { + const { stackTraceLimit } = Error; + Error.stackTraceLimit = 7; + try { + new Job().cancel(); + new Job().cancelStatic(); + assert.strictEqual(Error.stackTraceLimit, 7); + } finally { + Error.stackTraceLimit = stackTraceLimit; + } +}); + +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; + } +}); diff --git a/test/parallel/test-abortsignal-any.mjs b/test/parallel/test-abortsignal-any.mjs index ff5226055b86..ddc8fb5691c6 100644 --- a/test/parallel/test-abortsignal-any.mjs +++ b/test/parallel/test-abortsignal-any.mjs @@ -75,6 +75,25 @@ describe('AbortSignal.any()', { concurrency: !process.env.TEST_PARALLEL }, () => assert.strictEqual(signal.reason, 'reason'); }); + it('aborts every signal that shares a source', () => { + const controller = new AbortController(); + const other = new AbortController(); + const signals = [ + AbortSignal.any([controller.signal]), + AbortSignal.any([other.signal, controller.signal]), + AbortSignal.any([AbortSignal.any([controller.signal]), other.signal]), + ]; + for (const signal of signals) { + signal.addEventListener('abort', common.mustCall()); + } + controller.abort('reason'); + for (const signal of signals) { + assert.ok(signal.aborted); + assert.strictEqual(signal.reason, 'reason'); + } + other.abort('other reason'); + }); + it('handles deeply aborted signals', async () => { const controllers = Array.from({ length: 2 }, () => new AbortController()); const composedSignal1 = AbortSignal.any([controllers[0].signal]); diff --git a/test/parallel/test-abortsignal-drop-settled-signals.mjs b/test/parallel/test-abortsignal-drop-settled-signals.mjs index 03e0146722ec..4f9015c2bb49 100644 --- a/test/parallel/test-abortsignal-drop-settled-signals.mjs +++ b/test/parallel/test-abortsignal-drop-settled-signals.mjs @@ -271,6 +271,23 @@ it('drops an observed composite of a composite once its sources are collected', await gcUntil('outer composite is collected', () => outerRef.deref() === undefined); }); +it('drops observed composites that share a source once it is collected', async () => { + const handler = () => {}; + // Using a helper keeps the source and the composites off this stack. + const createObservedComposites = () => { + const controller = new AbortController(); + return Array.from({ length: 10 }, () => { + const composite = AbortSignal.any([controller.signal]); + composite.addEventListener('abort', handler); + return new WeakRef(composite); + }); + }; + const refs = createObservedComposites(); + + await gcUntil('observed composites are collected', + () => refs.every((ref) => ref.deref() === undefined)); +}); + it('drops settled signals even when there are listeners', (t, done) => { runWithOrphanListeners(limit, async (signalRefs) => { await gcUntil('all signals are GCed', () => { diff --git a/test/parallel/test-eventtarget.js b/test/parallel/test-eventtarget.js index 558ee58f8be5..60d55badeb9e 100644 --- a/test/parallel/test-eventtarget.js +++ b/test/parallel/test-eventtarget.js @@ -779,3 +779,34 @@ let asyncTest = Promise.resolve(); new Event('', null); new Event('', undefined); } + +{ + // Aborting the signal option only removes the listener that was registered + // with the same capture flag. + const et = new EventTarget(); + const controller = new AbortController(); + let calls = 0; + const listener = () => calls++; + et.addEventListener('foo', listener, { capture: true, signal: controller.signal }); + et.addEventListener('foo', listener, false); + et.dispatchEvent(new Event('foo')); + assert.strictEqual(calls, 2); + controller.abort(); + et.dispatchEvent(new Event('foo')); + assert.strictEqual(calls, 3); + et.removeEventListener('foo', listener); + et.dispatchEvent(new Event('foo')); + assert.strictEqual(calls, 3); +} + +{ + // The isTrusted getter returns false for any receiver that is not a trusted + // event, without throwing. + const { get } = Object.getOwnPropertyDescriptor(Event.prototype, 'isTrusted'); + for (const value of [ + undefined, null, 1, 'foo', Symbol('foo'), {}, () => {}, + { __proto__: Event.prototype }, new Event('foo'), new CustomEvent('foo'), + ]) { + assert.strictEqual(get.call(value), false); + } +}