Skip to content

Commit

Permalink
n-api: add methods to open/close callback scope
Browse files Browse the repository at this point in the history
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: nodejs#18089
Fixes: nodejs#15604
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
  • Loading branch information
mhdawson authored and Gabriel Schulhof committed Apr 12, 2018
1 parent 8923ad5 commit ed8ac32
Show file tree
Hide file tree
Showing 9 changed files with 314 additions and 4 deletions.
38 changes: 38 additions & 0 deletions doc/api/n-api.md
Original file line number Diff line number Diff line change
Expand Up @@ -3446,6 +3446,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
Expand Down Expand Up @@ -3725,6 +3761,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
Expand All @@ -3750,6 +3787,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
Expand Down
62 changes: 59 additions & 3 deletions src/node_api.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);

Expand All @@ -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(
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down
8 changes: 8 additions & 0 deletions src/node_api.h
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
4 changes: 3 additions & 1 deletion src/node_api_types.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down
138 changes: 138 additions & 0 deletions test/addons-napi/test_callback_scope/binding.cc
Original file line number Diff line number Diff line change
@@ -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)
9 changes: 9 additions & 0 deletions test/addons-napi/test_callback_scope/binding.gyp
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
'targets': [
{
'target_name': 'binding',
'defines': [ 'V8_DEPRECATION_WARNINGS=1' ],
'sources': [ 'binding.cc' ]
}
]
}
29 changes: 29 additions & 0 deletions test/addons-napi/test_callback_scope/test-async-hooks.js
Original file line number Diff line number Diff line change
@@ -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);
});
13 changes: 13 additions & 0 deletions test/addons-napi/test_callback_scope/test-resolve-async.js
Original file line number Diff line number Diff line change
@@ -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));
Loading

0 comments on commit ed8ac32

Please sign in to comment.