-
-
Notifications
You must be signed in to change notification settings - Fork 37.1k
Integrate C++ AsyncHooks Embedder API with native abstraction #13254
Copy link
Copy link
Closed
Labels
addonsIssues and PRs related to native addons.Issues and PRs related to native addons.async_hooksIssues and PRs related to the async hooks subsystem.Issues and PRs related to the async hooks subsystem.c++Issues and PRs that require attention from people who are familiar with C++.Issues and PRs that require attention from people who are familiar with C++.diag-agendaIssues and PRs to discuss during Diagnostics Working Group meetings.Issues and PRs to discuss during Diagnostics Working Group meetings.feature requestIssues requesting new Node.js features.Issues requesting new Node.js features.node-apiIssues and PRs related to Node-API.Issues and PRs related to Node-API.
Description
Activity
Metadata
Metadata
Assignees
Labels
addonsIssues and PRs related to native addons.Issues and PRs related to native addons.async_hooksIssues and PRs related to the async hooks subsystem.Issues and PRs related to the async hooks subsystem.c++Issues and PRs that require attention from people who are familiar with C++.Issues and PRs that require attention from people who are familiar with C++.diag-agendaIssues and PRs to discuss during Diagnostics Working Group meetings.Issues and PRs to discuss during Diagnostics Working Group meetings.feature requestIssues requesting new Node.js features.Issues requesting new Node.js features.node-apiIssues and PRs related to Node-API.Issues and PRs related to Node-API.
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)