From b6d312464d3a5063cab78038512f5cd10b945a55 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Tue, 22 Sep 2020 20:35:48 +0200 Subject: [PATCH] src: allow N-API addon in `AddLinkedBinding()` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `AddLinkedBinding()` can be used to load old-style Node.js addons, but currently not N-API addons. There’s no good reason not to support N-API addons as well, so add that. PR-URL: https://github.com/nodejs/node/pull/35301 Reviewed-By: Colin Ihrig Reviewed-By: Michael Dawson Reviewed-By: Zeyu Yang --- src/api/environment.cc | 4 ++ src/node.h | 4 ++ src/node_api.cc | 17 +++-- src/node_api.h | 2 +- src/node_internals.h | 2 + test/cctest/test_linked_binding.cc | 109 ++++++++++++++++++++++++++++- 6 files changed, 131 insertions(+), 7 deletions(-) diff --git a/src/api/environment.cc b/src/api/environment.cc index 6cd2c6c34a1c16..65ee6020933a27 100644 --- a/src/api/environment.cc +++ b/src/api/environment.cc @@ -677,6 +677,10 @@ void AddLinkedBinding(Environment* env, const node_module& mod) { prev_head->nm_link = &env->extra_linked_bindings()->back(); } +void AddLinkedBinding(Environment* env, const napi_module& mod) { + AddLinkedBinding(env, napi_module_to_node_module(&mod)); +} + void AddLinkedBinding(Environment* env, const char* name, addon_context_register_func fn, diff --git a/src/node.h b/src/node.h index d2c1b9cff3487f..c23cf45d564848 100644 --- a/src/node.h +++ b/src/node.h @@ -117,6 +117,8 @@ // Forward-declare libuv loop struct uv_loop_s; +struct napi_module; + // Forward-declare these functions now to stop MSVS from becoming // terminally confused when it's done in node_internals.h namespace node { @@ -819,6 +821,8 @@ extern "C" NODE_EXTERN void node_module_register(void* mod); // In each variant, the registration function needs to be usable at least for // the time during which the Environment exists. NODE_EXTERN void AddLinkedBinding(Environment* env, const node_module& mod); +NODE_EXTERN void AddLinkedBinding(Environment* env, + const struct napi_module& mod); NODE_EXTERN void AddLinkedBinding(Environment* env, const char* name, addon_context_register_func fn, diff --git a/src/node_api.cc b/src/node_api.cc index 93488146d56690..12f369a809734f 100644 --- a/src/node_api.cc +++ b/src/node_api.cc @@ -447,7 +447,7 @@ static void napi_module_register_cb(v8::Local exports, v8::Local context, void* priv) { napi_module_register_by_symbol(exports, module, context, - static_cast(priv)->nm_register_func); + static_cast(priv)->nm_register_func); } void napi_module_register_by_symbol(v8::Local exports, @@ -480,9 +480,9 @@ void napi_module_register_by_symbol(v8::Local exports, } } -// Registers a NAPI module. -void napi_module_register(napi_module* mod) { - node::node_module* nm = new node::node_module { +namespace node { +node_module napi_module_to_node_module(const napi_module* mod) { + return { -1, mod->nm_flags | NM_F_DELETEME, nullptr, @@ -490,9 +490,16 @@ void napi_module_register(napi_module* mod) { nullptr, napi_module_register_cb, mod->nm_modname, - mod, // priv + const_cast(mod), // priv nullptr, }; +} +} // namespace node + +// Registers a NAPI module. +void napi_module_register(napi_module* mod) { + node::node_module* nm = new node::node_module( + node::napi_module_to_node_module(mod)); node::node_module_register(nm); } diff --git a/src/node_api.h b/src/node_api.h index 577a1dcd949872..786988e296b8b2 100644 --- a/src/node_api.h +++ b/src/node_api.h @@ -31,7 +31,7 @@ struct uv_loop_s; // Forward declaration. typedef napi_value (*napi_addon_register_func)(napi_env env, napi_value exports); -typedef struct { +typedef struct napi_module { int nm_version; unsigned int nm_flags; const char* nm_filename; diff --git a/src/node_internals.h b/src/node_internals.h index dffaa084db409b..c8952e59a2b071 100644 --- a/src/node_internals.h +++ b/src/node_internals.h @@ -398,6 +398,8 @@ namespace fs { std::string Basename(const std::string& str, const std::string& extension); } // namespace fs +node_module napi_module_to_node_module(const napi_module* mod); + } // namespace node #endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS diff --git a/test/cctest/test_linked_binding.cc b/test/cctest/test_linked_binding.cc index 523acc86c63e2a..17c020429f0993 100644 --- a/test/cctest/test_linked_binding.cc +++ b/test/cctest/test_linked_binding.cc @@ -1,5 +1,5 @@ #include "node_test_fixture.h" -#include "node_internals.h" // RunBootstrapping() +#include "node_api.h" void InitializeBinding(v8::Local exports, v8::Local module, @@ -83,3 +83,110 @@ TEST_F(LinkedBindingTest, LocallyDefinedLinkedBindingTest) { CHECK_EQ(strcmp(*utf8val, "value"), 0); CHECK_EQ(calls, 1); } + +napi_value InitializeLocalNapiBinding(napi_env env, napi_value exports) { + napi_value key, value; + CHECK_EQ( + napi_create_string_utf8(env, "hello", NAPI_AUTO_LENGTH, &key), napi_ok); + CHECK_EQ( + napi_create_string_utf8(env, "world", NAPI_AUTO_LENGTH, &value), napi_ok); + CHECK_EQ(napi_set_property(env, exports, key, value), napi_ok); + return nullptr; +} + +static napi_module local_linked_napi = { + NAPI_MODULE_VERSION, + node::ModuleFlags::kLinked, + nullptr, + InitializeLocalNapiBinding, + "local_linked_napi", + nullptr, + {0}, +}; + +TEST_F(LinkedBindingTest, LocallyDefinedLinkedBindingNapiTest) { + const v8::HandleScope handle_scope(isolate_); + const Argv argv; + Env test_env {handle_scope, argv}; + + AddLinkedBinding(*test_env, local_linked_napi); + + v8::Local context = isolate_->GetCurrentContext(); + + const char* run_script = + "process._linkedBinding('local_linked_napi').hello"; + v8::Local script = v8::Script::Compile( + context, + v8::String::NewFromOneByte(isolate_, + reinterpret_cast(run_script)) + .ToLocalChecked()) + .ToLocalChecked(); + v8::Local completion_value = script->Run(context).ToLocalChecked(); + v8::String::Utf8Value utf8val(isolate_, completion_value); + CHECK_NOT_NULL(*utf8val); + CHECK_EQ(strcmp(*utf8val, "world"), 0); +} + +napi_value NapiLinkedWithInstanceData(napi_env env, napi_value exports) { + int* instance_data = new int(0); + CHECK_EQ( + napi_set_instance_data( + env, + instance_data, + [](napi_env env, void* data, void* hint) { + ++*static_cast(data); + }, nullptr), + napi_ok); + + napi_value key, value; + CHECK_EQ( + napi_create_string_utf8(env, "hello", NAPI_AUTO_LENGTH, &key), napi_ok); + CHECK_EQ( + napi_create_external(env, instance_data, nullptr, nullptr, &value), + napi_ok); + CHECK_EQ(napi_set_property(env, exports, key, value), napi_ok); + return nullptr; +} + +static napi_module local_linked_napi_id = { + NAPI_MODULE_VERSION, + node::ModuleFlags::kLinked, + nullptr, + NapiLinkedWithInstanceData, + "local_linked_napi_id", + nullptr, + {0}, +}; + +TEST_F(LinkedBindingTest, LocallyDefinedLinkedBindingNapiInstanceDataTest) { + const v8::HandleScope handle_scope(isolate_); + int* instance_data = nullptr; + + { + const Argv argv; + Env test_env {handle_scope, argv}; + + AddLinkedBinding(*test_env, local_linked_napi_id); + + v8::Local context = isolate_->GetCurrentContext(); + + const char* run_script = + "process._linkedBinding('local_linked_napi_id').hello"; + v8::Local script = v8::Script::Compile( + context, + v8::String::NewFromOneByte(isolate_, + reinterpret_cast(run_script)) + .ToLocalChecked()) + .ToLocalChecked(); + v8::Local completion_value = + script->Run(context).ToLocalChecked(); + CHECK(completion_value->IsExternal()); + instance_data = static_cast( + completion_value.As()->Value()); + CHECK_NE(instance_data, nullptr); + CHECK_EQ(*instance_data, 0); + } + + CHECK_EQ(*instance_data, 1); + delete instance_data; +}