From 8c8c90b714756433b3333146a2893572b473dfd6 Mon Sep 17 00:00:00 2001
From: Anna Henningsen <anna@addaleax.net>
Date: Tue, 8 Aug 2017 20:18:16 +0200
Subject: [PATCH] n-api: use AsyncResource for Work tracking
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Enable combining N-API async work with async-hooks.

PR-URL: https://github.com/nodejs/node/pull/14697
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jason Ginchereau <jasongin@microsoft.com>
Reviewed-By: Michael Dawson <mhdawson@ibm.com>
---
 doc/api/n-api.md                              | 20 +++++++
 src/node_api.cc                               | 32 ++++++++--
 src/node_api.h                                |  2 +
 .../test_async/test-async-hooks.js            | 60 +++++++++++++++++++
 test/addons-napi/test_async/test.js           |  4 +-
 test/addons-napi/test_async/test_async.cc     | 31 ++++++----
 6 files changed, 129 insertions(+), 20 deletions(-)
 create mode 100644 test/addons-napi/test_async/test-async-hooks.js

diff --git a/doc/api/n-api.md b/doc/api/n-api.md
index b9d06079fd..0a74d89ea0 100644
--- a/doc/api/n-api.md
+++ b/doc/api/n-api.md
@@ -3256,10 +3256,16 @@ callback invocation, even when it was cancelled.
 ### napi_create_async_work
 <!-- YAML
 added: v8.0.0
+changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/14697
+    description: Added `async_resource` and `async_resource_name` parameters.
 -->
 ```C
 NAPI_EXTERN
 napi_status napi_create_async_work(napi_env env,
+                                   napi_value async_resource,
+                                   const char* async_resource_name,
                                    napi_async_execute_callback execute,
                                    napi_async_complete_callback complete,
                                    void* data,
@@ -3267,6 +3273,10 @@ napi_status napi_create_async_work(napi_env env,
 ```
 
 - `[in] env`: The environment that the API is invoked under.
+- `[in] async_resource`: An optional object associated with the async work
+  that will be passed to possible async_hooks [`init` hooks][].
+- `[in] async_resource_name`: An identifier for the kind of resource that is
+being provided for diagnostic information exposed by the `async_hooks` API.
 - `[in] execute`: The native function which should be called to excute
 the logic asynchronously.
 - `[in] complete`: The native function which will be called when the
@@ -3282,6 +3292,14 @@ This API allocates a work object that is used to execute logic asynchronously.
 It should be freed using [`napi_delete_async_work`][] once the work is no longer
 required.
 
+`async_resource_name` should be a null-terminated, UTF-8-encoded string.
+
+*Note*: The `async_resource_name` identifier is provided by the user and should
+be representative of the type of async work being performed. It is also
+recommended to apply namespacing to the identifier, e.g. by including the
+module name. See the [`async_hooks` documentation][async_hooks `type`]
+for more information.
+
 ### napi_delete_async_work
 <!-- YAML
 added: v8.0.0
