Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

src: protect global state with mutexes #20542

Closed
wants to merge 2 commits into from
Closed

src: protect global state with mutexes #20542

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
945c9fe
src: protect global state with mutexes
addaleax May 5, 2018
9b9d2a8
[squash] drop process title mutex
addaleax May 6, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 21 additions & 4 deletions src/node.cc
View file
Open in desktop
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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not necessary, uv_get_process_title() and uv_set_process_title() already use locks internally.

(Perhaps uv_os_getenv(), uv_os_setenv() and uv_os_unsetenv() should do the same.)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, thanks for pointing this out.

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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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