[vm/unwind] Ensure no dart api calls happen after unwind error raised.

TEST=DartAPI_EnsureUnwindError*

Change-Id: Ic14e806c34635954cb04a759f5915809dec1ad34
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/214762
Commit-Queue: Alexander Aprelev <[email protected]>
Reviewed-by: Ryan Macnak <[email protected]>

Author: aam

runtime/lib/isolate.cc

@@ -302,8 +302,6 @@ DEFINE_NATIVE_ENTRY(SendPortImpl_sendAndExitInternal_, 0, 2) {
       isolate->group()->api_state()->AllocatePersistentHandle();
   handle->set_ptr(msg_array);
   isolate->bequeath(std::unique_ptr<Bequest>(new Bequest(handle, port.Id())));
-  // TODO(aam): Ensure there are no dart api calls after this point as we want
-  // to ensure that validated message won't get tampered with.
   Isolate::KillIfExists(isolate, Isolate::LibMsgId::kKillMsg);
   // Drain interrupts before running so any IMMEDIATE operations on the current
   // isolate happen synchronously.

runtime/vm/dart_api_impl.cc

@@ -485,6 +485,16 @@ Dart_Handle Api::AcquiredError(IsolateGroup* isolate_group) {
   return reinterpret_cast<Dart_Handle>(acquired_error_handle);
 }
 
+Dart_Handle Api::UnwindInProgressError() {
+  Thread* T = Thread::Current();
+  CHECK_API_SCOPE(T);
+  TransitionToVM transition(T);
+  HANDLESCOPE(T);
+  const String& message = String::Handle(
+      Z, String::New("No api calls are allowed while unwind is in progress"));
+  return Api::NewHandle(T, UnwindError::New(message));
+}
+
 bool Api::IsValid(Dart_Handle handle) {
   Isolate* isolate = Isolate::Current();
   Thread* thread = Thread::Current();

runtime/vm/dart_api_impl.h

@@ -194,6 +194,9 @@ class Api : AllStatic {
   // Gets the handle which holds the pre-created acquired error object.
   static Dart_Handle AcquiredError(IsolateGroup* isolate_group);
 
+  // Gets the handle for unwind-is-in-progress error.
+  static Dart_Handle UnwindInProgressError();
+
   // Returns true if the handle holds a Smi.
   static bool IsSmi(Dart_Handle handle) {
     // Important: we do not require current thread to be in VM state because
@@ -335,6 +338,9 @@ class Api : AllStatic {
   if (thread->no_callback_scope_depth() != 0) {                                \
     return reinterpret_cast<Dart_Handle>(                                      \
         Api::AcquiredError(thread->isolate_group()));                          \
+  }                                                                            \
+  if (thread->is_unwind_in_progress()) {                                       \
+    return reinterpret_cast<Dart_Handle>(Api::UnwindInProgressError());        \
   }
 
 #define ASSERT_CALLBACK_STATE(thread)                                          \

runtime/vm/dart_api_impl_test.cc

@@ -707,6 +707,83 @@ TEST_CASE(DartAPI_UnhandleExceptionError) {
   EXPECT_STREQ(kRegularString, exception_cstr);
 }
 
+void JustPropagateErrorNative(Dart_NativeArguments args) {
+  Dart_Handle closure = Dart_GetNativeArgument(args, 0);
+  EXPECT(Dart_IsClosure(closure));
+  Dart_Handle result = Dart_InvokeClosure(closure, 0, NULL);
+  EXPECT(Dart_IsError(result));
+  Dart_PropagateError(result);
+  UNREACHABLE();
+}
+
+static Dart_NativeFunction JustPropagateError_lookup(Dart_Handle name,
+                                                     int argument_count,
+                                                     bool* auto_setup_scope) {
+  ASSERT(auto_setup_scope != NULL);
+  *auto_setup_scope = true;
+  return JustPropagateErrorNative;
+}
+
+TEST_CASE(DartAPI_EnsureUnwindErrorHandled_WhenKilled) {
+  const char* kScriptChars = R"(
+import 'dart:isolate';
+
+exitRightNow() {
+  Isolate.current.kill(priority: Isolate.immediate);
+}
+
+@pragma("vm:external-name", "Test_nativeFunc")
+external void nativeFunc(closure);
+
+void Func1() {
+  nativeFunc(() => exitRightNow());
+}
+)";
+  Dart_Handle lib =
+      TestCase::LoadTestScript(kScriptChars, &JustPropagateError_lookup);
+  Dart_Handle result;
+
+  result = Dart_Invoke(lib, NewString("Func1"), 0, NULL);
+  EXPECT(Dart_IsError(result));
+  EXPECT_SUBSTRING("isolate terminated by Isolate.kill", Dart_GetError(result));
+
+  result = Dart_Invoke(lib, NewString("Func1"), 0, NULL);
+  EXPECT(Dart_IsError(result));
+  EXPECT_SUBSTRING("No api calls are allowed while unwind is in progress",
+                   Dart_GetError(result));
+}
+
+TEST_CASE(DartAPI_EnsureUnwindErrorHandled_WhenSendAndExit) {
+  const char* kScriptChars = R"(
+import 'dart:isolate';
+import 'dart:_internal' show sendAndExit;
+
+sendAndExitNow() {
+  final receivePort = ReceivePort();
+  sendAndExit(receivePort.sendPort, true);
+}
+
+@pragma("vm:external-name", "Test_nativeFunc")
+external void nativeFunc(closure);
+
+void Func1() {
+  nativeFunc(() => sendAndExitNow());
+}
+)";
+  Dart_Handle lib =
+      TestCase::LoadTestScript(kScriptChars, &JustPropagateError_lookup);
+  Dart_Handle result;
+
+  result = Dart_Invoke(lib, NewString("Func1"), 0, NULL);
+  EXPECT(Dart_IsError(result));
+  EXPECT_SUBSTRING("isolate terminated by Isolate.kill", Dart_GetError(result));
+
+  result = Dart_Invoke(lib, NewString("Func1"), 0, NULL);
+  EXPECT(Dart_IsError(result));
+  EXPECT_SUBSTRING("No api calls are allowed while unwind is in progress",
+                   Dart_GetError(result));
+}
+
 // Should we propagate the error via Dart_SetReturnValue?
 static bool use_set_return = false;
 

runtime/vm/isolate.cc

@@ -1126,6 +1126,7 @@ ErrorPtr IsolateMessageHandler::HandleLibMessage(const Array& message) {
       if (!obj.IsSmi()) return Error::null();
       const intptr_t priority = Smi::Cast(obj).Value();
       if (priority == Isolate::kImmediateAction) {
+        Thread::Current()->StartUnwindError();
         obj = message.At(2);
         if (I->VerifyTerminateCapability(obj)) {
           // We will kill the current isolate by returning an UnwindError.

runtime/vm/runtime_entry.cc

@@ -3635,6 +3635,9 @@ static Thread* GetThreadForNativeCallback(uword callback_id,
   if (thread->no_callback_scope_depth() != 0) {
     FATAL("Cannot invoke native callback when API callbacks are prohibited.");
   }
+  if (thread->is_unwind_in_progress()) {
+    FATAL("Cannot invoke native callback while unwind error propagates.");
+  }
   if (!thread->IsMutatorThread()) {
     FATAL("Native callbacks must be invoked on the mutator thread.");
   }

runtime/vm/thread.h

@@ -521,6 +521,10 @@ class Thread : public ThreadState {
     no_callback_scope_depth_ -= 1;
   }
 
+  bool is_unwind_in_progress() const { return is_unwind_in_progress_; }
+
+  void StartUnwindError() { is_unwind_in_progress_ = true; }
+
 #if defined(DEBUG)
   void EnterCompiler() {
     ASSERT(!IsInsideCompiler());
@@ -1182,6 +1186,8 @@ class Thread : public ThreadState {
   Thread* next_;  // Used to chain the thread structures in an isolate.
   bool is_mutator_thread_ = false;
 
+  bool is_unwind_in_progress_ = false;
+
 #if defined(DEBUG)
   bool inside_compiler_ = false;
 #endif