From f536cc2e89092c60bddfc9953838aeab432da9b9 Mon Sep 17 00:00:00 2001
From: legendecas <legendecas@gmail.com>
Date: Tue, 11 May 2021 00:45:24 +0800
Subject: [PATCH] src: fix fatal errors when a current isolate not exist

napi_fatal_error and node watchdog trigger fatal error but rather
running on a thread that hold no current isolate.

PR-URL: https://github.com/nodejs/node/pull/38624
Reviewed-By: Michael Dawson <midawson@redhat.com>
---
 src/node_errors.cc                       |  6 ++++++
 test/node-api/test_fatal/test.js         |  1 +
 test/node-api/test_fatal/test2.js        |  1 +
 test/node-api/test_fatal/test_fatal.c    | 21 +++++++++++++++++++++
 test/node-api/test_fatal/test_threads.js | 20 ++++++++++++++++++++
 5 files changed, 49 insertions(+)
 create mode 100644 test/node-api/test_fatal/test_threads.js

diff --git a/src/node_errors.cc b/src/node_errors.cc
index ab8b513cceee5d..47b13f6149545b 100644
--- a/src/node_errors.cc
+++ b/src/node_errors.cc
@@ -425,6 +425,12 @@ void OnFatalError(const char* location, const char* message) {
   }
 
   Isolate* isolate = Isolate::GetCurrent();
+  // TODO(legendecas): investigate failures on triggering node-report with
+  // nullptr isolates.
+  if (isolate == nullptr) {
+    fflush(stderr);
+    ABORT();
+  }
   Environment* env = Environment::GetCurrent(isolate);
   bool report_on_fatalerror;
   {
diff --git a/test/node-api/test_fatal/test.js b/test/node-api/test_fatal/test.js
index 7ff9a395635dce..c4af828ff154d1 100644
--- a/test/node-api/test_fatal/test.js
+++ b/test/node-api/test_fatal/test.js
@@ -16,3 +16,4 @@ const p = child_process.spawnSync(
 assert.ifError(p.error);
 assert.ok(p.stderr.toString().includes(
   'FATAL ERROR: test_fatal::Test fatal message'));
+assert.ok(p.status === 134 || p.signal === 'SIGABRT');
diff --git a/test/node-api/test_fatal/test2.js b/test/node-api/test_fatal/test2.js
index b9bde8f13016cc..6449f098458d76 100644
--- a/test/node-api/test_fatal/test2.js
+++ b/test/node-api/test_fatal/test2.js
@@ -16,3 +16,4 @@ const p = child_process.spawnSync(
 assert.ifError(p.error);
 assert.ok(p.stderr.toString().includes(
   'FATAL ERROR: test_fatal::Test fatal message'));
+assert.ok(p.status === 134 || p.signal === 'SIGABRT');
diff --git a/test/node-api/test_fatal/test_fatal.c b/test/node-api/test_fatal/test_fatal.c
index 9072bfd080bc2c..aebbda76888f4f 100644
--- a/test/node-api/test_fatal/test_fatal.c
+++ b/test/node-api/test_fatal/test_fatal.c
@@ -1,12 +1,32 @@
+// For the purpose of this test we use libuv's threading library. When deciding
+// on a threading library for a new project it bears remembering that in the
+// future libuv may introduce API changes which may render it non-ABI-stable,
+// which, in turn, may affect the ABI stability of the project despite its use
+// of N-API.
+#include <uv.h>
 #include <node_api.h>
 #include "../../js-native-api/common.h"
 
+static uv_thread_t uv_thread;
+
+static void work_thread(void* data) {
+  napi_fatal_error("work_thread", NAPI_AUTO_LENGTH,
+      "foobar", NAPI_AUTO_LENGTH);
+}
+
 static napi_value Test(napi_env env, napi_callback_info info) {
   napi_fatal_error("test_fatal::Test", NAPI_AUTO_LENGTH,
                    "fatal message", NAPI_AUTO_LENGTH);
   return NULL;
 }
 
+static napi_value TestThread(napi_env env, napi_callback_info info) {
+  NODE_API_ASSERT(env,
+      (uv_thread_create(&uv_thread, work_thread, NULL) == 0),
+      "Thread creation");
+  return NULL;
+}
+
 static napi_value TestStringLength(napi_env env, napi_callback_info info) {
   napi_fatal_error("test_fatal::TestStringLength", 16, "fatal message", 13);
   return NULL;
@@ -16,6 +36,7 @@ static napi_value Init(napi_env env, napi_value exports) {
   napi_property_descriptor properties[] = {
     DECLARE_NODE_API_PROPERTY("Test", Test),
     DECLARE_NODE_API_PROPERTY("TestStringLength", TestStringLength),
+    DECLARE_NODE_API_PROPERTY("TestThread", TestThread),
   };
 
   NODE_API_CALL(env, napi_define_properties(
diff --git a/test/node-api/test_fatal/test_threads.js b/test/node-api/test_fatal/test_threads.js
new file mode 100644
index 00000000000000..fd56f60cbe832f
--- /dev/null
+++ b/test/node-api/test_fatal/test_threads.js
@@ -0,0 +1,20 @@
+'use strict';
+const common = require('../../common');
+const assert = require('assert');
+const child_process = require('child_process');
+const test_fatal = require(`./build/${common.buildType}/test_fatal`);
+
+// Test in a child process because the test code will trigger a fatal error
+// that crashes the process.
+if (process.argv[2] === 'child') {
+  test_fatal.TestThread();
+  // Busy loop to allow the work thread to abort.
+  while (true) {}
+}
+
+const p = child_process.spawnSync(
+  process.execPath, [ __filename, 'child' ]);
+assert.ifError(p.error);
+assert.ok(p.stderr.toString().includes(
+  'FATAL ERROR: work_thread foobar'));
+assert.ok(p.status === 134 || p.signal === 'SIGABRT');