Repository: sinonjs/fake-timers
Pull request #497 (2024-08-23 08:24:57)

Fix listener leak for timers/promises
#### Purpose (TL;DR)
Fix abort event listener leak discussed in #495 by adding `abortListenerMap` to keep track of active abort listeners.

#### Solution
The leak is fixed by adding a map with listener functions as keys and the abort signals as values. During uninstall it iterates over the map and removes the listeners. If a timer is aborted or finishes on its own the listener signal pair will be removed from the map.




Patch:
diff --git a/src/fake-timers-src.js b/src/fake-timers-src.js
index 9dc8540..825454d 100644
--- a/src/fake-timers-src.js
+++ b/src/fake-timers-src.js
@@ -99,6 +99,7 @@ if (typeof require === "function" && typeof module === "object") {
  * @property {boolean} [shouldClearNativeTimers] inherited from config
  * @property {{methodName:string, original:any}[] | undefined} timersModuleMethods
  * @property {{methodName:string, original:any}[] | undefined} timersPromisesModuleMethods
+ * @property {Map<function(): void, AbortSignal>} abortListenerMap
  */
 /* eslint-enable jsdoc/require-property-description */
 
@@ -967,6 +968,11 @@ function withGlobal(_global) {
         // Prevent multiple executions which will completely remove these props
         clock.methods = [];
 
+        for (const [listener, signal] of clock.abortListenerMap.entries()) {
+            signal.removeEventListener("abort", listener);
+            clock.abortListenerMap.delete(listener);
+        }
+
         // return pending timers, to enable checking what timers remained on uninstall
         if (!clock.timers) {
             return [];
@@ -1789,6 +1795,8 @@ function withGlobal(_global) {
             return uninstall(clock, config);
         };
 
+        clock.abortListenerMap = new Map();
+
         clock.methods = config.toFake || [];
 
         if (clock.methods.length === 0) {
@@ -1895,6 +1903,8 @@ function withGlobal(_global) {
                                     "abort",
                                     abort,
                                 );
+                                clock.abortListenerMap.delete(abort);
+
                                 // This is safe, there is no code path that leads to this function
                                 // being invoked before handle has been assigned.
                                 // eslint-disable-next-line no-use-before-define
@@ -1903,21 +1913,30 @@ function withGlobal(_global) {
                             };
 
                             const handle = clock.setTimeout(() => {
-                                options.signal?.removeEventListener(
-                                    "abort",
-                                    abort,
-                                );
+                                if (options.signal) {
+                                    options.signal.removeEventListener(
+                                        "abort",
+                                        abort,
+                                    );
+                                    clock.abortListenerMap.delete(abort);
+                                }
 
                                 resolve(value);
                             }, delay);
 
-                            if (options.signal?.aborted) {
-                                abort();
-                            } else {
-                                options.signal?.addEventListener(
-                                    "abort",
-                                    abort,
-                                );
+                            if (options.signal) {
+                                if (options.signal.aborted) {
+                                    abort();
+                                } else {
+                                    options.signal.addEventListener(
+                                        "abort",
+                                        abort,
+                                    );
+                                    clock.abortListenerMap.set(
+                                        abort,
+                                        options.signal,
+                                    );
+                                }
                             }
                         });
                 } else if (nameOfMethodToReplace === "setImmediate") {
@@ -1933,6 +1952,8 @@ function withGlobal(_global) {
                                     "abort",
                                     abort,
                                 );
+                                clock.abortListenerMap.delete(abort);
+
                                 // This is safe, there is no code path that leads to this function
                                 // being invoked before handle has been assigned.
                                 // eslint-disable-next-line no-use-before-define
@@ -1941,21 +1962,30 @@ function withGlobal(_global) {
                             };
 
                             const handle = clock.setImmediate(() => {
-                                options.signal?.removeEventListener(
-                                    "abort",
-                                    abort,
-                                );
+                                if (options.signal) {
+                                    options.signal.removeEventListener(
+                                        "abort",
+                                        abort,
+                                    );
+                                    clock.abortListenerMap.delete(abort);
+                                }
 
                                 resolve(value);
                             });
 
-                            if (options.signal?.aborted) {
-                                abort();
-                            } else {
-                                options.signal?.addEventListener(
-                                    "abort",
-                                    abort,
-                                );
+                            if (options.signal) {
+                                if (options.signal.aborted) {
+                                    abort();
+                                } else {
+                                    options.signal.addEventListener(
+                                        "abort",
+                                        abort,
+                                    );
+                                    clock.abortListenerMap.set(
+                                        abort,
+                                        options.signal,
+                                    );
+                                }
                             }
                         });
                 } else if (nameOfMethodToReplace === "setInterval") {
@@ -2000,6 +2030,8 @@ function withGlobal(_global) {
                                     "abort",
                                     abort,
                                 );
+                                clock.abortListenerMap.delete(abort);
+
                                 clock.clearInterval(handle);
                                 done = true;
                                 for (const resolvable of nextQueue) {
@@ -2007,13 +2039,19 @@ function withGlobal(_global) {
                                 }
                             };
 
-                            if (options.signal?.aborted) {
-                                done = true;
-                            } else {
-                                options.signal?.addEventListener(
-                                    "abort",
-                                    abort,
-                                );
+                            if (options.signal) {
+                                if (options.signal.aborted) {
+                                    done = true;
+                                } else {
+                                    options.signal.addEventListener(
+                                        "abort",
+                                        abort,
+                                    );
+                                    clock.abortListenerMap.set(
+                                        abort,
+                                        options.signal,
+                                    );
+                                }
                             }
 
                             return {
@@ -2065,10 +2103,13 @@ function withGlobal(_global) {
                                     clock.clearInterval(handle);
                                     done = true;
 
-                                    options.signal?.removeEventListener(
-                                        "abort",
-                                        abort,
-                                    );
+                                    if (options.signal) {
+                                        options.signal.removeEventListener(
+                                            "abort",
+                                            abort,
+                                        );
+                                        clock.abortListenerMap.delete(abort);
+                                    }
 
                                     return { done: true, value: undefined };
                                 },


Test patch:
diff --git a/test/fake-timers-test.js b/test/fake-timers-test.js
index 4b072f0..be9e9b9 100644
--- a/test/fake-timers-test.js
+++ b/test/fake-timers-test.js
@@ -5068,6 +5068,49 @@ describe("FakeTimers", function () {
                     true,
                 );
             });
+
+            it("should remove abort listener when uninstalling", function () {
+                clock = FakeTimers.install();
+                const abortController = new AbortController();
+                abortController.signal.removeEventListener = sinon.stub();
+                timersPromisesModule.setTimeout(100, null, {
+                    signal: abortController.signal,
+                });
+
+                clock.uninstall();
+                clock = undefined;
+
+                assert.equals(
+                    abortController.signal.removeEventListener.called,
+                    true,
+                );
+            });
+
+            it("should remove listener from abort listener map when aborting", async function () {
+                clock = FakeTimers.install();
+                const abortController = new AbortController();
+                const promise = timersPromisesModule.setTimeout(100, null, {
+                    signal: abortController.signal,
+                });
+
+                abortController.abort();
+                await promise.catch(() => {});
+
+                assert.equals(clock.abortListenerMap.size, 0);
+            });
+
+            it("should remove listener from abort listener map when resolving", async function () {
+                clock = FakeTimers.install();
+                const abortController = new AbortController();
+                const promise = timersPromisesModule.setTimeout(100, null, {
+                    signal: abortController.signal,
+                });
+
+                clock.tick(100);
+                await promise;
+
+                assert.equals(clock.abortListenerMap.size, 0);
+            });
         });
 
         describe("The setImmediate function", function () {
@@ -5140,6 +5183,51 @@ describe("FakeTimers", function () {
                     true,
                 );
             });
+
+            it("should remove abort listener when uninstalling", function () {
+                clock = FakeTimers.install();
+                const abortController = new AbortController();
+                abortController.signal.removeEventListener = sinon.stub();
+                timersPromisesModule.setImmediate(null, {
+                    signal: abortController.signal,
+                });
+
+                clock.uninstall();
+                clock = undefined;
+
+                assert.equals(
+                    abortController.signal.removeEventListener.called,
+                    true,
+                );
+            });
+
+            it("should remove listener from abort listener map when aborting", async function () {
+                clock = FakeTimers.install();
+                const abortController = new AbortController();
+                abortController.signal.removeEventListener = sinon.stub();
+                const promise = timersPromisesModule.setImmediate(null, {
+                    signal: abortController.signal,
+                });
+
+                abortController.abort();
+                await promise.catch(() => {});
+
+                assert.equals(clock.abortListenerMap.size, 0);
+            });
+
+            it("should remove listener from abort listener map when resolving", async function () {
+                clock = FakeTimers.install();
+                const abortController = new AbortController();
+                abortController.signal.removeEventListener = sinon.stub();
+                const promise = timersPromisesModule.setImmediate(null, {
+                    signal: abortController.signal,
+                });
+
+                clock.tick(0);
+                await promise;
+
+                assert.equals(clock.abortListenerMap.size, 0);
+            });
         });
 
         describe("The setInterval function", function () {
@@ -5389,6 +5477,52 @@ describe("FakeTimers", function () {
                     true,
                 );
             });
+
+            it("should remove abort listener when uninstalling", function () {
+                clock = FakeTimers.install();
+                const abortController = new AbortController();
+                abortController.signal.removeEventListener = sinon.stub();
+                const iterable = timersPromisesModule.setInterval(100, null, {
+                    signal: abortController.signal,
+                });
+                iterable[Symbol.asyncIterator]();
+
+                clock.uninstall();
+                clock = undefined;
+
+                assert.equals(
+                    abortController.signal.removeEventListener.called,
+                    true,
+                );
+            });
+
+            it("should remove listener from abort listener map when aborting", function () {
+                clock = FakeTimers.install();
+                const abortController = new AbortController();
+                abortController.signal.removeEventListener = sinon.stub();
+                const iterable = timersPromisesModule.setInterval(100, null, {
+                    signal: abortController.signal,
+                });
+                iterable[Symbol.asyncIterator]();
+
+                abortController.abort();
+
+                assert.equals(clock.abortListenerMap.size, 0);
+            });
+
+            it("should remove listener from abort listener map when returning", async function () {
+                clock = FakeTimers.install();
+                const abortController = new AbortController();
+                abortController.signal.removeEventListener = sinon.stub();
+                const iterable = timersPromisesModule.setInterval(100, null, {
+                    signal: abortController.signal,
+                });
+                const iter = iterable[Symbol.asyncIterator]();
+
+                await iter.return();
+
+                assert.equals(clock.abortListenerMap.size, 0);
+            });
         });
     });
 });