Skip to content

Commit 8297e44

Browse files
authored
[ Hot Restart ] Fix possible hang due to unhandled exception in UI isolates on hot restart (#165693)
Possible fix for flutter/flutter#161466
1 parent 6b02598 commit 8297e44

File tree

5 files changed

+187
-12
lines changed

5 files changed

+187
-12
lines changed

packages/flutter_tools/lib/src/run_hot.dart

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -645,7 +645,7 @@ class HotRunner extends ResidentRunner {
645645
);
646646
operations.add(
647647
reloadIsolate.then((vm_service.Isolate? isolate) async {
648-
if ((isolate != null) && isPauseEvent(isolate.pauseEvent!.kind!)) {
648+
if (isolate != null) {
649649
// The embedder requires that the isolate is unpaused, because the
650650
// runInView method requires interaction with dart engine APIs that
651651
// are not thread-safe, and thus must be run on the same thread that
@@ -655,7 +655,7 @@ class HotRunner extends ResidentRunner {
655655
// or in a frequently called method) or an exception. Instead, all
656656
// breakpoints are first disabled and exception pause mode set to
657657
// None, and then the isolate resumed.
658-
// These settings to not need restoring as Hot Restart results in
658+
// These settings do not need restoring as Hot Restart results in
659659
// new isolates, which will be configured by the editor as they are
660660
// started.
661661
final List<Future<void>> breakpointAndExceptionRemoval = <Future<void>>[
@@ -667,12 +667,22 @@ class HotRunner extends ResidentRunner {
667667
device.vmService!.service.removeBreakpoint(isolate.id!, breakpoint.id!),
668668
];
669669
await Future.wait(breakpointAndExceptionRemoval);
670-
await device.vmService!.service.resume(view.uiIsolate!.id!);
670+
if (isPauseEvent(isolate.pauseEvent!.kind!)) {
671+
await device.vmService!.service.resume(view.uiIsolate!.id!);
672+
}
671673
}
672674
}),
673675
);
674676
}
675677

678+
// Wait for the UI isolates to have their breakpoints removed and exception pause mode
679+
// cleared while also ensuring the isolate's are no longer paused. If we don't clear
680+
// the exception pause mode before we start killing child isolates, it's possible that
681+
// any UI isolate waiting on a result from a child isolate could throw an unhandled
682+
// exception and re-pause the isolate, causing hot restart to hang.
683+
await Future.wait(operations);
684+
operations.clear();
685+
676686
// The engine handles killing and recreating isolates that it has spawned
677687
// ("uiIsolates"). The isolates that were spawned from these uiIsolates
678688
// will not be restarted, and so they must be manually killed.

packages/flutter_tools/test/general.shard/resident_runner_test.dart

Lines changed: 58 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -239,6 +239,14 @@ void main() {
239239
args: <String, Object?>{'isolateId': fakeUnpausedIsolate.id},
240240
jsonResponse: fakeUnpausedIsolate.toJson(),
241241
),
242+
FakeVmServiceRequest(
243+
method: 'setIsolatePauseMode',
244+
args: <String, Object?>{
245+
'isolateId': fakeUnpausedIsolate.id,
246+
'exceptionPauseMode': vm_service.ExceptionPauseMode.kNone,
247+
},
248+
jsonResponse: vm_service.Success().toJson(),
249+
),
242250
FakeVmServiceRequest(
243251
method: 'getVM',
244252
jsonResponse: vm_service.VM.parse(<String, Object>{})!.toJson(),
@@ -785,6 +793,14 @@ void main() {
785793
args: <String, Object?>{'isolateId': fakeUnpausedIsolate.id},
786794
jsonResponse: fakeUnpausedIsolate.toJson(),
787795
),
796+
FakeVmServiceRequest(
797+
method: 'setIsolatePauseMode',
798+
args: <String, Object?>{
799+
'isolateId': fakeUnpausedIsolate.id,
800+
'exceptionPauseMode': vm_service.ExceptionPauseMode.kNone,
801+
},
802+
jsonResponse: vm_service.Success().toJson(),
803+
),
788804
FakeVmServiceRequest(
789805
method: 'getVM',
790806
jsonResponse: vm_service.VM.parse(<String, Object>{})!.toJson(),
@@ -852,18 +868,28 @@ void main() {
852868
jsonResponse: fakePausedIsolate.toJson(),
853869
),
854870
FakeVmServiceRequest(
855-
method: 'getVM',
856-
jsonResponse: vm_service.VM.parse(<String, Object>{})!.toJson(),
857-
),
858-
const FakeVmServiceRequest(
859871
method: 'setIsolatePauseMode',
860-
args: <String, String>{'isolateId': '1', 'exceptionPauseMode': 'None'},
872+
args: <String, Object?>{
873+
'isolateId': fakeUnpausedIsolate.id,
874+
'exceptionPauseMode': vm_service.ExceptionPauseMode.kNone,
875+
},
876+
jsonResponse: vm_service.Success().toJson(),
861877
),
862-
const FakeVmServiceRequest(
878+
FakeVmServiceRequest(
863879
method: 'removeBreakpoint',
864-
args: <String, String>{'isolateId': '1', 'breakpointId': 'test-breakpoint'},
880+
args: <String, Object?>{
881+
'isolateId': fakeUnpausedIsolate.id,
882+
'breakpointId': 'test-breakpoint',
883+
},
884+
),
885+
FakeVmServiceRequest(
886+
method: 'resume',
887+
args: <String, Object?>{'isolateId': fakeUnpausedIsolate.id},
888+
),
889+
FakeVmServiceRequest(
890+
method: 'getVM',
891+
jsonResponse: vm_service.VM.parse(<String, Object>{})!.toJson(),
865892
),
866-
const FakeVmServiceRequest(method: 'resume', args: <String, String>{'isolateId': '1'}),
867893
listViews,
868894
const FakeVmServiceRequest(
869895
method: 'streamListen',
@@ -913,6 +939,14 @@ void main() {
913939
args: <String, Object?>{'isolateId': fakeUnpausedIsolate.id},
914940
jsonResponse: fakeUnpausedIsolate.toJson(),
915941
),
942+
FakeVmServiceRequest(
943+
method: 'setIsolatePauseMode',
944+
args: <String, Object?>{
945+
'isolateId': fakeUnpausedIsolate.id,
946+
'exceptionPauseMode': vm_service.ExceptionPauseMode.kNone,
947+
},
948+
jsonResponse: vm_service.Success().toJson(),
949+
),
916950
FakeVmServiceRequest(
917951
method: 'getVM',
918952
jsonResponse: vm_service.VM.parse(<String, Object>{})!.toJson(),
@@ -940,6 +974,14 @@ void main() {
940974
args: <String, Object?>{'isolateId': fakeUnpausedIsolate.id},
941975
jsonResponse: fakeUnpausedIsolate.toJson(),
942976
),
977+
FakeVmServiceRequest(
978+
method: 'setIsolatePauseMode',
979+
args: <String, Object?>{
980+
'isolateId': fakeUnpausedIsolate.id,
981+
'exceptionPauseMode': vm_service.ExceptionPauseMode.kNone,
982+
},
983+
jsonResponse: vm_service.Success().toJson(),
984+
),
943985
FakeVmServiceRequest(
944986
method: 'getVM',
945987
jsonResponse: vm_service.VM.parse(<String, Object>{})!.toJson(),
@@ -967,6 +1009,14 @@ void main() {
9671009
args: <String, Object?>{'isolateId': fakeUnpausedIsolate.id},
9681010
jsonResponse: fakeUnpausedIsolate.toJson(),
9691011
),
1012+
FakeVmServiceRequest(
1013+
method: 'setIsolatePauseMode',
1014+
args: <String, Object?>{
1015+
'isolateId': fakeUnpausedIsolate.id,
1016+
'exceptionPauseMode': vm_service.ExceptionPauseMode.kNone,
1017+
},
1018+
jsonResponse: vm_service.Success().toJson(),
1019+
),
9701020
FakeVmServiceRequest(
9711021
method: 'getVM',
9721022
jsonResponse: vm_service.VM.parse(<String, Object>{})!.toJson(),
Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
// Copyright 2014 The Flutter Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
5+
import 'dart:async';
6+
7+
import 'package:file/file.dart';
8+
import 'package:vm_service/vm_service.dart';
9+
import 'package:vm_service/vm_service_io.dart';
10+
11+
import '../src/common.dart';
12+
import 'test_data/hot_restart_with_paused_child_isolate_project.dart';
13+
import 'test_driver.dart';
14+
import 'test_utils.dart';
15+
16+
void main() {
17+
late Directory tempDir;
18+
final HotRestartWithPausedChildIsolateProject project = HotRestartWithPausedChildIsolateProject();
19+
late FlutterRunTestDriver flutter;
20+
21+
setUp(() async {
22+
tempDir = createResolvedTempDirectorySync('hot_restart_test.');
23+
await project.setUpIn(tempDir);
24+
flutter = FlutterRunTestDriver(tempDir);
25+
});
26+
27+
tearDown(() async {
28+
await flutter.stop();
29+
tryToDelete(tempDir);
30+
});
31+
32+
// Possible regression test for https://github.com/flutter/flutter/issues/161466
33+
testWithoutContext("Hot restart doesn't hang when an unhandled exception is "
34+
'thrown in the UI isolate', () async {
35+
await flutter.run(withDebugger: true, startPaused: true, pauseOnExceptions: true);
36+
final VmService vmService = await vmServiceConnectUri(flutter.vmServiceWsUri.toString());
37+
final Isolate root = await flutter.getFlutterIsolate();
38+
39+
// The UI isolate has already started paused. Setup a listener for the
40+
// child isolate that will spawn when the isolate resumes. Resume the
41+
// spawned child which will pause on start, and then wait for it to execute
42+
// the `debugger()` call.
43+
final Completer<void> childIsolatePausedCompleter = Completer<void>();
44+
vmService.onDebugEvent.listen((Event event) async {
45+
if (event.kind == EventKind.kPauseStart) {
46+
await vmService.resume(event.isolate!.id!);
47+
} else if (event.kind == EventKind.kPauseBreakpoint) {
48+
if (!childIsolatePausedCompleter.isCompleted) {
49+
await vmService.streamCancel(EventStreams.kDebug);
50+
childIsolatePausedCompleter.complete();
51+
}
52+
}
53+
});
54+
await vmService.streamListen(EventStreams.kDebug);
55+
56+
await vmService.resume(root.id!);
57+
await childIsolatePausedCompleter.future;
58+
59+
// This call will fail to return if the UI isolate pauses on an unhandled
60+
// exception due to the isolate spawned by `Isolate.run` not completing.
61+
await flutter.hotRestart();
62+
});
63+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
// Copyright 2014 The Flutter Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
5+
import 'project.dart';
6+
7+
// Reproduction case from
8+
// https://github.com/flutter/flutter/issues/161466#issuecomment-2743309718.
9+
class HotRestartWithPausedChildIsolateProject extends Project {
10+
@override
11+
final String pubspec = '''
12+
name: test
13+
environment:
14+
sdk: ^3.7.0-0
15+
16+
dependencies:
17+
flutter:
18+
sdk: flutter
19+
''';
20+
21+
@override
22+
final String main = r'''
23+
import 'dart:async';
24+
import 'dart:developer';
25+
import 'dart:isolate';
26+
27+
import 'package:flutter/material.dart';
28+
29+
void main() {
30+
WidgetsFlutterBinding.ensureInitialized().platformDispatcher.onError = (Object error, StackTrace? stack) {
31+
print('HERE');
32+
return true;
33+
};
34+
runApp(
35+
const Center(
36+
child: Text(
37+
'Hello, world!',
38+
key: Key('title'),
39+
textDirection: TextDirection.ltr,
40+
),
41+
),
42+
);
43+
44+
Isolate.run(() {
45+
print('COMPUTING');
46+
debugger();
47+
});
48+
}
49+
''';
50+
}

packages/flutter_tools/test/integration.shard/test_driver.dart

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,9 @@ abstract final class FlutterTestDriver {
148148
final Completer<void> isolateStarted = Completer<void>();
149149
_vmService!.onIsolateEvent.listen((Event event) {
150150
if (event.kind == EventKind.kIsolateStart) {
151-
isolateStarted.complete();
151+
if (!isolateStarted.isCompleted) {
152+
isolateStarted.complete();
153+
}
152154
} else if (event.kind == EventKind.kIsolateExit && event.isolate?.id == _flutterIsolateId) {
153155
// Hot restarts cause all the isolates to exit, so we need to refresh
154156
// our idea of what the Flutter isolate ID is.

0 commit comments

Comments
 (0)