From f1777942c22d0adb1e94f3a12dc2bf4043a01205 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Sun, 9 Jan 2022 19:50:49 +0100 Subject: [PATCH 1/2] src: use std::optional for Worker thread id Refs: https://github.com/nodejs/node/pull/41421 --- src/node_worker.cc | 20 +++++++++++--------- src/node_worker.h | 3 +-- 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/src/node_worker.cc b/src/node_worker.cc index 6e0475511d5770..c9743fcf583a08 100644 --- a/src/node_worker.cc +++ b/src/node_worker.cc @@ -376,10 +376,10 @@ bool Worker::CreateEnvMessagePort(Environment* env) { } void Worker::JoinThread() { - if (thread_joined_) + if (!tid_.has_value()) return; - CHECK_EQ(uv_thread_join(&tid_), 0); - thread_joined_ = true; + CHECK_EQ(uv_thread_join(&tid_.value()), 0); + tid_.reset(); env()->remove_sub_worker_context(this); @@ -406,7 +406,7 @@ void Worker::JoinThread() { MakeCallback(env()->onexit_string(), arraysize(args), args); } - // If we get here, the !thread_joined_ condition at the top of the function + // If we get here, the tid_.has_value() condition at the top of the function // implies that the thread was running. In that case, its final action will // be to schedule a callback on the parent thread which will delete this // object, so there's nothing more to do here. @@ -417,7 +417,7 @@ Worker::~Worker() { CHECK(stopped_); CHECK_NULL(env_); - CHECK(thread_joined_); + CHECK(!tid_.has_value()); Debug(this, "Worker %llu destroyed", thread_id_.id); } @@ -600,7 +600,9 @@ void Worker::StartThread(const FunctionCallbackInfo& args) { uv_thread_options_t thread_options; thread_options.flags = UV_THREAD_HAS_STACK_SIZE; thread_options.stack_size = w->stack_size_; - int ret = uv_thread_create_ex(&w->tid_, &thread_options, [](void* arg) { + + uv_thread_t* tid = &w->tid_.emplace(); // Create uv_thread_t instance + int ret = uv_thread_create_ex(tid, &thread_options, [](void* arg) { // XXX: This could become a std::unique_ptr, but that makes at least // gcc 6.3 detect undefined behaviour when there shouldn't be any. // gcc 7+ handles this well. @@ -627,7 +629,6 @@ void Worker::StartThread(const FunctionCallbackInfo& args) { // The object now owns the created thread and should not be garbage // collected until that finishes. w->ClearWeak(); - w->thread_joined_ = false; if (w->has_ref_) w->env()->add_refs(1); @@ -635,6 +636,7 @@ void Worker::StartThread(const FunctionCallbackInfo& args) { w->env()->add_sub_worker_context(w); } else { w->stopped_ = true; + w->tid_.reset(); char err_buf[128]; uv_err_name_r(ret, err_buf, sizeof(err_buf)); @@ -657,7 +659,7 @@ void Worker::StopThread(const FunctionCallbackInfo& args) { void Worker::Ref(const FunctionCallbackInfo& args) { Worker* w; ASSIGN_OR_RETURN_UNWRAP(&w, args.This()); - if (!w->has_ref_ && !w->thread_joined_) { + if (!w->has_ref_ && w->tid_.has_value()) { w->has_ref_ = true; w->env()->add_refs(1); } @@ -666,7 +668,7 @@ void Worker::Ref(const FunctionCallbackInfo& args) { void Worker::Unref(const FunctionCallbackInfo& args) { Worker* w; ASSIGN_OR_RETURN_UNWRAP(&w, args.This()); - if (w->has_ref_ && !w->thread_joined_) { + if (w->has_ref_ && w->tid_.has_value()) { w->has_ref_ = false; w->env()->add_refs(-1); } diff --git a/src/node_worker.h b/src/node_worker.h index 077d2b8390e6f8..0e6175d6b8c5b1 100644 --- a/src/node_worker.h +++ b/src/node_worker.h @@ -80,14 +80,13 @@ class Worker : public AsyncWrap { MultiIsolatePlatform* platform_; v8::Isolate* isolate_ = nullptr; - uv_thread_t tid_; + std::optional tid_; // Set while the thread is running std::unique_ptr inspector_parent_handle_; // This mutex protects access to all variables listed below it. mutable Mutex mutex_; - bool thread_joined_ = true; const char* custom_error_ = nullptr; std::string custom_error_str_; int exit_code_ = 0; From 8eb8c70febe308dce0e61487ef4beed5d09d594c Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Thu, 13 Jan 2022 21:08:34 +0100 Subject: [PATCH 2/2] fixup! src: use std::optional for Worker thread id --- src/node_worker.h | 1 + 1 file changed, 1 insertion(+) diff --git a/src/node_worker.h b/src/node_worker.h index 0e6175d6b8c5b1..d400c4c991dcbc 100644 --- a/src/node_worker.h +++ b/src/node_worker.h @@ -3,6 +3,7 @@ #if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS +#include #include #include "node_messaging.h" #include "uv.h"