Make cleanup of flutter processes in tests more reliable (#19307)

* Make cleanup of flutter processes in tests more reliable

* Fix quit signals

I confused SigInt&SigTerm for SigTerm&SigKill when I first did this. SigTerm can be blocked and doesn't guarantee the process will be terminated.

* Don't use deprecated constants

* Fix typo

* Add some additional info to debug buffer

* Fix return types on Futures

packages/flutter_tools/lib/src/base/io.dart

@@ -116,11 +116,12 @@ class ProcessSignal implements io.ProcessSignal {
   @visibleForTesting
   const ProcessSignal(this._delegate);
 
-  static const ProcessSignal SIGWINCH = const _PosixProcessSignal._(io.ProcessSignal.SIGWINCH); // ignore: deprecated_member_use
-  static const ProcessSignal SIGTERM = const _PosixProcessSignal._(io.ProcessSignal.SIGTERM); // ignore: deprecated_member_use
-  static const ProcessSignal SIGUSR1 = const _PosixProcessSignal._(io.ProcessSignal.SIGUSR1); // ignore: deprecated_member_use
-  static const ProcessSignal SIGUSR2 = const _PosixProcessSignal._(io.ProcessSignal.SIGUSR2); // ignore: deprecated_member_use
-  static const ProcessSignal SIGINT =  const ProcessSignal(io.ProcessSignal.SIGINT); // ignore: deprecated_member_use
+  static const ProcessSignal SIGWINCH = const _PosixProcessSignal._(io.ProcessSignal.sigwinch);
+  static const ProcessSignal SIGTERM = const _PosixProcessSignal._(io.ProcessSignal.sigterm);
+  static const ProcessSignal SIGUSR1 = const _PosixProcessSignal._(io.ProcessSignal.sigusr1);
+  static const ProcessSignal SIGUSR2 = const _PosixProcessSignal._(io.ProcessSignal.sigusr2);
+  static const ProcessSignal SIGINT =  const ProcessSignal(io.ProcessSignal.sigint);
+  static const ProcessSignal SIGKILL =  const ProcessSignal(io.ProcessSignal.sigkill);
 
   final io.ProcessSignal _delegate;
 

packages/flutter_tools/test/integration/test_driver.dart

@@ -20,10 +20,12 @@ import '../src/common.dart';
 const bool _printJsonAndStderr = false;
 const Duration defaultTimeout = const Duration(seconds: 20);
 const Duration appStartTimeout = const Duration(seconds: 60);
+const Duration quitTimeout = const Duration(seconds: 5);
 
 class FlutterTestDriver {
   Directory _projectFolder;
   Process _proc;
+  int _procPid;
   final StreamController<String> _stdout = new StreamController<String>.broadcast();
   final StreamController<String> _stderr = new StreamController<String>.broadcast();
   final StreamController<String> _allMessages = new StreamController<String>.broadcast();
@@ -62,6 +64,12 @@ class FlutterTestDriver {
     _stdout.stream.listen(_debugPrint);
     _stderr.stream.listen(_debugPrint);
 
+    // Stash the PID so that we can terminate the VM more reliably than using
+    // _proc.kill() (because _proc is a shell, because `flutter` is a shell
+    // script).
+    final Map<String, dynamic> connected = await _waitFor(event: 'daemon.connected');
+    _procPid = connected['params']['pid'];
+    
     // Set this up now, but we don't wait it yet. We want to make sure we don't
     // miss it while waiting for debugPort below.
     final Future<Map<String, dynamic>> started = _waitFor(event: 'app.started',
@@ -113,15 +121,37 @@ class FlutterTestDriver {
 
   Future<int> stop() async {
     if (vmService != null) {
-      await vmService.close();
+      _debugPrint('Closing VM service');
+      await vmService.close()
+          .timeout(quitTimeout,
+              onTimeout: () { _debugPrint('VM Service did not quit within $quitTimeout'); });
     }
     if (_currentRunningAppId != null) {
+      _debugPrint('Stopping app');
       await _sendRequest(
-          'app.stop',
-          <String, dynamic>{'appId': _currentRunningAppId}
+        'app.stop',
+        <String, dynamic>{'appId': _currentRunningAppId}
+      ).timeout(
+        quitTimeout,
+        onTimeout: () { _debugPrint('app.stop did not return within $quitTimeout'); }
       );
+      _currentRunningAppId = null;
     }
-    _currentRunningAppId = null;
+    _debugPrint('Waiting for process to end');
+    return _proc.exitCode.timeout(quitTimeout, onTimeout: _killGracefully);
+  }
+
+  Future<int> _killGracefully() async {
+    if (_procPid == null)
+      return -1;
+    _debugPrint('Sending SIGTERM to $_procPid..');
+    Process.killPid(_procPid);
+    return _proc.exitCode.timeout(quitTimeout, onTimeout: _killForcefully);
+  }
+
+  Future<int> _killForcefully() {
+    _debugPrint('Sending SIGKILL to $_procPid..');
+    Process.killPid(_procPid, ProcessSignal.SIGKILL);
     return _proc.exitCode;
   }