Repository navigation
Integrate C++ AsyncHooks Embedder API with native abstraction #13254
Description
Activity
I can’t really speak for NAN; my best guess is that it will want to build its own thing similar to
AsyncResource.As for N-API: I’m not sure. We could just proxy the low-level API to C and add a
napi_make_callbackvariant that takesidandtrigger_idparameters, but that might require committing to the native async_hooks API as it currently is, i.e. make that non-experimental?One thought I’ve had is that we could add a
CallbackScopeclass that basically does whatMakeCallbackdoes but without necessarily jumping into JS; N-API could use that to make async worker’scompletecallbacks automagically run in the correct async context. That wouldn’t require any API changes, too.- changed the title
[-]Integrate C++ AsyncHooks Embedder API with native abstaction[/-][+]Integrate C++ AsyncHooks Embedder API with native abstraction[/+]on May 27, 2017 I need to familiarize myself more with this, but initially I think @addaleax is right in that it would be better to make a new class for this (new) concept.
Nan::Callbackhas always been a convenience class which is mostly around for legacy reasons (legacy within NAN). It was useful in the Node 0.10 -> 0.12 transition with thePersistentchanges to reduce leaky and broken code. However, making it do something new seems like it could backfire in unforseen ways.Another question is that of compatibility. How far back can this be implemented without patching node.js?
All the way down to 0.10? NAN only offers functionality that works on all supported versions, but supports pick-and-mix with raw V8, libuv and anything else. That way one can write the bulk of code using NAN's abstractions and use, say, the V8 API for Futures if it will only run on versions of node that supports V8 Futures.However, making it do something new seems like it could backfire in unforseen ways.
I'm not exactly sure what "new" covers, I see this as adding annotation to existing functionality. My thoughts behind annotating
Nan::Callbackis that it would make the userland adoption transparent, but if very few are usingNan::Callbackthen that will of cause not be the case.Adoption by userland is critical in this case, because if just one native addon model is missing the annotation then the usefulness of AsyncHooks is dramatically decreased.
Another question is that of compatibility. How far back can this be implemented without patching node.js?
Currently, AsyncHooks will only be in node v8.x, we may backport it to v6.x and perhaps even v4.x. As said the AsyncHooks Embedder API should be seen as annotation, thus for node versions where the AsyncHooks Embedder API is not supported that annotation simply won't be added.
Adoption by userland is critical in this case, because if just one native addon model is missing the annotation then the usefulness of AsyncHooks is dramatically decreased.
I agree – once
async_hooksis no longer experimental, we should probably deprecate the old functionality.we may backport it to v6.x and perhaps even v4.x
I’m pretty sure we won’t backport to v4.x?
I do not know what the explicit usage rate of
Nan::Callbackis. It is however implicitly used inNan::AsyncWorker, which most addons use for asynchronous functions. Holdouts are mostly projects that have been around for a long time and simply updated their existing, working baton code at some point.I am not sure what "new" covers either, which is why I need to read up on what async hooks are supposed to do before I can make an informed statement.
However, the point of NAN is to offer a set of functionality that gives the same observable behavior on all supported versions of Node.js. There should not be anything that only works on specific versions, since that would defeat the point of having a version-independence abstraction layer. An end-user should be able to assume that all code does what it should on all supported versions. This also includes not having functionality that becomes a no-op on certain versions, since that hides that the code does not work as intended.
That being said, if all this does is adding some extra information which can be useful in some cases, there might be a way to add some of it to NAN so that code running on newer versions of node.js does something in a better or more efficient manner. For instance, if async hooks subsumes an older API which may then be deprecated, the NAN code for all newer versions that support this functionality could be made to use it instead of the older functionality, since that would fall within implementation, not version-independent functionality.
To make this a bit clearer: We can think of two aspects of NAN: One is the external, documented API. Only functionality which works on all supported versions goes here. The other is the internal implementation (actually there are several of them depending on node version), which has greater freedom. Here, the only restriction is: Given a program in state S which calls a part of NAN, at return from NAN, the program is in state S', regardless of which version-dependent implementation was used.
This is a truth with modification, but we try to keep it as close as possible. In practice, we may end up in a state S'' which is a superset of S'. While not ideal, this is sometimes the only practical way. What is not allowed is to sometimes end up in a state S''' which is a strict subset of S'.It is however implicitly used in
Nan::AsyncWorker, which most addons use for asynchronous functions.I think it should be easy enough to add support for everything we need to
AsyncWorkertransparently, because N-API is basically going to need the same things anyway.There should not be anything that only works on specific versions, since that would defeat the point of having a version-independence abstraction layer.
You mean, that works from the addon writer’s perspective, right? It would be okay if Nan had code that only worked well with the versions of Node to which
async_hookshas been ported, correct?Also: Would it help if Node had a
#definethat indicated the presence of the C++ embedder API?59 remaining items
Thanks @ofrobots for driving this. :)
- added a commit that references this issue
on Apr 16, 2018 - added a commit that references this issue
on May 8, 2018 - added a commit that references this issue
on Oct 3, 2024 - added a commit that references this issue
on Jul 27, 2026
The AsyncHooks Embedder API has now been merged, we need to integrate this into N-API and NAN such that userland add-ons can inform
async_hooksabout the context.I'm not very familiar with either APIs, but NAN is the API I know the best, so I will explain it from that perspective.
AsyncHooks allows userland to get notified about all asynchronous event and understand what caused the asynchronous job to be tasked. This requires 4 events to be emitted:
init: emitted with the asynchronous job is created (called a resource).EmitAsyncInitemits this.before,after: emitted with the asynchronous job calls back, this can happen multiple times.MakeCallbacknow emits these when two additional parameters are passed (async_idandtrigger_id).destroy: emitted when the resource can't call back anymore.EmitAsyncDestroyemits this.there is also a high-level API, a C++ class called
AsyncResourcebut I suspect this isn't useful for NAN or N-API.In terms of NAN I think there is almost a 1 to 1 mapping between
Nan::Callbackand the AsyncHooks API. I believe the following changes should be made:Callback::Callbackshould calltrigger_id = AsyncHooksGetTriggerId(isolate);anduid = EmitAsyncInit(isolate, resource, name, trigger_id);.Callback::Callshould callnode::MakeCallback(isolate, resource, callback, argc, argv, uid, trigger_id);Callback::~Callbackshould callEmitAsyncDestroy(isolate, uid);This is very similar to the
AsyncResourceclass. It is not clear what theresourceshould be asCallback::Callbackdoes not take such a parameter.I believe @mhdawson said during a diagnostics meeting that if NAN required changes then likely N-API would need changes too.
/cc @mhdawson @addaleax @trevnorris @nodejs/diagnostics @nodejs/n-api @nodejs/addon-api @nodejs/nan (@kkoopa)