From 7019d48ba1a379312693923af1e48d828fbb7ef3 Mon Sep 17 00:00:00 2001 From: Vladimir Morozov Date: Mon, 23 Jan 2023 07:15:34 -0800 Subject: [PATCH] node-api: deprecate napi_module_register PR-URL: https://github.com/nodejs/node/pull/46319 Reviewed-By: Chengzhong Wu Reviewed-By: Michael Dawson --- src/api/environment.cc | 30 ++- src/node.h | 8 +- src/node_api.h | 76 ++---- test/cctest/test_linked_binding.cc | 243 +++++++++++------- test/node-api/test_null_init/test_null_init.c | 52 +++- 5 files changed, 254 insertions(+), 155 deletions(-) diff --git a/src/api/environment.cc b/src/api/environment.cc index 0de7ae88c2a715..fb8c84027d56b5 100644 --- a/src/api/environment.cc +++ b/src/api/environment.cc @@ -780,7 +780,9 @@ void AddLinkedBinding(Environment* env, const node_module& mod) { } void AddLinkedBinding(Environment* env, const napi_module& mod) { - AddLinkedBinding(env, napi_module_to_node_module(&mod)); + node_module node_mod = napi_module_to_node_module(&mod); + node_mod.nm_flags = NM_F_LINKED; + AddLinkedBinding(env, node_mod); } void AddLinkedBinding(Environment* env, @@ -801,6 +803,32 @@ void AddLinkedBinding(Environment* env, AddLinkedBinding(env, mod); } +void AddLinkedBinding(Environment* env, + const char* name, + napi_addon_register_func fn) { + node_module mod = { + -1, + NM_F_LINKED, + nullptr, // nm_dso_handle + nullptr, // nm_filename + nullptr, // nm_register_func + [](v8::Local exports, + v8::Local module, + v8::Local context, + void* priv) { + napi_module_register_by_symbol( + exports, + module, + context, + reinterpret_cast(priv)); + }, + name, + reinterpret_cast(fn), + nullptr // nm_link + }; + AddLinkedBinding(env, mod); +} + static std::atomic next_thread_id{0}; ThreadId AllocateEnvironmentThreadId() { diff --git a/src/node.h b/src/node.h index 1dfe3d718450f1..648bbe0404b718 100644 --- a/src/node.h +++ b/src/node.h @@ -75,6 +75,9 @@ #include "v8-platform.h" // NOLINT(build/include_order) #include "node_version.h" // NODE_MODULE_VERSION +#define NAPI_EXPERIMENTAL +#include "node_api.h" + #include #include #include @@ -121,8 +124,6 @@ // 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 { @@ -1096,6 +1097,9 @@ NODE_EXTERN void AddLinkedBinding(Environment* env, const char* name, addon_context_register_func fn, void* priv); +NODE_EXTERN void AddLinkedBinding(Environment* env, + const char* name, + napi_addon_register_func fn); /* Registers a callback with the passed-in Environment instance. The callback * is called after the event loop exits, but before the VM is disposed. diff --git a/src/node_api.h b/src/node_api.h index caf987dbd8dd8b..4d1b50414e2e02 100644 --- a/src/node_api.h +++ b/src/node_api.h @@ -31,6 +31,7 @@ struct uv_loop_s; // Forward declaration. typedef napi_value(NAPI_CDECL* napi_addon_register_func)(napi_env env, napi_value exports); +// Used by deprecated registration method napi_module_register. typedef struct napi_module { int nm_version; unsigned int nm_flags; @@ -43,70 +44,15 @@ typedef struct napi_module { #define NAPI_MODULE_VERSION 1 -#if defined(_MSC_VER) -#if defined(__cplusplus) -#define NAPI_C_CTOR(fn) \ - static void NAPI_CDECL fn(void); \ - namespace { \ - struct fn##_ { \ - fn##_() { fn(); } \ - } fn##_v_; \ - } \ - static void NAPI_CDECL fn(void) -#else // !defined(__cplusplus) -#pragma section(".CRT$XCU", read) -// The NAPI_C_CTOR macro defines a function fn that is called during CRT -// initialization. -// C does not support dynamic initialization of static variables and this code -// simulates C++ behavior. Exporting the function pointer prevents it from being -// optimized. See for details: -// https://docs.microsoft.com/en-us/cpp/c-runtime-library/crt-initialization?view=msvc-170 -#define NAPI_C_CTOR(fn) \ - static void NAPI_CDECL fn(void); \ - __declspec(dllexport, allocate(".CRT$XCU")) void(NAPI_CDECL * fn##_)(void) = \ - fn; \ - static void NAPI_CDECL fn(void) -#endif // defined(__cplusplus) -#else -#define NAPI_C_CTOR(fn) \ - static void fn(void) __attribute__((constructor)); \ - static void fn(void) -#endif - -#define NAPI_MODULE_X(modname, regfunc, priv, flags) \ - EXTERN_C_START \ - static napi_module _module = { \ - NAPI_MODULE_VERSION, \ - flags, \ - __FILE__, \ - regfunc, \ - #modname, \ - priv, \ - {0}, \ - }; \ - NAPI_C_CTOR(_register_##modname) { napi_module_register(&_module); } \ - EXTERN_C_END - #define NAPI_MODULE_INITIALIZER_X(base, version) \ NAPI_MODULE_INITIALIZER_X_HELPER(base, version) #define NAPI_MODULE_INITIALIZER_X_HELPER(base, version) base##version #ifdef __wasm32__ -#define NAPI_WASM_INITIALIZER \ - NAPI_MODULE_INITIALIZER_X(napi_register_wasm_v, NAPI_MODULE_VERSION) -#define NAPI_MODULE(modname, regfunc) \ - EXTERN_C_START \ - NAPI_MODULE_EXPORT napi_value NAPI_WASM_INITIALIZER(napi_env env, \ - napi_value exports) { \ - return regfunc(env, exports); \ - } \ - EXTERN_C_END +#define NAPI_MODULE_INITIALIZER_BASE napi_register_wasm_v #else -#define NAPI_MODULE(modname, regfunc) \ - NAPI_MODULE_X(modname, regfunc, NULL, 0) // NOLINT (readability/null_usage) -#endif - #define NAPI_MODULE_INITIALIZER_BASE napi_register_module_v +#endif #define NAPI_MODULE_INITIALIZER \ NAPI_MODULE_INITIALIZER_X(NAPI_MODULE_INITIALIZER_BASE, NAPI_MODULE_VERSION) @@ -116,12 +62,24 @@ typedef struct napi_module { NAPI_MODULE_EXPORT napi_value NAPI_MODULE_INITIALIZER(napi_env env, \ napi_value exports); \ EXTERN_C_END \ - NAPI_MODULE(NODE_GYP_MODULE_NAME, NAPI_MODULE_INITIALIZER) \ napi_value NAPI_MODULE_INITIALIZER(napi_env env, napi_value exports) +#define NAPI_MODULE(modname, regfunc) \ + NAPI_MODULE_INIT() { return regfunc(env, exports); } + +// Deprecated. Use NAPI_MODULE. +#define NAPI_MODULE_X(modname, regfunc, priv, flags) \ + NAPI_MODULE(modname, regfunc) + EXTERN_C_START -NAPI_EXTERN void NAPI_CDECL napi_module_register(napi_module* mod); +// Deprecated. Replaced by symbol-based registration defined by NAPI_MODULE +// and NAPI_MODULE_INIT macros. +#if defined(__cplusplus) && __cplusplus >= 201402L +[[deprecated]] +#endif +NAPI_EXTERN void NAPI_CDECL +napi_module_register(napi_module* mod); NAPI_EXTERN NAPI_NO_RETURN void NAPI_CDECL napi_fatal_error(const char* location, diff --git a/test/cctest/test_linked_binding.cc b/test/cctest/test_linked_binding.cc index 7e40068b5db799..1a4e6a838b5d72 100644 --- a/test/cctest/test_linked_binding.cc +++ b/test/cctest/test_linked_binding.cc @@ -1,19 +1,19 @@ -#include "node_test_fixture.h" #include "node_api.h" +#include "node_test_fixture.h" void InitializeBinding(v8::Local exports, v8::Local module, v8::Local context, void* priv) { v8::Isolate* isolate = context->GetIsolate(); - exports->Set( - context, - v8::String::NewFromOneByte(isolate, - reinterpret_cast("key")) - .ToLocalChecked(), - v8::String::NewFromOneByte(isolate, - reinterpret_cast("value")) - .ToLocalChecked()) + exports + ->Set(context, + v8::String::NewFromOneByte(isolate, + reinterpret_cast("key")) + .ToLocalChecked(), + v8::String::NewFromOneByte( + isolate, reinterpret_cast("value")) + .ToLocalChecked()) .FromJust(); } @@ -24,18 +24,18 @@ class LinkedBindingTest : public EnvironmentTestFixture {}; TEST_F(LinkedBindingTest, SimpleTest) { const v8::HandleScope handle_scope(isolate_); const Argv argv; - Env test_env {handle_scope, argv}; + Env test_env{handle_scope, argv}; v8::Local context = isolate_->GetCurrentContext(); - const char* run_script = - "process._linkedBinding('cctest_linkedbinding').key"; - v8::Local script = v8::Script::Compile( - context, - v8::String::NewFromOneByte(isolate_, - reinterpret_cast(run_script)) - .ToLocalChecked()) - .ToLocalChecked(); + const char* run_script = "process._linkedBinding('cctest_linkedbinding').key"; + 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); @@ -48,35 +48,35 @@ void InitializeLocalBinding(v8::Local exports, void* priv) { ++*static_cast(priv); v8::Isolate* isolate = context->GetIsolate(); - exports->Set( - context, - v8::String::NewFromOneByte(isolate, - reinterpret_cast("key")) - .ToLocalChecked(), - v8::String::NewFromOneByte(isolate, - reinterpret_cast("value")) - .ToLocalChecked()) + exports + ->Set(context, + v8::String::NewFromOneByte(isolate, + reinterpret_cast("key")) + .ToLocalChecked(), + v8::String::NewFromOneByte( + isolate, reinterpret_cast("value")) + .ToLocalChecked()) .FromJust(); } TEST_F(LinkedBindingTest, LocallyDefinedLinkedBindingTest) { const v8::HandleScope handle_scope(isolate_); const Argv argv; - Env test_env {handle_scope, argv}; + Env test_env{handle_scope, argv}; int calls = 0; AddLinkedBinding(*test_env, "local_linked", InitializeLocalBinding, &calls); v8::Local context = isolate_->GetCurrentContext(); - const char* run_script = - "process._linkedBinding('local_linked').key"; - v8::Local script = v8::Script::Compile( - context, - v8::String::NewFromOneByte(isolate_, - reinterpret_cast(run_script)) - .ToLocalChecked()) - .ToLocalChecked(); + const char* run_script = "process._linkedBinding('local_linked').key"; + 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); @@ -86,41 +86,64 @@ TEST_F(LinkedBindingTest, LocallyDefinedLinkedBindingTest) { 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_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}, + 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}; + 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(); + 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); +} + +TEST_F(LinkedBindingTest, LocallyDefinedLinkedBindingNapiCallbackTest) { + const v8::HandleScope handle_scope(isolate_); + const Argv argv; + Env test_env{handle_scope, argv}; + + AddLinkedBinding(*test_env, "local_linked_napi", InitializeLocalNapiBinding); + + 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); @@ -129,33 +152,32 @@ TEST_F(LinkedBindingTest, LocallyDefinedLinkedBindingNapiTest) { 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); + 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_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}, + NAPI_MODULE_VERSION, + node::ModuleFlags::kLinked, + nullptr, + NapiLinkedWithInstanceData, + "local_linked_napi_id", + nullptr, + {0}, }; TEST_F(LinkedBindingTest, LocallyDefinedLinkedBindingNapiInstanceDataTest) { @@ -164,7 +186,7 @@ TEST_F(LinkedBindingTest, LocallyDefinedLinkedBindingNapiInstanceDataTest) { { const Argv argv; - Env test_env {handle_scope, argv}; + Env test_env{handle_scope, argv}; AddLinkedBinding(*test_env, local_linked_napi_id); @@ -172,17 +194,54 @@ TEST_F(LinkedBindingTest, LocallyDefinedLinkedBindingNapiInstanceDataTest) { 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 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()); + 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; +} + +TEST_F(LinkedBindingTest, + LocallyDefinedLinkedBindingNapiCallbackInstanceDataTest) { + 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", NapiLinkedWithInstanceData); + + 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); } @@ -194,7 +253,7 @@ TEST_F(LinkedBindingTest, LocallyDefinedLinkedBindingNapiInstanceDataTest) { TEST_F(LinkedBindingTest, ManyBindingsTest) { const v8::HandleScope handle_scope(isolate_); const Argv argv; - Env test_env {handle_scope, argv}; + Env test_env{handle_scope, argv}; int calls = 0; AddLinkedBinding(*test_env, "local_linked1", InitializeLocalBinding, &calls); @@ -209,16 +268,16 @@ TEST_F(LinkedBindingTest, ManyBindingsTest) { const char* run_script = "for (let i = 1; i <= 5; i++)process._linkedBinding(`local_linked${i}`);" "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 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); CHECK_EQ(calls, 5); } - diff --git a/test/node-api/test_null_init/test_null_init.c b/test/node-api/test_null_init/test_null_init.c index d9d2200488ce41..28c283b89240f7 100644 --- a/test/node-api/test_null_init/test_null_init.c +++ b/test/node-api/test_null_init/test_null_init.c @@ -1,3 +1,53 @@ #include -NAPI_MODULE(NODE_GYP_MODULE_NAME, NULL) +// This test uses old module initialization style deprecated in current code. +// The goal is to see that all previously compiled code continues to work the +// same way as before. +// The test has a copy of previous macro definitions which are removed from +// the node_api.h file. + +#if defined(_MSC_VER) +#if defined(__cplusplus) +#define NAPI_C_CTOR(fn) \ + static void NAPI_CDECL fn(void); \ + namespace { \ + struct fn##_ { \ + fn##_() { fn(); } \ + } fn##_v_; \ + } \ + static void NAPI_CDECL fn(void) +#else // !defined(__cplusplus) +#pragma section(".CRT$XCU", read) +// The NAPI_C_CTOR macro defines a function fn that is called during CRT +// initialization. +// C does not support dynamic initialization of static variables and this code +// simulates C++ behavior. Exporting the function pointer prevents it from being +// optimized. See for details: +// https://docs.microsoft.com/en-us/cpp/c-runtime-library/crt-initialization?view=msvc-170 +#define NAPI_C_CTOR(fn) \ + static void NAPI_CDECL fn(void); \ + __declspec(dllexport, allocate(".CRT$XCU")) void(NAPI_CDECL * fn##_)(void) = \ + fn; \ + static void NAPI_CDECL fn(void) +#endif // defined(__cplusplus) +#else +#define NAPI_C_CTOR(fn) \ + static void fn(void) __attribute__((constructor)); \ + static void fn(void) +#endif + +#define NAPI_MODULE_TEST(modname, regfunc) \ + EXTERN_C_START \ + static napi_module _module = { \ + NAPI_MODULE_VERSION, \ + 0, \ + __FILE__, \ + regfunc, \ + #modname, \ + NULL, \ + {0}, \ + }; \ + NAPI_C_CTOR(_register_##modname) { napi_module_register(&_module); } \ + EXTERN_C_END + +NAPI_MODULE_TEST(NODE_GYP_MODULE_NAME, NULL)