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 <vlad2t@hotmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
Reviewed-By: Andrey Pechkurov <apechkurov@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>

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