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

This reverts commit 4ed9d48f.

Reason for revert: UBSan failure https://ci.chromium.org/p/v8/builders/ci/V8%20Linux64%20UBSan/9084

Original change's description:
> [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}

TBR=yangguo@chromium.org,sigurds@chromium.org,verwaest@chromium.org

Change-Id: Iedefe5320d8bdc442a87e03698a20daf6a0ebf4f
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: chromium:1014415
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1943149Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Cr-Commit-Position: refs/heads/master@{#65256}

src/execution/execution.cc

@@ -336,17 +336,15 @@ 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);
       }
     }
   }
 
-  if (is_termination) {
-    // Reschedule terminate execution exception.
-    isolate->OptionalRescheduleException(false);
-  }
+  // Re-request terminate execution interrupt to trigger later.
+  if (is_termination) isolate->stack_guard()->RequestTerminateExecution();
 
   return maybe_result;
 }

src/execution/microtask-queue.cc

@@ -177,7 +177,6 @@ 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());
-  CHECK(exception.is_null());
-  // TryCall reschedules the termination exception.
-  CHECK(args.GetIsolate()->IsExecutionTerminating());
+  // TryCall ignores terminate execution, but rerequests the interrupt.
+  CHECK(!args.GetIsolate()->IsExecutionTerminating());
+  CHECK(CompileRun("1 + 1;").IsEmpty());
 }
 
 
@@ -781,13 +781,7 @@ 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());
@@ -817,14 +811,11 @@ TEST(TerminateAndTryCall) {
             ->Get(isolate->GetCurrentContext(), v8_str("terminate"))
             .ToLocalChecked();
     CHECK(value->IsFunction());
-    // Any further executions in this TryCatch scope fail.
-    CHECK(!isolate->IsExecutionTerminating());
+    // The first stack check after terminate has been re-requested fails.
     CHECK(CompileRun("1 + 1").IsEmpty());
     CHECK(isolate->IsExecutionTerminating());
-    CHECK(CompileRun("1 + 1").IsEmpty());
-    CHECK(CompileRun("1 + 1").IsEmpty());
   }
-  // Leaving the TryCatch cleared the termination exception.
+  // V8 then recovers.
   v8::Maybe<int32_t> result =
       CompileRun("2 + 2")->Int32Value(isolate->GetCurrentContext());
   CHECK_EQ(4, result.FromJust());
@@ -903,36 +894,26 @@ 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::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());
-    }
+    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(!CcTest::i_isolate()->stack_guard()->CheckTerminateExecution());
   CHECK(!isolate->IsExecutionTerminating());
 }
 
@@ -951,8 +932,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);
@@ -960,7 +941,6 @@ TEST(TerminateInApiMicrotask) {
     isolate->RunMicrotasks();
     CHECK(try_catch.HasCaught());
     CHECK(try_catch.HasTerminated());
-    CHECK(isolate->IsExecutionTerminating());
   }
   CHECK(!isolate->IsExecutionTerminating());
 }