From 8b810a26949561fd84cc7e75cd61a11a0e512a76 Mon Sep 17 00:00:00 2001 From: Ali Ijaz Sheikh Date: Tue, 6 Feb 2018 10:28:04 -0800 Subject: [PATCH] add async context versions of MakeCallback This commit adds support for the async context accepting versions of node::MakeCallback. An async_context concept has been added as a wrapper around node::async_context, along with APIs for initializing and destroying async context, similar to how [N-API][] exposes this functionality. 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 --- Makefile | 1 + doc/node_misc.md | 55 ++++++++++++++++++ nan.h | 88 +++++++++++++++++++++++++++++ test/binding.gyp | 4 ++ test/cpp/makecallbackcontext.cpp | 65 +++++++++++++++++++++ test/js/makecallbackcontext-test.js | 70 +++++++++++++++++++++++ 6 files changed, 283 insertions(+) create mode 100644 test/cpp/makecallbackcontext.cpp create mode 100644 test/js/makecallbackcontext-test.js diff --git a/Makefile b/Makefile index fc164f88..dfd45bd9 100644 --- a/Makefile +++ b/Makefile @@ -52,6 +52,7 @@ LINT_SOURCES = \ test/cpp/json-parse.cpp \ test/cpp/json-stringify.cpp \ test/cpp/makecallback.cpp \ + test/cpp/makecallbackcontext.cpp \ test/cpp/morenews.cpp \ test/cpp/multifile1.cpp \ test/cpp/multifile2.cpp \ diff --git a/doc/node_misc.md b/doc/node_misc.md index 8aa080f5..ee53abe6 100644 --- a/doc/node_misc.md +++ b/doc/node_misc.md @@ -1,10 +1,45 @@ ## Miscellaneous Node Helpers - Nan::MakeCallback() + - Nan::AsyncInit() + - Nan::AsyncDestory() - NAN_MODULE_INIT() - Nan::Export() + +### Nan::AsyncInit() + +When calling back into JavaScript asynchornously, special care must be taken to ensure that the runtime can properly track +async hops. `Nan::AsyncInit` is an object that wraps `node::EmitAsyncInit` and returns a wrapper for the +`node::async_context` structure. The `async_context` can be provided to `Nan::MakeCallback` to properly restore the correct +async execution context. + +Signatures: + +```c++ +Nan::async_context AsyncInit(v8::MaybeLocal maybe_resource, + const char* name); +Nan::async_context AsyncInit(v8::MaybeLocal maybe_resource, + v8::Local name); +``` + +* `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`. +* An opaque `async_context` structure is returned. This should be passed to any `Nan::MakeCallback` operations done later. + +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 `makecallbackcontext.cpp` example in the `test/cpp` directory. + + +### Nan::AsyncDestroy() + +Wrapper around `node::EmitAsyncDestroy`. + ### Nan::MakeCallback() @@ -15,6 +50,23 @@ Use `MakeCallback()` rather than using `v8::Function#Call()` directly in order t Signatures: ```c++ +v8::Local Nan::MakeCallback(v8::Local target, + v8::Local func, + int argc, + v8::Local* argv, + Nan::async_context async_context); +v8::Local Nan::MakeCallback(v8::Local target, + v8::Local symbol, + int argc, + v8::Local* argv, + Nan::async_context async_context); +v8::Local Nan::MakeCallback(v8::Local target, + const char* method, + int argc, + v8::Local* argv, + Nan::async_context async_context); + +// Legacy versions. We recommend the async context preserving versions above. v8::Local Nan::MakeCallback(v8::Local target, v8::Local func, int argc, @@ -61,3 +113,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..bb6d32b1 100644 --- a/nan.h +++ b/nan.h @@ -1273,6 +1273,94 @@ class Utf8String { #endif // NODE_MODULE_VERSION +//=== async_context and context aware MakeCallback ============================= + +#if NODE_MODULE_VERSION >= NODE_8_0_MODULE_VERSION + typedef node::async_context async_context; +#else + struct async_context {}; +#endif + + inline async_context AsyncInit( + MaybeLocal maybe_resource + , v8::Local resource_name) { +#if NODE_MODULE_VERSION < NODE_8_0_MODULE_VERSION + return async_context(); +#else + 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); + return static_cast(context); +#endif + } + + inline async_context AsyncInit( + MaybeLocal maybe_resource + , const char* name) { + return AsyncInit(maybe_resource, New(name).ToLocalChecked()); + } + + inline void AsyncDestroy(async_context context) { +#if NODE_MODULE_VERSION >= NODE_8_0_MODULE_VERSION + v8::Isolate* isolate = v8::Isolate::GetCurrent(); + node::async_context node_context = static_cast(context); + node::EmitAsyncDestroy(isolate, node_context); +#endif + } + + inline MaybeLocal MakeCallback( + v8::Local target + , v8::Local func + , int argc + , v8::Local* argv + , async_context asyncContext) { +#if NODE_MODULE_VERSION < NODE_8_0_MODULE_VERSION + // Ignore the async_context value. + return MakeCallback(target, func, argc, argv); +#else + return node::MakeCallback( + v8::Isolate::GetCurrent(), target, func, argc, argv, + static_cast(asyncContext)); +#endif + } + + inline MaybeLocal MakeCallback( + v8::Local target + , v8::Local symbol + , int argc + , v8::Local* argv + , async_context asyncContext) { +#if NODE_MODULE_VERSION < NODE_8_0_MODULE_VERSION + // Ignore the async_context value. + return MakeCallback(target, symbol, argc, argv); +#else + return node::MakeCallback( + v8::Isolate::GetCurrent(), target, symbol, argc, argv, + static_cast(asyncContext)); +#endif + } + + inline MaybeLocal MakeCallback( + v8::Local target + , const char* method + , int argc + , v8::Local* argv + , async_context asyncContext) { +#if NODE_MODULE_VERSION < NODE_8_0_MODULE_VERSION + // Ignore the async_context value. + return MakeCallback(target, method, argc, argv); +#else + return node::MakeCallback( + v8::Isolate::GetCurrent(), target, method, argc, argv, + static_cast(asyncContext)); +#endif + } + 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..55a2304c 100644 --- a/test/binding.gyp +++ b/test/binding.gyp @@ -90,6 +90,10 @@ "target_name" : "makecallback" , "sources" : [ "cpp/makecallback.cpp" ] } + , { + "target_name" : "makecallbackcontext" + , "sources" : [ "cpp/makecallbackcontext.cpp" ] + } , { "target_name" : "isolatedata" , "sources" : [ "cpp/isolatedata.cpp" ] diff --git a/test/cpp/makecallbackcontext.cpp b/test/cpp/makecallbackcontext.cpp new file mode 100644 index 00000000..19f3722c --- /dev/null +++ b/test/cpp/makecallbackcontext.cpp @@ -0,0 +1,65 @@ +/********************************************************************* + * NAN - Native Abstractions for Node.js + * + * Copyright (c) 2018 NAN contributors + * + * MIT License + ********************************************************************/ + +#include +#include + +using namespace Nan; // NOLINT(build/namespaces) + +class DelayRequest { + public: + DelayRequest(int milliseconds_, v8::Local callback_) + : milliseconds(milliseconds_) { + callback.Reset(callback_); + request.data = this; + asyncContext = AsyncInit(MaybeLocal(), "test.DelayRequest"); + } + ~DelayRequest() { + AsyncDestroy(asyncContext); + callback.Reset(); + } + + Persistent callback; + uv_work_t request; + async_context asyncContext; + 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(); + MakeCallback(target, callback, 0, argv, delay_request->asyncContext); + + 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, AfterDelay); +} + +NAN_MODULE_INIT(Init) { + Set(target, New("delay").ToLocalChecked(), + GetFunction(New(Delay)).ToLocalChecked()); +} + +NODE_MODULE(makecallbackcontext, Init) diff --git a/test/js/makecallbackcontext-test.js b/test/js/makecallbackcontext-test.js new file mode 100644 index 00000000..ef56b30f --- /dev/null +++ b/test/js/makecallbackcontext-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: 'makecallbackcontext' }).delay + , asyncHooks = require('async_hooks'); + +test('makecallbackcontext', 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 === '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); + }); +});