Skip to content

Commit efc77ec

Browse files
bnoordhuisMyles Borins
bnoordhuis
authored and
Myles Borins
committed
src: use RAII for mutexes and condition variables
We will be introducing many more critical sections in the upcoming multi-isolate changes, so let's make manual synchronization a thing of the past. PR-URL: #7334 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Trevor Norris <[email protected]>
1 parent bceac33 commit efc77ec

File tree

6 files changed

+212
-40
lines changed

6 files changed

+212
-40
lines changed

‎node.gyp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,7 @@
169169
'src/node_http_parser.h',
170170
'src/node_internals.h',
171171
'src/node_javascript.h',
172+
'src/node_mutex.h',
172173
'src/node_root_certs.h',
173174
'src/node_version.h',
174175
'src/node_watchdog.h',

‎src/debug-agent.cc

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -55,21 +55,14 @@ Agent::Agent(Environment* env) : state_(kNone),
5555
parent_env_(env),
5656
child_env_(nullptr),
5757
dispatch_handler_(nullptr) {
58-
int err;
59-
60-
err = uv_sem_init(&start_sem_, 0);
61-
CHECK_EQ(err, 0);
62-
63-
err = uv_mutex_init(&message_mutex_);
64-
CHECK_EQ(err, 0);
58+
CHECK_EQ(0, uv_sem_init(&start_sem_, 0));
6559
}
6660

6761

