Skip to content

Commit

Permalink
fix: work around setTimeout memory leak, improve wrappers (#75727)
Browse files Browse the repository at this point in the history
Backports:
- #75727

Co-authored-by: Janka Uryga <lolzatu2@gmail.com>
  • Loading branch information
ztanner and lubieowoce authored Feb 10, 2025
1 parent c25fa95 commit c536898
Showing 1 changed file with 41 additions and 5 deletions.
46 changes: 41 additions & 5 deletions packages/next/src/server/web/sandbox/resource-managers.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
abstract class ResourceManager<T, K> {
abstract class ResourceManager<T, Args> {
private resources: T[] = []

abstract create(resourceArgs: K): T
abstract create(resourceArgs: Args): T
abstract destroy(resource: T): void

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

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

destroy(timeout: number) {
clearTimeout(timeout)
}
}

function webSetIntervalPolyfill<TArgs extends any[]>(
callback: (...args: TArgs) => void,
ms?: number,
...args: TArgs
): number {
return setInterval(() => {
// node's `setInterval` sets `this` to the `Timeout` instance it returned,
// but web `setInterval` always sets `this` to `window`
// see: https://developer.mozilla.org/en-US/docs/Web/API/Window/setInterval#the_this_problem
return callback.apply(globalThis, args)
}, ms)[Symbol.toPrimitive]()
}

function webSetTimeoutPolyfill<TArgs extends any[]>(
callback: (...args: TArgs) => void,
ms?: number,
...args: TArgs
): number {
const wrappedCallback = () => {
try {
// node's `setTimeout` sets `this` to the `Timeout` instance it returned,
// but web `setTimeout` always sets `this` to `window`
// see: https://developer.mozilla.org/en-US/docs/Web/API/Window/setTimeout#the_this_problem
return callback.apply(globalThis, args)
} finally {
// On certain older node versions (<20.16.0, <22.4.0),
// a `setTimeout` whose Timeout was converted to a primitive will leak.
// See: /~https://github.com/nodejs/node/issues/53335
// We can work around this by explicitly calling `clearTimeout` after the callback runs.
clearTimeout(timeout)
}
}
const timeout = setTimeout(wrappedCallback, ms)
return timeout[Symbol.toPrimitive]()
}

export const intervalsManager = new IntervalsManager()
export const timeoutsManager = new TimeoutsManager()

0 comments on commit c536898

Please sign in to comment.