From 6f90a44716583fddefcba0455d3c8f4bd30eca7a Mon Sep 17 00:00:00 2001 From: White Autumn Date: Fri, 23 Aug 2024 09:35:56 +0200 Subject: [PATCH] Fix listener leak for timers/promises --- src/fake-timers-src.js | 107 +++++++++++++++++++++---------- test/fake-timers-test.js | 134 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 208 insertions(+), 33 deletions(-) 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} 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 }; }, 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); + }); }); }); });