src: enable wrapping Napi namespace with custom namespace (#1135)

Enable support for a `NAPI_CPP_CUSTOM_NAMESPACE` that would wrap the `Napi` namespace in a custom namespace. This can be helpful when there are multiple parts of a compiled shared library or executable that depend on `node-addon-api`, but different versions of it. In order to avoid symbol conflicts in these cases, namespacing can be used to make sure that the linker would not merge definitions that the compiler potentially generates for the node-addon-api methods.
nodejs · Feb 22, 2022 · 5b51864 · 5b51864
1 parent c54aeef
commit 5b51864
Show file tree

Hide file tree

Showing 8 changed files with 68 additions and 19 deletions.
diff --git a/napi-inl.h b/napi-inl.h
@@ -18,6 +18,10 @@
 
 namespace Napi {
 
+#ifdef NAPI_CPP_CUSTOM_NAMESPACE
+namespace NAPI_CPP_CUSTOM_NAMESPACE {
+#endif
+
 // Helpers to handle functions exposed from C++.
 namespace details {
 
@@ -6245,6 +6249,10 @@ bool Env::CleanupHook<Hook, Arg>::IsEmpty() const {
 }
 #endif  // NAPI_VERSION > 2
 
+#ifdef NAPI_CPP_CUSTOM_NAMESPACE
+}  // namespace NAPI_CPP_CUSTOM_NAMESPACE
+#endif
+
 } // namespace Napi
 
 #endif // SRC_NAPI_INL_H_
diff --git a/napi.h b/napi.h
@@ -140,6 +140,18 @@ static_assert(sizeof(char16_t) == sizeof(wchar_t), "Size mismatch between char16
 ////////////////////////////////////////////////////////////////////////////////
 namespace Napi {
 
+#ifdef NAPI_CPP_CUSTOM_NAMESPACE
+// NAPI_CPP_CUSTOM_NAMESPACE can be #define'd per-addon to avoid symbol
+// conflicts between different instances of node-addon-api
+
+// First dummy definition of the namespace to make sure that Napi::(name) still
+// refers to the right things inside this file.
+namespace NAPI_CPP_CUSTOM_NAMESPACE {}
+using namespace NAPI_CPP_CUSTOM_NAMESPACE;
+
+namespace NAPI_CPP_CUSTOM_NAMESPACE {
+#endif
+
   // Forward declarations
   class Env;
   class Value;
@@ -2979,6 +2991,10 @@ namespace Napi {
   };
 #endif  // NAPI_VERSION > 5
 
+#ifdef NAPI_CPP_CUSTOM_NAMESPACE
+  }  // namespace NAPI_CPP_CUSTOM_NAMESPACE
+#endif
+
 } // namespace Napi
 
 // Inline implementations of all the above class methods are included here.

diff --git a/package.json b/package.json
@@ -323,7 +323,6 @@
       "name": "WenheLI",
       "url": "https://github.com/WenheLI"
     },
-
     {
       "name": "Yohei Kishimoto",
       "url": "https://github.com/morokosi"

diff --git a/test/binding.gyp b/test/binding.gyp
@@ -114,5 +114,11 @@
       'sources': ['>@(build_sources_swallowexcept)'],
       'defines': ['NODE_API_SWALLOW_UNTHROWABLE_EXCEPTIONS']
     },
+    {
+      'target_name': 'binding_custom_namespace',
+      'includes': ['../noexcept.gypi'],
+      'sources': ['>@(build_sources)'],
+      'defines': ['NAPI_CPP_CUSTOM_NAMESPACE=cstm']
+    },
   ],
 }
