From 9d000a9f2e6881f1210a857fec45515d3ec25d25 Mon Sep 17 00:00:00 2001 From: theanarkh Date: Fri, 7 Jun 2024 23:51:44 +0800 Subject: [PATCH] lib: fix timer leak PR-URL: https://github.com/nodejs/node/pull/53337 Fixes: https://github.com/nodejs/node/issues/53335 Reviewed-By: Benjamin Gruenbaum Reviewed-By: Chemi Atlow Reviewed-By: Feng Yu Reviewed-By: Yagiz Nizipli --- lib/internal/timers.js | 12 ++++++++++ lib/timers.js | 6 +---- .../source_map_throw_set_immediate.snapshot | 2 +- test/parallel/test-primitive-timer-leak.js | 23 +++++++++++++++++++ 4 files changed, 37 insertions(+), 6 deletions(-) create mode 100644 test/parallel/test-primitive-timer-leak.js diff --git a/lib/internal/timers.js b/lib/internal/timers.js index aa7d3ec69f2189..5c56cf106350c1 100644 --- a/lib/internal/timers.js +++ b/lib/internal/timers.js @@ -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 = @@ -550,6 +555,9 @@ function getTimerCallbacks(runNextTicks) { if (!timer._destroyed) { timer._destroyed = true; + if (timer[kHasPrimitive]) + delete knownTimersById[asyncId]; + if (timer[kRefed]) timeoutInfo[0]--; @@ -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]--; @@ -683,4 +694,5 @@ module.exports = { timerListQueue, decRefCount, incRefCount, + knownTimersById, }; diff --git a/lib/timers.js b/lib/timers.js index 7c77ca5c04e2e0..9e40f8ce0b298e 100644 --- a/lib/timers.js +++ b/lib/timers.js @@ -52,6 +52,7 @@ const { active, unrefActive, insert, + knownTimersById, } = require('internal/timers'); const { promisify: { custom: customPromisify }, @@ -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) diff --git a/test/fixtures/source-map/output/source_map_throw_set_immediate.snapshot b/test/fixtures/source-map/output/source_map_throw_set_immediate.snapshot index e5054d01b8f91b..d2d838ca35a3de 100644 --- a/test/fixtures/source-map/output/source_map_throw_set_immediate.snapshot +++ b/test/fixtures/source-map/output/source_map_throw_set_immediate.snapshot @@ -6,6 +6,6 @@ Error: goodbye at Hello (*uglify-throw-original.js:5:9) at Immediate. (*uglify-throw-original.js:9:3) - at process.processImmediate (node:internal*timers:478:21) + at process.processImmediate (node:internal*timers:483:21) Node.js * diff --git a/test/parallel/test-primitive-timer-leak.js b/test/parallel/test-primitive-timer-leak.js new file mode 100644 index 00000000000000..45681af33d37e5 --- /dev/null +++ b/test/parallel/test-primitive-timer-leak.js @@ -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); +}