Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[coverage] Fix isolate resumption after service disposal #1189

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion pkgs/coverage/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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

Expand Down
10 changes: 8 additions & 2 deletions pkgs/coverage/lib/src/isolate_paused_listener.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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();
}
}
Expand Down
2 changes: 1 addition & 1 deletion pkgs/coverage/pubspec.yaml
Original file line number Diff line number Diff line change
@@ -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

Expand Down
62 changes: 62 additions & 0 deletions pkgs/coverage/test/isolate_paused_listener_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down