Skip to content
This repository was archived by the owner on Feb 22, 2023. It is now read-only.

Commit 5d31b07

Browse files
authored
[flutter_tools] [dap] Ensure DAP sends app.stop/app.detach during terminate (#108310)
* [flutter_tools] [dap] Ensure DAP sends app.stop/app.detach during terminate Fixes an issue where the flutter_tester device may not be cleaned up correctly if we just terminate the Flutter process. * Update integration test expectations * Revert accidental commit
1 parent 925bee9 commit 5d31b07

4 files changed

Lines changed: 69 additions & 0 deletions

File tree

packages/flutter_tools/lib/src/debug_adapters/flutter_adapter.dart

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -400,6 +400,16 @@ class FlutterDebugAdapter extends DartDebugAdapter<FlutterLaunchRequestArguments
400400
if (isAttach) {
401401
await preventBreakingAndResume();
402402
}
403+
404+
// Send a request to stop/detach to give Flutter chance to do some cleanup.
405+
// It's possible the Flutter process will terminate before we process the
406+
// response, so accept either a response or the process exiting.
407+
final String method = isAttach ? 'app.detach' : 'app.stop';
408+
await Future.any<void>(<Future<void>>[
409+
sendFlutterRequest(method, <String, Object?>{'appId': _appId}),
410+
_process?.exitCode ?? Future<void>.value(),
411+
]);
412+
403413
terminatePids(ProcessSignal.sigterm);
404414
await _process?.exitCode;
405415
}

packages/flutter_tools/test/general.shard/dap/flutter_adapter_test.dart

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
import 'dart:async';
66

7+
import 'package:dds/dap.dart';
78
import 'package:file/memory.dart';
89
import 'package:flutter_tools/src/base/platform.dart';
910
import 'package:flutter_tools/src/cache.dart';
@@ -96,6 +97,29 @@ void main() {
9697
// Ensure the VM's pid was not recorded.
9798
expect(adapter.pidsToTerminate, isNot(contains(123)));
9899
});
100+
101+
test('calls "app.stop" on terminateRequest', () async {
102+
final MockFlutterDebugAdapter adapter = MockFlutterDebugAdapter(
103+
fileSystem: MemoryFileSystem.test(style: fsStyle),
104+
platform: platform,
105+
);
106+
107+
final FlutterLaunchRequestArguments args = FlutterLaunchRequestArguments(
108+
cwd: '/project',
109+
program: 'foo.dart',
110+
);
111+
112+
await adapter.configurationDoneRequest(MockRequest(), null, () {});
113+
final Completer<void> launchCompleter = Completer<void>();
114+
await adapter.launchRequest(MockRequest(), args, launchCompleter.complete);
115+
await launchCompleter.future;
116+
117+
final Completer<void> terminateCompleter = Completer<void>();
118+
await adapter.terminateRequest(MockRequest(), TerminateArguments(restart: false), terminateCompleter.complete);
119+
await terminateCompleter.future;
120+
121+
expect(adapter.flutterRequests, contains('app.stop'));
122+
});
99123
});
100124

101125
group('attachRequest', () {
@@ -139,6 +163,28 @@ void main() {
139163
// Ensure the VM's pid was not recorded.
140164
expect(adapter.pidsToTerminate, isNot(contains(123)));
141165
});
166+
167+
test('calls "app.detach" on terminateRequest', () async {
168+
final MockFlutterDebugAdapter adapter = MockFlutterDebugAdapter(
169+
fileSystem: MemoryFileSystem.test(style: fsStyle),
170+
platform: platform,
171+
);
172+
173+
final FlutterAttachRequestArguments args = FlutterAttachRequestArguments(
174+
cwd: '/project',
175+
);
176+
177+
await adapter.configurationDoneRequest(MockRequest(), null, () {});
178+
final Completer<void> attachCompleter = Completer<void>();
179+
await adapter.attachRequest(MockRequest(), args, attachCompleter.complete);
180+
await attachCompleter.future;
181+
182+
final Completer<void> terminateCompleter = Completer<void>();
183+
await adapter.terminateRequest(MockRequest(), TerminateArguments(restart: false), terminateCompleter.complete);
184+
await terminateCompleter.future;
185+
186+
expect(adapter.flutterRequests, contains('app.detach'));
187+
});
142188
});
143189

144190
group('--start-paused', () {

packages/flutter_tools/test/general.shard/dap/mocks.dart

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ class MockFlutterDebugAdapter extends FlutterDebugAdapter {
4343
late String executable;
4444
late List<String> processArgs;
4545
late Map<String, String>? env;
46+
final List<String> flutterRequests = <String>[];
4647

4748
@override
4849
Future<void> launchAsProcess({
@@ -59,6 +60,16 @@ class MockFlutterDebugAdapter extends FlutterDebugAdapter {
5960
appStartedCompleter.complete();
6061
}
6162

63+
@override
64+
Future<Object?> sendFlutterRequest(
65+
String method,
66+
Map<String, Object?>? params, {
67+
bool failSilently = true,
68+
}) {
69+
flutterRequests.add(method);
70+
return super.sendFlutterRequest(method, params, failSilently: failSilently);
71+
}
72+
6273
@override
6374
Future<void> get debuggerInitialized {
6475
// If we were mocking debug mode, then simulate the debugger initializing.

packages/flutter_tools/test/integration.shard/debug_adapter/flutter_adapter_test.dart

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ void main() {
6464
'Launching $relativeMainPath on Flutter test device in debug mode...',
6565
startsWith('Connecting to VM Service at'),
6666
'topLevelFunction',
67+
'Application finished.',
6768
'',
6869
startsWith('Exited'),
6970
]);
@@ -94,6 +95,7 @@ void main() {
9495
expectLines(output, <Object>[
9596
'Launching $relativeMainPath on Flutter test device in debug mode...',
9697
'topLevelFunction',
98+
'Application finished.',
9799
'',
98100
startsWith('Exited'),
99101
]);

0 commit comments

Comments
 (0)