async_hooks: ensure AsyncLocalStore instances work isolated

Avoid that one AsyncLocalStore instance changes the state of another
AsyncLocalStore instance by restoring only the owned store instead
the complete AsyncContextFrame.

PR-URL: #58149
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Stephen Belanger <[email protected]>

Author: Flarna

lib/internal/async_local_storage/async_context_frame.js

@@ -1,6 +1,7 @@
 'use strict';
 
 const {
+  ObjectIs,
   ReflectApply,
 } = primordials;
 
@@ -53,12 +54,15 @@ class AsyncLocalStorage {
   }
 
   run(data, fn, ...args) {
-    const prior = AsyncContextFrame.current();
+    const prior = this.getStore();
+    if (ObjectIs(prior, data)) {
+      return ReflectApply(fn, null, args);
+    }
     this.enterWith(data);
     try {
       return ReflectApply(fn, null, args);
     } finally {
-      AsyncContextFrame.set(prior);
+      this.enterWith(prior);
     }
   }
 

test/async-hooks/test-async-local-storage-gcable.js

@@ -1,11 +1,12 @@
 'use strict';
-// Flags: --expose_gc
+// Flags: --expose_gc --expose-internals
 
 // This test ensures that AsyncLocalStorage gets gced once it was disabled
 // and no strong references remain in userland.
 
 const common = require('../common');
 const { AsyncLocalStorage } = require('async_hooks');
+const AsyncContextFrame = require('internal/async_context_frame');
 const { onGC } = require('../common/gc');
 
 let asyncLocalStorage = new AsyncLocalStorage();
@@ -16,5 +17,11 @@ asyncLocalStorage.run({}, () => {
   onGC(asyncLocalStorage, { ongc: common.mustCall() });
 });
 
+if (AsyncContextFrame.enabled) {
+  // This disable() is needed to remove reference form AsyncContextFrame
+  // created during exit of run() to the AsyncLocalStore instance.
+  asyncLocalStorage.disable();
+}
+
 asyncLocalStorage = null;
 global.gc();

test/parallel/test-async-local-storage-isolation.js

@@ -0,0 +1,67 @@
+'use strict';
+const common = require('../common');
+const { AsyncLocalStorage } = require('node:async_hooks');
+const assert = require('node:assert');
+
+// Verify that ALS instances are independent of each other.
+
+{
+  // Verify als2.enterWith() and als2.run inside als1.run()
+  const als1 = new AsyncLocalStorage();
+  const als2 = new AsyncLocalStorage();
+
+  assert.strictEqual(als1.getStore(), undefined);
+  assert.strictEqual(als2.getStore(), undefined);
+
+  als1.run('store1', common.mustCall(() => {
+    assert.strictEqual(als1.getStore(), 'store1');
+    assert.strictEqual(als2.getStore(), undefined);
+
+    als2.run('store2', common.mustCall(() => {
+      assert.strictEqual(als1.getStore(), 'store1');
+      assert.strictEqual(als2.getStore(), 'store2');
+    }));
+    assert.strictEqual(als1.getStore(), 'store1');
+    assert.strictEqual(als2.getStore(), undefined);
+
+    als2.enterWith('store3');
+    assert.strictEqual(als1.getStore(), 'store1');
+    assert.strictEqual(als2.getStore(), 'store3');
+  }));
+
+  assert.strictEqual(als1.getStore(), undefined);
+  assert.strictEqual(als2.getStore(), 'store3');
+}
+
+{
+  // Verify als1.disable() has no side effects to als2 and als3
+  const als1 = new AsyncLocalStorage();
+  const als2 = new AsyncLocalStorage();
+  const als3 = new AsyncLocalStorage();
+
+  als3.enterWith('store3');
+
+  als1.run('store1', common.mustCall(() => {
+    assert.strictEqual(als1.getStore(), 'store1');
+    assert.strictEqual(als2.getStore(), undefined);
+    assert.strictEqual(als3.getStore(), 'store3');
+
+    als2.run('store2', common.mustCall(() => {
+      assert.strictEqual(als1.getStore(), 'store1');
+      assert.strictEqual(als2.getStore(), 'store2');
+      assert.strictEqual(als3.getStore(), 'store3');
+
+      als1.disable();
+      assert.strictEqual(als1.getStore(), undefined);
+      assert.strictEqual(als2.getStore(), 'store2');
+      assert.strictEqual(als3.getStore(), 'store3');
+    }));
+    assert.strictEqual(als1.getStore(), undefined);
+    assert.strictEqual(als2.getStore(), undefined);
+    assert.strictEqual(als3.getStore(), 'store3');
+  }));
+
+  assert.strictEqual(als1.getStore(), undefined);
+  assert.strictEqual(als2.getStore(), undefined);
+  assert.strictEqual(als3.getStore(), 'store3');
+}