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.

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

src: use RAII for mutexes and condition variables (v4.x) #7715

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions node.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,7 @@
'src/node_http_parser.h',
'src/node_internals.h',
'src/node_javascript.h',
'src/node_mutex.h',
'src/node_root_certs.h',
'src/node_version.h',
'src/node_watchdog.h',
Expand Down
15 changes: 3 additions & 12 deletions src/debug-agent.cc
Original file line number Diff line number Diff line change
Expand Up @@ -55,21 +55,14 @@ Agent::Agent(Environment* env) : state_(kNone),
parent_env_(env),
child_env_(nullptr),
dispatch_handler_(nullptr) {
int err;

err = uv_sem_init(&start_sem_, 0);
CHECK_EQ(err, 0);

err = uv_mutex_init(&message_mutex_);
CHECK_EQ(err, 0);
CHECK_EQ(0, uv_sem_init(&start_sem_, 0));
}


Agent::~Agent() {
Stop();

uv_sem_destroy(&start_sem_);
uv_mutex_destroy(&message_mutex_);

while (AgentMessage* msg = messages_.PopFront())
delete msg;
Expand Down Expand Up @@ -274,7 +267,7 @@ void Agent::ChildSignalCb(uv_async_t* signal) {
HandleScope scope(isolate);
Local<Object> api = PersistentToLocal(isolate, a->api_);

uv_mutex_lock(&a->message_mutex_);
Mutex::ScopedLock scoped_lock(a->message_mutex_);
while (AgentMessage* msg = a->messages_.PopFront()) {
// Time to close everything
if (msg->data() == nullptr) {
Expand Down Expand Up @@ -305,14 +298,12 @@ void Agent::ChildSignalCb(uv_async_t* signal) {
argv);
delete msg;
}
uv_mutex_unlock(&a->message_mutex_);
}


void Agent::EnqueueMessage(AgentMessage* message) {
uv_mutex_lock(&message_mutex_);
Mutex::ScopedLock scoped_lock(message_mutex_);
messages_.PushBack(message);
uv_mutex_unlock(&message_mutex_);
uv_async_send(&child_signal_);
}

Expand Down
3 changes: 2 additions & 1 deletion src/debug-agent.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#ifndef SRC_DEBUG_AGENT_H_
#define SRC_DEBUG_AGENT_H_

#include "node_mutex.h"
#include "util.h"
#include "util-inl.h"
#include "uv.h"
Expand Down Expand Up @@ -115,7 +116,7 @@ class Agent {
bool wait_;

uv_sem_t start_sem_;
uv_mutex_t message_mutex_;
node::Mutex message_mutex_;
uv_async_t child_signal_;

uv_thread_t thread_;
Expand Down
30 changes: 14 additions & 16 deletions src/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ static double prog_start_time;
static bool debugger_running;
static uv_async_t dispatch_debug_messages_async;

static uv_mutex_t node_isolate_mutex;
static Mutex node_isolate_mutex;
static v8::Isolate* node_isolate;
static v8::Platform* default_platform;

Expand Down Expand Up @@ -3535,18 +3535,17 @@ static void EnableDebug(Environment* env) {

// Called from an arbitrary thread.
static void TryStartDebugger() {
uv_mutex_lock(&node_isolate_mutex);
Mutex::ScopedLock scoped_lock(node_isolate_mutex);
if (auto isolate = node_isolate) {
v8::Debug::DebugBreak(isolate);
uv_async_send(&dispatch_debug_messages_async);
}
uv_mutex_unlock(&node_isolate_mutex);
}


// Called from the main thread.
static void DispatchDebugMessagesAsyncCallback(uv_async_t* handle) {
uv_mutex_lock(&node_isolate_mutex);
Mutex::ScopedLock scoped_lock(node_isolate_mutex);
if (auto isolate = node_isolate) {
if (debugger_running == false) {
fprintf(stderr, "Starting debugger agent.\n");
Expand All @@ -3562,7 +3561,6 @@ static void DispatchDebugMessagesAsyncCallback(uv_async_t* handle) {
Isolate::Scope isolate_scope(isolate);
v8::Debug::ProcessDebugMessages();
}
uv_mutex_unlock(&node_isolate_mutex);
}


Expand Down Expand Up @@ -3888,8 +3886,6 @@ void Init(int* argc,
// Make inherited handles noninheritable.
uv_disable_stdio_inheritance();

CHECK_EQ(0, uv_mutex_init(&node_isolate_mutex));

// init async debug messages dispatching
// Main thread uses uv_default_loop
CHECK_EQ(0, uv_async_init(uv_default_loop(),
Expand Down Expand Up @@ -4177,12 +4173,13 @@ static void StartNodeInstance(void* arg) {
#endif
Isolate* isolate = Isolate::New(params);

uv_mutex_lock(&node_isolate_mutex);
if (instance_data->is_main()) {
CHECK_EQ(node_isolate, nullptr);
node_isolate = isolate;
{
Mutex::ScopedLock scoped_lock(node_isolate_mutex);
if (instance_data->is_main()) {
CHECK_EQ(node_isolate, nullptr);
node_isolate = isolate;
}
}
uv_mutex_unlock(&node_isolate_mutex);

if (track_heap_objects) {
isolate->GetHeapProfiler()->StartTrackingHeapObjects(true);
Expand Down Expand Up @@ -4251,10 +4248,11 @@ static void StartNodeInstance(void* arg) {
env = nullptr;
}

uv_mutex_lock(&node_isolate_mutex);
if (node_isolate == isolate)
node_isolate = nullptr;
uv_mutex_unlock(&node_isolate_mutex);
{
Mutex::ScopedLock scoped_lock(node_isolate_mutex);
if (node_isolate == isolate)
node_isolate = nullptr;
}

CHECK_NE(isolate, nullptr);
isolate->Dispose();
Expand Down
16 changes: 5 additions & 11 deletions src/node_crypto.cc
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ static X509_NAME *cnnic_ev_name =
d2i_X509_NAME(nullptr, &cnnic_ev_p,
sizeof(CNNIC_EV_ROOT_CA_SUBJECT_DATA)-1);

static uv_mutex_t* locks;
static Mutex* mutexes;

const char* const root_certs[] = {
#include "node_root_certs.h" // NOLINT(build/include_order)
Expand Down Expand Up @@ -172,25 +172,19 @@ static void crypto_threadid_cb(CRYPTO_THREADID* tid) {


static void crypto_lock_init(void) {
int i, n;

n = CRYPTO_num_locks();
locks = new uv_mutex_t[n];

for (i = 0; i < n; i++)
if (uv_mutex_init(locks + i))
ABORT();
mutexes = new Mutex[CRYPTO_num_locks()];
}


static void crypto_lock_cb(int mode, int n, const char* file, int line) {
CHECK(!(mode & CRYPTO_LOCK) ^ !(mode & CRYPTO_UNLOCK));
CHECK(!(mode & CRYPTO_READ) ^ !(mode & CRYPTO_WRITE));

auto mutex = &mutexes[n];
if (mode & CRYPTO_LOCK)
uv_mutex_lock(locks + n);
mutex->Lock();
else
uv_mutex_unlock(locks + n);
mutex->Unlock();
}


Expand Down
187 changes: 187 additions & 0 deletions src/node_mutex.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,187 @@
#ifndef SRC_NODE_MUTEX_H_
#define SRC_NODE_MUTEX_H_

#include "util.h"
#include "uv.h"

namespace node {

template <typename Traits> class ConditionVariableBase;
template <typename Traits> class MutexBase;
struct LibuvMutexTraits;

using ConditionVariable = ConditionVariableBase<LibuvMutexTraits>;
using Mutex = MutexBase<LibuvMutexTraits>;

template <typename Traits>
class MutexBase {
public:
inline MutexBase();
inline ~MutexBase();
inline void Lock();
inline void Unlock();

class ScopedLock;
class ScopedUnlock;

class ScopedLock {
public:
inline explicit ScopedLock(const MutexBase& mutex);
inline explicit ScopedLock(const ScopedUnlock& scoped_unlock);
inline ~ScopedLock();

private:
template <typename> friend class ConditionVariableBase;
friend class ScopedUnlock;
const MutexBase& mutex_;
DISALLOW_COPY_AND_ASSIGN(ScopedLock);
};

class ScopedUnlock {
public:
inline explicit ScopedUnlock(const ScopedLock& scoped_lock);
inline ~ScopedUnlock();

private:
friend class ScopedLock;
const MutexBase& mutex_;
DISALLOW_COPY_AND_ASSIGN(ScopedUnlock);
};

private:
template <typename> friend class ConditionVariableBase;
mutable typename Traits::MutexT mutex_;
DISALLOW_COPY_AND_ASSIGN(MutexBase);
};

template <typename Traits>
class ConditionVariableBase {
public:
using ScopedLock = typename MutexBase<Traits>::ScopedLock;

inline ConditionVariableBase();
inline ~ConditionVariableBase();
inline void Broadcast(const ScopedLock&);
inline void Signal(const ScopedLock&);
inline void Wait(const ScopedLock& scoped_lock);

private:
typename Traits::CondT cond_;
DISALLOW_COPY_AND_ASSIGN(ConditionVariableBase);
};

struct LibuvMutexTraits {
using CondT = uv_cond_t;
using MutexT = uv_mutex_t;

static inline int cond_init(CondT* cond) {
return uv_cond_init(cond);
}

static inline int mutex_init(MutexT* mutex) {
return uv_mutex_init(mutex);
}

static inline void cond_broadcast(CondT* cond) {
uv_cond_broadcast(cond);
}

static inline void cond_destroy(CondT* cond) {
uv_cond_destroy(cond);
}

static inline void cond_signal(CondT* cond) {
uv_cond_signal(cond);
}

static inline void cond_wait(CondT* cond, MutexT* mutex) {
uv_cond_wait(cond, mutex);
}

static inline void mutex_destroy(MutexT* mutex) {
uv_mutex_destroy(mutex);
}

static inline void mutex_lock(MutexT* mutex) {
uv_mutex_lock(mutex);
}

static inline void mutex_unlock(MutexT* mutex) {
uv_mutex_unlock(mutex);
}
};

template <typename Traits>
ConditionVariableBase<Traits>::ConditionVariableBase() {
CHECK_EQ(0, Traits::cond_init(&cond_));
}

template <typename Traits>
ConditionVariableBase<Traits>::~ConditionVariableBase() {
Traits::cond_destroy(&cond_);
}

template <typename Traits>
void ConditionVariableBase<Traits>::Broadcast(const ScopedLock&) {
Traits::cond_broadcast(&cond_);
}

template <typename Traits>
void ConditionVariableBase<Traits>::Signal(const ScopedLock&) {
Traits::cond_signal(&cond_);
}

template <typename Traits>
void ConditionVariableBase<Traits>::Wait(const ScopedLock& scoped_lock) {
Traits::cond_wait(&cond_, &scoped_lock.mutex_.mutex_);
}

template <typename Traits>
MutexBase<Traits>::MutexBase() {
CHECK_EQ(0, Traits::mutex_init(&mutex_));
}

template <typename Traits>
MutexBase<Traits>::~MutexBase() {
Traits::mutex_destroy(&mutex_);
}

template <typename Traits>
void MutexBase<Traits>::Lock() {
Traits::mutex_lock(&mutex_);
}

template <typename Traits>
void MutexBase<Traits>::Unlock() {
Traits::mutex_unlock(&mutex_);
}

template <typename Traits>
MutexBase<Traits>::ScopedLock::ScopedLock(const MutexBase& mutex)
: mutex_(mutex) {
Traits::mutex_lock(&mutex_.mutex_);
}

template <typename Traits>
MutexBase<Traits>::ScopedLock::ScopedLock(const ScopedUnlock& scoped_unlock)
: MutexBase(scoped_unlock.mutex_) {}

template <typename Traits>
MutexBase<Traits>::ScopedLock::~ScopedLock() {
Traits::mutex_unlock(&mutex_.mutex_);
}

template <typename Traits>
MutexBase<Traits>::ScopedUnlock::ScopedUnlock(const ScopedLock& scoped_lock)
: mutex_(scoped_lock.mutex_) {
Traits::mutex_unlock(&mutex_.mutex_);
}

template <typename Traits>
MutexBase<Traits>::ScopedUnlock::~ScopedUnlock() {
Traits::mutex_lock(&mutex_.mutex_);
}

} // namespace node

#endif // SRC_NODE_MUTEX_H_