From 99e85b1c5fb5d53654e85ad0e3f6715dc27cd1e8 Mon Sep 17 00:00:00 2001 From: Danny Tuppeny Date: Fri, 19 Nov 2021 06:23:09 +0000 Subject: [PATCH] Disable pause-on-exceptions for (outgoing) isolates during hot restart (#93411) --- packages/flutter_tools/lib/src/run_hot.dart | 15 +++-- .../general.shard/resident_runner_test.dart | 9 ++- .../debug_adapter/flutter_adapter_test.dart | 33 ++++++++++ .../debug_adapter/test_client.dart | 9 ++- .../test_data/basic_project.dart | 62 +++++++++++++++++++ 5 files changed, 120 insertions(+), 8 deletions(-) diff --git a/packages/flutter_tools/lib/src/run_hot.dart b/packages/flutter_tools/lib/src/run_hot.dart index 2b8c4199efb..d00dd3508b3 100644 --- a/packages/flutter_tools/lib/src/run_hot.dart +++ b/packages/flutter_tools/lib/src/run_hot.dart @@ -603,14 +603,19 @@ class HotRunner extends ResidentRunner { // are not thread-safe, and thus must be run on the same thread that // would be blocked by the pause. Simply un-pausing is not sufficient, // because this does not prevent the isolate from immediately hitting - // a breakpoint, for example if the breakpoint was placed in a loop - // or in a frequently called method. Instead, all breakpoints are first - // disabled and then the isolate resumed. - final List> breakpointRemoval = >[ + // a breakpoint (for example if the breakpoint was placed in a loop + // or in a frequently called method) or an exception. Instead, all + // breakpoints are first disabled and exception pause mode set to + // None, and then the isolate resumed. + // These settings to not need restoring as Hot Restart results in + // new isolates, which will be configured by the editor as they are + // started. + final List> breakpointAndExceptionRemoval = >[ + device.vmService.service.setExceptionPauseMode(isolate.id, 'None'), for (final vm_service.Breakpoint breakpoint in isolate.breakpoints) device.vmService.service.removeBreakpoint(isolate.id, breakpoint.id) ]; - await Future.wait(breakpointRemoval); + await Future.wait(breakpointAndExceptionRemoval); await device.vmService.service.resume(view.uiIsolate.id); } })); diff --git a/packages/flutter_tools/test/general.shard/resident_runner_test.dart b/packages/flutter_tools/test/general.shard/resident_runner_test.dart index 4386a4d2c48..d85a372f15b 100644 --- a/packages/flutter_tools/test/general.shard/resident_runner_test.dart +++ b/packages/flutter_tools/test/general.shard/resident_runner_test.dart @@ -906,7 +906,7 @@ void main() { Usage: () => TestUsage(), })); - testUsingContext('ResidentRunner can remove breakpoints from paused isolate during hot restart', () => testbed.run(() async { + testUsingContext('ResidentRunner can remove breakpoints and exception-pause-mode from paused isolate during hot restart', () => testbed.run(() async { fakeVmServiceHost = FakeVmServiceHost(requests: [ listViews, listViews, @@ -922,6 +922,13 @@ void main() { method: 'getVM', jsonResponse: vm_service.VM.parse({}).toJson(), ), + const FakeVmServiceRequest( + method: 'setExceptionPauseMode', + args: { + 'isolateId': '1', + 'mode': 'None', + } + ), const FakeVmServiceRequest( method: 'removeBreakpoint', args: { diff --git a/packages/flutter_tools/test/integration.shard/debug_adapter/flutter_adapter_test.dart b/packages/flutter_tools/test/integration.shard/debug_adapter/flutter_adapter_test.dart index f7d76140fd7..a01858c31c6 100644 --- a/packages/flutter_tools/test/integration.shard/debug_adapter/flutter_adapter_test.dart +++ b/packages/flutter_tools/test/integration.shard/debug_adapter/flutter_adapter_test.dart @@ -187,6 +187,39 @@ void main() { await dap.client.terminate(); }); + + testWithoutContext('can hot restart when exceptions occur on outgoing isolates', () async { + final BasicProjectThatThrows _project = BasicProjectThatThrows(); + await _project.setUpIn(tempDir); + + // Launch the app and wait for it to stop at an exception. + int originalThreadId, newThreadId; + await Future.wait(>[ + // Capture the thread ID of the stopped thread. + dap.client.stoppedEvents.first.then((StoppedEventBody event) => originalThreadId = event.threadId), + dap.client.start( + exceptionPauseMode: 'All', // Ensure we stop on all exceptions + launch: () => dap.client.launch( + cwd: _project.dir.path, + toolArgs: ['-d', 'flutter-tester'], + ), + ), + ], eagerError: true); + + // Hot restart, ensuring it completes and capturing the ID of the new thread + // to pause. + await Future.wait(>[ + // Capture the thread ID of the newly stopped thread. + dap.client.stoppedEvents.first.then((StoppedEventBody event) => newThreadId = event.threadId), + dap.client.hotRestart(), + ], eagerError: true); + + // We should not have stopped on the original thread, but the new thread + // from after the restart. + expect(newThreadId, isNot(equals(originalThreadId))); + + await dap.client.terminate(); + }); } /// Extracts the output from a set of [OutputEventBody], removing any diff --git a/packages/flutter_tools/test/integration.shard/debug_adapter/test_client.dart b/packages/flutter_tools/test/integration.shard/debug_adapter/test_client.dart index 2359787e73c..7426e0b1b80 100644 --- a/packages/flutter_tools/test/integration.shard/debug_adapter/test_client.dart +++ b/packages/flutter_tools/test/integration.shard/debug_adapter/test_client.dart @@ -56,6 +56,10 @@ class DapTestClient { Stream get outputEvents => events('output') .map((Event e) => OutputEventBody.fromJson(e.body! as Map)); + /// Returns a stream of [StoppedEventBody] events. + Stream get stoppedEvents => events('stopped') + .map((Event e) => StoppedEventBody.fromJson(e.body! as Map)); + /// Returns a stream of the string output from [OutputEventBody] events. Stream get output => outputEvents.map((OutputEventBody output) => output.output); @@ -172,10 +176,11 @@ class DapTestClient { Future start({ String? program, String? cwd, + String exceptionPauseMode = 'None', Future Function()? launch, }) { return Future.wait(>[ - initialize(), + initialize(exceptionPauseMode: exceptionPauseMode), launch?.call() ?? this.launch(program: program, cwd: cwd), ], eagerError: true); } @@ -201,7 +206,7 @@ class DapTestClient { } else { completer.completeError(message); } - } else if (message is Event) { + } else if (message is Event && !_eventController.isClosed) { _eventController.add(message); // When we see a terminated event, close the event stream so if any diff --git a/packages/flutter_tools/test/integration.shard/test_data/basic_project.dart b/packages/flutter_tools/test/integration.shard/test_data/basic_project.dart index 0090d690034..7f3cc0f8d57 100644 --- a/packages/flutter_tools/test/integration.shard/test_data/basic_project.dart +++ b/packages/flutter_tools/test/integration.shard/test_data/basic_project.dart @@ -53,6 +53,68 @@ class BasicProject extends Project { int get topLevelFunctionBreakpointLine => lineContaining(main, '// TOP LEVEL BREAKPOINT'); } +/// A project that throws multiple exceptions during Widget builds. +/// +/// A repro for the issue at https://github.com/Dart-Code/Dart-Code/issues/3448 +/// where Hot Restart could become stuck on exceptions and never complete. +class BasicProjectThatThrows extends Project { + + @override + final String pubspec = ''' + name: test + environment: + sdk: ">=2.12.0-0 <3.0.0" + + dependencies: + flutter: + sdk: flutter + '''; + + @override + final String main = r''' + import 'package:flutter/material.dart'; + + void a() { + throw Exception('a'); + } + + void b() { + try { + a(); + } catch (e) { + throw Exception('b'); + } + } + + void c() { + try { + b(); + } catch (e) { + throw Exception('c'); + } + } + + void main() { + runApp(App()); + } + + class App extends StatelessWidget { + @override + Widget build(BuildContext context) { + c(); + return MaterialApp( + debugShowCheckedModeBanner: false, + title: 'Study Flutter', + theme: ThemeData( + primarySwatch: Colors.green, + ), + home: Container(), + ); + } + } + '''; +} + class BasicProjectWithTimelineTraces extends Project { @override final String pubspec = '''