Skip to content

Commit f177794

Browse files
committed
src: use std::optional for Worker thread id
Refs: nodejs#41421
1 parent f17b73c commit f177794

File tree

2 files changed

+12
-11
lines changed

2 files changed

+12
-11
lines changed

src/node_worker.cc

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -376,10 +376,10 @@ bool Worker::CreateEnvMessagePort(Environment* env) {
376376
}
377377

378378
void Worker::JoinThread() {
379-
if (thread_joined_)
379+
if (!tid_.has_value())
380380
return;
381-
CHECK_EQ(uv_thread_join(&tid_), 0);
382-
thread_joined_ = true;
381+
CHECK_EQ(uv_thread_join(&tid_.value()), 0);
382+
tid_.reset();
383383

384384
env()->remove_sub_worker_context(this);
385385

@@ -406,7 +406,7 @@ void Worker::JoinThread() {
406406
MakeCallback(env()->onexit_string(), arraysize(args), args);
407407
}
408408

409-
// If we get here, the !thread_joined_ condition at the top of the function
409+
// If we get here, the tid_.has_value() condition at the top of the function
410410
// implies that the thread was running. In that case, its final action will
411411
// be to schedule a callback on the parent thread which will delete this
412412
// object, so there's nothing more to do here.
@@ -417,7 +417,7 @@ Worker::~Worker() {
417417

418418
CHECK(stopped_);
419419
CHECK_NULL(env_);
420-
CHECK(thread_joined_);
420+
CHECK(!tid_.has_value());
421421

422422
Debug(this, "Worker %llu destroyed", thread_id_.id);
423423
}
@@ -600,7 +600,9 @@ void Worker::StartThread(const FunctionCallbackInfo<Value>& args) {
600600
uv_thread_options_t thread_options;
601601
thread_options.flags = UV_THREAD_HAS_STACK_SIZE;
602602
thread_options.stack_size = w->stack_size_;
603-
int ret = uv_thread_create_ex(&w->tid_, &thread_options, [](void* arg) {
603+
604+
uv_thread_t* tid = &w->tid_.emplace(); // Create uv_thread_t instance
605+
int ret = uv_thread_create_ex(tid, &thread_options, [](void* arg) {
604606
// XXX: This could become a std::unique_ptr, but that makes at least
605607
// gcc 6.3 detect undefined behaviour when there shouldn't be any.
606608
// gcc 7+ handles this well.
@@ -627,14 +629,14 @@ void Worker::StartThread(const FunctionCallbackInfo<Value>& args) {
627629
// The object now owns the created thread and should not be garbage
628630
// collected until that finishes.
629631
w->ClearWeak();
630-
w->thread_joined_ = false;
631632

632633
if (w->has_ref_)
633634
w->env()->add_refs(1);
634635

635636
w->env()->add_sub_worker_context(w);
636637
} else {
637638
w->stopped_ = true;
639+
w->tid_.reset();
638640

639641
char err_buf[128];
640642
uv_err_name_r(ret, err_buf, sizeof(err_buf));
@@ -657,7 +659,7 @@ void Worker::StopThread(const FunctionCallbackInfo<Value>& args) {
657659
void Worker::Ref(const FunctionCallbackInfo<Value>& args) {
658660
Worker* w;
659661
ASSIGN_OR_RETURN_UNWRAP(&w, args.This());
660-
if (!w->has_ref_ && !w->thread_joined_) {
662+
if (!w->has_ref_ && w->tid_.has_value()) {
661663
w->has_ref_ = true;
662664
w->env()->add_refs(1);
663665
}
@@ -666,7 +668,7 @@ void Worker::Ref(const FunctionCallbackInfo<Value>& args) {
666668
void Worker::Unref(const FunctionCallbackInfo<Value>& args) {
667669
Worker* w;
668670
ASSIGN_OR_RETURN_UNWRAP(&w, args.This());
669-
if (w->has_ref_ && !w->thread_joined_) {
671+
if (w->has_ref_ && w->tid_.has_value()) {
670672
w->has_ref_ = false;
671673
w->env()->add_refs(-1);
672674
}

src/node_worker.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,14 +80,13 @@ class Worker : public AsyncWrap {
8080

8181
MultiIsolatePlatform* platform_;
8282
v8::Isolate* isolate_ = nullptr;
83-
uv_thread_t tid_;
83+
std::optional<uv_thread_t> tid_; // Set while the thread is running
8484

8585
std::unique_ptr<InspectorParentHandle> inspector_parent_handle_;
8686

8787
// This mutex protects access to all variables listed below it.
8888
mutable Mutex mutex_;
8989

90-
bool thread_joined_ = true;
9190
const char* custom_error_ = nullptr;
9291
std::string custom_error_str_;
9392
int exit_code_ = 0;

0 commit comments

Comments
 (0)