-
Notifications
You must be signed in to change notification settings - Fork 31.2k
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
async_hooks: add AsyncLocal #31746
async_hooks: add AsyncLocal #31746
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -133,6 +133,69 @@ function createHook(fns) { | |
} | ||
|
||
|
||
// AsyncLocal API // | ||
|
||
const locals = []; | ||
const localsHook = createHook({ | ||
init(asyncId, type, triggerAsyncId, resource) { | ||
const execRes = executionAsyncResource(); | ||
// Using var here instead of let because "for (var ...)" is faster than let. | ||
// Refs: https://github.com/nodejs/node/pull/30380#issuecomment-552948364 | ||
for (var i = 0; i < locals.length; i++) { | ||
locals[i][kPropagateSymbol](execRes, resource); | ||
} | ||
} | ||
}); | ||
|
||
const kPropagateSymbol = Symbol('propagate'); | ||
|
||
class AsyncLocal { | ||
constructor() { | ||
this.symbol = Symbol('async-local'); | ||
this.enable(); | ||
} | ||
|
||
[kPropagateSymbol](execRes, initRes) { | ||
initRes[this.symbol] = execRes[this.symbol]; | ||
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 this might miss a way to stop context propagation without disabling. Right now, this would continue adding 1 property per resource for each AsyncLocal even if unused right? 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.
|
||
} | ||
|
||
unwrap() { | ||
const resource = executionAsyncResource(); | ||
return resource[this.symbol]; | ||
} | ||
|
||
store(value) { | ||
const resource = executionAsyncResource(); | ||
resource[this.symbol] = value; | ||
} | ||
|
||
clear() { | ||
const resource = executionAsyncResource(); | ||
delete resource[this.symbol]; | ||
} | ||
|
||
enable() { | ||
const index = locals.indexOf(this); | ||
if (index === -1) { | ||
locals.push(this); | ||
localsHook.enable(); | ||
} | ||
} | ||
|
||
disable() { | ||
const index = locals.indexOf(this); | ||
if (index === -1) | ||
return; | ||
|
||
this.clear(); | ||
locals.splice(index, 1); | ||
if (locals.length === 0) { | ||
localsHook.disable(); | ||
} | ||
} | ||
} | ||
|
||
|
||
// Embedder API // | ||
|
||
const destroyedSymbol = Symbol('destroyed'); | ||
|
@@ -213,6 +276,7 @@ module.exports = { | |
executionAsyncId, | ||
triggerAsyncId, | ||
executionAsyncResource, | ||
AsyncLocal, | ||
// Embedder API | ||
AsyncResource, | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
'use strict'; | ||
|
||
require('../common'); | ||
const assert = require('assert'); | ||
const async_hooks = require('async_hooks'); | ||
const { AsyncLocal } = async_hooks; | ||
|
||
// This test ensures isolation of `AsyncLocal`s | ||
// from each other in terms of stored values | ||
|
||
const asyncLocalOne = new AsyncLocal(); | ||
const asyncLocalTwo = new AsyncLocal(); | ||
|
||
setTimeout(() => { | ||
assert.strictEqual(asyncLocalOne.unwrap(), undefined); | ||
assert.strictEqual(asyncLocalTwo.unwrap(), undefined); | ||
|
||
asyncLocalOne.store('foo'); | ||
asyncLocalTwo.store('bar'); | ||
assert.strictEqual(asyncLocalOne.unwrap(), 'foo'); | ||
assert.strictEqual(asyncLocalTwo.unwrap(), 'bar'); | ||
|
||
asyncLocalOne.store('baz'); | ||
asyncLocalTwo.store(42); | ||
setTimeout(() => { | ||
assert.strictEqual(asyncLocalOne.unwrap(), 'baz'); | ||
assert.strictEqual(asyncLocalTwo.unwrap(), 42); | ||
}, 0); | ||
}, 0); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
'use strict'; | ||
|
||
require('../common'); | ||
const assert = require('assert'); | ||
const async_hooks = require('async_hooks'); | ||
const { AsyncLocal } = async_hooks; | ||
|
||
// This test ensures correct work of the global hook | ||
// that serves for propagation of all `AsyncLocal`s | ||
// in the context of `.unwrap()`/`.store(value)` calls | ||
|
||
const asyncLocal = new AsyncLocal(); | ||
|
||
setTimeout(() => { | ||
assert.strictEqual(asyncLocal.unwrap(), undefined); | ||
|
||
asyncLocal.store('A'); | ||
setTimeout(() => { | ||
assert.strictEqual(asyncLocal.unwrap(), 'A'); | ||
|
||
asyncLocal.store('B'); | ||
setTimeout(() => { | ||
assert.strictEqual(asyncLocal.unwrap(), 'B'); | ||
}, 0); | ||
|
||
assert.strictEqual(asyncLocal.unwrap(), 'B'); | ||
}, 0); | ||
|
||
assert.strictEqual(asyncLocal.unwrap(), 'A'); | ||
}, 0); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
'use strict'; | ||
|
||
require('../common'); | ||
const assert = require('assert'); | ||
const async_hooks = require('async_hooks'); | ||
const { AsyncLocal } = async_hooks; | ||
|
||
// This test ensures correct work of the global hook | ||
// that serves for propagation of all `AsyncLocal`s | ||
// in the context of `.disable()` call | ||
|
||
const asyncLocalOne = new AsyncLocal(); | ||
asyncLocalOne.store(1); | ||
const asyncLocalTwo = new AsyncLocal(); | ||
asyncLocalTwo.store(2); | ||
|
||
setImmediate(() => { | ||
// Removal of one local should not affect others | ||
asyncLocalTwo.disable(); | ||
assert.strictEqual(asyncLocalOne.unwrap(), 1); | ||
assert.strictEqual(asyncLocalTwo.unwrap(), undefined); | ||
|
||
// Removal of the last active local should not | ||
// prevent propagation of locals created later | ||
asyncLocalOne.disable(); | ||
const asyncLocalThree = new AsyncLocal(); | ||
asyncLocalThree.store(3); | ||
setImmediate(() => { | ||
assert.strictEqual(asyncLocalThree.unwrap(), 3); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
'use strict'; | ||
|
||
const common = require('../common'); | ||
const assert = require('assert'); | ||
const async_hooks = require('async_hooks'); | ||
const { AsyncLocal } = async_hooks; | ||
|
||
const asyncLocal = new AsyncLocal(); | ||
|
||
async function asyncFunc() { | ||
return new Promise((resolve) => { | ||
setTimeout(resolve, 0); | ||
}); | ||
} | ||
|
||
async function testAwait() { | ||
asyncLocal.store('foo'); | ||
await asyncFunc(); | ||
assert.strictEqual(asyncLocal.unwrap(), 'foo'); | ||
} | ||
|
||
testAwait().then(common.mustCall(() => { | ||
assert.strictEqual(asyncLocal.unwrap(), 'foo'); | ||
})); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
'use strict'; | ||
|
||
require('../common'); | ||
const assert = require('assert'); | ||
const async_hooks = require('async_hooks'); | ||
const { AsyncLocal } = async_hooks; | ||
|
||
assert.strictEqual(new AsyncLocal().unwrap(), undefined); | ||
|
||
const asyncLocal = new AsyncLocal(); | ||
|
||
assert.strictEqual(asyncLocal.unwrap(), undefined); | ||
|
||
asyncLocal.store(42); | ||
assert.strictEqual(asyncLocal.unwrap(), 42); | ||
asyncLocal.store('foo'); | ||
assert.strictEqual(asyncLocal.unwrap(), 'foo'); | ||
const obj = {}; | ||
asyncLocal.store(obj); | ||
assert.strictEqual(asyncLocal.unwrap(), obj); | ||
|
||
asyncLocal.disable(); | ||
assert.strictEqual(asyncLocal.unwrap(), undefined); | ||
|
||
// Does not throw when disabled | ||
asyncLocal.store('bar'); | ||
|
||
// Subsequent .disable() does not throw | ||
asyncLocal.disable(); |
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.
I am curious how this relates to the domain problem as it does not enforce scoping. Is that a non-goal here?
In that case, it might be clearer if the doc highlights the pitfalls of the absence of scoping.
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.
The "scoping" is intentionally similar to lexical scoping--anywhere with access to the
asyncLocal
instance has access to the value it contains. At any given point in the lifetime of theasyncLocal
, the value can be branched by storing a new value, resulting in all following async tasks created in the remainder of that sync tick to follow from that branch. This is similar to the domain problem, but not quite the same as domains suffer from multiple-entry problems as there is logic that can be specifically depended on in enter/exit points, whereas here the only thing that can be "depended" on is the availability of the object, which is intentionally accessible in this way.The exit "cleanup" that most CLS implementations tend to do is something like
store[key] = undefined
to "restore" to a blank state after entering does something likestore[key] = getParentContext()
. However there's actually never any point at which theundefined
state would be reachable because it would always be in an "entered" state for the remainder of the async execution tree unless disabled which typically needs to be cleaned up differently anyway. Because there's now a resource stack, we can actually just use the resource itself to represent the "entered" or "exited" state and never actually need an event for it, relying only on an eager propagation in the "init" event and then a simple lookup later. The resource will remain constant for the entire remainder of the callback tick, so the descending tree is stable, it's only the sync tick where a branch/transition occurs that could be considered "unstable", but it functions that way intentionally. By branching mid-way through the tick, it's exactly the same as arun(callback)
method, except it prohibits the possible edge case of arun(...)
with more code after it that might be expected to be in-scope, but is not. Consider an express middleware:This will not work as expected, because the
next()
call was made outside of the run. I see user errors like this all the time, and they can be eliminated by changing from an enter/exit machine to a simpler transition state--you're never "outside", just transitioned to a different level. With the state transition model, you don't need an explicitrun(...)
call as it automatically ensures that any logically following code descends from the branch made at the point when the local value was stored.If you think about it, the "before" and "after" pairing in async_hooks is actually kind of redundant as you can be certain that any time a "before" is triggered it implies the "after" state of the previous callback tick has already been reached. Of course there are ergonomics reasons for this--using any of the id/resource fetching APIs would not give you the data you need to handle cleanup normally done in "after" events if you tried to directly move that logic to the "before" events. Also, technically you can nest enter/exits syncronously with
AsyncResource
, but that's an edge case that is already worked around automatically through state transitions gained byexecutionAsyncResource
observing the resource stack.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.
Fully agree here.
I tried to model the transitions in #27172 (similar as .NET but they even have an ExecutionContext class to signal) to hide the before/after callbacks of async hooks but it didn't make a lot people happy.