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.

Fixes #53408

Signed-off-by: Matteo Collina <hello@matteocollina.com>

Author: mcollina

lib/internal/async_hooks.js

@@ -105,6 +105,7 @@ const before_symbol = Symbol('before');
 const after_symbol = Symbol('after');
 const destroy_symbol = Symbol('destroy');
 const promise_resolve_symbol = Symbol('promiseResolve');
+const async_local_storage_context_symbol = Symbol('kAsyncLocalStorageContext');
 const emitBeforeNative = emitHookFactory(before_symbol, 'emitBeforeNative');
 const emitAfterNative = emitHookFactory(after_symbol, 'emitAfterNative');
 const emitDestroyNative = emitHookFactory(destroy_symbol, 'emitDestroyNative');
@@ -584,7 +585,8 @@ module.exports = {
   symbols: {
     async_id_symbol, trigger_async_id_symbol,
     init_symbol, before_symbol, after_symbol, destroy_symbol,
-    promise_resolve_symbol, owner_symbol,
+    promise_resolve_symbol, async_local_storage_context_symbol,
+    owner_symbol,
   },
   constants: {
     kInit, kBefore, kAfter, kDestroy, kTotals, kPromiseResolve,

lib/internal/async_local_storage/async_hooks.js

@@ -12,6 +12,11 @@ const {
 const {
   validateObject,
 } = require('internal/validators');
+const {
+  symbols: {
+    async_local_storage_context_symbol,
+  },
+} = require('internal/async_hooks');
 
 const {
   AsyncResource,
@@ -23,6 +28,11 @@ const RunScope = require('internal/async_local_storage/run_scope');
 const { kEmptyObject } = require('internal/util');
 
 const storageList = [];
+
+function getOrCreateResourceStore(resource) {
+  return resource[async_local_storage_context_symbol] ??= { __proto__: null };
+}
+
 const storageHook = createHook({
   init(asyncId, type, triggerAsyncId, resource) {
     const currentResource = executionAsyncResource();
@@ -91,16 +101,18 @@ class AsyncLocalStorage {
 
   // Propagate the context from a parent resource to a child one
   _propagate(resource, triggerResource, type) {
-    const store = triggerResource[this.kResourceStore];
+    const store = triggerResource[async_local_storage_context_symbol]?.[this.kResourceStore];
     if (this.enabled) {
-      resource[this.kResourceStore] = store;
+      const resourceStore = getOrCreateResourceStore(resource);
+      resourceStore[this.kResourceStore] = store;
     }
   }
 
   enterWith(store) {
     this._enable();
     const resource = executionAsyncResource();
-    resource[this.kResourceStore] = store;
+    const resourceStore = getOrCreateResourceStore(resource);
+    resourceStore[this.kResourceStore] = store;
   }
 
   run(store, callback, ...args) {
@@ -112,14 +124,15 @@ class AsyncLocalStorage {
     this._enable();
 
     const resource = executionAsyncResource();
-    const oldStore = resource[this.kResourceStore];
+    const resourceStore = getOrCreateResourceStore(resource);
+    const oldStore = resourceStore[this.kResourceStore];
 
-    resource[this.kResourceStore] = store;
+    resourceStore[this.kResourceStore] = store;
 
     try {
       return ReflectApply(callback, null, args);
     } finally {
-      resource[this.kResourceStore] = oldStore;
+      resourceStore[this.kResourceStore] = oldStore;
     }
   }
 
@@ -138,10 +151,11 @@ class AsyncLocalStorage {
   getStore() {
     if (this.enabled) {
       const resource = executionAsyncResource();
-      if (!(this.kResourceStore in resource)) {
+      const resourceStore = resource[async_local_storage_context_symbol];
+      if (resourceStore === undefined || !(this.kResourceStore in resourceStore)) {
         return this.#defaultValue;
       }
-      return resource[this.kResourceStore];
+      return resourceStore[this.kResourceStore];
     }
     return this.#defaultValue;
   }

lib/internal/timers.js

@@ -87,6 +87,9 @@ const {
   immediateInfo,
   timeoutInfo,
 } = binding;
+const {
+  enqueueMicrotask,
+} = internalBinding('task_queue');
 
 const {
   getDefaultTriggerAsyncId,
@@ -97,6 +100,9 @@ const {
   emitBefore,
   emitAfter,
   emitDestroy,
+  symbols: {
+    async_local_storage_context_symbol,
+  },
 } = require('internal/async_hooks');
 
 // Symbols for storing async id state.
@@ -125,6 +131,39 @@ const AsyncContextFrame = require('internal/async_context_frame');
 
 const async_context_frame = Symbol('kAsyncContextFrame');
 
+function removeStoresFromResource(resource) {
+  if (AsyncContextFrame.enabled) {
+    if (resource[async_context_frame] !== undefined) {
+      resource[async_context_frame] = undefined;
+    }
+  } else if (resource[async_local_storage_context_symbol] !== undefined) {
+    resource[async_local_storage_context_symbol] = undefined;
+  }
+}
+
+function cleanTimer(timer) {
+  removeStoresFromResource(timer);
+  timer._onTimeout = undefined;
+  timer._timerArgs = undefined;
+}
+
+function cleanImmediate(immediate) {
+  removeStoresFromResource(immediate);
+  immediate._onImmediate = undefined;
+  immediate._argv = undefined;
+}
+
+function enqueueRemoveStoresFromResource(resource) {
+  enqueueMicrotask(() => removeStoresFromResource(resource));
+}
+
+function enqueueRemoveStoresIfNotReinserted(resource) {
+  enqueueMicrotask(() => {
+    if (!resource._idleNext && !resource._idlePrev)
+      removeStoresFromResource(resource);
+  });
+}
+
 // *Must* match Environment::ImmediateInfo::Fields in src/env.h.
 const kCount = 0;
 const kRefCount = 1;
@@ -528,14 +567,22 @@ function getTimerCallbacks(runNextTicks) {
       const asyncId = immediate[async_id_symbol];
       emitBefore(asyncId, immediate[trigger_async_id_symbol], immediate);
 
+      let threw = true;
       try {
         const argv = immediate._argv;
         if (!argv)
           immediate._onImmediate();
         else
           immediate._onImmediate(...argv);
+        threw = false;
       } finally {
-        immediate._onImmediate = null;
+        if (threw) {
+          immediate._onImmediate = undefined;
+          immediate._argv = undefined;
+          enqueueRemoveStoresFromResource(immediate);
+        } else {
+          cleanImmediate(immediate);
+        }
 
         emitDestroy(asyncId);
 
@@ -607,6 +654,8 @@ function getTimerCallbacks(runNextTicks) {
         if (!timer._destroyed) {
           timer._destroyed = true;
 
+          cleanTimer(timer);
+
           if (timer[kHasPrimitive])
             delete knownTimersById[asyncId];
 
@@ -629,26 +678,42 @@ function getTimerCallbacks(runNextTicks) {
         start = binding.getLibuvNow();
       }
 
+      let threw = true;
       try {
         const args = timer._timerArgs;
         if (args === undefined)
           timer._onTimeout();
         else
           ReflectApply(timer._onTimeout, timer, args);
+        threw = false;
       } finally {
         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;
-
-          if (timer[kHasPrimitive])
-            delete knownTimersById[asyncId];
-
-          if (timer[kRefed])
-            timeoutInfo[0]--;
-
-          emitDestroy(asyncId);
+        } else if (!timer._idleNext && !timer._idlePrev) {
+          if (timer._destroyed) {
+            timer._onTimeout = undefined;
+            timer._timerArgs = undefined;
+            if (threw)
+              enqueueRemoveStoresIfNotReinserted(timer);
+            else
+              removeStoresFromResource(timer);
+          } else {
+            if (threw)
+              enqueueRemoveStoresIfNotReinserted(timer);
+            else
+              removeStoresFromResource(timer);
+
+            timer._destroyed = true;
+
+            if (timer[kHasPrimitive])
+              delete knownTimersById[asyncId];
+
+            if (timer[kRefed])
+              timeoutInfo[0]--;
+
+            emitDestroy(asyncId);
+          }
         }
       }
 
@@ -728,8 +793,11 @@ module.exports = {
   kTimeout: Symbol('timeout'), // For hiding Timeouts on other internals.
   async_id_symbol,
   trigger_async_id_symbol,
+  async_context_frame,
   Timeout,
   Immediate,
+  cleanImmediate,
+  cleanTimer,
   kRefed,
   kHasPrimitive,
   initAsyncResource,

lib/timers.js

@@ -38,6 +38,8 @@ const {
   async_id_symbol,
   Timeout,
   Immediate,
+  cleanTimer,
+  cleanImmediate,
   decRefCount,
   immediateInfoFields: {
     kCount,
@@ -69,9 +71,12 @@ const {
 
 // Remove a timer. Cancels the timeout and resets the relevant timer properties.
 function unenroll(item) {
-  if (item._destroyed)
+  if (item._destroyed) {
+    cleanTimer(item);
     return;
+  }
 
+  const wasEnrolled = item._idleNext !== null || item._idlePrev !== null;
   item._destroyed = true;
 
   if (item[kHasPrimitive])
@@ -99,6 +104,9 @@ function unenroll(item) {
     decRefCount();
   }
 
+  if (wasEnrolled)
+    cleanTimer(item);
+
   // If active is called later, then we want to make sure not to insert again
   item._idleTimeout = -1;
 }
@@ -238,7 +246,7 @@ function clearImmediate(immediate) {
 
   emitDestroy(immediate[async_id_symbol]);
 
-  immediate._onImmediate = null;
+  cleanImmediate(immediate);
 
   immediateQueue.remove(immediate);
 }