@@ -3636,3 +3654,5 @@ NAPI_EXTERN napi_status napi_run_script(napi_env env,
 [`napi_wrap`]: #n_api_napi_wrap
 
 [`process.release`]: process.html#process_process_release
+[`init` hooks]: async_hooks.html#async_hooks_init_asyncid_type_triggerasyncid_resource
+[async_hooks `type`]: async_hooks.html#async_hooks_type
diff --git a/src/node_api.cc b/src/node_api.cc
index 8c55e64b7c..f49dd6da68 100644
--- a/src/node_api.cc
+++ b/src/node_api.cc
@@ -3246,13 +3246,18 @@ static napi_status ConvertUVErrorCode(int code) {
 }
 
 // Wrapper around uv_work_t which calls user-provided callbacks.
-class Work {
+class Work : public node::AsyncResource {
  private:
   explicit Work(napi_env env,
-                napi_async_execute_callback execute = nullptr,
+                v8::Local<v8::Object> async_resource,
+                const char* async_resource_name,
+                napi_async_execute_callback execute,
                 napi_async_complete_callback complete = nullptr,
                 void* data = nullptr)
-    : _env(env),
+    : AsyncResource(env->isolate,
+                    async_resource,
+                    async_resource_name),
+    _env(env),
     _data(data),
     _execute(execute),
     _complete(complete) {
@@ -3264,10 +3269,13 @@ class Work {
 
  public:
   static Work* New(napi_env env,
+                   v8::Local<v8::Object> async_resource,
+                   const char* async_resource_name,
                    napi_async_execute_callback execute,
                    napi_async_complete_callback complete,
                    void* data) {
-    return new Work(env, execute, complete, data);
+    return new Work(env, async_resource, async_resource_name,
+                    execute, complete, data);
   }
 
   static void Delete(Work* work) {
@@ -3288,6 +3296,7 @@ class Work {
       // Establish a handle scope here so that every callback doesn't have to.
       // Also it is needed for the exception-handling below.
       v8::HandleScope scope(env->isolate);
+      CallbackScope callback_scope(work);
 
       work->_complete(env, ConvertUVErrorCode(status), work->_data);
 
@@ -3330,6 +3339,8 @@ class Work {
   } while (0)
 
 napi_status napi_create_async_work(napi_env env,
+                                   napi_value async_resource,
+                                   const char* async_resource_name,
                                    napi_async_execute_callback execute,
                                    napi_async_complete_callback complete,
                                    void* data,
@@ -3338,7 +3349,18 @@ napi_status napi_create_async_work(napi_env env,
   CHECK_ARG(env, execute);
   CHECK_ARG(env, result);
 
-  uvimpl::Work* work = uvimpl::Work::New(env, execute, complete, data);
+  v8::Local<v8::Object> resource;
+  if (async_resource != nullptr) {
+    auto value = v8impl::V8LocalValueFromJsValue(async_resource);
+    RETURN_STATUS_IF_FALSE(env, value->IsObject(), napi_invalid_arg);
+    resource = value.As<v8::Object>();
+  } else {
+    resource = v8::Object::New(env->isolate);
+  }
+
+  uvimpl::Work* work =
+      uvimpl::Work::New(env, resource, async_resource_name,
+                        execute, complete, data);
 
   *result = reinterpret_cast<napi_async_work>(work);
 
diff --git a/src/node_api.h b/src/node_api.h
index 807595777d..227f26a2ff 100644
--- a/src/node_api.h
+++ b/src/node_api.h
@@ -524,6 +524,8 @@ NAPI_EXTERN napi_status napi_get_dataview_info(napi_env env,
 // Methods to manage simple async operations
 NAPI_EXTERN
 napi_status napi_create_async_work(napi_env env,
+                                   napi_value async_resource,
+                                   const char* async_resource_name,
                                    napi_async_execute_callback execute,
                                    napi_async_complete_callback complete,
                                    void* data,
diff --git a/test/addons-napi/test_async/test-async-hooks.js b/test/addons-napi/test_async/test-async-hooks.js
new file mode 100644
index 0000000000..db6e0cbd16
--- /dev/null
+++ b/test/addons-napi/test_async/test-async-hooks.js
@@ -0,0 +1,60 @@
+'use strict';
+const common = require('../../common');
+const assert = require('assert');
+const async_hooks = require('async_hooks');
+const test_async = require(`./build/${common.buildType}/test_async`);
+
+const events = [];
+let testId;
+const initAsyncId = async_hooks.executionAsyncId();
+
+async_hooks.createHook({
+  init(id, provider, triggerAsyncId, resource) {
+    if (provider === 'TestResource') {
+      testId = id;
+      events.push({ type: 'init', id, provider, triggerAsyncId, resource });
+    }
+  },
+  before(id) {
+    if (testId === id) {
+      events.push({ type: 'before', id });
+    }
+  },
+  after(id) {
+    if (testId === id) {
+      events.push({ type: 'after', id });
+    }
+  },
+  destroy(id) {
+    if (testId === id) {
+      events.push({ type: 'destroy', id });
+    }
+  }
+}).enable();
+
+const resource = { foo: 'foo' };
+
+events.push({ type: 'start' });
+test_async.Test(5, resource, common.mustCall(function(err, val) {
+  assert.strictEqual(err, null);
+  assert.strictEqual(val, 10);
+  events.push({ type: 'complete' });
+  process.nextTick(common.mustCall());
+}));
+events.push({ type: 'scheduled' });
+
+process.on('exit', () => {
+  assert.deepStrictEqual(events, [
+    { type: 'start' },
+    { type: 'init',
+      id: testId,
+      provider: 'TestResource',
+      triggerAsyncId: initAsyncId,
+      resource },
+    { type: 'scheduled' },
+    { type: 'before', id: testId },
+    { type: 'complete' },
+    { type: 'after', id: testId },
+    { type: 'destroy', id: testId }
+  ]);
+});
diff --git a/test/addons-napi/test_async/test.js b/test/addons-napi/test_async/test.js
index cfd39a5b11..749c48e057 100644
--- a/test/addons-napi/test_async/test.js
+++ b/test/addons-napi/test_async/test.js
@@ -9,7 +9,7 @@ const testException = 'test_async_cb_exception';
 // Exception thrown from async completion callback.
 // (Tested in a spawned process because the exception is fatal.)
 if (process.argv[2] === 'child') {
-  test_async.Test(1, common.mustCall(function() {
+  test_async.Test(1, {}, common.mustCall(function() {
     throw new Error(testException);
   }));
   return;
@@ -20,7 +20,7 @@ assert.ifError(p.error);
 assert.ok(p.stderr.toString().includes(testException));
 
 // Successful async execution and completion callback.
-test_async.Test(5, common.mustCall(function(err, val) {
+test_async.Test(5, {}, common.mustCall(function(err, val) {
   assert.strictEqual(err, null);
   assert.strictEqual(val, 10);
   process.nextTick(common.mustCall());
diff --git a/test/addons-napi/test_async/test_async.cc b/test/addons-napi/test_async/test_async.cc
index fd5278d7d5..4ffc1d2fc9 100644
--- a/test/addons-napi/test_async/test_async.cc
+++ b/test/addons-napi/test_async/test_async.cc
@@ -61,37 +61,40 @@ void Complete(napi_env env, napi_status status, void* data) {
 
   napi_value result;
   NAPI_CALL_RETURN_VOID(env,
-    napi_make_callback(env, global, callback, 2, argv, &result));
+    napi_call_function(env, global, callback, 2, argv, &result));
 
   NAPI_CALL_RETURN_VOID(env, napi_delete_reference(env, c->_callback));
   NAPI_CALL_RETURN_VOID(env, napi_delete_async_work(env, c->_request));
 }
 
 napi_value Test(napi_env env, napi_callback_info info) {
-  size_t argc = 2;
-  napi_value argv[2];
+  size_t argc = 3;
+  napi_value argv[3];
   napi_value _this;
   void* data;
   NAPI_CALL(env,
     napi_get_cb_info(env, info, &argc, argv, &_this, &data));
-  NAPI_ASSERT(env, argc >= 2, "Not enough arguments, expected 2.");
+  NAPI_ASSERT(env, argc >= 3, "Not enough arguments, expected 2.");
 
   napi_valuetype t;
   NAPI_CALL(env, napi_typeof(env, argv[0], &t));
   NAPI_ASSERT(env, t == napi_number,
     "Wrong first argument, integer expected.");
   NAPI_CALL(env, napi_typeof(env, argv[1], &t));
+  NAPI_ASSERT(env, t == napi_object,
+    "Wrong second argument, object expected.");
+  NAPI_CALL(env, napi_typeof(env, argv[2], &t));
   NAPI_ASSERT(env, t == napi_function,
-    "Wrong second argument, function expected.");
+    "Wrong third argument, function expected.");
 
   the_carrier._output = 0;
 
   NAPI_CALL(env,
     napi_get_value_int32(env, argv[0], &the_carrier._input));
   NAPI_CALL(env,
-    napi_create_reference(env, argv[1], 1, &the_carrier._callback));
-  NAPI_CALL(env, napi_create_async_work(
-    env, Execute, Complete, &the_carrier, &the_carrier._request));
+    napi_create_reference(env, argv[2], 1, &the_carrier._callback));
+  NAPI_CALL(env, napi_create_async_work(env, argv[1], "TestResource",
+    Execute, Complete, &the_carrier, &the_carrier._request));
   NAPI_CALL(env,
     napi_queue_async_work(env, the_carrier._request));
 
@@ -116,7 +119,7 @@ void CancelComplete(napi_env env, napi_status status, void* data) {
     NAPI_CALL_RETURN_VOID(env, napi_get_global(env, &global));
     napi_value result;
     NAPI_CALL_RETURN_VOID(env,
-      napi_make_callback(env, global, callback, 0, nullptr, &result));
+      napi_call_function(env, global, callback, 0, nullptr, &result));
   }
 
   NAPI_CALL_RETURN_VOID(env, napi_delete_async_work(env, c->_request));
@@ -140,8 +143,9 @@ napi_value TestCancel(napi_env env, napi_callback_info info) {
   // make sure the work we are going to cancel will not be
   // able to start by using all the threads in the pool
   for (int i = 1; i < MAX_CANCEL_THREADS; i++) {
-    NAPI_CALL(env, napi_create_async_work(env, CancelExecute,
-      BusyCancelComplete, &async_carrier[i], &async_carrier[i]._request));
+    NAPI_CALL(env, napi_create_async_work(env, nullptr, "TestCancelBusy",
+      CancelExecute, BusyCancelComplete,
+      &async_carrier[i], &async_carrier[i]._request));
     NAPI_CALL(env, napi_queue_async_work(env, async_carrier[i]._request));
   }
 
@@ -151,8 +155,9 @@ napi_value TestCancel(napi_env env, napi_callback_info info) {
   // workers above.
   NAPI_CALL(env,
     napi_get_cb_info(env, info, &argc, argv, &_this, &data));
-  NAPI_CALL(env, napi_create_async_work(env, CancelExecute,
-    CancelComplete, &async_carrier[0], &async_carrier[0]._request));
+  NAPI_CALL(env, napi_create_async_work(env, nullptr, "TestCancelled",
+    CancelExecute, CancelComplete,
+    &async_carrier[0], &async_carrier[0]._request));
   NAPI_CALL(env,
     napi_create_reference(env, argv[0], 1, &async_carrier[0]._callback));
   NAPI_CALL(env, napi_queue_async_work(env, async_carrier[0]._request));