async_hooks: fix leak in AsyncLocalStorage exit

If exit is called and then run or enterWith are called within the
exit function, the als instace should not be added to the storageList
additional times. The correct behaviour is to remove the instance
from the storageList before executing the exit handler and then to
restore it after.

PR-URL: #35779
Reviewed-By: Vladimir de Turckheim <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Gerhard Stöbich <[email protected]>
Reviewed-By: Andrey Pechkurov <[email protected]>
Reviewed-By: Rich Trott <[email protected]>

lib/async_hooks.js

@@ -271,6 +271,14 @@ class AsyncLocalStorage {
     }
   }
 
+  _enable() {
+    if (!this.enabled) {
+      this.enabled = true;
+      storageList.push(this);
+      storageHook.enable();
+    }
+  }
+
   // Propagate the context from a parent resource to a child one
   _propagate(resource, triggerResource) {
     const store = triggerResource[this.kResourceStore];
@@ -280,11 +288,7 @@ class AsyncLocalStorage {
   }
 
   enterWith(store) {
-    if (!this.enabled) {
-      this.enabled = true;
-      storageList.push(this);
-      storageHook.enable();
-    }
+    this._enable();
     const resource = executionAsyncResource();
     resource[this.kResourceStore] = store;
   }
@@ -308,11 +312,11 @@ class AsyncLocalStorage {
     if (!this.enabled) {
       return callback(...args);
     }
-    this.enabled = false;
+    this.disable();
     try {
       return callback(...args);
     } finally {
-      this.enabled = true;
+      this._enable();
     }
   }
 

test/parallel/test-async-local-storage-exit-does-not-leak.js

@@ -0,0 +1,25 @@
+'use strict';
+const common = require('../common');
+const assert = require('assert');
+const { AsyncLocalStorage } = require('async_hooks');
+
+const als = new AsyncLocalStorage();
+
+// Make sure _propagate function exists.
+assert.ok(typeof als._propagate === 'function');
+
+// The als instance should be getting removed from the storageList in
+// lib/async_hooks.js when exit(...) is called, therefore when the nested runs
+// are called there should be no copy of the als in the storageList to run the
+// _propagate method on.
+als._propagate = common.mustNotCall('_propagate() should not be called');
+
+const done = common.mustCall();
+
+function run(count) {
+  if (count === 0) return done();
+  als.run({}, () => {
+    als.exit(run, --count);
+  });
+}
+run(100);