nodejs / node

Node.js JavaScript runtime ✨🐢🚀✨
https://nodejs.org
Other
107.88k stars 29.73k forks source link

Support loading dynamic addon modules (`.node` files) when embedding the Node.js shared library without needing to link with `node.def` #52282

Open segevfiner opened 7 months ago

segevfiner commented 7 months ago

What is the problem this feature will solve?

Node.js addon modules on Windows current import from the node.exe executable, when you build Node.js as a shared library, it reexports all the needed symbols from the node shared library from the executable using a generated node.def file.

If you embed the Node.js shared library into a different executable/program, for loading such addon modules to work, that executable will need to be linked with node.def to reexport the needed symbols.

node-gyp also adds win_delay_load_hook.cc to addons on Windows to make them work when the Node.js executable is renamed.

What is the feature you are proposing to solve the problem?

Maybe we can support loading such addon modules without this requirement?

For example, what if the delay load hook looked for an env var (e.g. NODE_LIBRARY) that it would then pass to GetModuleHandle instead of NULL if it is set, and when Node.js is built as a shared library, it would set this env var before dynamically loading a module?

Though I'm not sure what security implications such a mechanism may have...

What alternatives have you considered?

No response

segevfiner commented 7 months ago

So basically something like the following (Untested though): In node:

diff --git a/src/node_binding.cc b/src/node_binding.cc
index 00493f03f1..6f56f79dc5 100644
--- a/src/node_binding.cc
+++ b/src/node_binding.cc
@@ -457,6 +457,27 @@ void DLOpen(const FunctionCallbackInfo<Value>& args) {
     return;  // Exception pending.
   }

+#if defined(_WIN32) && defined(NODE_SHARED_MODE)
+  HMODULE mod;
+  if (!GetModuleHandleExA(
+      GET_MODULE_HANDLE_EX_FLAG_FROM_ADDRESS | GET_MODULE_HANDLE_EX_FLAG_UNCHANGED_REFCOUNT,
+      &DLOpen,
+      &mod
+    )) {
+      THROW_ERR_DLOPEN_FAILED(env, "TODO.");
+      return false;
+  }
+
+  // TODO Might want a dynamic allocation, with the possibility to opt into long path support in the future
+  char nodeLibrary[MAX_PATH];
+  if (!GetModuleFileNameA(mod, &nodeLibrary, sizeof(nodeLibrary))) {
+    THROW_ERR_DLOPEN_FAILED(env, "TODO.");
+    return false;
+  }
+
+  std::setenv("NODE_LIBRARY", nodeLibrary);
+#endif  // _WIN32
+
   node::Utf8Value filename(env->isolate(), args[1]);  // Cast
   env->TryLoadAddon(*filename, flags, [&](DLib* dlib) {
     static Mutex dlib_load_mutex;

In node-gyp:

diff --git a/src/win_delay_load_hook.cc b/src/win_delay_load_hook.cc
index 169f802..0c136ed 100644
--- a/src/win_delay_load_hook.cc
+++ b/src/win_delay_load_hook.cc
@@ -28,7 +28,7 @@ static FARPROC WINAPI load_exe_hook(unsigned int event, DelayLoadInfo* info) {
   if (_stricmp(info->szDll, HOST_BINARY) != 0)
     return NULL;

-  m = GetModuleHandle(NULL);
+  m = GetModuleHandle(std::getenv("NODE_LIBRARY"));
   return (FARPROC) m;
 }
github-actions[bot] commented 1 month ago

There has been no activity on this feature request for 5 months. To help maintain relevant open issues, please add the https://github.com/nodejs/node/labels/never-stale label or close this issue if it should be closed. If not, the issue will be automatically closed 6 months after the last non-automated comment. For more information on how the project manages feature requests, please consult the feature request management document.

segevfiner commented 1 month ago

Bump