Skip to content

events.once leaks signal listener #43337

Closed
@breavyn

Description

@breavyn

Version

v18.3.0

Platform

Linux

Subsystem

events

What steps will reproduce the bug?

const { EventEmitter, once } = require("events");

(async () => {
  const controller = new AbortController();
  const ee = new EventEmitter();

  setTimeout(() => {
    controller.abort();
  }, 500);

  while (true) {
    setTimeout(() => ee.emit("test"), 100);
    await once(ee, "test", { signal: controller.signal });

    console.dir(controller.signal);
  }
})();

How often does it reproduce? Is there a required condition?

No response

What is the expected behavior?

No response

What do you see instead?

The signal abort listeners do not get removed.

AbortSignal {
  [Symbol(kEvents)]: SafeMap(1) [Map] { 'abort' => { size: 1, next: [Listener] } },
  [Symbol(events.maxEventTargetListeners)]: 10,
  [Symbol(events.maxEventTargetListenersWarned)]: false,
  [Symbol(kAborted)]: false,
  [Symbol(kReason)]: undefined
}
AbortSignal {
  [Symbol(kEvents)]: SafeMap(1) [Map] { 'abort' => { size: 2, next: [Listener] } },
  [Symbol(events.maxEventTargetListeners)]: 10,
  [Symbol(events.maxEventTargetListenersWarned)]: false,
  [Symbol(kAborted)]: false,
  [Symbol(kReason)]: undefined
}
AbortSignal {
  [Symbol(kEvents)]: SafeMap(1) [Map] { 'abort' => { size: 3, next: [Listener] } },
  [Symbol(events.maxEventTargetListeners)]: 10,
  [Symbol(events.maxEventTargetListenersWarned)]: false,
  [Symbol(kAborted)]: false,
  [Symbol(kReason)]: undefined
}
AbortSignal {
  [Symbol(kEvents)]: SafeMap(1) [Map] { 'abort' => { size: 4, next: [Listener] } },
  [Symbol(events.maxEventTargetListeners)]: 10,
  [Symbol(events.maxEventTargetListenersWarned)]: false,
  [Symbol(kAborted)]: false,
  [Symbol(kReason)]: undefined
}

Additional information

When listeners are added to EventTarget's in events.on and events.once, the listener gets wrapped. As AbortSignal is an EventTarget, this means the abort listener can never be removed.

node/lib/events.js

Lines 1011 to 1013 in b631086

// EventTarget does not have `error` event semantics like Node
// EventEmitters, we do not listen to `error` events here.
emitter.addEventListener(name, (arg) => { listener(arg); }, flags);

I confirmed that the listeners do get removed correctly when simply passing the listener in directly, however, I'm unsure if this will have any other consequences hinted at by the comment.

Metadata

Metadata

Assignees

No one assigned

    Labels

    confirmed-bugIssues with confirmed bugs.eventsIssues and PRs related to the events subsystem / EventEmitter.

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions