From 4443d2d5612d508953f481fb90ee35edecdf906e Mon Sep 17 00:00:00 2001
From: Alexander Aprelev <aam@google.com>
Date: Tue, 31 Jan 2023 17:15:38 +0000
Subject: [PATCH] [vm/isolate] Ensure that isolate can correctly resume and
 exit if paused on exit.

It could happen that an isolate gets stuck unable to exit if paused on exit when using Isolate.exit.

Fixes https://github.com/dart-lang/sdk/issues/51164
TEST=isolate_exit_resume_test

Change-Id: I114686ce0637434827e56988821932cb91238032
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/280044
Commit-Queue: Alexander Aprelev <aam@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>
---
 .../test/isolate_exit_resume_test.dart        | 74 +++++++++++++++++++
 runtime/vm/message_handler.cc                 |  6 +-
 runtime/vm/message_handler.h                  |  3 +
 3 files changed, 82 insertions(+), 1 deletion(-)
 create mode 100644 pkg/vm_service/test/isolate_exit_resume_test.dart

diff --git a/pkg/vm_service/test/isolate_exit_resume_test.dart b/pkg/vm_service/test/isolate_exit_resume_test.dart
new file mode 100644
index 00000000000..3eb0ef9f812
--- /dev/null
+++ b/pkg/vm_service/test/isolate_exit_resume_test.dart
@@ -0,0 +1,74 @@
+// Copyright (c) 2023, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+//
+// Ensure that compute isolate can exit if paused on exit
+// BUG=https://github.com/dart-lang/sdk/issues/51164
+
+import 'dart:async';
+import 'dart:isolate' as iso;
+
+import 'package:vm_service/vm_service.dart';
+
+import 'common/test_helper.dart';
+
+Future<void> _compute() async {
+  iso.ReceivePort();
+  print('compute is done');
+}
+
+void testMain() async {
+  await iso.Isolate.run(_compute);
+  print('Done');
+}
+
+final tests = <IsolateTest>[
+  (VmService service, IsolateRef isolateRef) async {
+    // await service.setIsolatePauseMode(isolateRef.id!, shouldPauseOnExit: true);
+    // expect(await shouldPauseOnExit(service, isolateRef), true);
+    final computeCompleter = Completer<void>();
+    final mainCompleter = Completer<void>();
+
+    final mainIsolate = Completer<String>();
+    final computeIsolate = Completer<String>();
+
+    final stream = service.onDebugEvent;
+    final subscription = stream.listen((Event event) {
+      print('debug stream event: $event');
+      switch (event.kind) {
+        case EventKind.kPauseExit:
+          if (!computeCompleter.isCompleted) {
+            computeCompleter.complete();
+          } else {
+            mainCompleter.complete();
+          }
+          break;
+        case EventKind.kPauseStart:
+          if (!mainIsolate.isCompleted) {
+            mainIsolate.complete(event.isolate!.id!);
+          } else {
+            computeIsolate.complete(event.isolate!.id!);
+          }
+          service.resume(event.isolate!.id!);
+      }
+    });
+    await service.streamListen(EventStreams.kDebug);
+
+    // Wait for pause on exit for compute isolate
+    await computeCompleter.future;
+    // Resume compute isolate paused on exit
+    await service.resume(await computeIsolate.future);
+    // Ensure that main exits as well.
+    await mainCompleter.future;
+    await subscription.cancel();
+  },
+];
+
+void main([args = const <String>[]]) => runIsolateTests(
+      args,
+      tests,
+      'isolate_exit_resume_test.dart',
+      pause_on_start: true,
+      pause_on_exit: true,
+      testeeConcurrent: testMain,
+    );
diff --git a/runtime/vm/message_handler.cc b/runtime/vm/message_handler.cc
index bbb8211e483..6e5d26a5fde 100644
--- a/runtime/vm/message_handler.cc
+++ b/runtime/vm/message_handler.cc
@@ -66,6 +66,7 @@ MessageHandler::MessageHandler()
       should_pause_on_exit_(false),
       is_paused_on_start_(false),
       is_paused_on_exit_(false),
+      remembered_paused_on_exit_status_(kOK),
       paused_timestamp_(-1),
 #endif
       task_running_(false),
@@ -427,6 +428,7 @@ void MessageHandler::TaskCallback() {
         return;
       } else {
         PausedOnExitLocked(&ml, false);
+        status = remembered_paused_on_exit_status_;
       }
     }
 #endif  // !defined(PRODUCT)
@@ -462,9 +464,11 @@ void MessageHandler::TaskCallback() {
               "Use the Observatory to release it.\n",
               name());
         }
+        remembered_paused_on_exit_status_ = status;
         PausedOnExitLocked(&ml, true);
         // More messages may have come in while we released the monitor.
-        status = HandleMessages(&ml, false, false);
+        status = HandleMessages(&ml, /*allow_normal_messages=*/false,
+                                /*allow_multiple_normal_messagesfalse=*/false);
         if (ShouldPauseOnExit(status)) {
           // Still paused.
           ASSERT(oob_queue_->IsEmpty());
diff --git a/runtime/vm/message_handler.h b/runtime/vm/message_handler.h
index 228c745a92e..b9fc39e91e6 100644
--- a/runtime/vm/message_handler.h
+++ b/runtime/vm/message_handler.h
@@ -265,6 +265,9 @@ class MessageHandler {
   bool should_pause_on_exit_;
   bool is_paused_on_start_;
   bool is_paused_on_exit_;
+  // When isolate gets paused, remember the status of the message being
+  // processed so that we can resume correctly(into potentially not-OK status).
+  MessageStatus remembered_paused_on_exit_status_;
   int64_t paused_timestamp_;
 #endif
   bool task_running_;