fix: work around setTimeout memory leak, improve wrappers (#75727)
Potential fix for a leak reported in #74855 on older node versions (see
[comment](https://github.com/vercel/next.js/issues/74855#issuecomment-2638334820)).
### Background
When running middleware (or other edge functions) in `next start`, we
wrap them in an edge runtime sandbox. this includes polyfills of
`setTimeout` and `setInterval` which return `number` instead of
`NodeJS.Timeout`.
Unfortunately, on some older node versions, converting a
`NodeJS.Timeout` to a number will cause that timeout to leak:
https://github.com/nodejs/node/issues/53335
The leaked timeout will also hold onto the callback, thus also leaking
anything that was closed over (which can be a lot of things!)
### Solution
Ideally, users just upgrade to a Node version that includes the fix:
- [node v20.16.0](https://github.com/nodejs/node/pull/53945)
- [node v22.4.0](https://github.com/nodejs/node/pull/53583)
- node v23.0.0
But we're currently still supporting node 18, so we can't necessarily
rely on that. Luckily, as noted in the description of the nodejs issue,
calling `clearTimeout` seems to unleak the timeout, so we can just do
that after the callback runs!
### Unrelated stuff I did
While i was at it, I also fixed a (very niche) discrepancy from how
`setTimeout` and `setInterval` behave on the web. when running the
callback, node sets `this` to the Timeout instance:
```js
> void setTimeout(function () {console.log('this in setTimeout:', this) } )
undefined
> this in setTimeout: Timeout { ... }
```
but on the web, `this` is always set to `globalThis`. Our wrapper now
correctly does this.
### Testing
<details>
<summary>Collapsed because it's long</summary>
Verifying this is kinda tricky, so bear with me...
Here's a script that can verify whether calling `clearTimeout` fixes the
leak by using a FinalizationRegistry and triggering GC to observe
whether memory leaked or not.
`setTimeoutWithFix` is a simplified version of `webSetTimeoutPolyfill`
from the PR.
```js
// setTimeout-test.js
if (typeof gc !== 'function') {
console.log('this test must be run with --expose-gc')
process.exit(1)
}
function setTimeoutWithFix(callback, ms, ...args) {
const wrappedCallback = function () {
try {
return callback.apply(this, args)
} finally {
clearTimeout(timeout)
}
}
const timeout = setTimeout(wrappedCallback, ms)
return timeout
}
const didFinalize = {}
const registry = new FinalizationRegistry((id) => {
didFinalize[id] = true
})
{
const id = 'node setTimeout'.padEnd(26, ' ')
const timeout = setTimeout(() => {}, 0)
registry.register(timeout, id)
didFinalize[id] = false
}
{
const id = 'node setTimeout as number'.padEnd(26, ' ')
const timeout = setTimeout(() => {}, 0)
timeout[Symbol.toPrimitive]()
registry.register(timeout, id)
didFinalize[id] = false
}
{
const id = 'fixed setTimeout'.padEnd(26, ' ')
const timeout = setTimeoutWithFix(() => {}, 0)
registry.register(timeout, id)
didFinalize[id] = false
}
{
const id = 'fixed setTimeout as number'.padEnd(26, ' ')
const timeout = setTimeoutWithFix(() => {}, 0)
timeout[Symbol.toPrimitive]()
registry.register(timeout, id)
didFinalize[id] = false
}
// wait for the timeouts to run
void setTimeout(() => {
gc() // trigger garbage collection
void registry // ...but make sure we keep the registry alive
// wait a task so that finalization callbacks can run
setTimeout(() =>
console.log('did the Timeout get released after GC?', didFinalize)
)
}, 10)
```
To run it, Install the required node versions:
```bash
for ver in v20.15.0 v20.16.0 v22.3.0 v22.4.0 v23.0.0; do ( nvm install "$ver" ); done
```
And run the test:
```bash
for ver in v20.15.0 v20.16.0 v22.3.0 v22.4.0 v23.0.0; do
(
echo '-------------------'
nvm use "$ver" && node --expose-gc setTimeout-test.js
echo
);
done
```
The output on my machine is as follows. Note that the `node setTimeout
as number` case comes up as false on the older versions (because it
leaks and doesn't get finalized), but `fixed setTimeout as number`
(which calls `clearTimeout`) gets released fine, which is exactly what
we want.
```terminal
-------------------
Now using node v20.15.0 (npm v10.7.0)
did the Timeout get released after GC? {
'node setTimeout ': true,
'node setTimeout as number ': false,
'fixed setTimeout ': true,
'fixed setTimeout as number': true
}
-------------------
Now using node v20.16.0 (npm v10.8.1)
did the Timeout get released after GC? {
'node setTimeout ': true,
'node setTimeout as number ': true,
'fixed setTimeout ': true,
'fixed setTimeout as number': true
}
-------------------
Now using node v22.3.0 (npm v10.8.1)
did the Timeout get released after GC? {
'node setTimeout ': true,
'node setTimeout as number ': false,
'fixed setTimeout ': true,
'fixed setTimeout as number': true
}
-------------------
Now using node v22.4.0 (npm v10.8.1)
did the Timeout get released after GC? {
'node setTimeout ': true,
'node setTimeout as number ': true,
'fixed setTimeout ': true,
'fixed setTimeout as number': true
}
-------------------
Now using node v23.0.0 (npm v10.9.0)
did the Timeout get released after GC? {
'node setTimeout ': true,
'node setTimeout as number ': true,
'fixed setTimeout ': true,
'fixed setTimeout as number': true
}
```
</details>
next.js
ad565540 - fix: work around setTimeout memory leak, improve wrappers (#75727)
