Skip to content

Commit

Permalink
src: use std::optional for Worker thread id
Browse files Browse the repository at this point in the history
  • Loading branch information
addaleax committed Jan 9, 2022
1 parent f17b73c commit 2f968dc
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 11 deletions.
20 changes: 11 additions & 9 deletions src/node_worker.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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.
Expand All @@ -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);
}
Expand Down Expand Up @@ -600,7 +600,9 @@ void Worker::StartThread(const FunctionCallbackInfo<Value>& 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) {

w->tid_.emplace(); // Create uv_thread_t instance
int ret = uv_thread_create_ex(&w->tid_.value(), &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.
Expand All @@ -627,14 +629,14 @@ void Worker::StartThread(const FunctionCallbackInfo<Value>& 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);

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));
Expand All @@ -657,7 +659,7 @@ void Worker::StopThread(const FunctionCallbackInfo<Value>& args) {
void Worker::Ref(const FunctionCallbackInfo<Value>& 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);
}
Expand All @@ -666,7 +668,7 @@ void Worker::Ref(const FunctionCallbackInfo<Value>& args) {
void Worker::Unref(const FunctionCallbackInfo<Value>& 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);
}
Expand Down
3 changes: 1 addition & 2 deletions src/node_worker.h
Original file line number Diff line number Diff line change
Expand Up @@ -80,14 +80,13 @@ class Worker : public AsyncWrap {

MultiIsolatePlatform* platform_;
v8::Isolate* isolate_ = nullptr;
uv_thread_t tid_;
std::optional<uv_thread_t> tid_; // Set while the thread is running

std::unique_ptr<InspectorParentHandle> 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;
Expand Down

0 comments on commit 2f968dc

Please sign in to comment.