async_hooks: add AsyncLocal

[Qard]

This is a continuation of #31016 and is an alternative to #26540. I don't have a strong opinion on which of the two should be accepted, this is just intended as an experiment to show a possible simpler implementation without the safety concerns of the previous AsyncLocal implementation.

Checklist

• make -j4 test (UNIX), or vcbuild test (Windows) passes
• tests and/or benchmarks are included
• documentation is changed or added
• commit message follows commit guidelines

[puzpuzpuz]

@Qard
I'm surprised to see this PR TBH. I have closed #31016 only for the sake of removing AsyncLocal from the competition and letting AsyncContext (now it's called AsyncLocalStorage) to land soon enough, as it seemed that the process would take forever. Obviously, I still believe that AsyncLocal is a better fit for the core, but having one or another CLS API in core in the nearest future is more important than having a particular one.

As this PR competes with #26540, it makes me thinking of reopening #31016.

Update. I've reopened #31016.

[tniessen]

I am a little confused as to who actually wrote the code. This PR seems to duplicate a lot of code from previous proposals, but doesn't attribute it to anyone. (But I also cannot tell who the original authors of other proposals were, this is becoming confusing.)

[puzpuzpuz]

@tniessen I'm the author of #31016 which was used as a foundation of this PR (at least, it looks like this). I didn't use code from other PRs in #31016 and it wasn't a continuation of something, so that PR is standalone.

[Flarna]

I took the idea of AsyncLocal from .NET and ported it to node.js in #27172 which stalled fast.
@puzpuzpuz created #31016 as followup - removing parts, adding new stuff and porting on top of #30959

[puzpuzpuz]

I took the idea of AsyncLocal from .NET and ported it to node.js in #27172 which stalled fast.
@puzpuzpuz created #31016 as followup - removing parts, adding new stuff and porting on top of #30959

Thanks for this addition! Yes, my initial inspiration was @Flarna's #27172. But I've tried to build my take on AsyncLocal while looking at Java's ThreadLocal.

[tniessen]

You might want to attribute authorship via Co-authored-by in all these cases :)

[Qard]

Yep, @puzpuzpuz and @Flarna wrote a bunch of this. I basically took all the test/benchmark code from their PRs and rewrote the AsyncLocal class itself to show an approach mixing some of the ideas of AsyncStorage with some of the ideas of the past AsyncLocal PRs. I don't intend for this to block AsyncStorage, I'm just presenting an alternate implementation based on my own ideas. My hope is that we can land something soon, but progress has been moving quite slow. For what it's worth, I gave my approval to #26540 awhile ago. 🤷‍♂

[puzpuzpuz]

@tniessen

You might want to attribute authorship via Co-authored-by in all these cases :)

Thanks for the advice! I've attributed authorship correctly in #31016.

[vdeturckheim]

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?

[Qard]

asyncLocal.disable() is the intended way to stop propagation. It would remove the asyncLocal from the locals list, preventing the propagator method from being called at all. By writing it this way, it's possible to store falsy values and still have them propagate properly, whereas doing a truthy check of execRes[this.symbol] would not.

[vdeturckheim]

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.

[Qard]

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 the asyncLocal, 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 like store[key] = getParentContext(). However there's actually never any point at which the undefined 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 a run(callback) method, except it prohibits the possible edge case of a run(...) with more code after it that might be expected to be in-scope, but is not. Consider an express middleware:

const cls = new SomeCLS()

app.use((req, res, next) => {
  cls.run(store => {
    store.set('requestId', req.id)
  })
  next()
})

app.get('/', (req, res) => {
  const store = cls.getStore()
  res.end(store.get('requestId'))
})

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 explicit run(...) 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 by executionAsyncResource observing the resource stack.

[Flarna]

Fully agree here.

If you think about it, the "before" and "after" pairing in async_hooks is actually kind of redundant...

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.

[Qard]

Added Co-Authored-By tags to the commit. Sorry it took so long, I've been travelling and haven't had the time to get back to this until now. 😅

[Qard]

Closing as #26540 has landed.