Skip to content

Commit

Permalink
lib: add option to force handling stopped events
Browse files Browse the repository at this point in the history
PR-URL: #48301
Backport-PR-URL: #49587
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
  • Loading branch information
atlowChemi authored and ruyadorno committed Sep 12, 2023
1 parent 56249b0 commit db355d1
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 11 deletions.
11 changes: 8 additions & 3 deletions lib/events.js
Expand Up @@ -59,6 +59,7 @@ const {
} = require('internal/util/inspect');

let spliceOne;
let kResistStopPropagation;

const {
AbortError,
Expand Down Expand Up @@ -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.
Expand All @@ -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 });
}
});
}
Expand Down Expand Up @@ -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() {
Expand Down
34 changes: 26 additions & 8 deletions lib/internal/event_target.js
Expand Up @@ -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');
Expand Down Expand Up @@ -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
Expand All @@ -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;
Expand All @@ -431,6 +433,8 @@ class Listener {
flags |= kFlagNodeStyle;
if (weak)
flags |= kFlagWeak;
if (resistStopPropagation)
flags |= kFlagResistStopPropagation;
this.flags = flags;

this.removed = false;
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -564,6 +571,7 @@ class EventTarget {
signal,
isNodeStyleListener,
weak,
resistStopPropagation,
} = validateEventListenerOptions(options);

if (!validateEventListener(listener)) {
Expand All @@ -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,
Expand All @@ -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);
}

Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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]),
};
}
Expand Down Expand Up @@ -1099,5 +1116,6 @@ module.exports = {
kRemoveListener,
kEvents,
kWeakHandler,
kResistStopPropagation,
isEventTarget,
};
13 changes: 13 additions & 0 deletions test/parallel/test-events-once.js
Expand Up @@ -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();
Expand Down Expand Up @@ -270,6 +282,7 @@ Promise.all([
abortSignalAfterEvent(),
abortSignalRemoveListener(),
eventTargetAbortSignalBefore(),
eventTargetAbortSignalBeforeEvenWhenSignalPropagationStopped(),
eventTargetAbortSignalAfter(),
eventTargetAbortSignalAfterEvent(),
]).then(common.mustCall());
12 changes: 12 additions & 0 deletions test/parallel/test-eventtarget.js
Expand Up @@ -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'));
}

0 comments on commit db355d1

Please sign in to comment.