-
-
Notifications
You must be signed in to change notification settings - Fork 34k
Description
Version
21.2.0
Platform
Darwin cheetah.local 23.5.0 Darwin Kernel Version 23.5.0: Wed May 1 20:12:58 PDT 2024; root:xnu-10063.121.3~5/RELEASE_ARM64_T6000 arm64
Subsystem
timers.js
What steps will reproduce the bug?
On node calling setTimeout returns a Timeout object. That object is tracked in an internal list of timers and that list is maintained in two places. On the one hand in unenroll which is used by clearTimeout (and clearInterval) and one when the timer runs.
However only the unenroll path also removes a timer from the internal knownTimersById map. This map is updated whenever the Timeout is converted into a primitive. From that moment onwards a timer can be cleared by it's internal async id.
So to get a setTimeout to leak you just need to call +setTimeout(...) and it wait for it to complete. The entry from the knownTimersById map is not removed and we leak.
The repro case is trivial:
// leaks
for (i = 0; i < 500000; i++) {
+setTimeout(() => {}, 0);
}This will create 500000 un-collectable Timeouts that can be found in the knownTimersById map in timers.js. Removing the + fixes it.
Timer is removed here from the list but not from knownTimersById:
Lines 544 to 545 in 7d14d1f
| // The actual logic for when a timeout happens. | |
| L.remove(timer); |
Compare this to how unenroll clears:
Lines 86 to 93 in 7d14d1f
| if (item[kHasPrimitive]) | |
| delete knownTimersById[item[async_id_symbol]]; | |
| // Fewer checks may be possible, but these cover everything. | |
| if (destroyHooksExist() && item[async_id_symbol] !== undefined) | |
| emitDestroy(item[async_id_symbol]); | |
| L.remove(item); |
How often does it reproduce? Is there a required condition?
Always
What is the expected behavior? Why is that the expected behavior?
Not leak memory
What do you see instead?
Leaks memory
Additional information
We ran into this with the Sentry SDK though it's not entirely clear yet what actually converts the value there into a primitive. Might be some monkey patching going on somewhere.
The code looks the same on the latest version but I did not try to repro it there yet.