timers: do not retain a reference to the async store after firing

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,
 };

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;

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

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