Skip to content

Commit

Permalink
lib: fix timer leak
Browse files Browse the repository at this point in the history
PR-URL: #53337
Fixes: #53335
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: Feng Yu <F3n67u@outlook.com>
Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
  • Loading branch information
theanarkh authored and marco-ippolito committed Jul 19, 2024
1 parent a3e8cda commit d1a7800
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 6 deletions.
12 changes: 12 additions & 0 deletions lib/internal/timers.js
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,11 @@ const timerListQueue = new PriorityQueue(compareTimersLists, setPosition);
// - value = linked list
const timerListMap = { __proto__: null };

// This stores all the known timer async ids to allow users to clearTimeout and
// clearInterval using those ids, to match the spec and the rest of the web
// platform.
const knownTimersById = { __proto__: null };

function initAsyncResource(resource, type) {
const asyncId = resource[async_id_symbol] = newAsyncId();
const triggerAsyncId =
Expand Down Expand Up @@ -550,6 +555,9 @@ function getTimerCallbacks(runNextTicks) {
if (!timer._destroyed) {
timer._destroyed = true;

if (timer[kHasPrimitive])
delete knownTimersById[asyncId];

if (timer[kRefed])
timeoutInfo[0]--;

Expand Down Expand Up @@ -580,6 +588,9 @@ function getTimerCallbacks(runNextTicks) {
} else if (!timer._idleNext && !timer._idlePrev && !timer._destroyed) {
timer._destroyed = true;

if (timer[kHasPrimitive])
delete knownTimersById[asyncId];

if (timer[kRefed])
timeoutInfo[0]--;

Expand Down Expand Up @@ -683,4 +694,5 @@ module.exports = {
timerListQueue,
decRefCount,
incRefCount,
knownTimersById,
};
6 changes: 1 addition & 5 deletions lib/timers.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ const {
active,
unrefActive,
insert,
knownTimersById,
} = require('internal/timers');
const {
promisify: { custom: customPromisify },
Expand All @@ -71,11 +72,6 @@ const {
emitDestroy,
} = require('internal/async_hooks');

// This stores all the known timer async ids to allow users to clearTimeout and
// clearInterval using those ids, to match the spec and the rest of the web
// platform.
const knownTimersById = { __proto__: null };

// Remove a timer. Cancels the timeout and resets the relevant timer properties.
function unenroll(item) {
if (item._destroyed)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,6 @@
Error: goodbye
at Hello (*uglify-throw-original.js:5:9)
at Immediate.<anonymous> (*uglify-throw-original.js:9:3)
at process.processImmediate (node:internal*timers:478:21)
at process.processImmediate (node:internal*timers:483:21)

Node.js *
23 changes: 23 additions & 0 deletions test/parallel/test-primitive-timer-leak.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
'use strict';
// Flags: --expose-gc
require('../common');
const onGC = require('../common/ongc');

// See https://github.com/nodejs/node/issues/53335
const poller = setInterval(() => {
global.gc();
}, 100);

let count = 0;

for (let i = 0; i < 10; i++) {
const timer = setTimeout(() => {}, 0);
onGC(timer, {
ongc: () => {
if (++count === 10) {
clearInterval(poller);
}
}
});
console.log(+timer);
}

0 comments on commit d1a7800

Please sign in to comment.