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); + })); + })); + })); +}