From 965bcb8e6cce782d247e6c12ce68e42c777c29ee Mon Sep 17 00:00:00 2001
From: Santiago Gimeno <santiago.gimeno@gmail.com>
Date: Fri, 16 Sep 2022 13:25:16 +0200
Subject: [PATCH] src: avoid using v8 on Isolate termination

Fix multiple instances of those uncovered while running the tests on
debug builds.

Fixes: https://github.com/nodejs/node-v8/issues/227
---
 src/env.cc           | 26 ++++++++++++++++++--------
 src/node_http2.cc    | 24 ++++++++++++++----------
 src/node_platform.cc |  2 ++
 3 files changed, 34 insertions(+), 18 deletions(-)

diff --git a/src/env.cc b/src/env.cc
index 1dbf45bc7493a8..03e22a3146b7b0 100644
--- a/src/env.cc
+++ b/src/env.cc
@@ -163,14 +163,17 @@ bool AsyncHooks::pop_async_context(double async_id) {
 }
 
 void AsyncHooks::clear_async_id_stack() {
-  Isolate* isolate = env()->isolate();
-  HandleScope handle_scope(isolate);
-  if (!js_execution_async_resources_.IsEmpty()) {
-    USE(PersistentToLocal::Strong(js_execution_async_resources_)
-            ->Set(env()->context(),
-                  env()->length_string(),
-                  Integer::NewFromUnsigned(isolate, 0)));
+  if (env()->can_call_into_js()) {
+    Isolate* isolate = env()->isolate();
+    HandleScope handle_scope(isolate);
+    if (!js_execution_async_resources_.IsEmpty()) {
+      USE(PersistentToLocal::Strong(js_execution_async_resources_)
+              ->Set(env()->context(),
+                    env()->length_string(),
+                    Integer::NewFromUnsigned(isolate, 0)));
+    }
   }
+
   native_execution_async_resources_.clear();
   native_execution_async_resources_.shrink_to_fit();
 
@@ -1046,7 +1049,14 @@ void Environment::RunAndClearNativeImmediates(bool only_refed) {
   TRACE_EVENT0(TRACING_CATEGORY_NODE1(environment),
                "RunAndClearNativeImmediates");
   HandleScope handle_scope(isolate_);
-  InternalCallbackScope cb_scope(this, Object::New(isolate_), { 0, 0 });
+  // In case the Isolate is no longer accessible just use an empty Local. This
+  // is not an issue for InternalCallbackScope as this case is already handled
+  // in its constructor but we avoid calls into v8 which can crash the process
+  // in debug builds.
+  Local<Object> obj = can_call_into_js() ?
+                      Object::New(isolate_) :
+                      Local<Object>();
+  InternalCallbackScope cb_scope(this, obj, { 0, 0 });
 
   size_t ref_count = 0;
 
diff --git a/src/node_http2.cc b/src/node_http2.cc
index e4b9e2775939eb..ff6f3b4e3babf7 100644
--- a/src/node_http2.cc
+++ b/src/node_http2.cc
@@ -1120,13 +1120,15 @@ int Http2Session::OnStreamClose(nghttp2_session* handle,
   // It is possible for the stream close to occur before the stream is
   // ever passed on to the javascript side. If that happens, the callback
   // will return false.
-  Local<Value> arg = Integer::NewFromUnsigned(isolate, code);
-  MaybeLocal<Value> answer =
-    stream->MakeCallback(env->http2session_on_stream_close_function(),
-                          1, &arg);
-  if (answer.IsEmpty() || answer.ToLocalChecked()->IsFalse()) {
-    // Skip to destroy
-    stream->Destroy();
+  if (env->can_call_into_js()) {
+    Local<Value> arg = Integer::NewFromUnsigned(isolate, code);
+    MaybeLocal<Value> answer =
+      stream->MakeCallback(env->http2session_on_stream_close_function(),
+                            1, &arg);
+    if (answer.IsEmpty() || answer.ToLocalChecked()->IsFalse()) {
+      // Skip to destroy
+      stream->Destroy();
+    }
   }
   return 0;
 }
@@ -1629,9 +1631,11 @@ void Http2Session::MaybeScheduleWrite() {
 
       // Sending data may call arbitrary JS code, so keep track of
       // async context.
-      HandleScope handle_scope(env->isolate());
-      InternalCallbackScope callback_scope(this);
-      SendPendingData();
+      if (env->can_call_into_js()) {
+        HandleScope handle_scope(env->isolate());
+        InternalCallbackScope callback_scope(this);
+        SendPendingData();
+      }
     });
   }
 }
diff --git a/src/node_platform.cc b/src/node_platform.cc
index 9787cbb3edc2e2..0400cea5172244 100644
--- a/src/node_platform.cc
+++ b/src/node_platform.cc
@@ -406,6 +406,8 @@ int NodePlatform::NumberOfWorkerThreads() {
 }
 
 void PerIsolatePlatformData::RunForegroundTask(std::unique_ptr<Task> task) {
+  if (isolate_->IsExecutionTerminating())
+    return task->Run();
   DebugSealHandleScope scope(isolate_);
   Environment* env = Environment::GetCurrent(isolate_);
   if (env != nullptr) {