From 10a38e86576c840fff743d851ab212f8dc45f067 Mon Sep 17 00:00:00 2001 From: Ali Ijaz Sheikh Date: Wed, 7 Feb 2018 10:49:50 -0800 Subject: [PATCH] add AsyncResource convenience class --- Makefile | 1 + doc/node_misc.md | 86 +++++++++++++++++++++++------ nan.h | 51 ++++++++++++++++- test/binding.gyp | 4 ++ test/cpp/asyncresource.cpp | 69 +++++++++++++++++++++++ test/cpp/makecallbackcontext.cpp | 3 +- test/js/asyncresource-test.js | 70 +++++++++++++++++++++++ test/js/makecallbackcontext-test.js | 2 +- 8 files changed, 265 insertions(+), 21 deletions(-) create mode 100644 test/cpp/asyncresource.cpp create mode 100644 test/js/asyncresource-test.js diff --git a/Makefile b/Makefile index dfd45bd9..6a0d7697 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 ee53abe6..d8368f13 100644 --- a/doc/node_misc.md +++ b/doc/node_misc.md @@ -1,13 +1,14 @@ ## Miscellaneous Node Helpers - - Nan::MakeCallback() - Nan::AsyncInit() - Nan::AsyncDestory() + - Nan::AsyncResource + - Nan::MakeCallback() - 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 @@ -35,14 +36,63 @@ Nan::async_context AsyncInit(v8::MaybeLocal maybe_resource, 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. - +Note: It might be more convenient to use `Nan::AsyncResource` instead of using this directly. + + ### Nan::AsyncDestroy() Wrapper around `node::EmitAsyncDestroy`. +Note: It might be more convenient to use `Nan::AsyncResource` instead of using this directly. + + +### Nan::AsyncResource + +`Nan::AsyncResouce` is a convenience class that provides RAII wrapper around `Nan::AsyncInit`, `Nan::AsyncDestroy` and `Nan::MakeCallback`. It is analogous to the `AsyncResource` JavaScript class exposed by Node's [async_hooks][] API. + +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 example usage, see the `asyncresource.cpp` example in the `test/cpp` directory. + ### Nan::MakeCallback() +Note: It might be more convenient to use `Nan::AsyncResource` instead of using this directly. + Wrappers around `node::MakeCallback()` providing a consistent API across all supported versions of Node. 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. @@ -50,21 +100,21 @@ 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); +v8::MaybeLocal Nan::MakeCallback(v8::Local target, + v8::Local func, + int argc, + v8::Local* argv, + Nan::async_context async_context); +v8::MaybeLocal Nan::MakeCallback(v8::Local target, + v8::Local symbol, + int argc, + v8::Local* argv, + Nan::async_context async_context); +v8::MaybeLocal 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, diff --git a/nan.h b/nan.h index bb6d32b1..4632be98 100644 --- a/nan.h +++ b/nan.h @@ -1295,6 +1295,7 @@ class Utf8String { node::async_context context = node::EmitAsyncInit(isolate, resource, resource_name); + printf("in AsyncInit, context is <%f,%f>\n", context.async_id, context.trigger_async_id); return static_cast(context); #endif } @@ -1308,7 +1309,8 @@ class Utf8String { 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::async_context node_context = + static_cast(context); node::EmitAsyncDestroy(isolate, node_context); #endif } @@ -1323,6 +1325,7 @@ class Utf8String { // Ignore the async_context value. return MakeCallback(target, func, argc, argv); #else + printf("in MakeCallback, context is <%f,%f>\n", asyncContext.async_id, asyncContext.trigger_async_id); return node::MakeCallback( v8::Isolate::GetCurrent(), target, func, argc, argv, static_cast(asyncContext)); @@ -1361,6 +1364,52 @@ class Utf8String { #endif } +// === AsyncResource =========================================================== + + class AsyncResource { + public: + AsyncResource( + MaybeLocal maybe_resource + , v8::Local resource_name) { + asyncContext = AsyncInit(maybe_resource, resource_name); + } + + AsyncResource(MaybeLocal maybe_resource, const char* name) { + asyncContext = AsyncInit(maybe_resource, name); + } + + ~AsyncResource() { + AsyncDestroy(asyncContext); + } + + inline MaybeLocal runInAsyncScope( + v8::Local target + , v8::Local func + , int argc + , v8::Local* argv) { + return MakeCallback(target, func, argc, argv, asyncContext); + } + + inline MaybeLocal runInAsyncScope( + v8::Local target + , v8::Local symbol + , int argc + , v8::Local* argv) { + return MakeCallback(target, symbol, argc, argv, asyncContext); + } + + inline MaybeLocal runInAsyncScope( + v8::Local target + , const char* method + , int argc + , v8::Local* argv) { + return MakeCallback(target, method, argc, argv, asyncContext); + } + + protected: + 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 55a2304c..f0c78089 100644 --- a/test/binding.gyp +++ b/test/binding.gyp @@ -94,6 +94,10 @@ "target_name" : "makecallbackcontext" , "sources" : [ "cpp/makecallbackcontext.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/cpp/makecallbackcontext.cpp b/test/cpp/makecallbackcontext.cpp index 99a553a6..a9543b67 100644 --- a/test/cpp/makecallbackcontext.cpp +++ b/test/cpp/makecallbackcontext.cpp @@ -17,7 +17,8 @@ class DelayRequest { : milliseconds(milliseconds_) { callback.Reset(callback_); request.data = this; - asyncContext = AsyncInit(MaybeLocal(), "test.DelayRequest"); + asyncContext = AsyncInit(MaybeLocal(), + "nan:test.DelayRequest"); } ~DelayRequest() { AsyncDestroy(asyncContext); 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); + }); +}); diff --git a/test/js/makecallbackcontext-test.js b/test/js/makecallbackcontext-test.js index ef56b30f..6576975b 100644 --- a/test/js/makecallbackcontext-test.js +++ b/test/js/makecallbackcontext-test.js @@ -28,7 +28,7 @@ test('makecallbackcontext', function (t) { var hooks = asyncHooks.createHook({ init: function(asyncId, type, triggerAsyncId, resource) { - if (type === 'test.DelayRequest') { + if (type === 'nan:test.DelayRequest') { resourceAsyncId = asyncId; } },