[exceptions] Don't re-request interrupt in InvokeWithTryCatch

This CL changes InvokeWithTryCatch to not re-request the terminate
execution interrupt, but instead schedule the termination exception.
This ensures that leaving the outermost TryCatch scope will clear
the exception, and no interrupt remains.

Previously, the interrupt request could remain and prevent further
JavaScript execution even after the TryCatch scope was left.

Change-Id: I1e603dc822bbcb0def4cf0a898d59cf8d4b9d039
Bug: chromium:1014415
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1871910
Commit-Queue: Sigurd Schneider <sigurds@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Cr-Commit-Position: refs/heads/master@{#65255}

src/execution/execution.cc

@@ -336,15 +336,17 @@ MaybeHandle<Object> InvokeWithTryCatch(Isolate* isolate,
           DCHECK(isolate->external_caught_exception());
           *params.exception_out = v8::Utils::OpenHandle(*catcher.Exception());
         }
-      }
-      if (params.message_handling == Execution::MessageHandling::kReport) {
-        isolate->OptionalRescheduleException(true);
+        if (params.message_handling == Execution::MessageHandling::kReport) {
+          isolate->OptionalRescheduleException(true);
+        }
       }
     }
   }
 
-  // Re-request terminate execution interrupt to trigger later.
-  if (is_termination) isolate->stack_guard()->RequestTerminateExecution();
+  if (is_termination) {
+    // Reschedule terminate execution exception.
+    isolate->OptionalRescheduleException(false);
+  }
 
   return maybe_result;
 }

src/execution/microtask-queue.cc

@@ -177,6 +177,7 @@ int MicrotaskQueue::RunMicrotasks(Isolate* isolate) {
     capacity_ = 0;
     size_ = 0;
     start_ = 0;
+    DCHECK(isolate->has_scheduled_exception());
     isolate->SetTerminationOnExternalTryCatch();
     OnCompleted(isolate);
     return -1;

test/cctest/test-thread-termination.cc

@@ -760,9 +760,9 @@ void InnerTryCallTerminate(const v8::FunctionCallbackInfo<v8::Value>& args) {
                             v8::Utils::OpenHandle((*global)), 0, nullptr,
                             i::Execution::MessageHandling::kReport, &exception);
   CHECK(result.is_null());
-  // TryCall ignores terminate execution, but rerequests the interrupt.
-  CHECK(!args.GetIsolate()->IsExecutionTerminating());
-  CHECK(CompileRun("1 + 1;").IsEmpty());
+  CHECK(exception.is_null());
+  // TryCall reschedules the termination exception.
+  CHECK(args.GetIsolate()->IsExecutionTerminating());
 }
 
 
@@ -781,7 +781,13 @@ TEST(TerminationInInnerTryCall) {
     v8::TryCatch try_catch(isolate);
     CompileRun("inner_try_call_terminate()");
     CHECK(try_catch.HasTerminated());
+    // Any further exectutions in this TryCatch scope fail.
+    CHECK(isolate->IsExecutionTerminating());
+    CHECK(CompileRun("1 + 1").IsEmpty());
+    CHECK(CompileRun("1 + 1").IsEmpty());
+    CHECK(CompileRun("1 + 1").IsEmpty());
   }
+  // Leaving the TryCatch cleared the termination exception.
   v8::Maybe<int32_t> result =
       CompileRun("2 + 2")->Int32Value(isolate->GetCurrentContext());
   CHECK_EQ(4, result.FromJust());
@@ -811,11 +817,14 @@ TEST(TerminateAndTryCall) {
             ->Get(isolate->GetCurrentContext(), v8_str("terminate"))
             .ToLocalChecked();
     CHECK(value->IsFunction());
-    // The first stack check after terminate has been re-requested fails.
+    // Any further executions in this TryCatch scope fail.
+    CHECK(!isolate->IsExecutionTerminating());
     CHECK(CompileRun("1 + 1").IsEmpty());
     CHECK(isolate->IsExecutionTerminating());
+    CHECK(CompileRun("1 + 1").IsEmpty());
+    CHECK(CompileRun("1 + 1").IsEmpty());
   }
