Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 36.4k
async_hooks: add use() method to AsyncLocalStorage#58104
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -347,6 +347,43 @@ try { | ||
| } | ||
| ``` | ||
| ### `asyncLocalStorage.use(store)` | ||
| <!-- YAML | ||
| added: REPLACEME | ||
| --> | ||
| * `store` {any} | ||
| * Returns: {Disposable} A disposable object. | ||
| Transitions into the given context, and transitions back into the previous | ||
Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it's not the previous context. It's that one seen at the time e.g. a cast like this (condensed, in real world this would be distributed at independent locations): const{ AsyncLocalStorage }=require("node:async_hooks");constals1=newAsyncLocalStorage();constals2=newAsyncLocalStorage();constd1=als1.use(store1);constd2=als2.use(store2);constd3=als1.use(store3);als1.getStore();// Returns store3als2.getStore();// Returns store2d1[Symbol.dispose]();als1.getStore();// Returns undefinedals2.getStore();// Returns undefinedd2[Symbol.dispose]();als1.getStore();// Returns store1als2.getStore();// Returns undefinedd3[Symbol.dispose]();als1.getStore();// Returns store1als2.getStore();// Returns store2Likely a user problem and once In special that one ALS user might effect others is critical in my opinion. Note that only the Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think if folks are manually calling Symbol.dispose they're rather on their own at that point. It's a bit unfortunate that the ERM model gives them that ability but there's only do much we can do. Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That said, we'll want to be sure that this works as expected when using DisposableStack when that is available. Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I fully agree once Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I notices that the side effect to other ALS instances is not bound to wrong usage of dispose: constd1=als1.use("store1");als2.enterWith("store2");console.log(als1.getStore());// store1console.log(als2.getStore());// store2d1[Symbol.dispose]();console.log(als1.getStore());// undefinedconsole.log(als2.getStore());// undefined, but should be store2Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I noticed that this global side effect is not new in the MemberAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
| ||
| context when the returned disposable object is disposed. The store is | ||
| accessible to any asynchronous operations created before disposal. | ||
| Example: | ||
| ```js | ||
| const store1 = { id: 1 }; | ||
| const store2 = { id: 2 }; | ||
| function inner() { | ||
| // Once `using` syntax is supported, you can use that here, and omit the | ||
| // dispose call at the end of this function. | ||
| const disposable = asyncLocalStorage.use(store); | ||
Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
| ||
| asyncLocalStorage.getStore(); // Returns store2 | ||
| setTimeout(() => { | ||
| asyncLocalStorage.getStore(); // Returns store2 | ||
| }, 200); | ||
| disposable[Symbol.dispose](); | ||
| } | ||
| asyncLocalStorage.run(store1, () => { | ||
| asyncLocalStorage.getStore(); // Returns store1 | ||
| inner(); | ||
| asyncLocalStorage.getStore(); // Returns store1 | ||
| }); | ||
| ``` | ||
| ### `asyncLocalStorage.exit(callback[, ...args])` | ||
| <!-- YAML | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -2,6 +2,7 @@ | ||
| const { | ||
| ReflectApply, | ||
| SymbolDispose, | ||
| } = primordials; | ||
| const { | ||
| @@ -11,6 +12,19 @@ const { | ||
| const AsyncContextFrame = require('internal/async_context_frame'); | ||
| const { AsyncResource } = require('async_hooks'); | ||
| class DisposableStore { | ||
| #oldFrame = undefined; | ||
| constructor(store, storage) { | ||
| this.#oldFrame = AsyncContextFrame.current(); | ||
Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess by capturing/restoring only the owned store instead the complete frame the side effects to other ALS users could be avoided. Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed, an operation on an AsyncLocalStorage should not affect other instances. For reference: tc39/proposal-async-context-disposable#2 | ||
| storage.enterWith(store); | ||
| } | ||
| [SymbolDispose]() { | ||
| AsyncContextFrame.set(this.#oldFrame); | ||
| } | ||
| } | ||
| class AsyncLocalStorage { | ||
| #defaultValue = undefined; | ||
| #name = undefined; | ||
| @@ -62,6 +76,10 @@ class AsyncLocalStorage { | ||
| } | ||
| } | ||
| use(data) { | ||
| return new DisposableStore(data, this); | ||
| } | ||
| exit(fn, ...args) { | ||
| return this.run(undefined, fn, ...args); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| 'use strict'; | ||
| require('../common'); | ||
| const assert = require('assert'); | ||
| const { AsyncLocalStorage } = require('async_hooks'); | ||
| const storage = new AsyncLocalStorage(); | ||
| const store1 = {}; | ||
| const store2 = {}; | ||
| const store3 = {}; | ||
| function inner() { | ||
| // TODO(bengl): Once `using` is supported use that here and don't call | ||
| // dispose manually later. | ||
| const disposable = storage.use(store2); | ||
| assert.strictEqual(storage.getStore(), store2); | ||
| storage.enterWith(store3); | ||
| disposable[Symbol.dispose](); | ||
| } | ||
| storage.enterWith(store1); | ||
| assert.strictEqual(storage.getStore(), store1); | ||
| inner(); | ||
| assert.strictEqual(storage.getStore(), store1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it intended to be added as
stable?Otherwise add
> Stability: 1 - Experimentalhere.