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

This is a reland of 4ed9d48f

CompileRun leads to undefined behavior if the compile fails;
CompileRunChecked can be used to assert that the compile must
succeed. I've removed the attempt to compile and rely on a
simpler check in the tests now.

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, verwaest@chromium.org
Bug: chromium:1014415
Change-Id: I29444c4b7ea5a158865f54d4608f374914f7b133
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1943151Reviewed-by: Sigurd Schneider <sigurds@chromium.org>
Commit-Queue: Sigurd Schneider <sigurds@chromium.org>
Cr-Commit-Position: refs/heads/master@{#65258}

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,10 @@ TEST(TerminationInInnerTryCall) {
     v8::TryCatch try_catch(isolate);
     CompileRun("inner_try_call_terminate()");
     CHECK(try_catch.HasTerminated());
+    // Any further exectutions in this TryCatch scope would fail.
+    CHECK(isolate->IsExecutionTerminating());
   }
+  // 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 +814,12 @@ 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());
   }
-  // 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 +898,32 @@ 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());
+      // Any further exectutions in this TryCatch scope would fail.
+      CHECK(isolate->IsExecutionTerminating());
+      CHECK(!CcTest::i_isolate()->stack_guard()->CheckTerminateExecution());
+    }
   }
+  CHECK(!CcTest::i_isolate()->stack_guard()->CheckTerminateExecution());
   CHECK(!isolate->IsExecutionTerminating());
 }
 
@@ -932,8 +942,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 +951,7 @@ TEST(TerminateInApiMicrotask) {
     isolate->RunMicrotasks();
     CHECK(try_catch.HasCaught());
     CHECK(try_catch.HasTerminated());
+    CHECK(isolate->IsExecutionTerminating());
   }
   CHECK(!isolate->IsExecutionTerminating());
 }