Skip to content

Commit

Permalink
src: protect global state with mutexes
Browse files Browse the repository at this point in the history
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: ayojs/ayo#82
  • Loading branch information
addaleax committed May 5, 2018
1 parent 64348f5 commit 945c9fe
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 4 deletions.
25 changes: 21 additions & 4 deletions src/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -2600,6 +2608,7 @@ static void GetLinkedBinding(const FunctionCallbackInfo<Value>& args) {

static void ProcessTitleGetter(Local<Name> property,
const PropertyCallbackInfo<Value>& 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));
Expand All @@ -2610,7 +2619,7 @@ 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
Mutex::ScopedLock lock(process_title_mutex);
uv_set_process_title(*title);
}

Expand All @@ -2621,6 +2630,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);
Expand Down Expand Up @@ -2661,6 +2671,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);
Expand All @@ -2681,6 +2693,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__
Expand Down Expand Up @@ -2710,6 +2723,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);
Expand All @@ -2735,6 +2749,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])
Expand Down Expand Up @@ -2850,6 +2865,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) {
Expand All @@ -2865,6 +2881,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());
}

Expand Down
3 changes: 3 additions & 0 deletions src/node_crypto.cc
Original file line number Diff line number Diff line change
Expand Up @@ -721,6 +721,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]));
Expand Down
4 changes: 4 additions & 0 deletions src/node_internals.h
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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;

Expand Down
1 change: 1 addition & 0 deletions src/util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
Expand Down

0 comments on commit 945c9fe

Please sign in to comment.