Run callback passed to napi_module_register after dlopen returns instead of during call (#20478)

src/bun.js/bindings/BunProcess.cpp

@@ -535,6 +535,15 @@ JSC_DEFINE_HOST_FUNCTION(Process_functionDlopen, (JSC::JSGlobalObject * globalOb
 #endif
 
     if (callCountAtStart != globalObject->napiModuleRegisterCallCount) {
+        // Module self-registered via static constructor
+        if (globalObject->m_pendingNapiModule) {
+            // Execute the stored registration function now that dlopen has completed
+            Napi::executePendingNapiModule(globalObject);
+
+            // Clear the pending module
+            globalObject->m_pendingNapiModule = {};
+        }
+
         JSValue resultValue = globalObject->m_pendingNapiModuleAndExports[0].get();
         globalObject->napiModuleRegisterCallCount = 0;
         globalObject->m_pendingNapiModuleAndExports[0].clear();

src/bun.js/bindings/ZigGlobalObject.h

@@ -643,6 +643,9 @@ public:
     // We will add it to the resulting napi value.
     void* m_pendingNapiModuleDlopenHandle = nullptr;
 
+    // Store the napi module struct to defer calling nm_register_func until after dlopen completes
+    std::optional<napi_module> m_pendingNapiModule = {};
+
     JSObject* nodeErrorCache() const { return m_nodeErrorCache.getInitializedOnMainThread(this); }
 
     Structure* memoryFootprintStructure()

src/bun.js/bindings/napi.cpp

@@ -680,19 +680,20 @@ extern "C" napi_status napi_get_named_property(napi_env env, napi_value object,
 }
 
 extern "C" size_t Bun__napi_module_register_count;
-extern "C" void napi_module_register(napi_module* mod)
+void Napi::executePendingNapiModule(Zig::GlobalObject* globalObject)
 {
-    Zig::GlobalObject* globalObject = defaultGlobalObject();
-    napi_env env = globalObject->makeNapiEnv(*mod);
     JSC::VM& vm = JSC::getVM(globalObject);
-    auto keyStr = WTF::String::fromUTF8(mod->nm_modname);
-    globalObject->napiModuleRegisterCallCount++;
-    Bun__napi_module_register_count++;
+    auto scope = DECLARE_THROW_SCOPE(vm);
+
+    ASSERT(globalObject->m_pendingNapiModule);
+
+    auto& mod = *globalObject->m_pendingNapiModule;
+    napi_env env = globalObject->makeNapiEnv(mod);
+    auto keyStr = WTF::String::fromUTF8(mod.nm_modname);
     JSValue pendingNapiModule = globalObject->m_pendingNapiModuleAndExports[0].get();
     JSObject* object = (pendingNapiModule && pendingNapiModule.isObject()) ? pendingNapiModule.getObject()
                                                                            : nullptr;
 
-    auto scope = DECLARE_THROW_SCOPE(vm);
     JSC::Strong<JSC::JSObject> strongExportsObject;
 
     if (!object) {
@@ -715,8 +716,8 @@ extern "C" void napi_module_register(napi_module* mod)
     Bun::NapiHandleScope handleScope(globalObject);
     JSValue resultValue;
 
-    if (mod->nm_register_func) {
-        resultValue = toJS(mod->nm_register_func(env, toNapi(object, globalObject)));
+    if (mod.nm_register_func) {
+        resultValue = toJS(mod.nm_register_func(env, toNapi(object, globalObject)));
     } else {
         JSValue errorInstance = createError(globalObject, makeString("Module has no declared entry point."_s));
         globalObject->m_pendingNapiModuleAndExports[0].set(vm, globalObject, errorInstance);
@@ -757,6 +758,25 @@ extern "C" void napi_module_register(napi_module* mod)
     globalObject->m_pendingNapiModuleAndExports[1].set(vm, globalObject, object);
 }
 
+extern "C" void napi_module_register(napi_module* mod)
+{
+    Zig::GlobalObject* globalObject = defaultGlobalObject();
+    JSC::VM& vm = JSC::getVM(globalObject);
+    // Increment this one even if the module is invalid so that functionDlopen
+    // knows that napi_module_register was attempted
+    globalObject->napiModuleRegisterCallCount++;
+
+    // Store the entire module struct to be processed after dlopen completes
+    if (mod && mod->nm_register_func) {
+        globalObject->m_pendingNapiModule = *mod;
+        // Increment the counter to signal that a module registered itself
+        Bun__napi_module_register_count++;
+    } else {
+        JSValue errorInstance = createError(globalObject, makeString("Module has no declared entry point."_s));
+        globalObject->m_pendingNapiModuleAndExports[0].set(vm, globalObject, errorInstance);
+    }
+}
+
 static void wrap_cleanup(napi_env env, void* data, void* hint)
 {
     auto* ref = reinterpret_cast<NapiRef*>(data);

src/bun.js/bindings/napi.h

@@ -316,6 +316,7 @@ class JSSourceCode;
 }
 
 namespace Napi {
+
 JSC::SourceCode generateSourceCode(WTF::String keyString, JSC::VM& vm, JSC::JSObject* object, JSC::JSGlobalObject* globalObject);
 
 class NapiRefWeakHandleOwner final : public JSC::WeakHandleOwner {
@@ -341,6 +342,11 @@ public:
         return jscWeakValueHandleOwner;
     }
 };
+
+// If a module registered itself by calling napi_module_register in a static constructor, run this
+// to run the module's entrypoint.
+void executePendingNapiModule(Zig::GlobalObject* globalObject);
+
 }
 
 namespace Zig {

test/napi/napi-app/binding.gyp

@@ -111,6 +111,15 @@
                 "NODE_API_EXPERIMENTAL_NOGC_ENV_OPT_OUT=1",
             ],
         },
-
+        {
+            "target_name": "constructor_order_addon",
+            "sources": ["constructor_order_addon.cpp"],
+            "include_dirs": ["<!@(node -p \"require('node-addon-api').include\")"],
+            "libraries": [],
+            "dependencies": ["<!(node -p \"require('node-addon-api').gyp\")"],
+            "defines": [
+                "NAPI_DISABLE_CPP_EXCEPTIONS",
+            ],
+        },
     ]
 }

test/napi/napi-app/constructor_order_addon.cpp

@@ -0,0 +1,44 @@
+#include <js_native_api.h>
+#include <node_api.h>
+#include <stdio.h>
+
+napi_value register_cb(napi_env env, napi_value exports);
+
+static napi_module mod = {
+    1,                           // nm_version
+    0,                           // nm_flags
+    "constructor_order_addon.c", // nm_filename
+    register_cb,                 // nm_register_func
+    "constructor_order_addon",   // nm_modname
+    NULL,                        // nm_priv
+    {NULL}                       // reserved
+};
+
+class call_register {
+public:
+  call_register() {
+    // should be called first during dlopen
+    printf("call_register\n");
+    napi_module_register(&mod);
+  }
+};
+
+class init_static {
+public:
+  init_static() {
+    // should be called second during dlopen
+    printf("init_static\n");
+  }
+};
+
+// declare these so their constructors run
+static call_register constructor1;
+static init_static constructor2;
+
+napi_value register_cb(napi_env env, napi_value exports) {
+  // should be called third, after dlopen returns and bun runs the callback
+  // passed to napi_module_register
+  (void)env;
+  printf("register_cb\n");
+  return exports;
+}

test/napi/napi-app/module.js

@@ -694,4 +694,8 @@ nativeTests.test_get_value_string = () => {
   }
 };
 
+nativeTests.test_constructor_order = () => {
+  require("./build/Debug/constructor_order_addon.node");
+};
+
 module.exports = nativeTests;

test/napi/napi.test.ts

@@ -512,6 +512,10 @@ describe("napi", () => {
   it("works when the module register function throws", async () => {
     expect(() => require("./napi-app/build/Debug/throw_addon.node")).toThrow(new Error("oops!"));
   });
+
+  it("runs the napi_module_register callback after dlopen finishes", () => {
+    checkSameOutput("test_constructor_order", []);
+  });
 });
 
 async function checkSameOutput(test: string, args: any[] | string, envArgs: Record<string, string> = {}) {

test/napi/node-napi-tests/test/common/index.js

@@ -311,32 +311,17 @@ let knownGlobals = [
   setInterval,
   setTimeout,
   queueMicrotask,
-  addEventListener,
-  alert,
-  confirm,
-  dispatchEvent,
-  postMessage,
-  prompt,
-  removeEventListener,
-  reportError,
-  Bun,
   File,
   process,
   Blob,
   Buffer,
-  BuildError,
-  BuildMessage,
-  HTMLRewriter,
   Request,
-  ResolveError,
-  ResolveMessage,
   Response,
   TextDecoder,
   AbortSignal,
   BroadcastChannel,
   CloseEvent,
   DOMException,
-  ErrorEvent,
   Event,
   EventTarget,
   FormData,
@@ -351,11 +336,31 @@ let knownGlobals = [
   URL,
   URLSearchParams,
   WebSocket,
-  Worker,
-  onmessage,
-  onerror,
 ];
 
+if (typeof Bun === "object") {
+  knownGlobals.push(
+    addEventListener,
+    alert,
+    confirm,
+    dispatchEvent,
+    postMessage,
+    prompt,
+    removeEventListener,
+    Bun,
+    reportError,
+    BuildError,
+    BuildMessage,
+    HTMLRewriter,
+    ResolveError,
+    ResolveMessage,
+    ErrorEvent,
+    Worker,
+    onmessage,
+    onerror,
+  );
+}
+
 const globalKeys = [
   "gc",
   "navigator",