From 18ecaebdbb5446107ebcf56254f61588dacde45d Mon Sep 17 00:00:00 2001
From: Anna Henningsen <anna@addaleax.net>
Date: Mon, 27 Apr 2020 03:36:23 +0200
Subject: [PATCH] worker: unify custom error creation

Mostly, this introduces a pattern that makes sure that if a custom
error is reported, `stopped_` will be set to `true` correctly in
every cast, which was previously missing for the
`NewContext().IsEmpty()` case (which led to a hard crash from the
`Worker` destructor).

This also leaves TODO comments for a few cases in which
`ERR_WORKER_OUT_OF_MEMORY` was not used in accordance with the
documentation for that error code (or according to its intention).
Fixing that is semver-major.

Backport-PR-URL: https://github.com/nodejs/node/pull/35241
PR-URL: https://github.com/nodejs/node/pull/33084
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
---
 src/node_worker.cc | 30 +++++++++++++++++-------------
 src/node_worker.h  |  7 +++++--
 2 files changed, 22 insertions(+), 15 deletions(-)

diff --git a/src/node_worker.cc b/src/node_worker.cc
index bd43a9cfbab97f..db809d12f81310 100644
--- a/src/node_worker.cc
+++ b/src/node_worker.cc
@@ -136,9 +136,7 @@ class WorkerThreadData {
     if (ret != 0) {
       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->stopped_ = true;
+      w->Exit(1, "ERR_WORKER_INIT_FAILED", err_buf);
       return;
     }
     loop_init_failed_ = false;
@@ -152,9 +150,9 @@ class WorkerThreadData {
 
     Isolate* isolate = Isolate::Allocate();
     if (isolate == nullptr) {
-      w->custom_error_ = "ERR_WORKER_OUT_OF_MEMORY";
-      w->custom_error_str_ = "Failed to create new Isolate";
-      w->stopped_ = true;
+      // TODO(addaleax): This should be ERR_WORKER_INIT_FAILED,
+      // ERR_WORKER_OUT_OF_MEMORY is for reaching the per-Worker heap limit.
+      w->Exit(1, "ERR_WORKER_OUT_OF_MEMORY", "Failed to create new Isolate");
       return;
     }
 
@@ -237,9 +235,7 @@ class WorkerThreadData {
 size_t Worker::NearHeapLimit(void* data, size_t current_heap_limit,
                              size_t initial_heap_limit) {
   Worker* worker = static_cast<Worker*>(data);
-  worker->custom_error_ = "ERR_WORKER_OUT_OF_MEMORY";
-  worker->custom_error_str_ = "JS heap out of memory";
-  worker->Exit(1);
+  worker->Exit(1, "ERR_WORKER_OUT_OF_MEMORY", "JS heap out of memory");
   // Give the current GC some extra leeway to let it finish rather than
   // crash hard. We are not going to perform further allocations anyway.
   constexpr size_t kExtraHeapAllowance = 16 * 1024 * 1024;
@@ -301,8 +297,9 @@ void Worker::Run() {
         TryCatch try_catch(isolate_);
         context = NewContext(isolate_);
         if (context.IsEmpty()) {
-          custom_error_ = "ERR_WORKER_OUT_OF_MEMORY";
-          custom_error_str_ = "Failed to create new Context";
+          // TODO(addaleax): This should be ERR_WORKER_INIT_FAILED,
+          // ERR_WORKER_OUT_OF_MEMORY is for reaching the per-Worker heap limit.
+          Exit(1, "ERR_WORKER_OUT_OF_MEMORY", "Failed to create new Context");
           return;
         }
       }
@@ -685,9 +682,16 @@ Local<Float64Array> Worker::GetResourceLimits(Isolate* isolate) const {
   return Float64Array::New(ab, 0, kTotalResourceLimitCount);
 }
 
-void Worker::Exit(int code) {
+void Worker::Exit(int code, const char* error_code, const char* error_message) {
   Mutex::ScopedLock lock(mutex_);
-  Debug(this, "Worker %llu called Exit(%d)", thread_id_.id, code);
+  Debug(this, "Worker %llu called Exit(%d, %s, %s)",
+        thread_id_.id, code, error_code, error_message);
+
+  if (error_code != nullptr) {
+    custom_error_ = error_code;
+    custom_error_str_ = error_message;
+  }
+
   if (env_ != nullptr) {
     exit_code_ = code;
     Stop(env_);
diff --git a/src/node_worker.h b/src/node_worker.h
index a7d02914826563..069bd190fca5f7 100644
--- a/src/node_worker.h
+++ b/src/node_worker.h
@@ -34,8 +34,11 @@ class Worker : public AsyncWrap {
   void Run();
 
   // Forcibly exit the thread with a specified exit code. This may be called
-  // from any thread.
-  void Exit(int code);
+  // from any thread. `error_code` and `error_message` can be used to create
+  // a custom `'error'` event before emitting `'exit'`.
+  void Exit(int code,
+            const char* error_code = nullptr,
+            const char* error_message = nullptr);
 
   // Wait for the worker thread to stop (in a blocking manner).
   void JoinThread();