Skip to content

Commit

Permalink
[flutter_tool] Kill a timing out process before trying to drain its s…
Browse files Browse the repository at this point in the history
…tdout/err streams (flutter#40159)
  • Loading branch information
zanderso authored and Inconnu08 committed Sep 30, 2019
1 parent 490eb02 commit bc851af
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 7 deletions.
15 changes: 11 additions & 4 deletions packages/flutter_tools/lib/src/base/process.dart
Expand Up @@ -275,13 +275,23 @@ Future<RunResult> runAsync(

int exitCode;
exitCode = await process.exitCode.timeout(timeout, onTimeout: () {
// The process timed out. Kill it.
processManager.killPid(process.pid);
return null;
});

String stdoutString;
String stderrString;
try {
await Future.wait<void>(<Future<void>>[stdoutFuture, stderrFuture]);
Future<void> stdioFuture =
Future.wait<void>(<Future<void>>[stdoutFuture, stderrFuture]);
if (exitCode == null) {
// If we had to kill the process for a timeout, only wait a short time
// for the stdio streams to drain in case killing the process didn't
// work.
stdioFuture = stdioFuture.timeout(const Duration(seconds: 1));
}
await stdioFuture;
} catch (_) {
// Ignore errors on the process' stdout and stderr streams. Just capture
// whatever we got, and use the exit code
Expand All @@ -299,9 +309,6 @@ Future<RunResult> runAsync(
return runResult;
}

// The process timed out. Kill it.
processManager.killPid(process.pid);

// If we are out of timeoutRetries, throw a ProcessException.
if (timeoutRetries < 0) {
throw ProcessException(cmd[0], cmd.sublist(1),
Expand Down
28 changes: 25 additions & 3 deletions packages/flutter_tools/test/general.shard/base/process_test.dart
Expand Up @@ -2,6 +2,8 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

import 'dart:async';

import 'package:flutter_tools/src/base/io.dart';
import 'package:flutter_tools/src/base/logger.dart';
import 'package:flutter_tools/src/base/platform.dart';
Expand Down Expand Up @@ -103,13 +105,13 @@ void main() {
// MockProcessManager has an implementation of start() that returns the
// result of processFactory.
flakyProcessManager = MockProcessManager();
});

testUsingContext('flaky process fails without retry', () async {
flakyProcessManager.processFactory = flakyProcessFactory(
flakes: 1,
delay: delay,
);
});

testUsingContext('flaky process fails without retry', () async {
final RunResult result = await runAsync(
<String>['dummy'],
timeout: delay + const Duration(seconds: 1),
Expand All @@ -120,6 +122,10 @@ void main() {
});

testUsingContext('flaky process succeeds with retry', () async {
flakyProcessManager.processFactory = flakyProcessFactory(
flakes: 1,
delay: delay,
);
final RunResult result = await runAsync(
<String>['dummy'],
timeout: delay - const Duration(milliseconds: 500),
Expand All @@ -131,6 +137,22 @@ void main() {
});

testUsingContext('flaky process generates ProcessException on timeout', () async {
final Completer<List<int>> flakyStderr = Completer<List<int>>();
final Completer<List<int>> flakyStdout = Completer<List<int>>();
flakyProcessManager.processFactory = flakyProcessFactory(
flakes: 1,
delay: delay,
stderr: () => Stream<List<int>>.fromFuture(flakyStderr.future),
stdout: () => Stream<List<int>>.fromFuture(flakyStdout.future),
);
when(flakyProcessManager.killPid(any)).thenAnswer((_) {
// Don't let the stderr stream stop until the process is killed. This
// ensures that runAsync() does not delay killing the process until
// stdout and stderr are drained (which won't happen).
flakyStderr.complete(<int>[]);
flakyStdout.complete(<int>[]);
return true;
});
expect(() async => await runAsync(
<String>['dummy'],
timeout: delay - const Duration(milliseconds: 500),
Expand Down

0 comments on commit bc851af

Please sign in to comment.