From 832ee65daf887693e8ffd8016f571253a629dd34 Mon Sep 17 00:00:00 2001 From: Chemi Atlow Date: Thu, 22 Jun 2023 16:38:02 +0300 Subject: [PATCH] lib: add option to force handling stopped events PR-URL: https://github.com/nodejs/node/pull/48301 Reviewed-By: Benjamin Gruenbaum Reviewed-By: Moshe Atlow --- lib/events.js | 11 +++++++--- lib/internal/event_target.js | 34 +++++++++++++++++++++++-------- test/parallel/test-events-once.js | 13 ++++++++++++ test/parallel/test-eventtarget.js | 12 +++++++++++ 4 files changed, 59 insertions(+), 11 deletions(-) diff --git a/lib/events.js b/lib/events.js index f0373a6e938ef1..241b67b54a8932 100644 --- a/lib/events.js +++ b/lib/events.js @@ -59,6 +59,7 @@ const { } = require('internal/util/inspect'); let spliceOne; +let kResistStopPropagation; const { AbortError, @@ -981,7 +982,10 @@ async function once(emitter, name, options = kEmptyObject) { } resolve(args); }; - eventTargetAgnosticAddListener(emitter, name, resolver, { once: true }); + + kResistStopPropagation ??= require('internal/event_target').kResistStopPropagation; + const opts = { __proto__: null, once: true, [kResistStopPropagation]: true }; + eventTargetAgnosticAddListener(emitter, name, resolver, opts); if (name !== 'error' && typeof emitter.once === 'function') { // EventTarget does not have `error` event semantics like Node // EventEmitters, we listen to `error` events only on EventEmitters. @@ -994,7 +998,7 @@ async function once(emitter, name, options = kEmptyObject) { } if (signal != null) { eventTargetAgnosticAddListener( - signal, 'abort', abortListener, { once: true }); + signal, 'abort', abortListener, { __proto__: null, once: true, [kResistStopPropagation]: true }); } }); } @@ -1119,11 +1123,12 @@ function on(emitter, event, options = kEmptyObject) { } if (signal) { + kResistStopPropagation ??= require('internal/event_target').kResistStopPropagation; eventTargetAgnosticAddListener( signal, 'abort', abortListener, - { once: true }); + { __proto__: null, once: true, [kResistStopPropagation]: true }); } function abortListener() { diff --git a/lib/internal/event_target.js b/lib/internal/event_target.js index 4f6870fa19040c..42b810679dadf7 100644 --- a/lib/internal/event_target.js +++ b/lib/internal/event_target.js @@ -59,6 +59,7 @@ const kStop = Symbol('kStop'); const kTarget = Symbol('kTarget'); const kHandlers = Symbol('kHandlers'); const kWeakHandler = Symbol('kWeak'); +const kResistStopPropagation = Symbol('kResistStopPropagation'); const kHybridDispatch = SymbolFor('nodejs.internal.kHybridDispatch'); const kCreateEvent = Symbol('kCreateEvent'); @@ -403,6 +404,7 @@ const kFlagPassive = 1 << 2; const kFlagNodeStyle = 1 << 3; const kFlagWeak = 1 << 4; const kFlagRemoved = 1 << 5; +const kFlagResistStopPropagation = 1 << 6; // The listeners for an EventTarget are maintained as a linked list. // Unfortunately, the way EventTarget is defined, listeners are accounted @@ -413,7 +415,7 @@ const kFlagRemoved = 1 << 5; // slower. class Listener { constructor(previous, listener, once, capture, passive, - isNodeStyleListener, weak) { + isNodeStyleListener, weak, resistStopPropagation) { this.next = undefined; if (previous !== undefined) previous.next = this; @@ -431,6 +433,8 @@ class Listener { flags |= kFlagNodeStyle; if (weak) flags |= kFlagWeak; + if (resistStopPropagation) + flags |= kFlagResistStopPropagation; this.flags = flags; this.removed = false; @@ -468,6 +472,9 @@ class Listener { get weak() { return Boolean(this.flags & kFlagWeak); } + get resistStopPropagation() { + return Boolean(this.flags & kFlagResistStopPropagation); + } get removed() { return Boolean(this.flags & kFlagRemoved); } @@ -564,6 +571,7 @@ class EventTarget { signal, isNodeStyleListener, weak, + resistStopPropagation, } = validateEventListenerOptions(options); if (!validateEventListener(listener)) { @@ -588,16 +596,16 @@ class EventTarget { // not prevent the event target from GC. signal.addEventListener('abort', () => { this.removeEventListener(type, listener, options); - }, { once: true, [kWeakHandler]: this }); + }, { __proto__: null, once: true, [kWeakHandler]: this, [kResistStopPropagation]: true }); } let root = this[kEvents].get(type); if (root === undefined) { - root = { size: 1, next: undefined }; + root = { size: 1, next: undefined, resistStopPropagation: Boolean(resistStopPropagation) }; // This is the first handler in our linked list. new Listener(root, listener, once, capture, passive, - isNodeStyleListener, weak); + isNodeStyleListener, weak, resistStopPropagation); this[kNewListener]( root.size, type, @@ -624,8 +632,9 @@ class EventTarget { } new Listener(previous, listener, once, capture, passive, - isNodeStyleListener, weak); + isNodeStyleListener, weak, resistStopPropagation); root.size++; + root.resistStopPropagation ||= Boolean(resistStopPropagation); this[kNewListener](root.size, type, listener, once, capture, passive, weak); } @@ -709,14 +718,21 @@ class EventTarget { let handler = root.next; let next; - while (handler !== undefined && - (handler.passive || event?.[kStop] !== true)) { + const iterationCondition = () => { + if (handler === undefined) { + return false; + } + return root.resistStopPropagation || handler.passive || event?.[kStop] !== true; + }; + while (iterationCondition()) { // Cache the next item in case this iteration removes the current one next = handler.next; - if (handler.removed) { + if (handler.removed || (event?.[kStop] === true && !handler.resistStopPropagation)) { // Deal with the case an event is removed while event handlers are // Being processed (removeEventListener called from a listener) + // And the case of event.stopImmediatePropagation() being called + // For events not flagged as resistStopPropagation handler = next; continue; } @@ -984,6 +1000,7 @@ function validateEventListenerOptions(options) { passive: Boolean(options.passive), signal: options.signal, weak: options[kWeakHandler], + resistStopPropagation: options[kResistStopPropagation] ?? false, isNodeStyleListener: Boolean(options[kIsNodeStyleListener]), }; } @@ -1099,5 +1116,6 @@ module.exports = { kRemoveListener, kEvents, kWeakHandler, + kResistStopPropagation, isEventTarget, }; diff --git a/test/parallel/test-events-once.js b/test/parallel/test-events-once.js index 5ae5461d92676c..be05028faaf0c2 100644 --- a/test/parallel/test-events-once.js +++ b/test/parallel/test-events-once.js @@ -233,6 +233,18 @@ async function eventTargetAbortSignalBefore() { }); } +async function eventTargetAbortSignalBeforeEvenWhenSignalPropagationStopped() { + const et = new EventTarget(); + const ac = new AbortController(); + const { signal } = ac; + signal.addEventListener('abort', (e) => e.stopImmediatePropagation(), { once: true }); + + process.nextTick(() => ac.abort()); + return rejects(once(et, 'foo', { signal }), { + name: 'AbortError', + }); +} + async function eventTargetAbortSignalAfter() { const et = new EventTarget(); const ac = new AbortController(); @@ -270,6 +282,7 @@ Promise.all([ abortSignalAfterEvent(), abortSignalRemoveListener(), eventTargetAbortSignalBefore(), + eventTargetAbortSignalBeforeEvenWhenSignalPropagationStopped(), eventTargetAbortSignalAfter(), eventTargetAbortSignalAfterEvent(), ]).then(common.mustCall()); diff --git a/test/parallel/test-eventtarget.js b/test/parallel/test-eventtarget.js index fd10ceee49c3df..31446e216444ff 100644 --- a/test/parallel/test-eventtarget.js +++ b/test/parallel/test-eventtarget.js @@ -717,3 +717,15 @@ let asyncTest = Promise.resolve(); et.removeEventListener(Symbol('symbol'), () => {}); }, TypeError); } + +{ + // Test that event listeners are removed by signal even when + // signal's abort event propagation stopped + const controller = new AbortController(); + const { signal } = controller; + signal.addEventListener('abort', (e) => e.stopImmediatePropagation(), { once: true }); + const et = new EventTarget(); + et.addEventListener('foo', common.mustNotCall(), { signal }); + controller.abort(); + et.dispatchEvent(new Event('foo')); +}