Comment by lovasoa | Hacker News Reader

Comment by lovasoa | Hacker News Reader

Parent Full thread

I think the bug in this code is the interface that it exposes. If a listener A removes another listener B during the dispatch of an event, then

- if B was added after A, it will not receive the current event

- if A was added after B, then B has already received the event by the time it is removed.

If I encountered that during a code-review, I would be very suspicious.

The right thing to do here is probably the more simple thing that the AI also seems to be suggesting :

    for (const listener of this.listeners) listener(arg);

Your method of event dispatch is very common. NodeJS does it this way, as well as many other event dispatchers. It is also how I use to do it until I encountered a very nasty bug in Chrome DevTools.

Object A emits events.

Object B subscribes to A, and manages the lifetime of Object C

Object C subscribes to A in its constructor, and unsubscribes in its dispose function.

With the common event listener model, Object C could have its methods called after dispose was called, even though it appeared to clean up its event listener!

You can check out the DOM spec for the correct behavior, which both clones the event listeners arrray and sets a removed flag on them in case they were removed after cloning.

https://dom.spec.whatwg.org/#concept-event-listener-invoke

Reply on news.ycombinator.com