Skip to content

Commit

Permalink
n-api: fix false assumption on napi_async_context structures
Browse files Browse the repository at this point in the history
napi_async_context should be an opaque type and not be used as same as
node::async_context.

PR-URL: #32928
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
  • Loading branch information
legendecas authored and targos committed May 13, 2020
1 parent 5ea5c26 commit d08be9c
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 28 deletions.
31 changes: 8 additions & 23 deletions test/node-api/test_callback_scope/binding.cc
Expand Up @@ -4,21 +4,12 @@

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_value args[3];

NAPI_CALL(env, napi_get_cb_info(env, info, &argc, nullptr, nullptr, nullptr));
NAPI_ASSERT(env, argc == 4 , "Wrong number of arguments");
NAPI_ASSERT(env, argc == 3 , "Wrong number of arguments");

NAPI_CALL(env, napi_get_cb_info(env, info, &argc, args, nullptr, nullptr));

Expand All @@ -28,35 +19,29 @@ napi_value RunInCallbackScope(napi_env env, napi_callback_info info) {
"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_ASSERT(env, valuetype == napi_string,
"Wrong type of arguments. Expects a string 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_async_context context;
NAPI_CALL(env, napi_async_init(env, args[0], args[1], &context));

napi_callback_scope scope = nullptr;
NAPI_CALL(
env,
napi_open_callback_scope(env,
args[0],
reinterpret_cast<napi_async_context>(&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);
napi_call_function(env, args[0], args[2], 0, nullptr, &result);
if (function_call_result != napi_ok) {
GET_AND_THROW_LAST_ERROR((env));
}
Expand Down
16 changes: 13 additions & 3 deletions test/node-api/test_callback_scope/test-async-hooks.js
Expand Up @@ -12,18 +12,28 @@ process.emitWarning = () => {};

const { runInCallbackScope } = require(`./build/${common.buildType}/binding`);

const expectedResource = {};
const expectedResourceType = 'test-resource';
let insideHook = false;
let expectedId;
async_hooks.createHook({
init: common.mustCall((id, type, triggerAsyncId, resource) => {
if (type !== expectedResourceType) {
return;
}
assert.strictEqual(resource, expectedResource);
expectedId = id;
}),
before: common.mustCall((id) => {
assert.strictEqual(id, 1000);
assert.strictEqual(id, expectedId);
insideHook = true;
}),
after: common.mustCall((id) => {
assert.strictEqual(id, 1000);
assert.strictEqual(id, expectedId);
insideHook = false;
})
}).enable();

runInCallbackScope({}, 1000, 1000, () => {
runInCallbackScope(expectedResource, expectedResourceType, () => {
assert(insideHook);
});
4 changes: 2 additions & 2 deletions test/node-api/test_callback_scope/test.js
Expand Up @@ -4,14 +4,14 @@ const common = require('../../common');
const assert = require('assert');
const { runInCallbackScope } = require(`./build/${common.buildType}/binding`);

assert.strictEqual(runInCallbackScope({}, 0, 0, () => 42), 42);
assert.strictEqual(runInCallbackScope({}, 'test-resource', () => 42), 42);

{
process.once('uncaughtException', common.mustCall((err) => {
assert.strictEqual(err.message, 'foo');
}));

runInCallbackScope({}, 0, 0, () => {
runInCallbackScope({}, 'test-resource', () => {
throw new Error('foo');
});
}

0 comments on commit d08be9c

Please sign in to comment.