From 0ad04f6c3e4a382dde4684869e0904f78205ba45 Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Sun, 31 May 2026 18:14:57 +0000 Subject: [PATCH] timers: do not retain a reference to the async store after firing After firing timers, we can clean them up by iterating over all active stores and setting the relevant symbols to undefined. This also covers immediates and intervals. Refs: https://github.com/nodejs/node/issues/53408 PR-URL: https://github.com/nodejs/node/pull/53443 Fixes: https://github.com/nodejs/node/issues/53408 --- lib/async_hooks.js | 10 +++++ .../async_local_storage/async_hooks.js | 7 +++ lib/internal/timers.js | 32 ++++++++++--- test/parallel/timeout-async-store-leak.js | 45 +++++++++++++++++++ 4 files changed, 87 insertions(+), 7 deletions(-) create mode 100644 test/parallel/timeout-async-store-leak.js diff --git a/lib/async_hooks.js b/lib/async_hooks.js index 5922971b9be68b..4447c3f6f57517 100644 --- a/lib/async_hooks.js +++ b/lib/async_hooks.js @@ -279,6 +279,15 @@ class AsyncResource { // Placing all exports down here because the exported classes won't export // otherwise. +function clearStores(resource) { + if (AsyncContextFrame.enabled) { + // The async_context_frame variant does not store per-resource stores, + // so there is nothing to clear. + return; + } + require('internal/async_local_storage/async_hooks').clearStores(resource); +} + module.exports = { // Public API get AsyncLocalStorage() { @@ -293,4 +302,5 @@ module.exports = { asyncWrapProviders: ObjectFreeze({ __proto__: null, ...asyncWrap.Providers }), // Embedder API AsyncResource, + clearStores, }; diff --git a/lib/internal/async_local_storage/async_hooks.js b/lib/internal/async_local_storage/async_hooks.js index 552092d89bf305..e95aba88366d23 100644 --- a/lib/internal/async_local_storage/async_hooks.js +++ b/lib/internal/async_local_storage/async_hooks.js @@ -32,6 +32,12 @@ const storageHook = createHook({ }, }); +function clearStores(resource) { + for (let i = 0; i < storageList.length; ++i) { + resource[storageList[i].kResourceStore] = undefined; + } +} + class AsyncLocalStorage { #defaultValue = undefined; #name = undefined; @@ -151,3 +157,4 @@ class AsyncLocalStorage { } module.exports = AsyncLocalStorage; +module.exports.clearStores = clearStores; diff --git a/lib/internal/timers.js b/lib/internal/timers.js index 9c7366d6ca772f..164ad6d841cc4d 100644 --- a/lib/internal/timers.js +++ b/lib/internal/timers.js @@ -125,6 +125,13 @@ const AsyncContextFrame = require('internal/async_context_frame'); const async_context_frame = Symbol('kAsyncContextFrame'); +let asyncHooks = null; + +function removeStoresFromResource(resource) { + asyncHooks ??= require('async_hooks'); + asyncHooks.clearStores(resource); +} + // *Must* match Environment::ImmediateInfo::Fields in src/env.h. const kCount = 0; const kRefCount = 1; @@ -505,6 +512,7 @@ function getTimerCallbacks(runNextTicks) { else immediate._onImmediate(...argv); } finally { + removeStoresFromResource(immediate); immediate._onImmediate = null; emitDestroy(asyncId); @@ -577,6 +585,10 @@ function getTimerCallbacks(runNextTicks) { if (!timer._destroyed) { timer._destroyed = true; + removeStoresFromResource(timer); + timer._onTimeout = undefined; + timer._timerArgs = undefined; + if (timer[kHasPrimitive]) delete knownTimersById[asyncId]; @@ -609,16 +621,22 @@ function getTimerCallbacks(runNextTicks) { if (timer._repeat && timer._idleTimeout !== -1) { timer._idleTimeout = timer._repeat; insert(timer, timer._idleTimeout, start); - } else if (!timer._idleNext && !timer._idlePrev && !timer._destroyed) { - timer._destroyed = true; + } else { + removeStoresFromResource(timer); + timer._onTimeout = undefined; + timer._timerArgs = undefined; - if (timer[kHasPrimitive]) - delete knownTimersById[asyncId]; + if (!timer._idleNext && !timer._idlePrev && !timer._destroyed) { + timer._destroyed = true; - if (timer[kRefed]) - timeoutInfo[0]--; + if (timer[kHasPrimitive]) + delete knownTimersById[asyncId]; - emitDestroy(asyncId); + if (timer[kRefed]) + timeoutInfo[0]--; + + emitDestroy(asyncId); + } } } diff --git a/test/parallel/timeout-async-store-leak.js b/test/parallel/timeout-async-store-leak.js new file mode 100644 index 00000000000000..1c8dd03521e3cf --- /dev/null +++ b/test/parallel/timeout-async-store-leak.js @@ -0,0 +1,45 @@ +'use strict'; + +const common = require('../common'); +const { AsyncLocalStorage } = require('async_hooks'); +const assert = require('assert'); + +// Test that setTimeout does not retain a reference to the async store +// after firing. +{ + const asyncLocalStorage = new AsyncLocalStorage(); + asyncLocalStorage.run({}, common.mustCall(() => { + const timeout = setTimeout(common.mustCall(() => { + setImmediate(common.mustCall(() => { + assert.strictEqual(timeout[asyncLocalStorage.kResourceStore], undefined); + assert.strictEqual(timeout._onTimeout, undefined); + })); + })); + })); +} + +// Test that clearTimeout cleans up the store even before firing. +{ + const asyncLocalStorage = new AsyncLocalStorage(); + asyncLocalStorage.run({}, common.mustCall(() => { + const timeout = setTimeout(() => { + // This should never be called because we clear it + assert.fail('should not be called'); + }, 1000); + clearTimeout(timeout); + assert.strictEqual(timeout._onTimeout, undefined); + })); +} + +// Test that setImmediate does not retain a reference to the async store +// after firing. +{ + const asyncLocalStorage = new AsyncLocalStorage(); + asyncLocalStorage.run({}, common.mustCall(() => { + const immediate = setImmediate(common.mustCall(() => { + setImmediate(common.mustCall(() => { + assert.strictEqual(immediate[asyncLocalStorage.kResourceStore], undefined); + })); + })); + })); +}