From 09d346994ec8dc1a5240e16ae7df1f9ddb2533c9 Mon Sep 17 00:00:00 2001
From: Anna Henningsen <anna@addaleax.net>
Date: Mon, 30 Mar 2020 10:35:16 +0200
Subject: [PATCH 1/2] src: clean up worker thread creation code

Instead of setting and then in the case of error un-setting properties,
only set them when no error occurs.

Refs: https://github.com/nodejs/node/pull/32344
---
 src/node_worker.cc | 31 ++++++++++++++-----------------
 1 file changed, 14 insertions(+), 17 deletions(-)

diff --git a/src/node_worker.cc b/src/node_worker.cc
index 26c6ce11cef677..432425c116d52a 100644
--- a/src/node_worker.cc
+++ b/src/node_worker.cc
@@ -588,16 +588,7 @@ void Worker::StartThread(const FunctionCallbackInfo<Value>& args) {
   ASSIGN_OR_RETURN_UNWRAP(&w, args.This());
   Mutex::ScopedLock lock(w->mutex_);
 
-  // The object now owns the created thread and should not be garbage collected
-  // until that finishes.
-  w->ClearWeak();
-
-  w->env()->add_sub_worker_context(w);
   w->stopped_ = false;
-  w->thread_joined_ = false;
-
-  if (w->has_ref_)
-    w->env()->add_refs(1);
 
   uv_thread_options_t thread_options;
   thread_options.flags = UV_THREAD_HAS_STACK_SIZE;
@@ -624,21 +615,27 @@ void Worker::StartThread(const FunctionCallbackInfo<Value>& args) {
           // implicitly delete w
         });
   }, static_cast<void*>(w));
-  if (ret != 0) {
+
+  if (ret == 0) {
+    // 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;
+
     char err_buf[128];
     uv_err_name_r(ret, err_buf, sizeof(err_buf));
-    w->custom_error_ = "ERR_WORKER_INIT_FAILED";
-    w->custom_error_str_ = err_buf;
-    w->loop_init_failed_ = true;
-    w->thread_joined_ = true;
-    w->stopped_ = true;
-    w->env()->remove_sub_worker_context(w);
     {
       Isolate* isolate = w->env()->isolate();
       HandleScope handle_scope(isolate);
       THROW_ERR_WORKER_INIT_FAILED(isolate, err_buf);
     }
-    w->MakeWeak();
   }
 }
 

From 09003e3e952638d96ce41433989807593eba812a Mon Sep 17 00:00:00 2001
From: Anna Henningsen <anna@addaleax.net>
Date: Mon, 30 Mar 2020 10:37:57 +0200
Subject: [PATCH 2/2] src: remove loop_init_failed_ from Worker class
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

There’s no reason for this to not be stored alongside the loop
data structure itself.
---
 src/node_worker.cc | 11 +++++++----
 src/node_worker.h  |  1 -
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/src/node_worker.cc b/src/node_worker.cc
index 432425c116d52a..b3becc491cd31f 100644
--- a/src/node_worker.cc
+++ b/src/node_worker.cc
@@ -135,10 +135,10 @@ class WorkerThreadData {
       uv_err_name_r(ret, err_buf, sizeof(err_buf));
       w->custom_error_ = "ERR_WORKER_INIT_FAILED";
       w->custom_error_str_ = err_buf;
-      w->loop_init_failed_ = true;
       w->stopped_ = true;
       return;
     }
+    loop_init_failed_ = false;
 
     std::shared_ptr<ArrayBufferAllocator> allocator =
         ArrayBufferAllocator::Create();
@@ -194,6 +194,7 @@ class WorkerThreadData {
     }
 
     if (isolate != nullptr) {
+      CHECK(!loop_init_failed_);
       bool platform_finished = false;
 
       isolate_data_.reset();
@@ -212,18 +213,20 @@ class WorkerThreadData {
 
       // Wait until the platform has cleaned up all relevant resources.
       while (!platform_finished) {
-        CHECK(!w_->loop_init_failed_);
         uv_run(&loop_, UV_RUN_ONCE);
       }
     }
-    if (!w_->loop_init_failed_) {
+    if (!loop_init_failed_) {
       CheckedUvLoopClose(&loop_);
     }
   }
 
+  bool loop_is_usable() const { return !loop_init_failed_; }
+
  private:
   Worker* const w_;
   uv_loop_t loop_;
+  bool loop_init_failed_ = true;
   DeleteFnPtr<IsolateData, FreeIsolateData> isolate_data_;
 
   friend class Worker;
@@ -253,7 +256,7 @@ void Worker::Run() {
 
   WorkerThreadData data(this);
   if (isolate_ == nullptr) return;
-  CHECK(!data.w_->loop_init_failed_);
+  CHECK(data.loop_is_usable());
 
   Debug(this, "Starting worker with id %llu", thread_id_.id);
   {
diff --git a/src/node_worker.h b/src/node_worker.h
index 384a9f160e09e8..b962e5757a6f75 100644
--- a/src/node_worker.h
+++ b/src/node_worker.h
@@ -83,7 +83,6 @@ class Worker : public AsyncWrap {
   bool thread_joined_ = true;
   const char* custom_error_ = nullptr;
   std::string custom_error_str_;
-  bool loop_init_failed_ = false;
   int exit_code_ = 0;
   ThreadId thread_id_;
   uintptr_t stack_base_ = 0;