From 53f19cee9ca613eb53901eaef82a27f230cc1e00 Mon Sep 17 00:00:00 2001
From: rogertyang <rogertyang@tencent.com>
Date: Mon, 20 Feb 2023 17:00:32 +0800
Subject: [PATCH] src: fix cb scope bugs involved in termination

Be more aggresive to clean up the async id stack,
and ensure the cleanup when terminating.

Do not call SetIdle() when terminating to tolerate
https://bugs.chromium.org/p/v8/issues/detail?id=13464
---
 src/api/callback.cc                                 |  4 ++--
 src/api/environment.cc                              |  1 +
 .../test-unhandled-exception-with-worker-inuse.js   | 13 ++++++++-----
 3 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/src/api/callback.cc b/src/api/callback.cc
index 156fd5b6d6ee03..3a8bd9155a8545 100644
--- a/src/api/callback.cc
+++ b/src/api/callback.cc
@@ -101,14 +101,14 @@ void InternalCallbackScope::Close() {
   // async id stack or pop the topmost one from it
 
   auto perform_stopping_check = [&]() {
-    if (!env_->can_call_into_js()) {
+    if (env_->is_stopping()) {
       MarkAsFailed();
       env_->async_hooks()->clear_async_id_stack();
     }
   };
   perform_stopping_check();
 
-  if (!env_->can_call_into_js()) return;
+  if (env_->is_stopping()) return;
 
   Isolate* isolate = env_->isolate();
   auto idle = OnScopeLeave([&]() { isolate->SetIdle(true); });
diff --git a/src/api/environment.cc b/src/api/environment.cc
index 389c6666dc6966..19c50398fa3da5 100644
--- a/src/api/environment.cc
+++ b/src/api/environment.cc
@@ -881,6 +881,7 @@ ThreadId AllocateEnvironmentThreadId() {
 }
 
 void DefaultProcessExitHandlerInternal(Environment* env, ExitCode exit_code) {
+  env->set_stopping(true);
   env->set_can_call_into_js(false);
   env->stop_sub_worker_contexts();
   env->isolate()->DumpAndResetStats();
diff --git a/test/parallel/test-unhandled-exception-with-worker-inuse.js b/test/parallel/test-unhandled-exception-with-worker-inuse.js
index 463aafad6a76b4..a3e823ca70bf0f 100644
--- a/test/parallel/test-unhandled-exception-with-worker-inuse.js
+++ b/test/parallel/test-unhandled-exception-with-worker-inuse.js
@@ -3,14 +3,17 @@ const common = require('../common');
 
 // https://github.com/nodejs/node/issues/45421
 //
-// Check that node will not call v8::Isolate::SetIdle() when exiting
+// Check that node will NOT call v8::Isolate::SetIdle() when exiting
 // due to an unhandled exception, otherwise the assertion(enabled in
-// debug build only) in the SetIdle() will fail.
+// debug build only) in the SetIdle(), which checks that the vm state
+// is either EXTERNAL or IDLE will fail.
 //
 // The root cause of this issue is that before PerIsolateMessageListener()
-// is invoked by v8, v8 preserves the vm state, which is JS. However,
-// SetIdle() requires the vm state is either EXTERNAL or IDLE when embedder
-// calling it.
+// is invoked by v8, v8 preserves the JS vm state, although it should
+// switch to EXTERNEL. https://bugs.chromium.org/p/v8/issues/detail?id=13464
+//
+// Therefore, this commit can be considered as an workaround of the v8 bug,
+// but we also find it not useful to call SetIdle() when terminating.
 
 if (process.argv[2] === 'child') {
   const { Worker } = require('worker_threads');