Skip to content

Commit 68656cf

Browse files
legendecasBethGriggs
authored andcommitted
n-api: fix false assumption on napi_async_context structures
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>
1 parent 343c6ac commit 68656cf

File tree

3 files changed

+23
-28
lines changed

3 files changed

+23
-28
lines changed

test/node-api/test_callback_scope/binding.cc

+8-23
Original file line numberDiff line numberDiff line change
@@ -4,21 +4,12 @@
44

55
namespace {
66

7-
// the test needs to fake out the async structure, so we need to use
8-
// the raw structure here and then cast as done behind the scenes
9-
// in napi calls.
10-
struct async_context {
11-
double async_id;
12-
double trigger_async_id;
13-
};
14-
15-
167
napi_value RunInCallbackScope(napi_env env, napi_callback_info info) {
178
size_t argc;
18-
napi_value args[4];
9+
napi_value args[3];
1910

2011
NAPI_CALL(env, napi_get_cb_info(env, info, &argc, nullptr, nullptr, nullptr));
21-
NAPI_ASSERT(env, argc == 4 , "Wrong number of arguments");
12+
NAPI_ASSERT(env, argc == 3 , "Wrong number of arguments");
2213

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

@@ -28,35 +19,29 @@ napi_value RunInCallbackScope(napi_env env, napi_callback_info info) {
2819
"Wrong type of arguments. Expects an object as first argument.");
2920

3021
NAPI_CALL(env, napi_typeof(env, args[1], &valuetype));
31-
NAPI_ASSERT(env, valuetype == napi_number,
32-
"Wrong type of arguments. Expects a number as second argument.");
22+
NAPI_ASSERT(env, valuetype == napi_string,
23+
"Wrong type of arguments. Expects a string as second argument.");
3324

3425
NAPI_CALL(env, napi_typeof(env, args[2], &valuetype));
35-
NAPI_ASSERT(env, valuetype == napi_number,
36-
"Wrong type of arguments. Expects a number as third argument.");
37-
38-
NAPI_CALL(env, napi_typeof(env, args[3], &valuetype));
3926
NAPI_ASSERT(env, valuetype == napi_function,
4027
"Wrong type of arguments. Expects a function as third argument.");
4128

42-
struct async_context context;
43-
NAPI_CALL(env, napi_get_value_double(env, args[1], &context.async_id));
44-
NAPI_CALL(env,
45-
napi_get_value_double(env, args[2], &context.trigger_async_id));
29+
napi_async_context context;
30+
NAPI_CALL(env, napi_async_init(env, args[0], args[1], &context));
4631

4732
napi_callback_scope scope = nullptr;
4833
NAPI_CALL(
4934
env,
5035
napi_open_callback_scope(env,
5136
args[0],
52-
reinterpret_cast<napi_async_context>(&context),
37+
context,
5338
&scope));
5439

5540
// if the function has an exception pending after the call that is ok
5641
// so we don't use NAPI_CALL as we must close the callback scope regardless
5742
napi_value result = nullptr;
5843
napi_status function_call_result =
59-
napi_call_function(env, args[0], args[3], 0, nullptr, &result);
44+
napi_call_function(env, args[0], args[2], 0, nullptr, &result);
6045
if (function_call_result != napi_ok) {
6146
GET_AND_THROW_LAST_ERROR((env));
6247
}

test/node-api/test_callback_scope/test-async-hooks.js

+13-3
Original file line numberDiff line numberDiff line change
@@ -12,18 +12,28 @@ process.emitWarning = () => {};
1212

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

15+
const expectedResource = {};
16+
const expectedResourceType = 'test-resource';
1517
let insideHook = false;
18+
let expectedId;
1619
async_hooks.createHook({
20+
init: common.mustCall((id, type, triggerAsyncId, resource) => {
21+
if (type !== expectedResourceType) {
22+
return;
23+
}
24+
assert.strictEqual(resource, expectedResource);
25+
expectedId = id;
26+
}),
1727
before: common.mustCall((id) => {
18-
assert.strictEqual(id, 1000);
28+
assert.strictEqual(id, expectedId);
1929
insideHook = true;
2030
}),
2131
after: common.mustCall((id) => {
22-
assert.strictEqual(id, 1000);
32+
assert.strictEqual(id, expectedId);
2333
insideHook = false;
2434
})
2535
}).enable();
2636

27-
runInCallbackScope({}, 1000, 1000, () => {
37+
runInCallbackScope(expectedResource, expectedResourceType, () => {
2838
assert(insideHook);
2939
});

test/node-api/test_callback_scope/test.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,14 @@ const common = require('../../common');
44
const assert = require('assert');
55
const { runInCallbackScope } = require(`./build/${common.buildType}/binding`);
66

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

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

14-
runInCallbackScope({}, 0, 0, () => {
14+
runInCallbackScope({}, 'test-resource', () => {
1515
throw new Error('foo');
1616
});
1717
}

0 commit comments

Comments
 (0)