Commit 68656cf
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
3 files changed
Lines changed: 23 additions & 28 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -4,21 +4,12 @@ | |||
| 4 | 4 | ||
| 5 | 5 | namespace { | |
| 6 | 6 | ||
| 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 | - | ||
| 16 | 7 | napi_value RunInCallbackScope(napi_env env, napi_callback_info info) { | |
| 17 | 8 | size_t argc; | |
| 18 | - napi_value args[4]; | ||
| 9 | + napi_value args[3]; | ||
| 19 | 10 | ||
| 20 | 11 | 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"); | ||
| 22 | 13 | ||
| 23 | 14 | NAPI_CALL(env, napi_get_cb_info(env, info, &argc, args, nullptr, nullptr)); | |
| 24 | 15 | ||
@@ -28,35 +19,29 @@ napi_value RunInCallbackScope(napi_env env, napi_callback_info info) { | |||
| 28 | 19 | "Wrong type of arguments. Expects an object as first argument."); | |
| 29 | 20 | ||
| 30 | 21 | 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."); | ||
| 33 | 24 | ||
| 34 | 25 | 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)); | ||
| 39 | 26 | NAPI_ASSERT(env, valuetype == napi_function, | |
| 40 | 27 | "Wrong type of arguments. Expects a function as third argument."); | |
| 41 | 28 | ||
| 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)); | ||
| 46 | 31 | ||
| 47 | 32 | napi_callback_scope scope = nullptr; | |
| 48 | 33 | NAPI_CALL( | |
| 49 | 34 | env, | |
| 50 | 35 | napi_open_callback_scope(env, | |
| 51 | 36 | args[0], | |
| 52 | - reinterpret_cast<napi_async_context>(&context), | ||
| 37 | + context, | ||
| 53 | 38 | &scope)); | |
| 54 | 39 | ||
| 55 | 40 | // if the function has an exception pending after the call that is ok | |
| 56 | 41 | // so we don't use NAPI_CALL as we must close the callback scope regardless | |
| 57 | 42 | napi_value result = nullptr; | |
| 58 | 43 | 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); | ||
| 60 | 45 | if (function_call_result != napi_ok) { | |
| 61 | 46 | GET_AND_THROW_LAST_ERROR((env)); | |
| 62 | 47 | } | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -12,18 +12,28 @@ process.emitWarning = () => {}; | |||
| 12 | 12 | ||
| 13 | 13 | const { runInCallbackScope } = require(`./build/${common.buildType}/binding`); | |
| 14 | 14 | ||
| 15 | + const expectedResource = {}; | ||
| 16 | + const expectedResourceType = 'test-resource'; | ||
| 15 | 17 | let insideHook = false; | |
| 18 | + let expectedId; | ||
| 16 | 19 | 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 | + }), | ||
| 17 | 27 | before: common.mustCall((id) => { | |
| 18 | - assert.strictEqual(id, 1000); | ||
| 28 | + assert.strictEqual(id, expectedId); | ||
| 19 | 29 | insideHook = true; | |
| 20 | 30 | }), | |
| 21 | 31 | after: common.mustCall((id) => { | |
| 22 | - assert.strictEqual(id, 1000); | ||
| 32 | + assert.strictEqual(id, expectedId); | ||
| 23 | 33 | insideHook = false; | |
| 24 | 34 | }) | |
| 25 | 35 | }).enable(); | |
| 26 | 36 | ||
| 27 | - runInCallbackScope({}, 1000, 1000, () => { | ||
| 37 | + runInCallbackScope(expectedResource, expectedResourceType, () => { | ||
| 28 | 38 | assert(insideHook); | |
| 29 | 39 | }); | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -4,14 +4,14 @@ const common = require('../../common'); | |||
| 4 | 4 | const assert = require('assert'); | |
| 5 | 5 | const { runInCallbackScope } = require(`./build/${common.buildType}/binding`); | |
| 6 | 6 | ||
| 7 | - assert.strictEqual(runInCallbackScope({}, 0, 0, () => 42), 42); | ||
| 7 | + assert.strictEqual(runInCallbackScope({}, 'test-resource', () => 42), 42); | ||
| 8 | 8 | ||
| 9 | 9 | { | |
| 10 | 10 | process.once('uncaughtException', common.mustCall((err) => { | |
| 11 | 11 | assert.strictEqual(err.message, 'foo'); | |
| 12 | 12 | })); | |
| 13 | 13 | ||
| 14 | - runInCallbackScope({}, 0, 0, () => { | ||
| 14 | + runInCallbackScope({}, 'test-resource', () => { | ||
| 15 | 15 | throw new Error('foo'); | |
| 16 | 16 | }); | |
| 17 | 17 | } | |
0 commit comments