Skip to content

Commit

Permalink
Fix listener leak for timers/promises
Browse files Browse the repository at this point in the history
  • Loading branch information
@WhiteAutumn @fatso83
WhiteAutumn authored and fatso83 committed Aug 23, 2024
1 parent 0145f3b commit 6f90a44
Show file tree
Hide file tree
Showing 2 changed files with 208 additions and 33 deletions.
107 changes: 74 additions & 33 deletions src/fake-timers-src.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 */

Expand Down Expand Up @@ -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 [];
Expand Down Expand Up @@ -1789,6 +1795,8 @@ function withGlobal(_global) {
return uninstall(clock, config);
};

clock.abortListenerMap = new Map();

clock.methods = config.toFake || [];

if (clock.methods.length === 0) {
Expand Down Expand Up @@ -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
Expand All @@ -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") {
Expand All @@ -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
Expand All @@ -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") {
Expand Down Expand Up @@ -2000,20 +2030,28 @@ function withGlobal(_global) {
"abort",
abort,
);
clock.abortListenerMap.delete(abort);

clock.clearInterval(handle);
done = true;
for (const resolvable of nextQueue) {
resolvable.resolve();
}
};

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 {
Expand Down Expand Up @@ -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 };
},
Expand Down
134 changes: 134 additions & 0 deletions test/fake-timers-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 () {
Expand Down Expand Up @@ -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 () {
Expand Down Expand Up @@ -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);
});
});
});
});
Expand Down

0 comments on commit 6f90a44

Please sign in to comment.