From 3afca3a95330e56e4357a026a9226f5de42e30a5 Mon Sep 17 00:00:00 2001 From: Ali Ijaz Sheikh Date: Wed, 26 Sep 2018 15:58:45 -0700 Subject: [PATCH 1/3] src: ready background workers before bootstrap Make sure background workers are ready before proceeding with the bootstrap or post-bootstrap execution of any code that may trigger `process.exit()`. Fixes: https://github.com/nodejs/node/issues/23065 --- src/node_platform.cc | 37 +++++++++++++++++++++++++++++++++++-- 1 file changed, 35 insertions(+), 2 deletions(-) diff --git a/src/node_platform.cc b/src/node_platform.cc index 1c237159f2d2e9..f39021f50443d6 100644 --- a/src/node_platform.cc +++ b/src/node_platform.cc @@ -18,10 +18,30 @@ using v8::TracingController; namespace { +static Mutex platform_workers_mutex; +static ConditionVariable platform_workers_ready; +static int pending_platform_workers; + +struct PlatformWorkerData { + TaskQueue* task_queue; + int id; +}; + static void PlatformWorkerThread(void* data) { + std::unique_ptr + worker_data(static_cast(data)); + + TaskQueue* pending_worker_tasks = worker_data->task_queue; TRACE_EVENT_METADATA1("__metadata", "thread_name", "name", "PlatformWorkerThread"); - TaskQueue* pending_worker_tasks = static_cast*>(data); + + // Notify the main thread that the platform worker is ready. + { + Mutex::ScopedLock lock(platform_workers_mutex); + pending_platform_workers--; + platform_workers_ready.Signal(lock); + } + while (std::unique_ptr task = pending_worker_tasks->BlockingPop()) { task->Run(); pending_worker_tasks->NotifyOfCompletion(); @@ -148,17 +168,30 @@ class WorkerThreadsTaskRunner::DelayedTaskScheduler { }; WorkerThreadsTaskRunner::WorkerThreadsTaskRunner(int thread_pool_size) { + Mutex::ScopedLock lock(platform_workers_mutex); + pending_platform_workers = thread_pool_size; + delayed_task_scheduler_.reset( new DelayedTaskScheduler(&pending_worker_tasks_)); threads_.push_back(delayed_task_scheduler_->Start()); + for (int i = 0; i < thread_pool_size; i++) { + PlatformWorkerData* worker_data = new PlatformWorkerData{ + &pending_worker_tasks_, i + }; std::unique_ptr t { new uv_thread_t() }; if (uv_thread_create(t.get(), PlatformWorkerThread, - &pending_worker_tasks_) != 0) { + worker_data) != 0) { break; } threads_.push_back(std::move(t)); } + + // Wait for platform workers to initialize before continuing with the + // bootstrap. + while (pending_platform_workers > 0) { + platform_workers_ready.Wait(lock); + } } void WorkerThreadsTaskRunner::PostTask(std::unique_ptr task) { From ebca997d3fd7feadc8ab4987d2ac19b7b89134c7 Mon Sep 17 00:00:00 2001 From: Ali Ijaz Sheikh Date: Wed, 3 Oct 2018 16:24:30 -0700 Subject: [PATCH 2/3] [squash] address addaleax comments --- src/node_platform.cc | 24 ++++++++++++------------ src/node_platform.h | 4 ++++ 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/src/node_platform.cc b/src/node_platform.cc index f39021f50443d6..eb2cc72211cf56 100644 --- a/src/node_platform.cc +++ b/src/node_platform.cc @@ -18,12 +18,11 @@ using v8::TracingController; namespace { -static Mutex platform_workers_mutex; -static ConditionVariable platform_workers_ready; -static int pending_platform_workers; - struct PlatformWorkerData { TaskQueue* task_queue; + Mutex* platform_workers_mutex; + ConditionVariable* platform_workers_ready; + int* pending_platform_workers; int id; }; @@ -37,9 +36,9 @@ static void PlatformWorkerThread(void* data) { // Notify the main thread that the platform worker is ready. { - Mutex::ScopedLock lock(platform_workers_mutex); - pending_platform_workers--; - platform_workers_ready.Signal(lock); + Mutex::ScopedLock lock(*worker_data->platform_workers_mutex); + *worker_data->pending_platform_workers--; + worker_data->platform_workers_ready->Signal(lock); } while (std::unique_ptr task = pending_worker_tasks->BlockingPop()) { @@ -168,8 +167,8 @@ class WorkerThreadsTaskRunner::DelayedTaskScheduler { }; WorkerThreadsTaskRunner::WorkerThreadsTaskRunner(int thread_pool_size) { - Mutex::ScopedLock lock(platform_workers_mutex); - pending_platform_workers = thread_pool_size; + Mutex::ScopedLock lock(platform_workers_mutex_); + pending_platform_workers_ = thread_pool_size; delayed_task_scheduler_.reset( new DelayedTaskScheduler(&pending_worker_tasks_)); @@ -177,7 +176,8 @@ WorkerThreadsTaskRunner::WorkerThreadsTaskRunner(int thread_pool_size) { for (int i = 0; i < thread_pool_size; i++) { PlatformWorkerData* worker_data = new PlatformWorkerData{ - &pending_worker_tasks_, i + &pending_worker_tasks_, &platform_workers_mutex_, + &platform_workers_ready_, &pending_platform_workers_, i }; std::unique_ptr t { new uv_thread_t() }; if (uv_thread_create(t.get(), PlatformWorkerThread, @@ -189,8 +189,8 @@ WorkerThreadsTaskRunner::WorkerThreadsTaskRunner(int thread_pool_size) { // Wait for platform workers to initialize before continuing with the // bootstrap. - while (pending_platform_workers > 0) { - platform_workers_ready.Wait(lock); + while (pending_platform_workers_ > 0) { + platform_workers_ready_.Wait(lock); } } diff --git a/src/node_platform.h b/src/node_platform.h index 9b9720f63804d5..197d5cee3968a7 100644 --- a/src/node_platform.h +++ b/src/node_platform.h @@ -116,6 +116,10 @@ class WorkerThreadsTaskRunner { std::unique_ptr delayed_task_scheduler_; std::vector> threads_; + + Mutex platform_workers_mutex_; + ConditionVariable platform_workers_ready_; + int pending_platform_workers_; }; class NodePlatform : public MultiIsolatePlatform { From 89a7d1b7e1b69f45e75469dba41b9f7a04701bb4 Mon Sep 17 00:00:00 2001 From: Ali Ijaz Sheikh Date: Wed, 3 Oct 2018 17:29:42 -0700 Subject: [PATCH 3/3] [squash] operator precedence --- src/node_platform.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/node_platform.cc b/src/node_platform.cc index eb2cc72211cf56..b583ce85423fb1 100644 --- a/src/node_platform.cc +++ b/src/node_platform.cc @@ -37,7 +37,7 @@ static void PlatformWorkerThread(void* data) { // Notify the main thread that the platform worker is ready. { Mutex::ScopedLock lock(*worker_data->platform_workers_mutex); - *worker_data->pending_platform_workers--; + (*worker_data->pending_platform_workers)--; worker_data->platform_workers_ready->Signal(lock); }