Merge pull request #12955 from CesiumGS/event-fix

Fixes event bug with changes to listeners during callback

Author: jjhembd

CHANGES.md

@@ -1,5 +1,13 @@
 # Change Log
 
+## 1.135 - 2025-11-01
+
+### @cesium/engine
+
+#### Fixes :wrench:
+
+- Fixes an event bug following recent changes, where adding a new listener during an event callback caused an infinite loop. [#12955](https://github.com/CesiumGS/cesium/pull/12955)
+
 ## 1.134 - 2025-10-01
 
 - [Sandcastle](https://sandcastle.cesium.com/) has been updated at `https://sandcastle.cesium.com`! The [legacy Sandcastle app](https://cesium.com/downloads/cesiumjs/releases/1.134/Apps/Sandcastle/index.html) will remain available through November 3, 2025.

packages/engine/Source/Core/Event.js

@@ -27,8 +27,17 @@ function Event() {
    * @private
    */
   this._listeners = new Map();
-  this._toRemove = [];
-  this._insideRaiseEvent = false;
+  /**
+   * @type {Map<Listener,Set<object>>}
+   * @private
+   */
+  this._toRemove = new Map();
+  /**
+   * @type {Map<Listener,Set<object>>}
+   * @private
+   */
+  this._toAdd = new Map();
+  this._invokingListeners = false;
   this._listenerCount = 0; // Tracks number of listener + scope pairs
 }
 
@@ -63,22 +72,34 @@ Event.prototype.addEventListener = function (listener, scope) {
   //>>includeStart('debug', pragmas.debug);
   Check.typeOf.func("listener", listener);
   //>>includeEnd('debug');
+  const event = this;
 
-  if (!this._listeners.has(listener)) {
-    this._listeners.set(listener, new Set());
-  }
-  const scopes = this._listeners.get(listener);
-  if (!scopes.has(scope)) {
-    scopes.add(scope);
-    this._listenerCount++;
+  const listenerMap = event._invokingListeners
+    ? event._toAdd
+    : event._listeners;
+  const added = addEventListener(this, listenerMap, listener, scope);
+  if (added) {
+    event._listenerCount++;
   }
 
-  const event = this;
   return function () {
     event.removeEventListener(listener, scope);
   };
 };
 
+function addEventListener(event, listenerMap, listener, scope) {
+  if (!listenerMap.has(listener)) {
+    listenerMap.set(listener, new Set());
+  }
+  const scopes = listenerMap.get(listener);
+  if (!scopes.has(scope)) {
+    scopes.add(scope);
+    return true;
+  }
+
+  return false;
+}
+
 /**
  * Unregisters a previously registered callback.
  *
@@ -94,23 +115,47 @@ Event.prototype.removeEventListener = function (listener, scope) {
   Check.typeOf.func("listener", listener);
   //>>includeEnd('debug');
 
-  const scopes = this._listeners.get(listener);
+  const removedFromListeners = removeEventListener(
+    this,
+    this._listeners,
+    listener,
+    scope,
+  );
+  const removedFromToAdd = removeEventListener(
+    this,
+    this._toAdd,
+    listener,
+    scope,
+  );
+
+  const removed = removedFromListeners || removedFromToAdd;
+  if (removed) {
+    this._listenerCount--;
+  }
+
+  return removed;
+};
+
+function removeEventListener(event, listenerMap, listener, scope) {
+  const scopes = listenerMap.get(listener);
   if (!scopes || !scopes.has(scope)) {
     return false;
   }
 
-  if (this._insideRaiseEvent) {
-    this._toRemove.push({ listener, scope });
+  if (event._invokingListeners) {
+    if (!addEventListener(event, event._toRemove, listener, scope)) {
+      // Already marked for removal
+      return false;
+    }
   } else {
     scopes.delete(scope);
     if (scopes.size === 0) {
-      this._listeners.delete(listener);
+      listenerMap.delete(listener);
     }
   }
 
-  this._listenerCount--;
   return true;
-};
+}
 
 /**
  * Raises the event by calling each registered listener with all supplied arguments.
@@ -121,7 +166,7 @@ Event.prototype.removeEventListener = function (listener, scope) {
  * @see Event#removeEventListener
  */
 Event.prototype.raiseEvent = function () {
-  this._insideRaiseEvent = true;
+  this._invokingListeners = true;
 
   for (const [listener, scopes] of this._listeners.entries()) {
     if (!defined(listener)) {
@@ -133,19 +178,23 @@ Event.prototype.raiseEvent = function () {
     }
   }
 
-  // Actually remove items marked for removal
-  if (this._toRemove.length > 0) {
-    for (const { listener, scope } of this._toRemove) {
-      const scopes = this._listeners.get(listener);
-      scopes.delete(scope);
-      if (scopes.size === 0) {
-        this._listeners.delete(listener);
-      }
+  this._invokingListeners = false;
+
+  // Actually add items marked for addition
+  for (const [listener, scopes] of this._toAdd.entries()) {
+    for (const scope of scopes) {
+      addEventListener(this, this._listeners, listener, scope);
     }
-    this._toRemove.length = 0;
   }
+  this._toAdd.clear();
 
-  this._insideRaiseEvent = false;
+  // Actually remove items marked for removal
+  for (const [listener, scopes] of this._toRemove.entries()) {
+    for (const scope of scopes) {
+      removeEventListener(this, this._listeners, listener, scope);
+    }
+  }
+  this._toRemove.clear();
 };
 
 /**

packages/engine/Specs/Core/EventSpec.js

@@ -91,6 +91,37 @@ describe("Core/Event", function () {
     expect(event.numberOfListeners).toEqual(5);
   });
 
+  it("can add a listener from within a callback", function () {
+    const doNothing = function (evt) {};
+
+    const addEventCb = function (evt) {
+      event.addEventListener(doNothing);
+    };
+
+    event.addEventListener(addEventCb);
+    event.raiseEvent();
+    expect(event.numberOfListeners).toEqual(2);
+
+    event.removeEventListener(doNothing);
+    event.removeEventListener(addEventCb);
+    expect(event.numberOfListeners).toEqual(0);
+  });
+
+  it("can add multiple listeners within a callback", function () {
+    const addEvent0 = function () {
+      event.addEventListener(function () {});
+    };
+    const addEvent1 = function () {
+      event.addEventListener(function () {});
+    };
+
+    event.addEventListener(addEvent0);
+    event.addEventListener(addEvent1);
+    expect(event.numberOfListeners).toEqual(2);
+    event.raiseEvent();
+    expect(event.numberOfListeners).toEqual(4);
+  });
+
   it("addEventListener and removeEventListener works with same function of different scopes", function () {
     const Scope = function () {
       this.timesCalled = 0;