Commit 269931f
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 <legendecas@gmail.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>1 parent 5656c74 commit 269931f
3 files changed
Lines changed: 81 additions & 3 deletions
File tree
- lib/internal/async_local_storage
- test
- async-hooks
- parallel
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -1,6 +1,7 @@ | |||
| 1 | 1 | 'use strict'; | |
| 2 | 2 | ||
| 3 | 3 | const { | |
| 4 | + ObjectIs, | ||
| 4 | 5 | ReflectApply, | |
| 5 | 6 | } = primordials; | |
| 6 | 7 | ||
@@ -26,12 +27,15 @@ class AsyncLocalStorage { | |||
| 26 | 27 | } | |
| 27 | 28 | ||
| 28 | 29 | run(data, fn, ...args) { | |
| 29 | - const prior = AsyncContextFrame.current(); | ||
| 30 | + const prior = this.getStore(); | ||
| 31 | + if (ObjectIs(prior, data)) { | ||
| 32 | + return ReflectApply(fn, null, args); | ||
| 33 | + } | ||
| 30 | 34 | this.enterWith(data); | |
| 31 | 35 | try { | |
| 32 | 36 | return ReflectApply(fn, null, args); | |
| 33 | 37 | } finally { | |
| 34 | - AsyncContextFrame.set(prior); | ||
| 38 | + this.enterWith(prior); | ||
| 35 | 39 | } | |
| 36 | 40 | } | |
| 37 | 41 | ||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -1,11 +1,12 @@ | |||
| 1 | 1 | 'use strict'; | |
| 2 | - // Flags: --expose_gc | ||
| 2 | + // Flags: --expose_gc --expose-internals | ||
| 3 | 3 | ||
| 4 | 4 | // This test ensures that AsyncLocalStorage gets gced once it was disabled | |
| 5 | 5 | // and no strong references remain in userland. | |
| 6 | 6 | ||
| 7 | 7 | const common = require('../common'); | |
| 8 | 8 | const { AsyncLocalStorage } = require('async_hooks'); | |
| 9 | + const AsyncContextFrame = require('internal/async_context_frame'); | ||
| 9 | 10 | const { onGC } = require('../common/gc'); | |
| 10 | 11 | ||
| 11 | 12 | let asyncLocalStorage = new AsyncLocalStorage(); | |
@@ -16,5 +17,11 @@ asyncLocalStorage.run({}, () => { | |||
| 16 | 17 | onGC(asyncLocalStorage, { ongc: common.mustCall() }); | |
| 17 | 18 | }); | |
| 18 | 19 | ||
| 20 | + if (AsyncContextFrame.enabled) { | ||
| 21 | + // This disable() is needed to remove reference form AsyncContextFrame | ||
| 22 | + // created during exit of run() to the AsyncLocalStore instance. | ||
| 23 | + asyncLocalStorage.disable(); | ||
| 24 | + } | ||
| 25 | + | ||
| 19 | 26 | asyncLocalStorage = null; | |
| 20 | 27 | global.gc(); | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -0,0 +1,67 @@ | |||
| 1 | + 'use strict'; | ||
| 2 | + const common = require('../common'); | ||
| 3 | + const { AsyncLocalStorage } = require('node:async_hooks'); | ||
| 4 | + const assert = require('node:assert'); | ||
| 5 | + | ||
| 6 | + // Verify that ALS instances are independent of each other. | ||
| 7 | + | ||
| 8 | + { | ||
| 9 | + // Verify als2.enterWith() and als2.run inside als1.run() | ||
| 10 | + const als1 = new AsyncLocalStorage(); | ||
| 11 | + const als2 = new AsyncLocalStorage(); | ||
| 12 | + | ||
| 13 | + assert.strictEqual(als1.getStore(), undefined); | ||
| 14 | + assert.strictEqual(als2.getStore(), undefined); | ||
| 15 | + | ||
| 16 | + als1.run('store1', common.mustCall(() => { | ||
| 17 | + assert.strictEqual(als1.getStore(), 'store1'); | ||
| 18 | + assert.strictEqual(als2.getStore(), undefined); | ||
| 19 | + | ||
| 20 | + als2.run('store2', common.mustCall(() => { | ||
| 21 | + assert.strictEqual(als1.getStore(), 'store1'); | ||
| 22 | + assert.strictEqual(als2.getStore(), 'store2'); | ||
| 23 | + })); | ||
| 24 | + assert.strictEqual(als1.getStore(), 'store1'); | ||
| 25 | + assert.strictEqual(als2.getStore(), undefined); | ||
| 26 | + | ||
| 27 | + als2.enterWith('store3'); | ||
| 28 | + assert.strictEqual(als1.getStore(), 'store1'); | ||
| 29 | + assert.strictEqual(als2.getStore(), 'store3'); | ||
| 30 | + })); | ||
| 31 | + | ||
| 32 | + assert.strictEqual(als1.getStore(), undefined); | ||
| 33 | + assert.strictEqual(als2.getStore(), 'store3'); | ||
| 34 | + } | ||
| 35 | + | ||
| 36 | + { | ||
| 37 | + // Verify als1.disable() has no side effects to als2 and als3 | ||
| 38 | + const als1 = new AsyncLocalStorage(); | ||
| 39 | + const als2 = new AsyncLocalStorage(); | ||
| 40 | + const als3 = new AsyncLocalStorage(); | ||
| 41 | + | ||
| 42 | + als3.enterWith('store3'); | ||
| 43 | + | ||
| 44 | + als1.run('store1', common.mustCall(() => { | ||
| 45 | + assert.strictEqual(als1.getStore(), 'store1'); | ||
| 46 | + assert.strictEqual(als2.getStore(), undefined); | ||
| 47 | + assert.strictEqual(als3.getStore(), 'store3'); | ||
| 48 | + | ||
| 49 | + als2.run('store2', common.mustCall(() => { | ||
| 50 | + assert.strictEqual(als1.getStore(), 'store1'); | ||
| 51 | + assert.strictEqual(als2.getStore(), 'store2'); | ||
| 52 | + assert.strictEqual(als3.getStore(), 'store3'); | ||
| 53 | + | ||
| 54 | + als1.disable(); | ||
| 55 | + assert.strictEqual(als1.getStore(), undefined); | ||
| 56 | + assert.strictEqual(als2.getStore(), 'store2'); | ||
| 57 | + assert.strictEqual(als3.getStore(), 'store3'); | ||
| 58 | + })); | ||
| 59 | + assert.strictEqual(als1.getStore(), undefined); | ||
| 60 | + assert.strictEqual(als2.getStore(), undefined); | ||
| 61 | + assert.strictEqual(als3.getStore(), 'store3'); | ||
| 62 | + })); | ||
| 63 | + | ||
| 64 | + assert.strictEqual(als1.getStore(), undefined); | ||
| 65 | + assert.strictEqual(als2.getStore(), undefined); | ||
| 66 | + assert.strictEqual(als3.getStore(), 'store3'); | ||
| 67 | + } | ||
0 commit comments