Skip to content

Commit c536898

Browse files
ztannerlubieowoce
andauthored
fix: work around setTimeout memory leak, improve wrappers (#75727)
Backports: - #75727 Co-authored-by: Janka Uryga <[email protected]>
1 parent c25fa95 commit c536898

File tree

1 file changed

+41
-5
lines changed

1 file changed

+41
-5
lines changed

packages/next/src/server/web/sandbox/resource-managers.ts

+41-5
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
1-
abstract class ResourceManager<T, K> {
1+
abstract class ResourceManager<T, Args> {
22
private resources: T[] = []
33

4-
abstract create(resourceArgs: K): T
4+
abstract create(resourceArgs: Args): T
55
abstract destroy(resource: T): void
66

7-
add(resourceArgs: K) {
7+
add(resourceArgs: Args) {
88
const resource = this.create(resourceArgs)
99
this.resources.push(resource)
1010
return resource
@@ -27,7 +27,7 @@ class IntervalsManager extends ResourceManager<
2727
> {
2828
create(args: Parameters<typeof setInterval>) {
2929
// TODO: use the edge runtime provided `setInterval` instead
30-
return setInterval(...args)[Symbol.toPrimitive]()
30+
return webSetIntervalPolyfill(...args)
3131
}
3232

3333
destroy(interval: number) {
@@ -41,13 +41,49 @@ class TimeoutsManager extends ResourceManager<
4141
> {
4242
create(args: Parameters<typeof setTimeout>) {
4343
// TODO: use the edge runtime provided `setTimeout` instead
44-
return setTimeout(...args)[Symbol.toPrimitive]()
44+
return webSetTimeoutPolyfill(...args)
4545
}
4646

4747
destroy(timeout: number) {
4848
clearTimeout(timeout)
4949
}
5050
}
5151

52+
function webSetIntervalPolyfill<TArgs extends any[]>(
53+
callback: (...args: TArgs) => void,
54+
ms?: number,
55+
...args: TArgs
56+
): number {
57+
return setInterval(() => {
58+
// node's `setInterval` sets `this` to the `Timeout` instance it returned,
59+
// but web `setInterval` always sets `this` to `window`
60+
// see: https://developer.mozilla.org/en-US/docs/Web/API/Window/setInterval#the_this_problem
61+
return callback.apply(globalThis, args)
62+
}, ms)[Symbol.toPrimitive]()
63+
}
64+
65+
function webSetTimeoutPolyfill<TArgs extends any[]>(
66+
callback: (...args: TArgs) => void,
67+
ms?: number,
68+
...args: TArgs
69+
): number {
70+
const wrappedCallback = () => {
71+
try {
72+
// node's `setTimeout` sets `this` to the `Timeout` instance it returned,
73+
// but web `setTimeout` always sets `this` to `window`
74+
// see: https://developer.mozilla.org/en-US/docs/Web/API/Window/setTimeout#the_this_problem
75+
return callback.apply(globalThis, args)
76+
} finally {
77+
// On certain older node versions (<20.16.0, <22.4.0),
78+
// a `setTimeout` whose Timeout was converted to a primitive will leak.
79+
// See: https://github.com/nodejs/node/issues/53335
80+
// We can work around this by explicitly calling `clearTimeout` after the callback runs.
81+
clearTimeout(timeout)
82+
}
83+
}
84+
const timeout = setTimeout(wrappedCallback, ms)
85+
return timeout[Symbol.toPrimitive]()
86+
}
87+
5288
export const intervalsManager = new IntervalsManager()
5389
export const timeoutsManager = new TimeoutsManager()

0 commit comments

Comments
 (0)