diff --git a/test/common/index.js b/test/common/index.js
@@ -81,7 +81,8 @@ exports.runTest = async function (test, buildType, buildPathRoot = process.env.B
   const bindings = [
     path.join(buildPathRoot, `../build/${buildType}/binding.node`),
     path.join(buildPathRoot, `../build/${buildType}/binding_noexcept.node`),
-    path.join(buildPathRoot, `../build/${buildType}/binding_noexcept_maybe.node`)
+    path.join(buildPathRoot, `../build/${buildType}/binding_noexcept_maybe.node`),
+    path.join(buildPathRoot, `../build/${buildType}/binding_custom_namespace.node`)
   ].map(it => require.resolve(it));
 
   for (const item of bindings) {
@@ -96,7 +97,8 @@ exports.runTestWithBindingPath = async function (test, buildType, buildPathRoot
   const bindings = [
     path.join(buildPathRoot, `../build/${buildType}/binding.node`),
     path.join(buildPathRoot, `../build/${buildType}/binding_noexcept.node`),
-    path.join(buildPathRoot, `../build/${buildType}/binding_noexcept_maybe.node`)
+    path.join(buildPathRoot, `../build/${buildType}/binding_noexcept_maybe.node`),
+    path.join(buildPathRoot, `../build/${buildType}/binding_custom_namespace.node`)
   ].map(it => require.resolve(it));
 
   for (const item of bindings) {

diff --git a/test/common/test_helper.h b/test/common/test_helper.h
@@ -3,6 +3,12 @@
 
 namespace Napi {
 
+// Needs this here since the MaybeUnwrap() functions need to be in the
+// same namespace as their arguments for C++ argument-dependent lookup
+#ifdef NAPI_CPP_CUSTOM_NAMESPACE
+namespace NAPI_CPP_CUSTOM_NAMESPACE {
+#endif
+
 // Use this when a variable or parameter is unused in order to explicitly
 // silence a compiler warning about that.
 template <typename T>
@@ -58,4 +64,8 @@ inline bool MaybeUnwrapTo(MaybeOrValue<T> maybe, T* out) {
 #endif
 }
 
+#ifdef NAPI_CPP_CUSTOM_NAMESPACE
+}  // namespace NAPI_CPP_CUSTOM_NAMESPACE
+#endif
+
 }  // namespace Napi
diff --git a/test/error_terminating_environment.js b/test/error_terminating_environment.js
@@ -70,6 +70,7 @@ test(`./build/${buildType}/binding.node`, true);
 test(`./build/${buildType}/binding_noexcept.node`, true);
 test(`./build/${buildType}/binding_swallowexcept.node`, false);
 test(`./build/${buildType}/binding_swallowexcept_noexcept.node`, false);
+test(`./build/${buildType}/binding_custom_namespace.node`, true);
 
 function test(bindingPath, process_should_abort) {
   const number_of_test_cases = 5;

diff --git a/unit-test/binding.gyp b/unit-test/binding.gyp
@@ -11,23 +11,23 @@
   },
   'targets': [
     {
-			"target_name": "generateBindingCC",
-			"type": "none",
-			"actions": [ {
-				"action_name": "generateBindingCC",
-				"message": "Generating binding cc file",
-				"outputs": ["generated/binding.cc"],
-				"conditions": [
-					[ "'true'=='true'", {
-						"inputs": [""],
-						"action": [
-							"node",
-							"generate-binding-cc.js",
+      "target_name": "generateBindingCC",
+      "type": "none",
+      "actions": [ {
+        "action_name": "generateBindingCC",
+        "message": "Generating binding cc file",
+        "outputs": ["generated/binding.cc"],
+        "conditions": [
+          [ "'true'=='true'", {
+            "inputs": [""],
+            "action": [
+              "node",
+              "generate-binding-cc.js",
               "<!@(node -p \"require('./injectTestParams').filesForBinding()\" )"
-						]
-					} ]
-				]
-			} ]
+            ]
+          } ]
+        ]
+      } ]
     },
     {
       'target_name': 'binding',
@@ -61,5 +61,12 @@
       'defines': ['NODE_API_SWALLOW_UNTHROWABLE_EXCEPTIONS'],
       'dependencies': [ 'generateBindingCC' ]
     },
+    {
+      'target_name': 'binding_custom_namespace',
+      'includes': ['../noexcept.gypi'],
+      'sources': ['>@(build_sources)'],
+      'defines': ['NAPI_CPP_CUSTOM_NAMESPACE=cstm'],
+      'dependencies': [ 'generateBindingCC' ]
+    },
   ],
 }