Skip to content

events.once leaks signal listener #43337

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.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    confirmed-bugIssues and PRs for confirmed bugs.eventsIssues and PRs related to EventEmitter and the events module.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions