From a89cc2886ecec5bd9b8b3f803a6658afb90e21b2 Mon Sep 17 00:00:00 2001
From: Anna Henningsen <anna@addaleax.net>
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 with
mutexes, to better accommodate 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
PR-URL: https://github.com/nodejs/node/pull/20542
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
---
 src/node.cc          | 22 ++++++++++++++++++----
 src/node_crypto.cc   |  3 +++
 src/node_internals.h |  1 +
 3 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/src/node.cc b/src/node.cc
index 693457d3ed0aae..704bdc334b2f9c 100644
--- a/src/node.cc
+++ b/src/node.cc
@@ -171,6 +171,9 @@ using v8::Value;
 
 using AsyncHooks = Environment::AsyncHooks;
 
+static Mutex process_mutex;
+static Mutex environ_mutex;
+
 static bool print_eval = false;
 static bool force_repl = false;
 static bool syntax_check_only = false;
@@ -698,9 +701,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:
@@ -1358,6 +1364,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();
@@ -2624,7 +2631,6 @@ static void ProcessTitleSetter(Local<Name> property,
                                Local<Value> value,
                                const PropertyCallbackInfo<void>& info) {
   node::Utf8Value title(info.GetIsolate(), value);
-  // TODO(piscisaureus): protect with a lock
   uv_set_process_title(*title);
 }
 
@@ -2635,6 +2641,7 @@ static void EnvGetter(Local<Name> 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);
@@ -2675,6 +2682,8 @@ static void EnvSetter(Local<Name> property,
           "DEP0104").IsNothing())
       return;
   }
+
+  Mutex::ScopedLock lock(environ_mutex);
 #ifdef __POSIX__
   node::Utf8Value key(info.GetIsolate(), property);
   node::Utf8Value val(info.GetIsolate(), value);
@@ -2695,6 +2704,7 @@ static void EnvSetter(Local<Name> property,
 
 static void EnvQuery(Local<Name> property,
                      const PropertyCallbackInfo<Integer>& info) {
+  Mutex::ScopedLock lock(environ_mutex);
   int32_t rc = -1;  // Not found unless proven otherwise.
   if (property->IsString()) {
 #ifdef __POSIX__
@@ -2724,6 +2734,7 @@ static void EnvQuery(Local<Name> property,
 
 static void EnvDeleter(Local<Name> property,
                        const PropertyCallbackInfo<Boolean>& info) {
+  Mutex::ScopedLock lock(environ_mutex);
   if (property->IsString()) {
 #ifdef __POSIX__
     node::Utf8Value key(info.GetIsolate(), property);
@@ -2749,6 +2760,7 @@ static void EnvEnumerator(const PropertyCallbackInfo<Array>& info) {
   Local<Value> argv[NODE_PUSH_VAL_TO_ARRAY_MAX];
   size_t idx = 0;
 
+  Mutex::ScopedLock lock(environ_mutex);
 #ifdef __POSIX__
   int size = 0;
   while (environ[size])
@@ -2864,6 +2876,7 @@ static Local<Object> GetFeatures(Environment* env) {
 
 static void DebugPortGetter(Local<Name> property,
                             const PropertyCallbackInfo<Value>& info) {
+  Mutex::ScopedLock lock(process_mutex);
   int port = debug_options.port();
 #if HAVE_INSPECTOR
   if (port == 0) {
@@ -2879,6 +2892,7 @@ static void DebugPortGetter(Local<Name> property,
 static void DebugPortSetter(Local<Name> property,
                             Local<Value> value,
                             const PropertyCallbackInfo<void>& 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 d5f27994307696..4d07546e0a213e 100644
--- a/src/node_crypto.cc
+++ b/src/node_crypto.cc
@@ -715,6 +715,9 @@ void SecureContext::SetCert(const FunctionCallbackInfo<Value>& args) {
 
 static X509_STORE* NewRootCertStore() {
   static std::vector<X509*> 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..e15df78ffdfee3 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"