From 7f2c8bf9b19dc3fe7761d8b71964dec1349492ee Mon Sep 17 00:00:00 2001 From: Ryan Petrich Date: Sat, 30 Jun 2018 13:59:37 -0400 Subject: [PATCH 1/2] src: fix race on modpending by migrating it to Environment Fixes a rare race condition on modpending when two native modules are loaded simultaneously on different threads by migrating modpending to be a member of Environment. --- src/node.cc | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/src/node.cc b/src/node.cc index 0db313fd36b81e..0c0239fc404a30 100644 --- a/src/node.cc +++ b/src/node.cc @@ -185,7 +185,8 @@ static int v8_thread_pool_size = v8_default_thread_pool_size; static bool prof_process = false; static bool v8_is_profiling = false; static bool node_is_initialized = false; -static node_module* modpending; +static uv_once_t init_once = UV_ONCE_INIT; +static uv_key_t thread_local_modpending; static node_module* modlist_builtin; static node_module* modlist_internal; static node_module* modlist_linked; @@ -1144,7 +1145,7 @@ extern "C" void node_module_register(void* m) { mp->nm_link = modlist_linked; modlist_linked = mp; } else { - modpending = mp; + uv_key_set(&thread_local_modpending, mp); } } @@ -1258,6 +1259,10 @@ inline napi_addon_register_func GetNapiInitializerCallback(DLib* dlib) { reinterpret_cast(dlib->GetSymbolAddress(name)); } +void InitDLOpenOnce() { + CHECK_EQ(0, uv_key_create(&thread_local_modpending)); +} + // DLOpen is process.dlopen(module, filename, flags). // Used to load 'module.node' dynamically shared objects. // @@ -1268,7 +1273,8 @@ static void DLOpen(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); auto context = env->context(); - CHECK_NULL(modpending); + uv_once(&init_once, InitDLOpenOnce); + CHECK_NULL(uv_key_get(&thread_local_modpending)); if (args.Length() < 2) { env->ThrowError("process.dlopen needs at least 2 arguments."); @@ -1296,8 +1302,9 @@ static void DLOpen(const FunctionCallbackInfo& args) { // 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; + node_module* const mp = static_cast( + uv_key_get(&thread_local_modpending)); + uv_key_set(&thread_local_modpending, nullptr); if (!is_opened) { Local errmsg = OneByteString(env->isolate(), dlib.errmsg_.c_str()); From da4632a3f0f1cc05c88cd4d171e9cd838cc74702 Mon Sep 17 00:00:00 2001 From: Gabriel Schulhof Date: Fri, 27 Jul 2018 16:59:34 -0400 Subject: [PATCH 2/2] src: squash! fix naming --- src/node.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/node.cc b/src/node.cc index 0c0239fc404a30..6dfe0543461300 100644 --- a/src/node.cc +++ b/src/node.cc @@ -185,7 +185,7 @@ static int v8_thread_pool_size = v8_default_thread_pool_size; static bool prof_process = false; static bool v8_is_profiling = false; static bool node_is_initialized = false; -static uv_once_t init_once = UV_ONCE_INIT; +static uv_once_t init_modpending_once = UV_ONCE_INIT; static uv_key_t thread_local_modpending; static node_module* modlist_builtin; static node_module* modlist_internal; @@ -1259,7 +1259,7 @@ inline napi_addon_register_func GetNapiInitializerCallback(DLib* dlib) { reinterpret_cast(dlib->GetSymbolAddress(name)); } -void InitDLOpenOnce() { +void InitModpendingOnce() { CHECK_EQ(0, uv_key_create(&thread_local_modpending)); } @@ -1273,7 +1273,7 @@ static void DLOpen(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); auto context = env->context(); - uv_once(&init_once, InitDLOpenOnce); + uv_once(&init_modpending_once, InitModpendingOnce); CHECK_NULL(uv_key_get(&thread_local_modpending)); if (args.Length() < 2) {