From 93eecc8fe2ecefe054c69de12da68920bf3f0fda Mon Sep 17 00:00:00 2001 From: Vladimir Morozov Date: Thu, 24 Mar 2022 08:37:35 -0700 Subject: [PATCH 1/4] node-api,src: fix module registration in MSVC C++ --- src/node.h | 27 +++++++++++ src/node_api.h | 22 +++++++++ test/js-native-api/common.h | 3 ++ test/node-api/test_init_order/binding.gyp | 8 ++++ test/node-api/test_init_order/test.js | 10 ++++ .../test_init_order/test_init_order.cc | 47 +++++++++++++++++++ 6 files changed, 117 insertions(+) create mode 100644 test/node-api/test_init_order/binding.gyp create mode 100644 test/node-api/test_init_order/test.js create mode 100644 test/node-api/test_init_order/test_init_order.cc diff --git a/src/node.h b/src/node.h index b96b3ea1e49d4a..f903bd0b3af12c 100644 --- a/src/node.h +++ b/src/node.h @@ -822,17 +822,44 @@ extern "C" NODE_EXTERN void node_module_register(void* mod); #ifdef NODE_SHARED_MODE # define NODE_CTOR_PREFIX +# define NODE_CTOR_ANONYMOUS_NAMESPACE_START +# define NODE_CTOR_ANONYMOUS_NAMESPACE_END #else # define NODE_CTOR_PREFIX static +# define NODE_CTOR_NAMESPACE namespace +# define NODE_CTOR_ANONYMOUS_NAMESPACE_START NODE_CTOR_NAMESPACE { +# define NODE_CTOR_ANONYMOUS_NAMESPACE_END } #endif #if defined(_MSC_VER) +#if defined(__cplusplus) +// The NODE_C_CTOR macro defines a function fn that is called during dynamic +// initialization of static variables. +// The order of the dynamic initialization is not defined and code in fn +// function must avoid using other static variables with dynamic initialization. +#define NODE_C_CTOR(fn) \ + NODE_CTOR_PREFIX void __cdecl fn(void); \ + NODE_CTOR_ANONYMOUS_NAMESPACE_START \ + struct fn##_ { \ + static int Call##fn() { return (fn(), 0); } \ + static inline const int x = Call##fn(); \ + }; \ + NODE_CTOR_ANONYMOUS_NAMESPACE_END \ + NODE_CTOR_PREFIX void __cdecl fn(void) +#else #pragma section(".CRT$XCU", read) +// The NODE_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 NODE_C_CTOR(fn) \ NODE_CTOR_PREFIX void __cdecl fn(void); \ __declspec(dllexport, allocate(".CRT$XCU")) \ void (__cdecl*fn ## _)(void) = fn; \ NODE_CTOR_PREFIX void __cdecl fn(void) +#endif #else #define NODE_C_CTOR(fn) \ NODE_CTOR_PREFIX void fn(void) __attribute__((constructor)); \ diff --git a/src/node_api.h b/src/node_api.h index e8e903b62a893b..a6be639c37b1bb 100644 --- a/src/node_api.h +++ b/src/node_api.h @@ -44,12 +44,34 @@ typedef struct napi_module { #define NAPI_MODULE_VERSION 1 #if defined(_MSC_VER) +#if defined(__cplusplus) +// The NAPI_C_CTOR macro defines a function fn that is called during dynamic +// initialization of static variables. +// The order of the dynamic initialization is not defined and code in fn +// function must avoid using other static variables with dynamic initialization. +#define NAPI_C_CTOR(fn) \ + static void __cdecl fn(void); \ + namespace { \ + struct fn##_ { \ + static int Call##fn() { return (fn(), 0); } \ + static inline const int x = Call##fn(); \ + }; \ + } \ + static void __cdecl fn(void) +#else #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 __cdecl fn(void); \ __declspec(dllexport, allocate(".CRT$XCU")) void(__cdecl * fn##_)(void) = \ fn; \ static void __cdecl fn(void) +#endif // defined(__cplusplus) #else #define NAPI_C_CTOR(fn) \ static void fn(void) __attribute__((constructor)); \ diff --git a/test/js-native-api/common.h b/test/js-native-api/common.h index 73f60906630140..46784059a1f70a 100644 --- a/test/js-native-api/common.h +++ b/test/js-native-api/common.h @@ -62,6 +62,9 @@ #define DECLARE_NODE_API_GETTER(name, func) \ { (name), NULL, NULL, (func), NULL, NULL, napi_default, NULL } +#define DECLARE_NODE_API_PROPERTY_VALUE(name, value) \ + { (name), NULL, NULL, NULL, NULL, (value), napi_default, NULL } + void add_returned_status(napi_env env, const char* key, napi_value object, diff --git a/test/node-api/test_init_order/binding.gyp b/test/node-api/test_init_order/binding.gyp new file mode 100644 index 00000000000000..2ffb81838d1189 --- /dev/null +++ b/test/node-api/test_init_order/binding.gyp @@ -0,0 +1,8 @@ +{ + "targets": [ + { + "target_name": "test_init_order", + "sources": [ "test_init_order.cc" ] + } + ] +} diff --git a/test/node-api/test_init_order/test.js b/test/node-api/test_init_order/test.js new file mode 100644 index 00000000000000..0bfeb8f94afebb --- /dev/null +++ b/test/node-api/test_init_order/test.js @@ -0,0 +1,10 @@ +'use strict'; + +// This test verifies that C++ static variable dynamic initialization is called +// correctly and does not interfere with the module initialization. +const common = require('../../common'); +const test_init_order = require(`./build/${common.buildType}/test_init_order`); +const assert = require('assert'); + +assert.strictEqual(test_init_order.cppIntValue, 42); +assert.strictEqual(test_init_order.cppStringValue, '123'); diff --git a/test/node-api/test_init_order/test_init_order.cc b/test/node-api/test_init_order/test_init_order.cc new file mode 100644 index 00000000000000..222a1c28bb0053 --- /dev/null +++ b/test/node-api/test_init_order/test_init_order.cc @@ -0,0 +1,47 @@ +#include +#include +#include +#include "../../js-native-api/common.h" + +namespace { + +inline std::string testString = "123"; + +struct ValueHolder { + int value{42}; +}; + +class MyClass { + public: + // Ensure that the static variable is initialized by a dynamic static + // initializer. + static std::unique_ptr valueHolder; +}; + +std::unique_ptr MyClass::valueHolder{new ValueHolder()}; + +} // namespace + +EXTERN_C_START +napi_value Init(napi_env env, napi_value exports) { + napi_value cppIntValue, cppStringValue; + NODE_API_CALL( + env, napi_create_int32(env, MyClass::valueHolder->value, &cppIntValue)); + NODE_API_CALL( + env, + napi_create_string_utf8( + env, testString.c_str(), NAPI_AUTO_LENGTH, &cppStringValue)); + + napi_property_descriptor descriptors[] = { + DECLARE_NODE_API_PROPERTY_VALUE("cppIntValue", cppIntValue), + DECLARE_NODE_API_PROPERTY_VALUE("cppStringValue", cppStringValue)}; + + NODE_API_CALL(env, + napi_define_properties( + env, exports, std::size(descriptors), descriptors)); + + return exports; +} +EXTERN_C_END + +NAPI_MODULE(NODE_GYP_MODULE_NAME, Init) From 2b38a2a085b91a177a43c64c57605fdd1c637295 Mon Sep 17 00:00:00 2001 From: Vladimir Morozov Date: Wed, 30 Mar 2022 11:45:49 -0700 Subject: [PATCH 2/4] add a guard for the new C++17 features --- src/node.h | 2 +- src/node_api.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/node.h b/src/node.h index f903bd0b3af12c..21e750449b282b 100644 --- a/src/node.h +++ b/src/node.h @@ -832,7 +832,7 @@ extern "C" NODE_EXTERN void node_module_register(void* mod); #endif #if defined(_MSC_VER) -#if defined(__cplusplus) +#if defined(__cplusplus) && defined(__cpp_inline_variables) // The NODE_C_CTOR macro defines a function fn that is called during dynamic // initialization of static variables. // The order of the dynamic initialization is not defined and code in fn diff --git a/src/node_api.h b/src/node_api.h index a6be639c37b1bb..f084cc0b29553c 100644 --- a/src/node_api.h +++ b/src/node_api.h @@ -44,7 +44,7 @@ typedef struct napi_module { #define NAPI_MODULE_VERSION 1 #if defined(_MSC_VER) -#if defined(__cplusplus) +#if defined(__cplusplus) && defined(__cpp_inline_variables) // The NAPI_C_CTOR macro defines a function fn that is called during dynamic // initialization of static variables. // The order of the dynamic initialization is not defined and code in fn From 69a6843913706d93e80911bc3eb9b97875e77b0c Mon Sep 17 00:00:00 2001 From: Vladimir Morozov Date: Sat, 2 Apr 2022 15:09:07 -0700 Subject: [PATCH 3/4] change code to use simple C++ initialization --- src/node.h | 33 ++++----------------------------- src/node_api.h | 13 ++++--------- 2 files changed, 8 insertions(+), 38 deletions(-) diff --git a/src/node.h b/src/node.h index 21e750449b282b..dec8dd6805fac9 100644 --- a/src/node.h +++ b/src/node.h @@ -822,45 +822,20 @@ extern "C" NODE_EXTERN void node_module_register(void* mod); #ifdef NODE_SHARED_MODE # define NODE_CTOR_PREFIX -# define NODE_CTOR_ANONYMOUS_NAMESPACE_START -# define NODE_CTOR_ANONYMOUS_NAMESPACE_END #else # define NODE_CTOR_PREFIX static -# define NODE_CTOR_NAMESPACE namespace -# define NODE_CTOR_ANONYMOUS_NAMESPACE_START NODE_CTOR_NAMESPACE { -# define NODE_CTOR_ANONYMOUS_NAMESPACE_END } #endif #if defined(_MSC_VER) -#if defined(__cplusplus) && defined(__cpp_inline_variables) -// The NODE_C_CTOR macro defines a function fn that is called during dynamic -// initialization of static variables. -// The order of the dynamic initialization is not defined and code in fn -// function must avoid using other static variables with dynamic initialization. #define NODE_C_CTOR(fn) \ NODE_CTOR_PREFIX void __cdecl fn(void); \ - NODE_CTOR_ANONYMOUS_NAMESPACE_START \ + namespace { \ struct fn##_ { \ - static int Call##fn() { return (fn(), 0); } \ - static inline const int x = Call##fn(); \ - }; \ - NODE_CTOR_ANONYMOUS_NAMESPACE_END \ + fn##_() { fn(); }; \ + } fn##_v_; \ + } \ NODE_CTOR_PREFIX void __cdecl fn(void) #else -#pragma section(".CRT$XCU", read) -// The NODE_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 NODE_C_CTOR(fn) \ - NODE_CTOR_PREFIX void __cdecl fn(void); \ - __declspec(dllexport, allocate(".CRT$XCU")) \ - void (__cdecl*fn ## _)(void) = fn; \ - NODE_CTOR_PREFIX void __cdecl fn(void) -#endif -#else #define NODE_C_CTOR(fn) \ NODE_CTOR_PREFIX void fn(void) __attribute__((constructor)); \ NODE_CTOR_PREFIX void fn(void) diff --git a/src/node_api.h b/src/node_api.h index f084cc0b29553c..d95046676a699d 100644 --- a/src/node_api.h +++ b/src/node_api.h @@ -44,21 +44,16 @@ typedef struct napi_module { #define NAPI_MODULE_VERSION 1 #if defined(_MSC_VER) -#if defined(__cplusplus) && defined(__cpp_inline_variables) -// The NAPI_C_CTOR macro defines a function fn that is called during dynamic -// initialization of static variables. -// The order of the dynamic initialization is not defined and code in fn -// function must avoid using other static variables with dynamic initialization. +#if defined(__cplusplus) #define NAPI_C_CTOR(fn) \ static void __cdecl fn(void); \ namespace { \ struct fn##_ { \ - static int Call##fn() { return (fn(), 0); } \ - static inline const int x = Call##fn(); \ - }; \ + fn##_() { fn(); } \ + } fn##_v_; \ } \ static void __cdecl fn(void) -#else +#else // !defined(__cplusplus) #pragma section(".CRT$XCU", read) // The NAPI_C_CTOR macro defines a function fn that is called during CRT // initialization. From ec61c248e2665e28a3fa0a45a13a3851b6ecc418 Mon Sep 17 00:00:00 2001 From: Vladimir Morozov Date: Mon, 4 Apr 2022 08:51:04 -0700 Subject: [PATCH 4/4] Remove C++17 specific code from tests --- .../test_init_order/test_init_order.cc | 35 +++++++++++-------- 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/test/node-api/test_init_order/test_init_order.cc b/test/node-api/test_init_order/test_init_order.cc index 222a1c28bb0053..fc4174c09d8dba 100644 --- a/test/node-api/test_init_order/test_init_order.cc +++ b/test/node-api/test_init_order/test_init_order.cc @@ -3,34 +3,39 @@ #include #include "../../js-native-api/common.h" -namespace { - -inline std::string testString = "123"; +// This test verifies that use of the NAPI_MODULE in C++ code does not +// interfere with the C++ dynamic static initializers. -struct ValueHolder { - int value{42}; -}; +namespace { -class MyClass { - public: - // Ensure that the static variable is initialized by a dynamic static - // initializer. - static std::unique_ptr valueHolder; +// This class uses dynamic static initializers for the test. +// In production code developers must avoid dynamic static initializers because +// they affect the start up time. They must prefer static initialization such as +// use of constexpr functions or classes with constexpr constructors. E.g. +// instead of using std::string, it is preferrable to use const char[], or +// constexpr std::string_view starting with C++17, or even constexpr +// std::string starting with C++20. +struct MyClass { + static const std::unique_ptr valueHolder; + static const std::string testString; }; -std::unique_ptr MyClass::valueHolder{new ValueHolder()}; +const std::unique_ptr MyClass::valueHolder = + std::unique_ptr(new int(42)); +// NOLINTNEXTLINE(runtime/string) +const std::string MyClass::testString = std::string("123"); } // namespace EXTERN_C_START napi_value Init(napi_env env, napi_value exports) { napi_value cppIntValue, cppStringValue; - NODE_API_CALL( - env, napi_create_int32(env, MyClass::valueHolder->value, &cppIntValue)); + NODE_API_CALL(env, + napi_create_int32(env, *MyClass::valueHolder, &cppIntValue)); NODE_API_CALL( env, napi_create_string_utf8( - env, testString.c_str(), NAPI_AUTO_LENGTH, &cppStringValue)); + env, MyClass::testString.c_str(), NAPI_AUTO_LENGTH, &cppStringValue)); napi_property_descriptor descriptors[] = { DECLARE_NODE_API_PROPERTY_VALUE("cppIntValue", cppIntValue),