From 945c9fe838c74e3109a0009294c5323de4a42b75 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Sat, 5 May 2018 17:47:02 +0200 Subject: [PATCH] src: protect global state with mutexes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Protect environment variables and inherently per-process state (e.g. the process title) with mutexes, to better accomodate Node’s usage in multi-threading environments. Thanks to Stephen Belanger for reviewing this change in its original PR. Refs: https://github.com/ayojs/ayo/pull/82 --- src/node.cc | 25 +++++++++++++++++++++---- src/node_crypto.cc | 3 +++ src/node_internals.h | 4 ++++ src/util.cc | 1 + 4 files changed, 29 insertions(+), 4 deletions(-) diff --git a/src/node.cc b/src/node.cc index ee114257d5d126..c9ea94b5b6cc66 100644 --- a/src/node.cc +++ b/src/node.cc @@ -172,6 +172,10 @@ using v8::Value; using AsyncHooks = Environment::AsyncHooks; +Mutex process_title_mutex; +static Mutex process_mutex; +static Mutex environ_mutex; + static bool print_eval = false; static bool force_repl = false; static bool syntax_check_only = false; @@ -699,9 +703,12 @@ bool SafeGetenv(const char* key, std::string* text) { goto fail; #endif - if (const char* value = getenv(key)) { - *text = value; - return true; + { + Mutex::ScopedLock lock(environ_mutex); + if (const char* value = getenv(key)) { + *text = value; + return true; + } } fail: @@ -1359,6 +1366,7 @@ void AppendExceptionLine(Environment* env, if (!can_set_arrow || (mode == FATAL_ERROR && !err_obj->IsNativeError())) { if (env->printed_error()) return; + Mutex::ScopedLock lock(process_mutex); env->set_printed_error(true); uv_tty_reset_mode(); @@ -2600,6 +2608,7 @@ static void GetLinkedBinding(const FunctionCallbackInfo& args) { static void ProcessTitleGetter(Local property, const PropertyCallbackInfo& info) { + Mutex::ScopedLock lock(process_title_mutex); char buffer[512]; uv_get_process_title(buffer, sizeof(buffer)); info.GetReturnValue().Set(String::NewFromUtf8(info.GetIsolate(), buffer)); @@ -2610,7 +2619,7 @@ static void ProcessTitleSetter(Local property, Local value, const PropertyCallbackInfo& info) { node::Utf8Value title(info.GetIsolate(), value); - // TODO(piscisaureus): protect with a lock + Mutex::ScopedLock lock(process_title_mutex); uv_set_process_title(*title); } @@ -2621,6 +2630,7 @@ static void EnvGetter(Local property, if (property->IsSymbol()) { return info.GetReturnValue().SetUndefined(); } + Mutex::ScopedLock lock(environ_mutex); #ifdef __POSIX__ node::Utf8Value key(isolate, property); const char* val = getenv(*key); @@ -2661,6 +2671,8 @@ static void EnvSetter(Local property, "DEP0104").IsNothing()) return; } + + Mutex::ScopedLock lock(environ_mutex); #ifdef __POSIX__ node::Utf8Value key(info.GetIsolate(), property); node::Utf8Value val(info.GetIsolate(), value); @@ -2681,6 +2693,7 @@ static void EnvSetter(Local property, static void EnvQuery(Local property, const PropertyCallbackInfo& info) { + Mutex::ScopedLock lock(environ_mutex); int32_t rc = -1; // Not found unless proven otherwise. if (property->IsString()) { #ifdef __POSIX__ @@ -2710,6 +2723,7 @@ static void EnvQuery(Local property, static void EnvDeleter(Local property, const PropertyCallbackInfo& info) { + Mutex::ScopedLock lock(environ_mutex); if (property->IsString()) { #ifdef __POSIX__ node::Utf8Value key(info.GetIsolate(), property); @@ -2735,6 +2749,7 @@ static void EnvEnumerator(const PropertyCallbackInfo& info) { Local argv[NODE_PUSH_VAL_TO_ARRAY_MAX]; size_t idx = 0; + Mutex::ScopedLock lock(environ_mutex); #ifdef __POSIX__ int size = 0; while (environ[size]) @@ -2850,6 +2865,7 @@ static Local GetFeatures(Environment* env) { static void DebugPortGetter(Local property, const PropertyCallbackInfo& info) { + Mutex::ScopedLock lock(process_mutex); int port = debug_options.port(); #if HAVE_INSPECTOR if (port == 0) { @@ -2865,6 +2881,7 @@ static void DebugPortGetter(Local property, static void DebugPortSetter(Local property, Local value, const PropertyCallbackInfo& info) { + Mutex::ScopedLock lock(process_mutex); debug_options.set_port(value->Int32Value()); } diff --git a/src/node_crypto.cc b/src/node_crypto.cc index ce9c41deaebcd2..f611f81f16a819 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -721,6 +721,9 @@ void SecureContext::SetCert(const FunctionCallbackInfo& args) { static X509_STORE* NewRootCertStore() { static std::vector root_certs_vector; + static Mutex root_certs_vector_mutex; + Mutex::ScopedLock lock(root_certs_vector_mutex); + if (root_certs_vector.empty()) { for (size_t i = 0; i < arraysize(root_certs); i++) { BIO* bp = NodeBIO::NewFixed(root_certs[i], strlen(root_certs[i])); diff --git a/src/node_internals.h b/src/node_internals.h index e5ea575ebc5981..9304b8692b8de3 100644 --- a/src/node_internals.h +++ b/src/node_internals.h @@ -25,6 +25,7 @@ #if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS #include "node.h" +#include "node_mutex.h" #include "node_persistent.h" #include "util-inl.h" #include "env-inl.h" @@ -215,6 +216,9 @@ extern bool v8_initialized; // Used in node_config.cc. extern node::DebugOptions debug_options; +// Used to protect the process title in multi-threading situations. +extern Mutex process_title_mutex; + // Forward declaration class Environment; diff --git a/src/util.cc b/src/util.cc index 77824acb03a610..e081c6a21f7d2f 100644 --- a/src/util.cc +++ b/src/util.cc @@ -114,6 +114,7 @@ std::string GetHumanReadableProcessName() { void GetHumanReadableProcessName(char (*name)[1024]) { char title[1024] = "Node.js"; + Mutex::ScopedLock lock(process_title_mutex); uv_get_process_title(title, sizeof(title)); snprintf(*name, sizeof(*name), "%s[%u]", title, uv_os_getpid()); }