fix: prohibit retroactive subscriptions to published events

packages/messenger/CHANGELOG.md

@@ -13,6 +13,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   - Unlike `delegate`, this method requires all external actions and events to be listed, producing a TypeScript error showing exactly which items are missing.
 - Add `MessengerNamespace` utility type to extract the namespace from a Messenger type ([#8338](https://github.com/MetaMask/core/pull/8338))
 
+### Fixed
+
+- Prohibit retroactive delivery of published events: a handler subscribed while an event is being published no longer receives the in-flight event ([#9773](https://github.com/MetaMask/core/pull/9773))
+  - The subscriber collection is now snapshotted before it is iterated during publish, so only handlers registered at the time the event was published are called, matching the behavior of `EventEmitter`. Consequently, a handler unsubscribed while an event is being published still receives the in-flight event.
+
 ## [2.0.0]
 
 ### Added

packages/messenger/src/Messenger.test.ts

@@ -583,6 +583,64 @@ describe('Messenger', () => {
       expect(handler2.mock.calls).toHaveLength(1);
     });
 
+    it('does not publish event to subscriber that was added during publish', () => {
+      type MessageEvent = { type: 'Fixture:message'; payload: [string] };
+      const messenger = new Messenger<'Fixture', never, MessageEvent>({
+        namespace: 'Fixture',
+      });
+
+      const addedDuringPublishHandler = jest.fn();
+      const handler = jest.fn(() => {
+        messenger.subscribe('Fixture:message', addedDuringPublishHandler);
+      });
+      messenger.subscribe('Fixture:message', handler);
+      messenger.publish('Fixture:message', 'hello');
+
+      expect(handler.mock.calls).toHaveLength(1);
+      expect(addedDuringPublishHandler).not.toHaveBeenCalled();
+    });
+
+    it('publishes subsequent event to subscriber that was added during publish of an earlier event', () => {
+      type MessageEvent = { type: 'Fixture:message'; payload: [string] };
+      const messenger = new Messenger<'Fixture', never, MessageEvent>({
+        namespace: 'Fixture',
+      });
+
+      const addedDuringPublishHandler = jest.fn();
+      const handler = jest.fn(() => {
+        messenger.subscribe('Fixture:message', addedDuringPublishHandler);
+      });
+      messenger.subscribe('Fixture:message', handler);
+      messenger.publish('Fixture:message', 'hello');
+      messenger.unsubscribe('Fixture:message', handler);
+      messenger.publish('Fixture:message', 'there');
+
+      expect(addedDuringPublishHandler).toHaveBeenCalledWith('there');
+      expect(addedDuringPublishHandler.mock.calls).toHaveLength(1);
+    });
+
+    it('publishes event to subscriber that was removed during publish of the same event', () => {
+      type MessageEvent = { type: 'Fixture:message'; payload: [string] };
+      const messenger = new Messenger<'Fixture', never, MessageEvent>({
+        namespace: 'Fixture',
+      });
+
+      const removedDuringPublishHandler = jest.fn();
+      const handler = jest.fn(() => {
+        messenger.unsubscribe('Fixture:message', removedDuringPublishHandler);
+      });
+      messenger.subscribe('Fixture:message', handler);
+      messenger.subscribe('Fixture:message', removedDuringPublishHandler);
+      messenger.publish('Fixture:message', 'hello');
+
+      expect(removedDuringPublishHandler).toHaveBeenCalledWith('hello');
+      expect(removedDuringPublishHandler.mock.calls).toHaveLength(1);
+
+      messenger.publish('Fixture:message', 'there');
+
+      expect(removedDuringPublishHandler.mock.calls).toHaveLength(1);
+    });
+
     describe('on first state change with an initial payload function registered', () => {
       it('publishes event if selected payload differs', () => {
         const state = {

packages/messenger/src/Messenger.ts

@@ -652,7 +652,12 @@ export class Messenger<
     const subscribers = this.#events.get(eventType);
 
     if (subscribers) {
-      for (const [handler, { selector }] of subscribers.entries()) {
+      // The subscriber collection is snapshotted before iterating so that
+      // mutating it during publish (e.g. a handler subscribing or
+      // unsubscribing) does not affect which handlers are called for this
+      // event. Only the handlers registered at the time the event was
+      // published are called, matching the behavior of `EventEmitter`.
+      for (const [handler, { selector }] of [...subscribers.entries()]) {
         try {
           if (selector) {
             const previousValue = this.#eventPayloadCache.get(handler);