6862
Agent::~Agent() {
6963
Stop();
7064

7165
uv_sem_destroy(&start_sem_);
72-
uv_mutex_destroy(&message_mutex_);
7366

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

277-
uv_mutex_lock(&a->message_mutex_);
270+
Mutex::ScopedLock scoped_lock(a->message_mutex_);
278271
while (AgentMessage* msg = a->messages_.PopFront()) {
279272
// Time to close everything
280273
if (msg->data() == nullptr) {
@@ -305,14 +298,12 @@ void Agent::ChildSignalCb(uv_async_t* signal) {
305298
argv);
306299
delete msg;
307300
}
308-
uv_mutex_unlock(&a->message_mutex_);
309301
}
310302

311303

312304
void Agent::EnqueueMessage(AgentMessage* message) {
313-
uv_mutex_lock(&message_mutex_);
305+
Mutex::ScopedLock scoped_lock(message_mutex_);
314306
messages_.PushBack(message);
315-
uv_mutex_unlock(&message_mutex_);
316307
uv_async_send(&child_signal_);
317308
}
318309

‎src/debug-agent.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
#ifndef SRC_DEBUG_AGENT_H_
2323
#define SRC_DEBUG_AGENT_H_
2424

25+
#include "node_mutex.h"
2526
#include "util.h"
2627
#include "util-inl.h"
2728
#include "uv.h"
@@ -115,7 +116,7 @@ class Agent {
115116
bool wait_;
116117

117118
uv_sem_t start_sem_;
118-
uv_mutex_t message_mutex_;
119+
node::Mutex message_mutex_;
119120
uv_async_t child_signal_;
120121

121122
uv_thread_t thread_;

‎src/node.cc

Lines changed: 14 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,7 @@ static double prog_start_time;
159159
static bool debugger_running;
160160
static uv_async_t dispatch_debug_messages_async;
161161

162-
static uv_mutex_t node_isolate_mutex;
162+
static Mutex node_isolate_mutex;
163163
static v8::Isolate* node_isolate;
164164
static v8::Platform* default_platform;
165165

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

35363536
// Called from an arbitrary thread.
35373537
static void TryStartDebugger() {
3538-
uv_mutex_lock(&node_isolate_mutex);
3538+
Mutex::ScopedLock scoped_lock(node_isolate_mutex);
35393539
if (auto isolate = node_isolate) {
35403540
v8::Debug::DebugBreak(isolate);
35413541
uv_async_send(&dispatch_debug_messages_async);
35423542
}
3543-
uv_mutex_unlock(&node_isolate_mutex);
35443543
}
35453544

35463545

35473546
// Called from the main thread.
35483547
static void DispatchDebugMessagesAsyncCallback(uv_async_t* handle) {
3549-
uv_mutex_lock(&node_isolate_mutex);
3548+
Mutex::ScopedLock scoped_lock(node_isolate_mutex);
35503549
if (auto isolate = node_isolate) {
35513550
if (debugger_running == false) {
35523551
fprintf(stderr, "Starting debugger agent.\n");
@@ -3562,7 +3561,6 @@ static void DispatchDebugMessagesAsyncCallback(uv_async_t* handle) {
35623561
Isolate::Scope isolate_scope(isolate);
35633562
v8::Debug::ProcessDebugMessages();
35643563
}
3565-
uv_mutex_unlock(&node_isolate_mutex);
35663564
}
35673565

35683566

@@ -3888,8 +3886,6 @@ void Init(int* argc,
38883886
// Make inherited handles noninheritable.
38893887
uv_disable_stdio_inheritance();
38903888

3891-
CHECK_EQ(0, uv_mutex_init(&node_isolate_mutex));
3892-
38933889
// init async debug messages dispatching
38943890
// Main thread uses uv_default_loop
38953891
CHECK_EQ(0, uv_async_init(uv_default_loop(),
@@ -4177,12 +4173,13 @@ static void StartNodeInstance(void* arg) {
41774173
#endif
41784174
Isolate* isolate = Isolate::New(params);
41794175

4180-
uv_mutex_lock(&node_isolate_mutex);
4181-
if (instance_data->is_main()) {
4182-
CHECK_EQ(node_isolate, nullptr);
4183-
node_isolate = isolate;
4176+
{
4177+
Mutex::ScopedLock scoped_lock(node_isolate_mutex);
4178+
if (instance_data->is_main()) {
4179+
CHECK_EQ(node_isolate, nullptr);
4180+
node_isolate = isolate;
4181+
}
41844182
}
4185-
uv_mutex_unlock(&node_isolate_mutex);
41864183

41874184
if (track_heap_objects) {
41884185
isolate->GetHeapProfiler()->StartTrackingHeapObjects(true);
@@ -4251,10 +4248,11 @@ static void StartNodeInstance(void* arg) {
42514248
env = nullptr;
42524249
}
42534250

4254-
uv_mutex_lock(&node_isolate_mutex);
4255-
if (node_isolate == isolate)
4256-
node_isolate = nullptr;
4257-
uv_mutex_unlock(&node_isolate_mutex);
4251+
{
4252+
Mutex::ScopedLock scoped_lock(node_isolate_mutex);
4253+
if (node_isolate == isolate)
4254+
node_isolate = nullptr;
4255+
}
42584256

42594257
CHECK_NE(isolate, nullptr);
42604258
isolate->Dispose();

‎src/node_crypto.cc

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ static X509_NAME *cnnic_ev_name =
114114
d2i_X509_NAME(nullptr, &cnnic_ev_p,
115115
sizeof(CNNIC_EV_ROOT_CA_SUBJECT_DATA)-1);
116116

117-
static uv_mutex_t* locks;
117+
static Mutex* mutexes;
118118

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

173173

174174
static void crypto_lock_init(void) {
175-
int i, n;
176-
177-
n = CRYPTO_num_locks();
178-
locks = new uv_mutex_t[n];
179-
180-
for (i = 0; i < n; i++)
181-
if (uv_mutex_init(locks + i))
182-
ABORT();
175+
mutexes = new Mutex[CRYPTO_num_locks()];
183176
}
184177

185178

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

183+
auto mutex = &mutexes[n];
190184
if (mode & CRYPTO_LOCK)
191-
uv_mutex_lock(locks + n);
185+
mutex->Lock();
192186
else
193-
uv_mutex_unlock(locks + n);
187+
mutex->Unlock();
194188
}
195189

196190

‎src/node_mutex.h

Lines changed: 187 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,187 @@
1+
#ifndef SRC_NODE_MUTEX_H_
2+
#define SRC_NODE_MUTEX_H_
3+
4+
#include "util.h"
5+
#include "uv.h"
6+
7+
namespace node {
8+
9+
template <typename Traits> class ConditionVariableBase;
10+
template <typename Traits> class MutexBase;
11+
struct LibuvMutexTraits;
12+
13+
using ConditionVariable = ConditionVariableBase<LibuvMutexTraits>;
14+
using Mutex = MutexBase<LibuvMutexTraits>;
15+
16+
template <typename Traits>
17+
class MutexBase {
18+
public:
19+
inline MutexBase();
20+
inline ~MutexBase();
21+
inline void Lock();
22+
inline void Unlock();
23+
24+
class ScopedLock;
25+
class ScopedUnlock;
26+
27+
class ScopedLock {
28+
public:
29+
inline explicit ScopedLock(const MutexBase& mutex);
30+
inline explicit ScopedLock(const ScopedUnlock& scoped_unlock);
31+
inline ~ScopedLock();
32+
33+
private:
34+
template <typename> friend class ConditionVariableBase;
35+
friend class ScopedUnlock;
36+
const MutexBase& mutex_;
37+
DISALLOW_COPY_AND_ASSIGN(ScopedLock);
38+
};
39+
40+
class ScopedUnlock {
41+
public:
42+
inline explicit ScopedUnlock(const ScopedLock& scoped_lock);
43+
inline ~ScopedUnlock();
44+
45+
private:
46+
friend class ScopedLock;
47+
const MutexBase& mutex_;
48+
DISALLOW_COPY_AND_ASSIGN(ScopedUnlock);
49+
};
50+
51+
private:
52+
template <typename> friend class ConditionVariableBase;
53+
mutable typename Traits::MutexT mutex_;
54+
DISALLOW_COPY_AND_ASSIGN(MutexBase);
55+
};
56+
57+
template <typename Traits>
58+
class ConditionVariableBase {
59+
public:
60+
using ScopedLock = typename MutexBase<Traits>::ScopedLock;
61+
62+
inline ConditionVariableBase();
63+
inline ~ConditionVariableBase();
64+
inline void Broadcast(const ScopedLock&);
65+
inline void Signal(const ScopedLock&);
66+
inline void Wait(const ScopedLock& scoped_lock);
67+
68+
private:
69+
typename Traits::CondT cond_;
70+
DISALLOW_COPY_AND_ASSIGN(ConditionVariableBase);
71+
};
72+
73+
struct LibuvMutexTraits {
74+
using CondT = uv_cond_t;
75+
using MutexT = uv_mutex_t;
76+
77+
static inline int cond_init(CondT* cond) {
78+
return uv_cond_init(cond);
79+
}
80+
81+
static inline int mutex_init(MutexT* mutex) {
82+
return uv_mutex_init(mutex);
83+
}
84+
85+
static inline void cond_broadcast(CondT* cond) {
86+
uv_cond_broadcast(cond);
87+
}
88+
89+
static inline void cond_destroy(CondT* cond) {
90+
uv_cond_destroy(cond);
91+
}
92+
93+
static inline void cond_signal(CondT* cond) {
94+
uv_cond_signal(cond);
95+
}
96+
97+
static inline void cond_wait(CondT* cond, MutexT* mutex) {
98+
uv_cond_wait(cond, mutex);
99+
}
100+
101+
static inline void mutex_destroy(MutexT* mutex) {
102+
uv_mutex_destroy(mutex);
103+
}
104+
105+
static inline void mutex_lock(MutexT* mutex) {
106+
uv_mutex_lock(mutex);
107+
}
108+
109+
static inline void mutex_unlock(MutexT* mutex) {
110+
uv_mutex_unlock(mutex);
111+
}
112+
};
113+
114+
template <typename Traits>
115+
ConditionVariableBase<Traits>::ConditionVariableBase() {
116+
CHECK_EQ(0, Traits::cond_init(&cond_));
117+
}
118+
119+
template <typename Traits>
120+
ConditionVariableBase<Traits>::~ConditionVariableBase() {
121+
Traits::cond_destroy(&cond_);
122+
}
123+
124+
template <typename Traits>
125+
void ConditionVariableBase<Traits>::Broadcast(const ScopedLock&) {
126+
Traits::cond_broadcast(&cond_);
127+
}
128+
129+
template <typename Traits>
130+
void ConditionVariableBase<Traits>::Signal(const ScopedLock&) {
131+
Traits::cond_signal(&cond_);
132+
}
133+
134+
template <typename Traits>
135+
void ConditionVariableBase<Traits>::Wait(const ScopedLock& scoped_lock) {
136+
Traits::cond_wait(&cond_, &scoped_lock.mutex_.mutex_);
137+
}
138+
139+
template <typename Traits>
140+
MutexBase<Traits>::MutexBase() {
141+
CHECK_EQ(0, Traits::mutex_init(&mutex_));
142+
}
143+
144+
template <typename Traits>
145+
MutexBase<Traits>::~MutexBase() {
146+
Traits::mutex_destroy(&mutex_);
147+
}
148+
149+
template <typename Traits>
150+
void MutexBase<Traits>::Lock() {
151+
Traits::mutex_lock(&mutex_);
152+
}
153+
154+
template <typename Traits>
155+
void MutexBase<Traits>::Unlock() {
156+
Traits::mutex_unlock(&mutex_);
157+
}
158+
159+
template <typename Traits>
160+
MutexBase<Traits>::ScopedLock::ScopedLock(const MutexBase& mutex)
161+
: mutex_(mutex) {
162+
Traits::mutex_lock(&mutex_.mutex_);
163+
}
164+
165+
template <typename Traits>
166+
MutexBase<Traits>::ScopedLock::ScopedLock(const ScopedUnlock& scoped_unlock)
167+
: MutexBase(scoped_unlock.mutex_) {}
168+
169+
template <typename Traits>
170+
MutexBase<Traits>::ScopedLock::~ScopedLock() {
171+
Traits::mutex_unlock(&mutex_.mutex_);
172+
}
173+
174+
template <typename Traits>
175+
MutexBase<Traits>::ScopedUnlock::ScopedUnlock(const ScopedLock& scoped_lock)
176+
: mutex_(scoped_lock.mutex_) {
177+
Traits::mutex_unlock(&mutex_.mutex_);
178+
}
179+
180+
template <typename Traits>
181+
MutexBase<Traits>::ScopedUnlock::~ScopedUnlock() {
182+
Traits::mutex_lock(&mutex_.mutex_);
183+
}
184+
185+
} // namespace node
186+
187+
#endif // SRC_NODE_MUTEX_H_

0 commit comments

Comments
 (0)