-  // V8 then recovers.
+  // Leaving the TryCatch cleared the termination exception.
   v8::Maybe<int32_t> result =
       CompileRun("2 + 2")->Int32Value(isolate->GetCurrentContext());
   CHECK_EQ(4, result.FromJust());
@@ -894,26 +903,36 @@ TEST(TerminateInMicrotask) {
       isolate, TerminateCurrentThread, DoLoopCancelTerminate);
   v8::Local<v8::Context> context1 = v8::Context::New(isolate, nullptr, global);
   v8::Local<v8::Context> context2 = v8::Context::New(isolate, nullptr, global);
-  v8::TryCatch try_catch(isolate);
-  {
-    v8::Context::Scope context_scope(context1);
-    CHECK(!isolate->IsExecutionTerminating());
-    CHECK(!CompileRun("Promise.resolve().then(function() {"
-                      "terminate(); loop(); fail();})")
-               .IsEmpty());
-    CHECK(!try_catch.HasCaught());
-  }
   {
-    v8::Context::Scope context_scope(context2);
-    CHECK(context2 == isolate->GetCurrentContext());
-    CHECK(context2 == isolate->GetEnteredOrMicrotaskContext());
-    CHECK(!isolate->IsExecutionTerminating());
-    isolate->RunMicrotasks();
-    CHECK(context2 == isolate->GetCurrentContext());
-    CHECK(context2 == isolate->GetEnteredOrMicrotaskContext());
-    CHECK(try_catch.HasCaught());
-    CHECK(try_catch.HasTerminated());
+    v8::TryCatch try_catch(isolate);
+    {
+      v8::Context::Scope context_scope(context1);
+      CHECK(!isolate->IsExecutionTerminating());
+      CHECK(!CompileRun("Promise.resolve().then(function() {"
+                        "terminate(); loop(); fail();})")
+                 .IsEmpty());
+      CHECK(!try_catch.HasCaught());
+    }
+    {
+      v8::Context::Scope context_scope(context2);
+      CHECK(context2 == isolate->GetCurrentContext());
+      CHECK(context2 == isolate->GetEnteredOrMicrotaskContext());
+      CHECK(!isolate->IsExecutionTerminating());
+      isolate->RunMicrotasks();
+      CHECK(context2 == isolate->GetCurrentContext());
+      CHECK(context2 == isolate->GetEnteredOrMicrotaskContext());
+      CHECK(try_catch.HasCaught());
+      CHECK(try_catch.HasTerminated());
+      CHECK(isolate->IsExecutionTerminating());
+      CHECK(!CcTest::i_isolate()->stack_guard()->CheckTerminateExecution());
+      // Any further exectutions in this TryCatch scope fail.
+      CHECK(isolate->IsExecutionTerminating());
+      CHECK(CompileRun("1 + 1").IsEmpty());
+      CHECK(CompileRun("1 + 1").IsEmpty());
+      CHECK(CompileRun("1 + 1").IsEmpty());
+    }
   }
+  CHECK(!CcTest::i_isolate()->stack_guard()->CheckTerminateExecution());
   CHECK(!isolate->IsExecutionTerminating());
 }
 
@@ -932,8 +951,8 @@ TEST(TerminateInApiMicrotask) {
   v8::Local<v8::ObjectTemplate> global = CreateGlobalTemplate(
       isolate, TerminateCurrentThread, DoLoopCancelTerminate);
   v8::Local<v8::Context> context = v8::Context::New(isolate, nullptr, global);
-  v8::TryCatch try_catch(isolate);
   {
+    v8::TryCatch try_catch(isolate);
     v8::Context::Scope context_scope(context);
     CHECK(!isolate->IsExecutionTerminating());
     isolate->EnqueueMicrotask(TerminationMicrotask);
@@ -941,6 +960,7 @@ TEST(TerminateInApiMicrotask) {
     isolate->RunMicrotasks();
     CHECK(try_catch.HasCaught());
     CHECK(try_catch.HasTerminated());
+    CHECK(isolate->IsExecutionTerminating());
   }
   CHECK(!isolate->IsExecutionTerminating());
 }