src: use RAII for mutexes and condition variables · nodejs/node@0593351 · GitHub
Skip to content

Commit 0593351

Browse files
bnoordhuisFishrock123
bnoordhuis
authored and
Fishrock123
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 <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Trevor Norris <trev.norris@gmail.com>
1 parent da8e510 commit 0593351

7 files changed

Lines changed: 227 additions & 65 deletions

File tree

‎node.gyp‎

Lines changed: 1 addition & 0 deletions

‎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;
@@ -276,7 +269,7 @@ void Agent::ChildSignalCb(uv_async_t* signal) {
276269
HandleScope scope(isolate);
277270
Local<Object> api = PersistentToLocal(isolate, a->api_);
278271

279-
uv_mutex_lock(&a->message_mutex_);
272+
Mutex::ScopedLock scoped_lock(a->message_mutex_);
280273
while (AgentMessage* msg = a->messages_.PopFront()) {
281274
// Time to close everything
282275
if (msg->data() == nullptr) {
@@ -307,14 +300,12 @@ void Agent::ChildSignalCb(uv_async_t* signal) {
307300
argv);
308301
delete msg;
309302
}
310-
uv_mutex_unlock(&a->message_mutex_);
311303
}
312304

313305

314306
void Agent::EnqueueMessage(AgentMessage* message) {
315-
uv_mutex_lock(&message_mutex_);
307+
Mutex::ScopedLock scoped_lock(message_mutex_);
316308
messages_.PushBack(message);
317-
uv_mutex_unlock(&message_mutex_);
318309
uv_async_send(&child_signal_);
319310
}
320311

‎src/debug-agent.h‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424

2525
#if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
2626

27+
#include "node_mutex.h"
2728
#include "util.h"
2829
#include "util-inl.h"
2930
#include "uv.h"
@@ -117,7 +118,7 @@ class Agent {
117118
bool wait_;
118119

119120
uv_sem_t start_sem_;
120-
uv_mutex_t message_mutex_;
121+
node::Mutex message_mutex_;
121122
uv_async_t child_signal_;
122123

123124
uv_thread_t thread_;

‎src/inspector_agent.cc‎

Lines changed: 15 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
#include "env.h"
55
#include "env-inl.h"
66
#include "node.h"
7+
#include "node_mutex.h"
78
#include "node_version.h"
89
#include "v8-platform.h"
910
#include "util.h"
@@ -189,9 +190,9 @@ class AgentImpl {
189190
void Write(const std::string& message);
190191

191192
uv_sem_t start_sem_;
192-
uv_cond_t pause_cond_;
193-
uv_mutex_t queue_lock_;
194-
uv_mutex_t pause_lock_;
193+
ConditionVariable pause_cond_;
194+
Mutex pause_lock_;
195+
Mutex queue_lock_;
195196
uv_thread_t thread_;
196197
uv_loop_t child_loop_;
197198

@@ -290,9 +291,10 @@ class V8NodeInspector : public blink::V8Inspector {
290291
terminated_ = false;
291292
running_nested_loop_ = true;
292293
do {
293-
uv_mutex_lock(&agent_->pause_lock_);
294-
uv_cond_wait(&agent_->pause_cond_, &agent_->pause_lock_);
295-
uv_mutex_unlock(&agent_->pause_lock_);
294+
{
295+
Mutex::ScopedLock scoped_lock(agent_->pause_lock_);
296+
agent_->pause_cond_.Wait(scoped_lock);
297+
}
296298
while (v8::platform::PumpMessageLoop(platform_, isolate_))
297299
{}
298300
} while (!terminated_);
@@ -321,19 +323,14 @@ AgentImpl::AgentImpl(Environment* env) : port_(0),
321323
inspector_(nullptr),
322324
platform_(nullptr),
323325
dispatching_messages_(false) {
324-
int err;
325-
err = uv_sem_init(&start_sem_, 0);
326-
CHECK_EQ(err, 0);
326+
CHECK_EQ(0, uv_sem_init(&start_sem_, 0));
327327
memset(&data_written_, 0, sizeof(data_written_));
328328
memset(&io_thread_req_, 0, sizeof(io_thread_req_));
329329
}
330330

331331
AgentImpl::~AgentImpl() {
332332
if (!inspector_)
333333
return;
334-
uv_mutex_destroy(&queue_lock_);
335-
uv_mutex_destroy(&pause_lock_);
336-
uv_cond_destroy(&pause_cond_);
337334
uv_close(reinterpret_cast<uv_handle_t*>(&data_written_), nullptr);
338335
}
339336

@@ -349,12 +346,6 @@ void AgentImpl::Start(v8::Platform* platform, int port, bool wait) {
349346
CHECK_EQ(err, 0);
350347
err = uv_async_init(env->event_loop(), &data_written_, nullptr);
351348
CHECK_EQ(err, 0);
352-
err = uv_mutex_init(&queue_lock_);
353-
CHECK_EQ(err, 0);
354-
err = uv_mutex_init(&pause_lock_);
355-
CHECK_EQ(err, 0);
356-
err = uv_cond_init(&pause_cond_);
357-
CHECK_EQ(err, 0);
358349

359350
uv_unref(reinterpret_cast<uv_handle_t*>(&data_written_));
360351

@@ -441,6 +432,7 @@ void AgentImpl::OnRemoteDataIO(uv_stream_t* stream,
441432
const uv_buf_t* b) {
442433
inspector_socket_t* socket = static_cast<inspector_socket_t*>(stream->data);
443434
AgentImpl* agent = static_cast<AgentImpl*>(socket->data);
435+
Mutex::ScopedLock scoped_lock(agent->pause_lock_);
444436
if (read > 0) {
445437
std::string str(b->base, read);
446438
agent->PushPendingMessage(&agent->message_queue_, str);
@@ -470,21 +462,19 @@ void AgentImpl::OnRemoteDataIO(uv_stream_t* stream,
470462
}
471463
DisconnectAndDisposeIO(socket);
472464
}
473-
uv_cond_broadcast(&agent->pause_cond_);
465+
agent->pause_cond_.Broadcast(scoped_lock);
474466
}
475467

476468
void AgentImpl::PushPendingMessage(std::vector<std::string>* queue,
477-
const std::string& message) {
478-
uv_mutex_lock(&queue_lock_);
469+
const std::string& message) {
470+
Mutex::ScopedLock scoped_lock(queue_lock_);
479471
queue->push_back(message);
480-
uv_mutex_unlock(&queue_lock_);
481472
}
482473

483474
void AgentImpl::SwapBehindLock(std::vector<std::string> AgentImpl::*queue,
484-
std::vector<std::string>* output) {
485-
uv_mutex_lock(&queue_lock_);
475+
std::vector<std::string>* output) {
476+
Mutex::ScopedLock scoped_lock(queue_lock_);
486477
(this->*queue).swap(*output);
487-
uv_mutex_unlock(&queue_lock_);
488478
}
489479

490480
// static

‎src/node.cc‎

Lines changed: 14 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,7 @@ static double prog_start_time;
182182
static bool debugger_running;
183183
static uv_async_t dispatch_debug_messages_async;
184184

185-
static uv_mutex_t node_isolate_mutex;
185+
static Mutex node_isolate_mutex;
186186
static v8::Isolate* node_isolate;
187187
static v8::Platform* default_platform;
188188

@@ -3782,18 +3782,17 @@ static void EnableDebug(Environment* env) {
37823782

37833783
// Called from an arbitrary thread.
37843784
static void TryStartDebugger() {
3785-
uv_mutex_lock(&node_isolate_mutex);
3785+
Mutex::ScopedLock scoped_lock(node_isolate_mutex);
37863786
if (auto isolate = node_isolate) {
37873787
v8::Debug::DebugBreak(isolate);
37883788
uv_async_send(&dispatch_debug_messages_async);
37893789
}
3790-
uv_mutex_unlock(&node_isolate_mutex);
37913790
}
37923791

37933792

37943793
// Called from the main thread.
37953794
static void DispatchDebugMessagesAsyncCallback(uv_async_t* handle) {
3796-
uv_mutex_lock(&node_isolate_mutex);
3795+
Mutex::ScopedLock scoped_lock(node_isolate_mutex);
37973796
if (auto isolate = node_isolate) {
37983797
if (debugger_running == false) {
37993798
fprintf(stderr, "Starting debugger agent.\n");
@@ -3809,7 +3808,6 @@ static void DispatchDebugMessagesAsyncCallback(uv_async_t* handle) {
38093808
Isolate::Scope isolate_scope(isolate);
38103809
v8::Debug::ProcessDebugMessages(isolate);
38113810
}
3812-
uv_mutex_unlock(&node_isolate_mutex);
38133811
}
38143812

38153813

@@ -4143,8 +4141,6 @@ void Init(int* argc,
41434141
// Make inherited handles noninheritable.
41444142
uv_disable_stdio_inheritance();
41454143

4146-
CHECK_EQ(0, uv_mutex_init(&node_isolate_mutex));
4147-
41484144
// init async debug messages dispatching
41494145
// Main thread uses uv_default_loop
41504146
uv_async_init(uv_default_loop(),
@@ -4428,12 +4424,13 @@ static void StartNodeInstance(void* arg) {
44284424
#endif
44294425
Isolate* isolate = Isolate::New(params);
44304426

4431-
uv_mutex_lock(&node_isolate_mutex);
4432-
if (instance_data->is_main()) {
4433-
CHECK_EQ(node_isolate, nullptr);
4434-
node_isolate = isolate;
4427+
{
4428+
Mutex::ScopedLock scoped_lock(node_isolate_mutex);
4429+
if (instance_data->is_main()) {
4430+
CHECK_EQ(node_isolate, nullptr);
4431+
node_isolate = isolate;
4432+
}
44354433
}
4436-
uv_mutex_unlock(&node_isolate_mutex);
44374434

44384435
if (track_heap_objects) {
44394436
isolate->GetHeapProfiler()->StartTrackingHeapObjects(true);
@@ -4503,10 +4500,11 @@ static void StartNodeInstance(void* arg) {
45034500
env = nullptr;
45044501
}
45054502

4506-
uv_mutex_lock(&node_isolate_mutex);
4507-
if (node_isolate == isolate)
4508-
node_isolate = nullptr;
4509-
uv_mutex_unlock(&node_isolate_mutex);
4503+
{
4504+
Mutex::ScopedLock scoped_lock(node_isolate_mutex);
4505+
if (node_isolate == isolate)
4506+
node_isolate = nullptr;
4507+
}
45104508

45114509
CHECK_NE(isolate, nullptr);
45124510
isolate->Dispose();

‎src/node_crypto.cc‎

Lines changed: 5 additions & 11 deletions

0 commit comments

Comments
 (0)