[coverage] Fix isolate resumption after service disposal (#1189)

dart-lang · Nov 10, 2024 · a6603a4 · a6603a4
1 parent 66afa68
commit a6603a4
Show file tree

Hide file tree

Showing 4 changed files with 79 additions and 4 deletions.
diff --git a/pkgs/coverage/CHANGELOG.md b/pkgs/coverage/CHANGELOG.md
@@ -1,6 +1,13 @@
+## 1.11.0
+
+- Fix a [bug](https://github.com/dart-lang/tools/issues/685) where the tool
+  would occasionally try to resume an isolate after the VM service had been
+  disposed.
+
 ## 1.10.0
 
-- Fix bug where tests involving multiple isolates never finish (#520).
+- Fix a [bug](https://github.com/dart-lang/tools/issues/520) where tests
+  involving multiple isolates never finish.
 
 ## 1.9.2
 

diff --git a/pkgs/coverage/lib/src/isolate_paused_listener.dart b/pkgs/coverage/lib/src/isolate_paused_listener.dart
@@ -46,7 +46,8 @@ class IsolatePausedListener {
     try {
       if (_mainIsolate != null) {
         if (await _mainIsolatePaused.future) {
-          await _runCallbackAndResume(_mainIsolate!, true);
+          await _runCallbackAndResume(
+              _mainIsolate!, !_getGroup(_mainIsolate!).collected);
         }
       }
     } finally {
@@ -114,8 +115,13 @@ class IsolatePausedListener {
     }
   }
 
+  bool get _mainRunning =>
+      _mainIsolate != null && !_mainIsolatePaused.isCompleted;
+
   void _checkCompleted() {
-    if (_numNonMainIsolates == 0 && !_allNonMainIsolatesExited.isCompleted) {
+    if (_numNonMainIsolates == 0 &&
+        !_mainRunning &&
+        !_allNonMainIsolatesExited.isCompleted) {
       _allNonMainIsolatesExited.complete();
     }
   }

diff --git a/pkgs/coverage/pubspec.yaml b/pkgs/coverage/pubspec.yaml
@@ -1,5 +1,5 @@
 name: coverage
-version: 1.10.0
+version: 1.11.0
 description: Coverage data manipulation and formatting
 repository: https://github.com/dart-lang/tools/tree/main/pkgs/coverage
 issue_tracker: https://github.com/dart-lang/tools/issues?q=is%3Aissue+is%3Aopen+label%3Apackage%3Acoverage

diff --git a/pkgs/coverage/test/isolate_paused_listener_test.dart b/pkgs/coverage/test/isolate_paused_listener_test.dart
@@ -700,6 +700,68 @@ void main() {
       ]);
     });
 
+    test(
+        'all other isolates exit before main isolate pauses, then main '
+        'starts another isolate, then pauses before they exit', () async {
+      startEvent('A', '1', 'main');
+      startEvent('B', '1');
+      pauseEvent('B', '1');
+      exitEvent('B', '1');
+
+      await Future<void>.delayed(Duration.zero);
+
+      startEvent('C', '1');
+      pauseEvent('C', '1');
+      pauseEvent('A', '1', 'main');
+      exitEvent('C', '1');
+
+      await Future<void>.delayed(Duration.zero);
+
+      exitEvent('A', '1', 'main');
+
+      await endTest();
+
+      expect(received, [
+        'Pause B. Last in group 1? No',
+        'Resume B',
+        'Pause C. Last in group 1? No',
+        'Resume C',
+        'Pause A. Last in group 1? Yes',
+        'Resume A',
+      ]);
+    });
+
+    test(
+        'all other isolates exit before main isolate pauses, then main '
+        'starts another isolate, then pauses before they pause', () async {
+      startEvent('A', '1', 'main');
+      startEvent('B', '1');
+      pauseEvent('B', '1');
+      exitEvent('B', '1');
+
+      await Future<void>.delayed(Duration.zero);
+
+      startEvent('C', '1');
+      pauseEvent('A', '1', 'main');
+      pauseEvent('C', '1');
+      exitEvent('C', '1');
+
+      await Future<void>.delayed(Duration.zero);
+
+      exitEvent('A', '1', 'main');
+
+      await endTest();
+
+      expect(received, [
+        'Pause B. Last in group 1? No',
+        'Resume B',
+        'Pause C. Last in group 1? Yes',
+        'Resume C',
+        'Pause A. Last in group 1? No',
+        'Resume A',
+      ]);
+    });
+
     test('group reopened', () async {
       // If an isolate is reported in a group after the group as believed to be
       // closed, reopen the group. This double counts some coverage, but at