ee-safe: removing a listener during emit throws \"ev[i] is not a function\" (and can skip listeners)

[bhousel]

Summary

In the ee-safe emitter, if a listener removes another listener for the same event while it is being dispatched, emit throws TypeError: ev[i] is not a function. In other cases it can silently skip listeners that should have been called.

This is a common pattern: a listener that, as a side effect, tears down other subscriptions (e.g. entering a new state/mode during pointer handling that unbinds sibling handlers).

Observed with tseep@1.3.1, and the same code path is present on master.

Root cause

emit caches the listener-array length once, then indexes the live array:

// src/ee-safe.ts
function emit(this: EventEmitter, event, a, b, c, d, e) {
  const ev = this.events[event];
  if (ev) {
    if (ev.length === 0) return false;
    if (arguments.length < 6) {
      for (let i = 0, len = ev.length; i < len; ++i) {
        ev[i](a, b, c, d, e);   // <-- ev[i] can be undefined mid-loop
      }
    }
    ...

removeListener mutates that same array in place via _fast_remove_single (shift / splice / arr.length--):

function removeListener(this, event, listener) {
  const evt = this.events[event];
  if (evt) {
    _fast_remove_single(evt, evt.indexOf(listener));
  }
  ...
}

So when a listener invoked during emit removes listeners from the same event:

• The array shrinks, but the cached len does not, so trailing iterations read ev[i] === undefined → ev[i] is not a function.
• Because removal shifts later elements down by one, a not-yet-called listener can be moved into an index the loop has already passed, so it is skipped entirely (a silent correctness bug, even when no crash occurs).

emitHasOnce has the same loop and the same problem.

Despite the ee-safe name, the loop is only safe against this.events[event] being reassigned during dispatch, not against the array being mutated in place.

Minimal reproduction

const { EventEmitter } = require('tseep/lib/ee-safe');

const ee = new EventEmitter();
const a = () => {};
const b = () => {};
const c = () => {};

// First listener removes two later listeners during dispatch
const first = () => {
  ee.off('evt', a);
  ee.off('evt', b);
};

ee.on('evt', first);
ee.on('evt', a);
ee.on('evt', b);
ee.on('evt', c);   // c should still fire

ee.emit('evt');    // throws: TypeError: ev[i] is not a function

Expected behavior

Node's EventEmitter fixes the listener set for the duration of a single emit: listeners added/removed during dispatch take effect on the next emit, and removed listeners still run for the in-flight event. emit should not throw or skip listeners when the listener list changes during dispatch.

Possible fix

Iterate a snapshot of the listener array:

const ev = this.events[event];
if (!ev || ev.length === 0) return false;
const snapshot = ev.slice();
for (let i = 0, len = snapshot.length; i < len; ++i) {
  snapshot[i](a, b, c, d, e);
}

This does add one array copy per emit. If the hot path must stay allocation-free, an alternative is to guard if (ev[i] !== undefined) and re-read ev.length each iteration, but that still has the shift-skip correctness issue, so a snapshot (or a copy-on-write when a removal happens mid-dispatch) is the more robust option.

Happy to send a PR if you'd like.

[Morglod]

Hi! Thank you for this report.

Yeah I'm thinking about copy-on-write or some kind of double buffering.