From 9058383a2139e96de6c5fff97339ef10d03f9e4f Mon Sep 17 00:00:00 2001 From: Amit Parida Date: Tue, 6 Feb 2018 10:28:04 -0800 Subject: [PATCH] introduce AsyncResource class This commit adds support for the AsyncResource API to allow native modules to asynchronously call back into JavaScript while preserving node's async context. This acts as a higher level alternative to the MakeCallback API. This is analogous to the AsyncResource JavaScript class exposed by [async_hooks][] and similar to the `napi_async_init`, `napi_async_destroy` and `napi_make_callback` APIs, albeit wrapped in a convenient RAII form-factor. Ref: https://github.com/nodejs/node/issues/13254 [N-API]: https://nodejs.org/dist/latest-v9.x/docs/api/n-api.html#n_api_custom_asynchronous_operations [async_hooks]: https://nodejs.org/api/async_hooks.html --- Makefile | 1 + doc/node_misc.md | 60 ++++++++++++++++++++- nan.h | 98 +++++++++++++++++++++++++++++++++++ test/binding.gyp | 4 ++ test/cpp/asyncresource.cpp | 69 ++++++++++++++++++++++++ test/js/asyncresource-test.js | 70 +++++++++++++++++++++++++ 6 files changed, 300 insertions(+), 2 deletions(-) create mode 100644 test/cpp/asyncresource.cpp create mode 100644 test/js/asyncresource-test.js diff --git a/Makefile b/Makefile index fc164f88..1b97e3f6 100644 --- a/Makefile +++ b/Makefile @@ -35,6 +35,7 @@ LINT_SOURCES = \ nan_weak.h \ test/cpp/accessors.cpp \ test/cpp/accessors2.cpp \ + test/cpp/asyncresource.cpp \ test/cpp/asyncworker.cpp \ test/cpp/asyncprogressworker.cpp \ test/cpp/asyncprogressworkerstream.cpp \ diff --git a/doc/node_misc.md b/doc/node_misc.md index 8aa080f5..01343520 100644 --- a/doc/node_misc.md +++ b/doc/node_misc.md @@ -1,16 +1,69 @@ ## Miscellaneous Node Helpers + - Nan::AsyncResource - Nan::MakeCallback() - NAN_MODULE_INIT() - Nan::Export() + +### Nan::AsyncResource + +This class is analogous to the `AsyncResource` JavaScript class exposed by Node's [async_hooks][] API. + +When calling back into JavaScript asynchornously, special care must be taken to ensure that the runtime can properly track +async hops. `Nan::AsyncResource` is a class that provides an RAII wrapper around `node::EmitAsyncInit`, `node::EmitAsyncDestroy`, +and `node::MakeCallback`. Using this mechanism to call back into JavaScript, as opposed to `Nan::MakeCallback` or +`v8::Function::Call` ensures that the callback is executed in the correct async context. This ensures that async mechanisms +such as domains and [async_hooks][] function correctly. + +Definition: + +```c++ +class AsyncResource { + public: + AsyncResource(MaybeLocal maybe_resource, v8::Local name); + AsyncResource(MaybeLocal maybe_resource, const char* name); + ~AsyncResource(); + + v8::MaybeLocal runInAsyncScope(v8::Local target, + v8::Local func, + int argc, + v8::Local* argv, + Nan::async_context async_context); + v8::MaybeLocal runInAsyncScope(v8::Local target, + v8::Local symbol, + int argc, + v8::Local* argv, + Nan::async_context async_context); + v8::MaybeLocal runInAsyncScope(v8::Local target, + const char* method, + int argc, + v8::Local* argv, + Nan::async_context async_context); +}; +``` + +* `maybe_resource`: An optional object associated with the async work that will be passed to the possible [async_hooks][] + `init` hook. +* `name`: Identified for the kind of resource that is being provided for diagnostics information exposed by the [async_hooks][] + API. This will be passed to the possible `init` hook as the `type`. To avoid name collisions with other modules we recommend + that the name include the name of the owning module as a prefix. For example `mysql` module could use something like + `mysql:batch-db-query-resource`. +* When calling JS on behalf of this resource, one can use `runInAsyncScope`. This will ensure that the callback runs in the + correct async execution context. +* `AsyncDestroy` is automatically called when an AsyncResource object is destroyed. + +For more details, see the Node [async_hooks][] documentation. You might also want to take a look at the documentation for the +[N-API counterpart][napi]. For example usage, see the `asyncresource.cpp` example in the `test/cpp` directory. ### Nan::MakeCallback() -Wrappers around `node::MakeCallback()` providing a consistent API across all supported versions of Node. +Wrappers around the legacy `node::MakeCallback()` APIs. -Use `MakeCallback()` rather than using `v8::Function#Call()` directly in order to properly process internal Node functionality including domains, async hooks, the microtask queue, and other debugging functionality. +We recommend that you use the `AsyncResource` class and `AsyncResource::runInAsyncScope` instead of using `Nan::MakeCallback` or +`v8::Function#Call()` directly. `AsyncResource` properly takes care of running the callback in the correct async execution +context – something that is essential for functionality like domains, async_hooks and async debugging. Signatures: @@ -61,3 +114,6 @@ NAN_MODULE_INIT(Init) { NAN_EXPORT(target, Foo); } ``` + +[async_hooks]: https://nodejs.org/dist/latest-v9.x/docs/api/async_hooks.html +[napi]: https://nodejs.org/dist/latest-v9.x/docs/api/n-api.html#n_api_custom_asynchronous_operations diff --git a/nan.h b/nan.h index 7c7699ff..ae4894f3 100644 --- a/nan.h +++ b/nan.h @@ -1273,6 +1273,104 @@ class Utf8String { #endif // NODE_MODULE_VERSION +//=== async_context ============================================================ + +#if NODE_MODULE_VERSION >= NODE_8_0_MODULE_VERSION + typedef node::async_context async_context; +#else + struct async_context {}; +#endif + +// === AsyncResource =========================================================== + + class AsyncResource { + public: + AsyncResource( + MaybeLocal maybe_resource + , v8::Local resource_name) { +#if NODE_MODULE_VERSION >= NODE_8_0_MODULE_VERSION + v8::Isolate* isolate = v8::Isolate::GetCurrent(); + + v8::Local resource = + maybe_resource.IsEmpty() ? New() + : maybe_resource.ToLocalChecked(); + + node::async_context context = + node::EmitAsyncInit(isolate, resource, resource_name); + asyncContext = static_cast(context); +#endif + } + + AsyncResource(MaybeLocal maybe_resource, const char* name) { +#if NODE_MODULE_VERSION >= NODE_8_0_MODULE_VERSION + v8::Isolate* isolate = v8::Isolate::GetCurrent(); + + v8::Local resource = + maybe_resource.IsEmpty() ? New() + : maybe_resource.ToLocalChecked(); + v8::Local name_string = + New(name).ToLocalChecked(); + node::async_context context = + node::EmitAsyncInit(isolate, resource, name_string); + asyncContext = static_cast(context); +#endif + } + + ~AsyncResource() { +#if NODE_MODULE_VERSION >= NODE_8_0_MODULE_VERSION + v8::Isolate* isolate = v8::Isolate::GetCurrent(); + node::async_context node_context = + static_cast(asyncContext); + node::EmitAsyncDestroy(isolate, node_context); +#endif + } + + inline MaybeLocal runInAsyncScope( + v8::Local target + , v8::Local func + , int argc + , v8::Local* argv) { +#if NODE_MODULE_VERSION < NODE_8_0_MODULE_VERSION + return MakeCallback(target, func, argc, argv); +#else + return node::MakeCallback( + v8::Isolate::GetCurrent(), target, func, argc, argv, + static_cast(asyncContext)); +#endif + } + + inline MaybeLocal runInAsyncScope( + v8::Local target + , v8::Local symbol + , int argc + , v8::Local* argv) { +#if NODE_MODULE_VERSION < NODE_8_0_MODULE_VERSION + return MakeCallback(target, symbol, argc, argv); +#else + return node::MakeCallback( + v8::Isolate::GetCurrent(), target, symbol, argc, argv, + static_cast(asyncContext)); +#endif + } + + inline MaybeLocal runInAsyncScope( + v8::Local target + , const char* method + , int argc + , v8::Local* argv) { +#if NODE_MODULE_VERSION < NODE_8_0_MODULE_VERSION + return MakeCallback(target, method, argc, argv); +#else + return node::MakeCallback( + v8::Isolate::GetCurrent(), target, method, argc, argv, + static_cast(asyncContext)); +#endif + } + + private: + async_context asyncContext; + }; + typedef void (*FreeCallback)(char *data, void *hint); typedef const FunctionCallbackInfo& NAN_METHOD_ARGS_TYPE; diff --git a/test/binding.gyp b/test/binding.gyp index 2c70cb9e..28d51a4a 100644 --- a/test/binding.gyp +++ b/test/binding.gyp @@ -90,6 +90,10 @@ "target_name" : "makecallback" , "sources" : [ "cpp/makecallback.cpp" ] } + , { + "target_name" : "asyncresource" + , "sources" : [ "cpp/asyncresource.cpp" ] + } , { "target_name" : "isolatedata" , "sources" : [ "cpp/isolatedata.cpp" ] diff --git a/test/cpp/asyncresource.cpp b/test/cpp/asyncresource.cpp new file mode 100644 index 00000000..8777e661 --- /dev/null +++ b/test/cpp/asyncresource.cpp @@ -0,0 +1,69 @@ +/********************************************************************* + * NAN - Native Abstractions for Node.js + * + * Copyright (c) 2018 NAN contributors + * + * MIT License + ********************************************************************/ + +#include +#include + +using namespace Nan; // NOLINT(build/namespaces) + +class DelayRequest : public AsyncResource { + public: + DelayRequest(int milliseconds_, v8::Local callback_) + : AsyncResource(MaybeLocal(), "nan:test.DelayRequest"), + milliseconds(milliseconds_) { + callback.Reset(callback_); + request.data = this; + } + ~DelayRequest() { + callback.Reset(); + } + + Persistent callback; + uv_work_t request; + int milliseconds; +}; + +void Delay(uv_work_t* req) { + DelayRequest *delay_request = static_cast(req->data); + sleep(delay_request->milliseconds / 1000); +} + +void AfterDelay(uv_work_t* req, int status) { + HandleScope scope; + + DelayRequest *delay_request = static_cast(req->data); + v8::Local callback = New(delay_request->callback); + v8::Local argv[0] = {}; + + v8::Local target = New(); + + // Run the callback in the async context. + delay_request->runInAsyncScope(target, callback, 0, argv); + + delete delay_request; +} + +NAN_METHOD(Delay) { + int delay = To(info[0]).FromJust(); + v8::Local cb = To(info[1]).ToLocalChecked(); + + DelayRequest* delay_request = new DelayRequest(delay, cb); + + uv_queue_work( + uv_default_loop() + , &delay_request->request + , Delay + , reinterpret_cast(AfterDelay)); +} + +NAN_MODULE_INIT(Init) { + Set(target, New("delay").ToLocalChecked(), + GetFunction(New(Delay)).ToLocalChecked()); +} + +NODE_MODULE(asyncresource, Init) diff --git a/test/js/asyncresource-test.js b/test/js/asyncresource-test.js new file mode 100644 index 00000000..5e093fa7 --- /dev/null +++ b/test/js/asyncresource-test.js @@ -0,0 +1,70 @@ +/********************************************************************* + * NAN - Native Abstractions for Node.js + * + * Copyright (c) 2018 NAN contributors + * + * MIT License + ********************************************************************/ + +try { + require('async_hooks'); +} catch (e) { + process.exit(0); +} + +const test = require('tap').test + , testRoot = require('path').resolve(__dirname, '..') + , delay = require('bindings')({ module_root: testRoot, bindings: 'asyncresource' }).delay + , asyncHooks = require('async_hooks'); + +test('asyncresource', function (t) { + t.plan(7); + + var resourceAsyncId; + var originalExecutionAsyncId; + var beforeCalled = false; + var afterCalled = false; + var destroyCalled = false; + + var hooks = asyncHooks.createHook({ + init: function(asyncId, type, triggerAsyncId, resource) { + if (type === 'nan:test.DelayRequest') { + resourceAsyncId = asyncId; + } + }, + before: function(asyncId) { + if (asyncId === resourceAsyncId) { + beforeCalled = true; + } + }, + after: function(asyncId) { + if (asyncId === resourceAsyncId) { + afterCalled = true; + } + }, + destroy: function(asyncId) { + if (asyncId === resourceAsyncId) { + destroyCalled = true; + } + } + + }); + hooks.enable(); + + originalExecutionAsyncId = asyncHooks.executionAsyncId(); + delay(1000, function() { + t.equal(asyncHooks.executionAsyncId(), resourceAsyncId, + 'callback should have the correct execution context'); + t.equal(asyncHooks.triggerAsyncId(), originalExecutionAsyncId, + 'callback should have the correct trigger context'); + t.ok(beforeCalled, 'before should have been called'); + t.notOk(afterCalled, 'after should not have been called yet'); + setTimeout(function() { + t.ok(afterCalled, 'after should have been called'); + t.ok(destroyCalled, 'destroy should have been called'); + t.equal(asyncHooks.triggerAsyncId(), resourceAsyncId, + 'setTimeout should have been triggered by the async resource'); + hooks.disable(); + }, 1); + }); +});