From 41c6739d3e633bd2815fdc48319761f9ecc5de25 Mon Sep 17 00:00:00 2001 From: Kamil Paradowski Date: Thu, 24 Jul 2025 17:21:38 +0200 Subject: [PATCH 1/4] feat: added support to three async functions --- packages/host/cpp/RuntimeNodeApiAsync.cpp | 156 ++++++++++-------- packages/host/cpp/RuntimeNodeApiAsync.hpp | 16 ++ .../generate-weak-node-api-injector.ts | 3 + 3 files changed, 108 insertions(+), 67 deletions(-) diff --git a/packages/host/cpp/RuntimeNodeApiAsync.cpp b/packages/host/cpp/RuntimeNodeApiAsync.cpp index dd4c87c3..c1501125 100644 --- a/packages/host/cpp/RuntimeNodeApiAsync.cpp +++ b/packages/host/cpp/RuntimeNodeApiAsync.cpp @@ -2,87 +2,74 @@ #include #include "Logger.hpp" -struct AsyncJob { - using IdType = uint64_t; - enum State { Created, Queued, Completed, Cancelled, Deleted }; +using IdType = uint64_t; +struct AsyncContext { IdType id{}; - State state{}; napi_env env; napi_value async_resource; napi_value async_resource_name; - napi_async_execute_callback execute; - napi_async_complete_callback complete; - void* data{nullptr}; - static AsyncJob* fromWork(napi_async_work work) { - return reinterpret_cast(work); - } - static napi_async_work toWork(AsyncJob* job) { - return reinterpret_cast(job); - } + AsyncContext( + napi_env env, napi_value async_resource, napi_value async_resource_name) + : env{env}, + async_resource{async_resource}, + async_resource_name{async_resource_name} {} }; -class AsyncWorkRegistry { - public: - using IdType = AsyncJob::IdType; +struct AsyncJob : AsyncContext { + enum State { Created, Queued, Completed, Cancelled, Deleted }; - std::shared_ptr create(napi_env env, + State state{State::Created}; + napi_async_execute_callback execute; + napi_async_complete_callback complete; + void* data{nullptr}; + + AsyncJob(napi_env env, napi_value async_resource, napi_value async_resource_name, napi_async_execute_callback execute, napi_async_complete_callback complete, - void* data) { - const auto job = std::shared_ptr(new AsyncJob{ - .id = next_id(), - .state = AsyncJob::State::Created, - .env = env, - .async_resource = async_resource, - .async_resource_name = async_resource_name, - .execute = execute, - .complete = complete, - .data = data, - }); - - jobs_[job->id] = job; - return job; - } + void* data) + : AsyncContext{env, async_resource, async_resource_name}, + execute{execute}, + complete{complete}, + data{data} {} +}; - std::shared_ptr get(napi_async_work work) const { - const auto job = AsyncJob::fromWork(work); - if (!job) { - return {}; - } - if (const auto it = jobs_.find(job->id); it != jobs_.end()) { - return it->second; - } - return {}; +template +class Container { + public: + void push(std::shared_ptr&& obj) { + const auto id = nextId(); + obj->id = id; + map_[id] = std::move(obj); } - - bool release(IdType id) { - if (const auto it = jobs_.find(id); it != jobs_.end()) { - it->second->state = AsyncJob::State::Deleted; - jobs_.erase(it); - return true; - } - return false; + std::shared_ptr get(const U obj) { + return map_.contains(id(obj)) ? map_[id(obj)] : nullptr; } + bool release(const U obj) { return map_.erase(id(obj)) > 0; } private: - IdType next_id() { - if (current_id_ == std::numeric_limits::max()) [[unlikely]] { - current_id_ = 0; + IdType id(const U obj) const { + auto casted = reinterpret_cast(obj); + return casted ? casted->id : 0; + } + IdType nextId() { + if (currentId_ == std::numeric_limits::max()) [[unlikely]] { + currentId_ = 0; } - return ++current_id_; + return ++currentId_; } - IdType current_id_{0}; - std::unordered_map> jobs_; + IdType currentId_{0}; + std::unordered_map> map_; }; static std::unordered_map> callInvokers; -static AsyncWorkRegistry asyncWorkRegistry; +static Container jobs_; +static Container contexts_; namespace callstack::nodeapihost { @@ -104,20 +91,16 @@ napi_status napi_create_async_work(napi_env env, napi_async_complete_callback complete, void* data, napi_async_work* result) { - const auto job = asyncWorkRegistry.create( + auto job = std::make_shared( env, async_resource, async_resource_name, execute, complete, data); - if (!job) { - log_debug("Error: Failed to create async work job"); - return napi_generic_failure; - } - - *result = AsyncJob::toWork(job.get()); + *result = reinterpret_cast(job.get()); + jobs_.push(std::move(job)); return napi_ok; } napi_status napi_queue_async_work( node_api_basic_env env, napi_async_work work) { - const auto job = asyncWorkRegistry.get(work); + const auto job = jobs_.get(work); if (!job) { log_debug("Error: Received null job in napi_queue_async_work"); return napi_invalid_arg; @@ -151,13 +134,14 @@ napi_status napi_queue_async_work( napi_status napi_delete_async_work( node_api_basic_env env, napi_async_work work) { - const auto job = asyncWorkRegistry.get(work); + const auto job = jobs_.get(work); if (!job) { log_debug("Error: Received non-existent job in napi_delete_async_work"); return napi_invalid_arg; } - if (!asyncWorkRegistry.release(job->id)) { + job->state = AsyncJob::State::Deleted; + if (!jobs_.release(work)) { log_debug("Error: Failed to release async work job"); return napi_generic_failure; } @@ -167,7 +151,7 @@ napi_status napi_delete_async_work( napi_status napi_cancel_async_work( node_api_basic_env env, napi_async_work work) { - const auto job = asyncWorkRegistry.get(work); + const auto job = jobs_.get(work); if (!job) { log_debug("Error: Received null job in napi_cancel_async_work"); return napi_invalid_arg; @@ -187,4 +171,42 @@ napi_status napi_cancel_async_work( job->state = AsyncJob::State::Cancelled; return napi_ok; } + +napi_status napi_async_init(napi_env env, + napi_value async_resource, + napi_value async_resource_name, + napi_async_context* result) { + if (!result) { + return napi_invalid_arg; + } + auto context = + std::make_shared(env, async_resource, async_resource_name); + *result = reinterpret_cast(context.get()); + contexts_.push(std::move(context)); + return napi_ok; +} + +napi_status napi_async_destroy(napi_env env, napi_async_context async_context) { + if (!async_context) { + return napi_invalid_arg; + } + if (!contexts_.release(async_context)) { + return napi_generic_failure; + } + return napi_ok; +} + +napi_status napi_make_callback(napi_env env, + napi_async_context async_context, + napi_value recv, + napi_value func, + size_t argc, + const napi_value* argv, + napi_value* result) { + const auto status = napi_call_function(env, recv, func, argc, argv, result); + if (status == napi_pending_exception && async_context) { + contexts_.release(async_context); + } + return status; +} } // namespace callstack::nodeapihost diff --git a/packages/host/cpp/RuntimeNodeApiAsync.hpp b/packages/host/cpp/RuntimeNodeApiAsync.hpp index f0108e6d..3f9c6171 100644 --- a/packages/host/cpp/RuntimeNodeApiAsync.hpp +++ b/packages/host/cpp/RuntimeNodeApiAsync.hpp @@ -23,4 +23,20 @@ napi_status napi_delete_async_work( napi_status napi_cancel_async_work( node_api_basic_env env, napi_async_work work); + +napi_status napi_async_init(napi_env env, + napi_value async_resource, + napi_value async_resource_name, + napi_async_context* result); + +napi_status napi_async_destroy(napi_env env, napi_async_context async_context); + +napi_status napi_make_callback(napi_env env, + napi_async_context async_context, + napi_value recv, + napi_value func, + size_t argc, + const napi_value* argv, + napi_value* result); + } // namespace callstack::nodeapihost diff --git a/packages/host/scripts/generate-weak-node-api-injector.ts b/packages/host/scripts/generate-weak-node-api-injector.ts index d5adfd83..87c75a2b 100644 --- a/packages/host/scripts/generate-weak-node-api-injector.ts +++ b/packages/host/scripts/generate-weak-node-api-injector.ts @@ -20,6 +20,9 @@ const IMPLEMENTED_RUNTIME_FUNCTIONS = [ "napi_fatal_error", "napi_get_node_version", "napi_get_version", + "napi_async_init", + "napi_async_destroy", + "napi_make_callback", ]; /** From 5887992e5880e218b7487e9e1f17f6a02b35d7bf Mon Sep 17 00:00:00 2001 From: Kamil Paradowski Date: Thu, 24 Jul 2025 17:27:50 +0200 Subject: [PATCH 2/4] chore: adopt node make_callback tests --- packages/node-addon-examples/src/index.ts | 5 +- .../tests/make_callback/CMakeLists.txt | 15 +++ .../tests/make_callback/addon.c | 68 ++++++++++++ .../tests/make_callback/addon.js | 103 ++++++++++++++++++ .../tests/make_callback/binding.gyp | 8 ++ .../tests/make_callback/package.json | 14 +++ 6 files changed, 210 insertions(+), 3 deletions(-) create mode 100644 packages/node-addon-examples/tests/make_callback/CMakeLists.txt create mode 100644 packages/node-addon-examples/tests/make_callback/addon.c create mode 100644 packages/node-addon-examples/tests/make_callback/addon.js create mode 100644 packages/node-addon-examples/tests/make_callback/binding.gyp create mode 100644 packages/node-addon-examples/tests/make_callback/package.json diff --git a/packages/node-addon-examples/src/index.ts b/packages/node-addon-examples/src/index.ts index 869d4825..4704f8f9 100644 --- a/packages/node-addon-examples/src/index.ts +++ b/packages/node-addon-examples/src/index.ts @@ -15,9 +15,7 @@ function assertLogs(cb: () => void, expectedMessages: string[]) { cb(); if (expectedMessages.length > 0) { errors.push( - new Error( - `Missing expected message(s): ${expectedMessages.join(", ")}`, - ), + new Error(`Missing expected message(s): ${expectedMessages.join(", ")}`) ); } } finally { @@ -85,5 +83,6 @@ export const suites: Record< require("../tests/buffers/addon.js"); }, async: () => require("../tests/async/addon.js") as () => Promise, + make_callback: () => require("../tests/make_callback/addon.js"), }, }; diff --git a/packages/node-addon-examples/tests/make_callback/CMakeLists.txt b/packages/node-addon-examples/tests/make_callback/CMakeLists.txt new file mode 100644 index 00000000..e353cc27 --- /dev/null +++ b/packages/node-addon-examples/tests/make_callback/CMakeLists.txt @@ -0,0 +1,15 @@ +cmake_minimum_required(VERSION 3.15) +project(tests-make_callback) + +add_compile_definitions(NAPI_VERSION=8) + +add_library(addon SHARED addon.c ${CMAKE_JS_SRC}) +set_target_properties(addon PROPERTIES PREFIX "" SUFFIX ".node") +target_include_directories(addon PRIVATE ${CMAKE_JS_INC}) +target_link_libraries(addon PRIVATE ${CMAKE_JS_LIB}) +target_compile_features(addon PRIVATE cxx_std_17) + +if(MSVC AND CMAKE_JS_NODELIB_DEF AND CMAKE_JS_NODELIB_TARGET) + # Generate node.lib + execute_process(COMMAND ${CMAKE_AR} /def:${CMAKE_JS_NODELIB_DEF} /out:${CMAKE_JS_NODELIB_TARGET} ${CMAKE_STATIC_LINKER_FLAGS}) +endif() \ No newline at end of file diff --git a/packages/node-addon-examples/tests/make_callback/addon.c b/packages/node-addon-examples/tests/make_callback/addon.c new file mode 100644 index 00000000..11a7234a --- /dev/null +++ b/packages/node-addon-examples/tests/make_callback/addon.c @@ -0,0 +1,68 @@ +#include +#include +#include +#include "../RuntimeNodeApiTestsCommon.h" + +#define MAX_ARGUMENTS 10 +#define RESERVED_ARGS 3 + +static napi_value MakeCallback(napi_env env, napi_callback_info info) { + size_t argc = MAX_ARGUMENTS; + size_t n; + napi_value args[MAX_ARGUMENTS]; + // NOLINTNEXTLINE (readability/null_usage) + NODE_API_CALL(env, napi_get_cb_info(env, info, &argc, args, NULL, NULL)); + + NODE_API_ASSERT(env, argc > 0, "Wrong number of arguments"); + printf("Make callback called with %zu arguments\n", argc); + + napi_value resource = args[0]; + napi_value recv = args[1]; + napi_value func = args[2]; + + napi_value argv[MAX_ARGUMENTS - RESERVED_ARGS]; + for (n = RESERVED_ARGS; n < argc; n += 1) { + argv[n - RESERVED_ARGS] = args[n]; + } + + napi_valuetype func_type; + + NODE_API_CALL(env, napi_typeof(env, func, &func_type)); + + napi_value resource_name; + NODE_API_CALL(env, + napi_create_string_utf8(env, "test", NAPI_AUTO_LENGTH, &resource_name)); + + napi_async_context context; + NODE_API_CALL(env, napi_async_init(env, resource, resource_name, &context)); + + napi_value result; + if (func_type == napi_function) { + NODE_API_CALL(env, + napi_make_callback( + env, context, recv, func, argc - RESERVED_ARGS, argv, &result)); + } else { + NODE_API_ASSERT(env, false, "Unexpected argument type"); + } + + NODE_API_CALL(env, napi_async_destroy(env, context)); + + return result; +} + +static napi_value Init(napi_env env, napi_value exports) { + napi_value fn; + NODE_API_CALL(env, + napi_create_function( + // NOLINTNEXTLINE (readability/null_usage) + env, + NULL, + NAPI_AUTO_LENGTH, + MakeCallback, + NULL, + &fn)); + NODE_API_CALL(env, napi_set_named_property(env, exports, "makeCallback", fn)); + return exports; +} + +NAPI_MODULE(NODE_GYP_MODULE_NAME, Init) \ No newline at end of file diff --git a/packages/node-addon-examples/tests/make_callback/addon.js b/packages/node-addon-examples/tests/make_callback/addon.js new file mode 100644 index 00000000..67b7b3bf --- /dev/null +++ b/packages/node-addon-examples/tests/make_callback/addon.js @@ -0,0 +1,103 @@ +/* eslint-disable @typescript-eslint/no-require-imports */ +/* eslint-disable no-undef */ +const assert = require("assert"); +const binding = require("bindings")("addon.node"); +const makeCallback = binding.makeCallback; + +/** + * Resource should be able to be arbitrary objects without special internal + * slots. Testing with plain object here. + */ + +function myMultiArgFunc(arg1, arg2, arg3) { + assert.strictEqual(arg1, 1); + assert.strictEqual(arg2, 2); + assert.strictEqual(arg3, 3); + return 42; +} + +module.exports = () => { + const resource = {}; + const process = (module.exports = {}); + + assert.strictEqual( + makeCallback(resource, process, function () { + assert.strictEqual(arguments.length, 0); + assert.strictEqual(this, process); + return 42; + }), + 42 + ); + + assert.strictEqual( + makeCallback( + resource, + process, + function (x) { + assert.strictEqual(arguments.length, 1); + assert.strictEqual(this, process); + assert.strictEqual(x, 1337); + return 42; + }, + 1337 + ), + 42 + ); + + assert.strictEqual( + makeCallback(resource, process, myMultiArgFunc, 1, 2, 3), + 42 + ); + + // TODO(node-api): napi_make_callback needs to support + // strings passed for the func argument + // + // const recv = { + // one: function () { + // assert.strictEqual(0, arguments.length); + // assert.strictEqual(this, recv); + // return 42; + // }, + // two: function (x) { + // assert.strictEqual(1, arguments.length); + // assert.strictEqual(this, recv); + // assert.strictEqual(x, 1339); + // return 42; + // }, + // }; + + // assert.strictEqual(makeCallback(recv, "one"), 42); + // assert.strictEqual(makeCallback(recv, "two", 1337), 42); + // + // Check that callbacks on a receiver from a different context works. + // const foreignObject = vm.runInNewContext('({ fortytwo() { return 42; } })'); + // const foreignObject = { fortytwo: () => 42 }; + // assert.strictEqual(makeCallback(foreignObject, "fortytwo"), 42); + + // Check that the callback is made in the context of the receiver. + + // const target = vm.runInNewContext(` + // (function($Object) { + // if (Object === $Object) + // throw new Error('bad'); + // return Object; + // }) + // `); + // assert.notStrictEqual(makeCallback(resource, process, target, Object), Object); + + // // Runs in inner context. + // const forward = vm.runInNewContext(` + // (function(forward) { + // return forward(Object); + // }) + // `); + + // // Runs in outer context. + // function endpoint($Object) { + // if (Object === $Object) + // throw new Error('bad'); + // return Object; + // } + + // assert.strictEqual(makeCallback(resource, process, forward, endpoint), Object); +}; diff --git a/packages/node-addon-examples/tests/make_callback/binding.gyp b/packages/node-addon-examples/tests/make_callback/binding.gyp new file mode 100644 index 00000000..80f9fa87 --- /dev/null +++ b/packages/node-addon-examples/tests/make_callback/binding.gyp @@ -0,0 +1,8 @@ +{ + "targets": [ + { + "target_name": "addon", + "sources": [ "addon.c" ] + } + ] +} diff --git a/packages/node-addon-examples/tests/make_callback/package.json b/packages/node-addon-examples/tests/make_callback/package.json new file mode 100644 index 00000000..30e62b51 --- /dev/null +++ b/packages/node-addon-examples/tests/make_callback/package.json @@ -0,0 +1,14 @@ +{ + "name": "make-callback-test", + "version": "0.0.0", + "description": "Tests of runtime async functions", + "main": "addon.js", + "private": true, + "dependencies": { + "bindings": "~1.5.0" + }, + "scripts": { + "test": "node addon.js" + }, + "gypfile": true +} From 8908c218d8035c270e1720f17b30941a4f6bb38b Mon Sep 17 00:00:00 2001 From: Kamil Paradowski Date: Thu, 24 Jul 2025 17:28:04 +0200 Subject: [PATCH 3/4] chore: changeset --- .changeset/social-parks-take.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/social-parks-take.md diff --git a/.changeset/social-parks-take.md b/.changeset/social-parks-take.md new file mode 100644 index 00000000..252dc31a --- /dev/null +++ b/.changeset/social-parks-take.md @@ -0,0 +1,5 @@ +--- +"react-native-node-api": minor +--- + +Provided implementation of napi_async_init, napi_async_destroy, napi_make_callback From c91b9f6d2491336e7f6559efcc914d297f1d9daa Mon Sep 17 00:00:00 2001 From: Kamil Paradowski Date: Thu, 24 Jul 2025 17:33:16 +0200 Subject: [PATCH 4/4] refactor: cleanup --- .../tests/make_callback/addon.c | 2 +- .../tests/make_callback/addon.js | 52 ------------------- 2 files changed, 1 insertion(+), 53 deletions(-) diff --git a/packages/node-addon-examples/tests/make_callback/addon.c b/packages/node-addon-examples/tests/make_callback/addon.c index 11a7234a..20d291c0 100644 --- a/packages/node-addon-examples/tests/make_callback/addon.c +++ b/packages/node-addon-examples/tests/make_callback/addon.c @@ -65,4 +65,4 @@ static napi_value Init(napi_env env, napi_value exports) { return exports; } -NAPI_MODULE(NODE_GYP_MODULE_NAME, Init) \ No newline at end of file +NAPI_MODULE(NODE_GYP_MODULE_NAME, Init) diff --git a/packages/node-addon-examples/tests/make_callback/addon.js b/packages/node-addon-examples/tests/make_callback/addon.js index 67b7b3bf..ef9c8047 100644 --- a/packages/node-addon-examples/tests/make_callback/addon.js +++ b/packages/node-addon-examples/tests/make_callback/addon.js @@ -48,56 +48,4 @@ module.exports = () => { makeCallback(resource, process, myMultiArgFunc, 1, 2, 3), 42 ); - - // TODO(node-api): napi_make_callback needs to support - // strings passed for the func argument - // - // const recv = { - // one: function () { - // assert.strictEqual(0, arguments.length); - // assert.strictEqual(this, recv); - // return 42; - // }, - // two: function (x) { - // assert.strictEqual(1, arguments.length); - // assert.strictEqual(this, recv); - // assert.strictEqual(x, 1339); - // return 42; - // }, - // }; - - // assert.strictEqual(makeCallback(recv, "one"), 42); - // assert.strictEqual(makeCallback(recv, "two", 1337), 42); - // - // Check that callbacks on a receiver from a different context works. - // const foreignObject = vm.runInNewContext('({ fortytwo() { return 42; } })'); - // const foreignObject = { fortytwo: () => 42 }; - // assert.strictEqual(makeCallback(foreignObject, "fortytwo"), 42); - - // Check that the callback is made in the context of the receiver. - - // const target = vm.runInNewContext(` - // (function($Object) { - // if (Object === $Object) - // throw new Error('bad'); - // return Object; - // }) - // `); - // assert.notStrictEqual(makeCallback(resource, process, target, Object), Object); - - // // Runs in inner context. - // const forward = vm.runInNewContext(` - // (function(forward) { - // return forward(Object); - // }) - // `); - - // // Runs in outer context. - // function endpoint($Object) { - // if (Object === $Object) - // throw new Error('bad'); - // return Object; - // } - - // assert.strictEqual(makeCallback(resource, process, forward, endpoint), Object); };