n-api: add methods to open/close callback scope

mhdawson · MylesBorins · commit a7d1735e4112 · 2018-02-21T09:18:10.000-05:00
Add support for the following methods; napi_open_callback_scope napi_close_callback_scope These are needed when running asynchronous methods directly using uv. PR-URL: #18089 Fixes: #15604 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
diff --git a/doc/api/n-api.md b/doc/api/n-api.md
@@ -3431,6 +3431,42 @@ is sufficient and appropriate. Use of the `napi_make_callback` function
 may be required when implementing custom async behavior that does not use
 `napi_create_async_work`.
 
+### *napi_open_callback_scope*
+<!-- YAML
+added: REPLACEME
+-->
+```C
+NAPI_EXTERN napi_status napi_open_callback_scope(napi_env env,
+                                                 napi_value resource_object,
+                                                 napi_async_context context,
+                                                 napi_callback_scope* result)
+```
+- `[in] env`: The environment that the API is invoked under.
+- `[in] resource_object`: An optional object associated with the async work
+  that will be passed to possible async_hooks [`init` hooks][].
+- `[in] context`: Context for the async operation that is
+invoking the callback. This should be a value previously obtained
+from [`napi_async_init`][].
+- `[out] result`: The newly created scope.
+
+There are cases(for example resolving promises) where it is
+necessary to have the equivalent of the scope associated with a callback
+in place when making certain N-API calls.  If there is no other script on
+the stack the [`napi_open_callback_scope`][] and
+[`napi_close_callback_scope`][] functions can be used to open/close
+the required scope.
+
+### *napi_close_callback_scope*
+<!-- YAML
+added: REPLACEME
+-->
+```C
+NAPI_EXTERN napi_status napi_close_callback_scope(napi_env env,
+                                                  napi_callback_scope scope)
+```
+- `[in] env`: The environment that the API is invoked under.
+- `[in] scope`: The scope to be closed.
+
 ## Version Management
 
 ### napi_get_node_version
