From 68656cf5889aed1c8b9d79289c19f804f2fe97a3 Mon Sep 17 00:00:00 2001 From: legendecas Date: Sun, 19 Apr 2020 17:45:02 +0800 Subject: [PATCH] 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: https://github.com/nodejs/node/pull/32928 Reviewed-By: Michael Dawson Reviewed-By: Anna Henningsen --- test/node-api/test_callback_scope/binding.cc | 31 +++++-------------- .../test_callback_scope/test-async-hooks.js | 16 ++++++++-- test/node-api/test_callback_scope/test.js | 4 +-- 3 files changed, 23 insertions(+), 28 deletions(-) diff --git a/test/node-api/test_callback_scope/binding.cc b/test/node-api/test_callback_scope/binding.cc index 1a06d04fce53e3..153428a65bd6e4 100644 --- a/test/node-api/test_callback_scope/binding.cc +++ b/test/node-api/test_callback_scope/binding.cc @@ -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)); @@ -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(&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)); } diff --git a/test/node-api/test_callback_scope/test-async-hooks.js b/test/node-api/test_callback_scope/test-async-hooks.js index 1a11bf60398f9b..22074428883d6b 100644 --- a/test/node-api/test_callback_scope/test-async-hooks.js +++ b/test/node-api/test_callback_scope/test-async-hooks.js @@ -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); }); diff --git a/test/node-api/test_callback_scope/test.js b/test/node-api/test_callback_scope/test.js index 2f2efe5f47b98a..abf9e41b8f5028 100644 --- a/test/node-api/test_callback_scope/test.js +++ b/test/node-api/test_callback_scope/test.js @@ -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'); }); }