async_hooks,worker: fix process.exit() crashes in async fn with async_hook enabled

src/api/callback.cc

@@ -84,9 +84,13 @@ void InternalCallbackScope::Close() {
   closed_ = true;
 
   if (!env_->can_call_into_js()) return;
-  if (failed_ && !env_->is_main_thread() && env_->is_stopping()) {
-    env_->async_hooks()->clear_async_id_stack();
-  }
+  auto perform_stopping_check = [&]() {
+    if (env_->is_stopping()) {
+      MarkAsFailed();
+      env_->async_hooks()->clear_async_id_stack();
+    }
+  };
+  perform_stopping_check();
 
   if (!failed_ && async_context_.async_id != 0 && !skip_hooks_) {
     AsyncWrap::EmitAfter(env_, async_context_.async_id);
@@ -109,6 +113,8 @@ void InternalCallbackScope::Close() {
 
   if (!tick_info->has_tick_scheduled()) {
     MicrotasksScope::PerformCheckpoint(env_->isolate());
+
+    perform_stopping_check();
   }
 
   // Make sure the stack unwound properly. If there are nested MakeCallback's
@@ -136,6 +142,7 @@ void InternalCallbackScope::Close() {
   if (tick_callback->Call(env_->context(), process, 0, nullptr).IsEmpty()) {
     failed_ = true;
   }
+  perform_stopping_check();
 }
 
 MaybeLocal<Value> InternalMakeCallback(Environment* env,

src/node_worker.cc

@@ -260,6 +260,11 @@ void Worker::Run() {
 
     DeleteFnPtr<Environment, FreeEnvironment> env_;
     auto cleanup_env = OnScopeLeave([&]() {
+      // TODO(addaleax): This call is harmless but should not be necessary.
+      // Figure out why V8 is raising a DCHECK() here without it
+      // (in test/parallel/test-async-hooks-worker-asyncfn-terminate-4.js).
+      isolate_->CancelTerminateExecution();
+
       if (!env_) return;
       env_->set_can_call_into_js(false);
 

test/parallel/test-async-hooks-worker-asyncfn-terminate-1.js

@@ -0,0 +1,15 @@
+'use strict';
+const common = require('../common');
+const { Worker } = require('worker_threads');
+
+const w = new Worker(`
+const { createHook } = require('async_hooks');
+
+setImmediate(async () => {
+  createHook({ init() {} }).enable();
+  await 0;
+  process.exit();
+});
+`, { eval: true });
+
+w.on('exit', common.mustCall());

test/parallel/test-async-hooks-worker-asyncfn-terminate-2.js

@@ -0,0 +1,21 @@
+'use strict';
+const common = require('../common');
+const { Worker } = require('worker_threads');
+
+// Like test-async-hooks-worker-promise.js but with the `await` and `createHook`
+// lines switched, because that resulted in different assertion failures
+// (one a Node.js assertion and one a V8 DCHECK) and it seems prudent to
+// cover both of those failures.
+
+const w = new Worker(`
+const { createHook } = require('async_hooks');
+
+setImmediate(async () => {
+  await 0;
+  createHook({ init() {} }).enable();
+  process.exit();
+});
+`, { eval: true });
+
+w.postMessage({});
+w.on('exit', common.mustCall());

test/parallel/test-async-hooks-worker-asyncfn-terminate-3.js

@@ -0,0 +1,20 @@
+'use strict';
+const common = require('../common');
+const { Worker } = require('worker_threads');
+
+// Like test-async-hooks-worker-promise.js but with an additional statement
+// after the `process.exit()` call, that shouldn’t really make a difference
+// but apparently does.
+
+const w = new Worker(`
+const { createHook } = require('async_hooks');
+
+setImmediate(async () => {
+  createHook({ init() {} }).enable();
+  await 0;
+  process.exit();
+  process._rawDebug('THIS SHOULD NEVER BE REACHED');
+});
+`, { eval: true });
+
+w.on('exit', common.mustCall());

test/parallel/test-async-hooks-worker-asyncfn-terminate-4.js

@@ -0,0 +1,24 @@
+'use strict';
+const common = require('../common');
+const assert = require('assert');
+const { Worker } = require('worker_threads');
+
+// Like test-async-hooks-worker-promise.js but doing a trivial counter increase
+// after process.exit(). This should not make a difference, but apparently it
+// does. This is *also* different from test-async-hooks-worker-promise-3.js,
+// in that the statement is an ArrayBuffer access rather than a full method,
+// which *also* makes a difference even though it shouldn’t.
+
+const workerData = new Int32Array(new SharedArrayBuffer(4));
+const w = new Worker(`
+const { createHook } = require('async_hooks');
+
+setImmediate(async () => {
+  createHook({ init() {} }).enable();
+  await 0;
+  process.exit();
+  workerData[0]++;
+});
+`, { eval: true, workerData });
+
+w.on('exit', common.mustCall(() => assert.strictEqual(workerData[0], 0)));