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: #53408
PR-URL: #53443
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');
@@ -595,7 +596,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,36 @@ const AsyncContextFrame = require('internal/async_context_frame');
 
 const async_context_frame = Symbol('kAsyncContextFrame');
 
+function removeStoresFromResource(resource) {
+  if (!AsyncContextFrame.enabled &&
+      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;
@@ -498,14 +534,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);
 
@@ -577,6 +621,8 @@ function getTimerCallbacks(runNextTicks) {
         if (!timer._destroyed) {
           timer._destroyed = true;
 
+          cleanTimer(timer);
+
           if (timer[kHasPrimitive])
             delete knownTimersById[asyncId];
 
@@ -599,26 +645,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);
+          }
         }
       }
 
@@ -703,6 +765,8 @@ module.exports = {
   kRefed,
   kHasPrimitive,
   initAsyncResource,
+  cleanImmediate,
+  cleanTimer,
   setUnrefTimeout,
   getTimerDuration,
   immediateQueue,

lib/timers.js

@@ -38,6 +38,8 @@ const {
   async_id_symbol,
   Timeout,
   Immediate,
+  cleanImmediate,
+  cleanTimer,
   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);
 }