@@ -3716,6 +3752,7 @@ NAPI_EXTERN napi_status napi_get_uv_event_loop(napi_env env,
 [`napi_async_init`]: #n_api_napi_async_init
 [`napi_cancel_async_work`]: #n_api_napi_cancel_async_work
 [`napi_close_escapable_handle_scope`]: #n_api_napi_close_escapable_handle_scope
+[`napi_close_callback_scope`]: #n_api_napi_close_callback_scope
 [`napi_close_handle_scope`]: #n_api_napi_close_handle_scope
 [`napi_create_async_work`]: #n_api_napi_create_async_work
 [`napi_create_error`]: #n_api_napi_create_error
@@ -3741,6 +3778,7 @@ NAPI_EXTERN napi_status napi_get_uv_event_loop(napi_env env,
 [`napi_get_last_error_info`]: #n_api_napi_get_last_error_info
 [`napi_get_and_clear_last_exception`]: #n_api_napi_get_and_clear_last_exception
 [`napi_make_callback`]: #n_api_napi_make_callback
+[`napi_open_callback_scope`]: #n_api_napi_open_callback_scope
 [`napi_open_escapable_handle_scope`]: #n_api_napi_open_escapable_handle_scope
 [`napi_open_handle_scope`]: #n_api_napi_open_handle_scope
 [`napi_property_descriptor`]: #n_api_napi_property_descriptor
diff --git a/src/node_api.cc b/src/node_api.cc
@@ -44,6 +44,7 @@ struct napi_env__ {
   v8::Persistent<v8::ObjectTemplate> accessor_data_template;
   napi_extended_error_info last_error;
   int open_handle_scopes = 0;
+  int open_callback_scopes = 0;
   uv_loop_t* loop = nullptr;
 };
 
@@ -253,6 +254,18 @@ V8EscapableHandleScopeFromJsEscapableHandleScope(
   return reinterpret_cast<EscapableHandleScopeWrapper*>(s);
 }
 
+static
+napi_callback_scope JsCallbackScopeFromV8CallbackScope(
+    node::CallbackScope* s) {
+  return reinterpret_cast<napi_callback_scope>(s);
+}
+
+static
+node::CallbackScope* V8CallbackScopeFromJsCallbackScope(
+    napi_callback_scope s) {
+  return reinterpret_cast<node::CallbackScope*>(s);
+}
+
 //=== Conversion between V8 Handles and napi_value ========================
 
 // This asserts v8::Local<> will always be implemented with a single
@@ -544,6 +557,7 @@ class CallbackWrapperBase : public CallbackWrapper {
     napi_clear_last_error(env);
 
     int open_handle_scopes = env->open_handle_scopes;
+    int open_callback_scopes = env->open_callback_scopes;
 
     napi_value result = cb(env, cbinfo_wrapper);
 
@@ -552,6 +566,7 @@ class CallbackWrapperBase : public CallbackWrapper {
     }
 
     CHECK_EQ(env->open_handle_scopes, open_handle_scopes);
+    CHECK_EQ(env->open_callback_scopes, open_callback_scopes);
 
     if (!env->last_exception.IsEmpty()) {
       isolate->ThrowException(
@@ -911,7 +926,8 @@ const char* error_messages[] = {nullptr,
                                 "An exception is pending",
                                 "The async work item was cancelled",
                                 "napi_escape_handle already called on scope",
-                                "Invalid handle scope usage"};
+                                "Invalid handle scope usage",
+                                "Invalid callback scope usage"};
 
 static inline napi_status napi_clear_last_error(napi_env env) {
   env->last_error.error_code = napi_ok;
@@ -942,9 +958,9 @@ napi_status napi_get_last_error_info(napi_env env,
   // We don't have a napi_status_last as this would result in an ABI
   // change each time a message was added.
   static_assert(
-      node::arraysize(error_messages) == napi_handle_scope_mismatch + 1,
+      node::arraysize(error_messages) == napi_callback_scope_mismatch + 1,
       "Count of error messages must match count of error values");
-  CHECK_LE(env->last_error.error_code, napi_handle_scope_mismatch);
+  CHECK_LE(env->last_error.error_code, napi_callback_scope_mismatch);
 
   // Wait until someone requests the last error information to fetch the error
   // message string
@@ -2633,6 +2649,46 @@ napi_status napi_escape_handle(napi_env env,
   return napi_set_last_error(env, napi_escape_called_twice);
 }
 
+napi_status napi_open_callback_scope(napi_env env,
+                                     napi_value resource_object,
+                                     napi_async_context async_context_handle,
+                                     napi_callback_scope* result) {
+  // Omit NAPI_PREAMBLE and GET_RETURN_STATUS because V8 calls here cannot throw
+  // JS exceptions.
+  CHECK_ENV(env);
+  CHECK_ARG(env, result);
+
+  v8::Local<v8::Context> context = env->isolate->GetCurrentContext();
+
+  node::async_context* node_async_context =
+      reinterpret_cast<node::async_context*>(async_context_handle);
+
+  v8::Local<v8::Object> resource;
+  CHECK_TO_OBJECT(env, context, resource, resource_object);
+
+  *result = v8impl::JsCallbackScopeFromV8CallbackScope(
+      new node::CallbackScope(env->isolate,
+                              resource,
+                              *node_async_context));
+
+  env->open_callback_scopes++;
+  return napi_clear_last_error(env);
+}
+
+napi_status napi_close_callback_scope(napi_env env, napi_callback_scope scope) {
+  // Omit NAPI_PREAMBLE and GET_RETURN_STATUS because V8 calls here cannot throw
+  // JS exceptions.
+  CHECK_ENV(env);
+  CHECK_ARG(env, scope);
+  if (env->open_callback_scopes == 0) {
+    return napi_callback_scope_mismatch;
+  }
+
+  env->open_callback_scopes--;
+  delete v8impl::V8CallbackScopeFromJsCallbackScope(scope);
+  return napi_clear_last_error(env);
+}
+
 napi_status napi_new_instance(napi_env env,
                               napi_value constructor,
                               size_t argc,
diff --git a/src/node_api.h b/src/node_api.h
@@ -424,6 +424,14 @@ NAPI_EXTERN napi_status napi_escape_handle(napi_env env,
                                            napi_value escapee,
                                            napi_value* result);
 
+NAPI_EXTERN napi_status napi_open_callback_scope(napi_env env,
+                                                 napi_value resource_object,
+                                                 napi_async_context context,
+                                                 napi_callback_scope* result);
+
+NAPI_EXTERN napi_status napi_close_callback_scope(napi_env env,
+                                                  napi_callback_scope scope);
+
 // Methods to support error handling
 NAPI_EXTERN napi_status napi_throw(napi_env env, napi_value error);
 NAPI_EXTERN napi_status napi_throw_error(napi_env env,
diff --git a/src/node_api_types.h b/src/node_api_types.h
@@ -15,6 +15,7 @@ typedef struct napi_value__ *napi_value;
 typedef struct napi_ref__ *napi_ref;
 typedef struct napi_handle_scope__ *napi_handle_scope;
 typedef struct napi_escapable_handle_scope__ *napi_escapable_handle_scope;
+typedef struct napi_callback_scope__ *napi_callback_scope;
 typedef struct napi_callback_info__ *napi_callback_info;
 typedef struct napi_async_context__ *napi_async_context;
 typedef struct napi_async_work__ *napi_async_work;
@@ -70,7 +71,8 @@ typedef enum {
   napi_pending_exception,
   napi_cancelled,
   napi_escape_called_twice,
-  napi_handle_scope_mismatch
+  napi_handle_scope_mismatch,
+  napi_callback_scope_mismatch
 } napi_status;
 
 typedef napi_value (*napi_callback)(napi_env env,
diff --git a/test/addons-napi/test_callback_scope/binding.cc b/test/addons-napi/test_callback_scope/binding.cc
@@ -0,0 +1,138 @@
+#include "node_api.h"
+#include "uv.h"
+#include "../common.h"
+
+namespace {
+
+// the test needs to fake out the async structure, so we need to use
+// the raw structure here and then cast as done behind the scenes
+// in napi calls.
+struct async_context {
+  double async_id;
+  double trigger_async_id;
+};
+
+
+napi_value RunInCallbackScope(napi_env env, napi_callback_info info) {
+  size_t argc;
+  napi_value args[4];
+
+  NAPI_CALL(env, napi_get_cb_info(env, info, &argc, nullptr, nullptr, nullptr));
+  NAPI_ASSERT(env, argc == 4 , "Wrong number of arguments");
+
+  NAPI_CALL(env, napi_get_cb_info(env, info, &argc, args, nullptr, nullptr));
+
+  napi_valuetype valuetype;
+  NAPI_CALL(env, napi_typeof(env, args[0], &valuetype));
+  NAPI_ASSERT(env, valuetype == napi_object,
+      "Wrong type of arguments. Expects an object as first argument.");
+
+  NAPI_CALL(env, napi_typeof(env, args[1], &valuetype));
+  NAPI_ASSERT(env, valuetype == napi_number,
+      "Wrong type of arguments. Expects a number as second argument.");
+
+  NAPI_CALL(env, napi_typeof(env, args[2], &valuetype));
+  NAPI_ASSERT(env, valuetype == napi_number,
+      "Wrong type of arguments. Expects a number as third argument.");
+
+  NAPI_CALL(env, napi_typeof(env, args[3], &valuetype));
+  NAPI_ASSERT(env, valuetype == napi_function,
+      "Wrong type of arguments. Expects a function as third argument.");
+
+  struct async_context context;
+  NAPI_CALL(env, napi_get_value_double(env, args[1], &context.async_id));
+  NAPI_CALL(env,
+      napi_get_value_double(env, args[2], &context.trigger_async_id));
+
+  napi_callback_scope scope = nullptr;
+  NAPI_CALL(
+      env,
+      napi_open_callback_scope(env,
+                               args[0],
+                               reinterpret_cast<napi_async_context>(&context),
+                               &scope));
+
+  // if the function has an exception pending after the call that is ok
+  // so we don't use NAPI_CALL as we must close the callback scope regardless
+  napi_value result = nullptr;
+  napi_status function_call_result =
+      napi_call_function(env, args[0], args[3], 0, nullptr, &result);
+  if (function_call_result != napi_ok) {
+    GET_AND_THROW_LAST_ERROR((env));
+  }
+
+  NAPI_CALL(env, napi_close_callback_scope(env, scope));
+
+  return result;
+}
+
+static napi_env shared_env = nullptr;
+static napi_deferred deferred = nullptr;
+
+static void Callback(uv_work_t* req, int ignored) {
+  napi_env env = shared_env;
+
+  napi_handle_scope handle_scope = nullptr;
+  NAPI_CALL_RETURN_VOID(env, napi_open_handle_scope(env, &handle_scope));
+
+  napi_value resource_name;
+  NAPI_CALL_RETURN_VOID(env, napi_create_string_utf8(
+      env, "test", NAPI_AUTO_LENGTH, &resource_name));
+  napi_async_context context;
+  NAPI_CALL_RETURN_VOID(env,
+                        napi_async_init(env, nullptr, resource_name, &context));
+
+  napi_value resource_object;
+  NAPI_CALL_RETURN_VOID(env, napi_create_object(env, &resource_object));
+
+  napi_value undefined_value;
+  NAPI_CALL_RETURN_VOID(env, napi_get_undefined(env, &undefined_value));
+
+  napi_callback_scope scope = nullptr;
+  NAPI_CALL_RETURN_VOID(env, napi_open_callback_scope(env,
+                                                      resource_object,
+                                                      context,
+                                                      &scope));
+
+  NAPI_CALL_RETURN_VOID(env,
+                        napi_resolve_deferred(env, deferred, undefined_value));
+
+  NAPI_CALL_RETURN_VOID(env, napi_close_callback_scope(env, scope));
+
+  NAPI_CALL_RETURN_VOID(env, napi_close_handle_scope(env, handle_scope));
+  delete req;
+}
+
+napi_value TestResolveAsync(napi_env env, napi_callback_info info) {
+  napi_value promise = nullptr;
+  if (deferred == nullptr) {
+    shared_env = env;
+    NAPI_CALL(env, napi_create_promise(env, &deferred, &promise));
+
+    uv_loop_t* loop = nullptr;
+    NAPI_CALL(env, napi_get_uv_event_loop(env, &loop));
+
+    uv_work_t* req = new uv_work_t();
+    uv_queue_work(loop,
+                  req,
+                  [](uv_work_t*) {},
+                  Callback);
+  }
+  return promise;
+}
+
+napi_value Init(napi_env env, napi_value exports) {
+  napi_property_descriptor descriptors[] = {
+    DECLARE_NAPI_PROPERTY("runInCallbackScope", RunInCallbackScope),
+    DECLARE_NAPI_PROPERTY("testResolveAsync", TestResolveAsync)
+  };
+
+  NAPI_CALL(env, napi_define_properties(
+      env, exports, sizeof(descriptors) / sizeof(*descriptors), descriptors));
+
+  return exports;
+}
+
+}  // anonymous namespace
+
+NAPI_MODULE(NODE_GYP_MODULE_NAME, Init)
diff --git a/test/addons-napi/test_callback_scope/binding.gyp b/test/addons-napi/test_callback_scope/binding.gyp
@@ -0,0 +1,9 @@
+{
+  'targets': [
+    {
+      'target_name': 'binding',
+      'defines': [ 'V8_DEPRECATION_WARNINGS=1' ],
+      'sources': [ 'binding.cc' ]
+    }
+  ]
+}
diff --git a/test/addons-napi/test_callback_scope/test-async-hooks.js b/test/addons-napi/test_callback_scope/test-async-hooks.js
@@ -0,0 +1,29 @@
+'use strict';
+
+const common = require('../../common');
+const assert = require('assert');
+const async_hooks = require('async_hooks');
+
+// The async_hook that we enable would register the process.emitWarning()
+// call from loading the N-API addon as asynchronous activity because
+// it contains a process.nextTick() call. Monkey patch it to be a no-op
+// before we load the addon in order to avoid this.
+process.emitWarning = () => {};
+
+const { runInCallbackScope } = require(`./build/${common.buildType}/binding`);
+
+let insideHook = false;
+async_hooks.createHook({
+  before: common.mustCall((id) => {
+    assert.strictEqual(id, 1000);
+    insideHook = true;
+  }),
+  after: common.mustCall((id) => {
+    assert.strictEqual(id, 1000);
+    insideHook = false;
+  })
+}).enable();
+
+runInCallbackScope({}, 1000, 1000, () => {
+  assert(insideHook);
+});
diff --git a/test/addons-napi/test_callback_scope/test-resolve-async.js b/test/addons-napi/test_callback_scope/test-resolve-async.js
@@ -0,0 +1,13 @@
+'use strict';
+
+const common = require('../../common');
+const assert = require('assert');
+const { testResolveAsync } = require(`./build/${common.buildType}/binding`);
+
+let called = false;
+testResolveAsync().then(common.mustCall(() => {
+  called = true;
+}));
+
+setTimeout(common.mustCall(() => { assert(called); }),
+           common.platformTimeout(20));
diff --git a/test/addons-napi/test_callback_scope/test.js b/test/addons-napi/test_callback_scope/test.js
