src: fix addon loader regression

Fix a regression that was introduced in commit a38b9178 by removing the
bad check.  Also rearrange the addon loading logic to ensure that the
list of pending addons remains in a consistent state when the shared
object fails to load; in particular, when an addon self-registers first,
then hits a dynamic linker error in a later constructor.

Fixes the following asserting when loading a .node shared object:

    node: ../src/node.cc:1944: void node::node_module_register(void*):
    Assertion `(modpending) != (nullptr)' failed.

Fixes strongloop/strongops#233.

PR-URL: https://github.com/iojs/io.js/pull/154
Reviewed-By: Ryan Graham <ryan@strongloop.com>

diff --git a/src/node.cc b/src/node.cc
index cbc1f06..a48e228 100644 (file)
--- a/src/node.cc
+++ b/src/node.cc
@@ -1939,9 +1939,6 @@ extern "C" void node_module_register(void* m) {
     mp->nm_link = modlist_linked;
     modlist_linked = mp;
   } else {
-    // Once node::Init was called we can only register dynamic modules.
-    // See DLOpen.
-    CHECK_NE(modpending, nullptr);
     modpending = mp;
   }
 }
@@ -1980,9 +1977,10 @@ typedef void (UV_DYNAMIC* extInit)(Handle<Object> exports);
 // cache that's a plain C list or hash table that's shared across contexts?
 void DLOpen(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
-  struct node_module* mp;
   uv_lib_t lib;
 
+  CHECK_EQ(modpending, nullptr);
+
   if (args.Length() < 2) {
     env->ThrowError("process.dlopen takes exactly 2 arguments.");
     return;
@@ -1990,11 +1988,15 @@ void DLOpen(const FunctionCallbackInfo<Value>& args) {
 
   Local<Object> module = args[0]->ToObject();  // Cast
   node::Utf8Value filename(args[1]);  // Cast
+  const bool is_dlopen_error = uv_dlopen(*filename, &lib);
 
-  Local<String> exports_string = env->exports_string();
-  Local<Object> exports = module->Get(exports_string)->ToObject();
+  // Objects containing v14 or later modules will have registered themselves
+  // on the pending list.  Activate all of them now.  At present, only one
+  // module per object is supported.
+  node_module* const mp = modpending;
+  modpending = nullptr;
 
-  if (uv_dlopen(*filename, &lib)) {
+  if (is_dlopen_error) {
     Local<String> errmsg = OneByteString(env->isolate(), uv_dlerror(&lib));
 #ifdef _WIN32
     // Windows needs to add the filename into the error message
@@ -2004,14 +2006,6 @@ void DLOpen(const FunctionCallbackInfo<Value>& args) {
     return;
   }
 
-  /*
-   * Objects containing v14 or later modules will have registered themselves
-   * on the pending list.  Activate all of them now.  At present, only one
-   * module per object is supported.
-   */
-  mp = modpending;
-  modpending = nullptr;
-
   if (mp == nullptr) {
     env->ThrowError("Module did not self-register.");
     return;
@@ -2034,6 +2028,9 @@ void DLOpen(const FunctionCallbackInfo<Value>& args) {
   mp->nm_link = modlist_addon;
   modlist_addon = mp;
 
+  Local<String> exports_string = env->exports_string();
+  Local<Object> exports = module->Get(exports_string)->ToObject();
+
   if (mp->nm_context_register_func != nullptr) {
     mp->nm_context_register_func(exports, module, env->context(), mp->nm_priv);
   } else if (mp->nm_register_func != nullptr) {