Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit ec9071f

Browse filesBrowse files
addaleaxtargos
authored andcommitted
src: use std::optional for Worker thread id
Refs: #41421 PR-URL: #41453 Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Michael Dawson <midawson@redhat.com> Reviewed-By: Darshan Sen <raisinten@gmail.com>
1 parent 106ef0c commit ec9071f
Copy full SHA for ec9071f

File tree

Expand file treeCollapse file tree

2 files changed

+13
-11
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

2 files changed

+13
-11
lines changed
Open diff view settings
Collapse file

‎src/node_worker.cc‎

Copy file name to clipboardExpand all lines: src/node_worker.cc
+11-9Lines 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
}
@@ -598,7 +598,9 @@ void Worker::StartThread(const FunctionCallbackInfo<Value>& args) {
598598
uv_thread_options_t thread_options;
599599
thread_options.flags = UV_THREAD_HAS_STACK_SIZE;
600600
thread_options.stack_size = w->stack_size_;
601-
int ret = uv_thread_create_ex(&w->tid_, &thread_options, [](void* arg) {
601+
602+
uv_thread_t* tid = &w->tid_.emplace(); // Create uv_thread_t instance
603+
int ret = uv_thread_create_ex(tid, &thread_options, [](void* arg) {
602604
// XXX: This could become a std::unique_ptr, but that makes at least
603605
// gcc 6.3 detect undefined behaviour when there shouldn't be any.
604606
// gcc 7+ handles this well.
@@ -625,14 +627,14 @@ void Worker::StartThread(const FunctionCallbackInfo<Value>& args) {
625627
// The object now owns the created thread and should not be garbage
626628
// collected until that finishes.
627629
w->ClearWeak();
628-
w->thread_joined_ = false;
629630

630631
if (w->has_ref_)
631632
w->env()->add_refs(1);
632633

633634
w->env()->add_sub_worker_context(w);
634635
} else {
635636
w->stopped_ = true;
637+
w->tid_.reset();
636638

637639
char err_buf[128];
638640
uv_err_name_r(ret, err_buf, sizeof(err_buf));
@@ -655,7 +657,7 @@ void Worker::StopThread(const FunctionCallbackInfo<Value>& args) {
655657
void Worker::Ref(const FunctionCallbackInfo<Value>& args) {
656658
Worker* w;
657659
ASSIGN_OR_RETURN_UNWRAP(&w, args.This());
658-
if (!w->has_ref_ && !w->thread_joined_) {
660+
if (!w->has_ref_ && w->tid_.has_value()) {
659661
w->has_ref_ = true;
660662
w->env()->add_refs(1);
661663
}
@@ -664,7 +666,7 @@ void Worker::Ref(const FunctionCallbackInfo<Value>& args) {
664666
void Worker::Unref(const FunctionCallbackInfo<Value>& args) {
665667
Worker* w;
666668
ASSIGN_OR_RETURN_UNWRAP(&w, args.This());
667-
if (w->has_ref_ && !w->thread_joined_) {
669+
if (w->has_ref_ && w->tid_.has_value()) {
668670
w->has_ref_ = false;
669671
w->env()->add_refs(-1);
670672
}
Collapse file

‎src/node_worker.h‎

Copy file name to clipboardExpand all lines: src/node_worker.h
+2-2Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
#if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
55

6+
#include <optional>
67
#include <unordered_map>
78
#include "node_messaging.h"
89
#include "uv.h"
@@ -80,14 +81,13 @@ class Worker : public AsyncWrap {
8081

8182
MultiIsolatePlatform* platform_;
8283
v8::Isolate* isolate_ = nullptr;
83-
uv_thread_t tid_;
84+
std::optional<uv_thread_t> tid_; // Set while the thread is running
8485

8586
std::unique_ptr<InspectorParentHandle> inspector_parent_handle_;
8687

8788
// This mutex protects access to all variables listed below it.
8889
mutable Mutex mutex_;
8990

90-
bool thread_joined_ = true;
9191
const char* custom_error_ = nullptr;
9292
std::string custom_error_str_;
9393
int exit_code_ = 0;

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.