From ecfdd7e1ea96be3c762af6a1bd6be63d9e4d9682 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Fri, 22 Mar 2019 14:32:36 -0700 Subject: [PATCH] Detect and cleanup leaky processes (#29196) * Detect and cleanup leaky processes * Add flaky tests for detecting leaked processes --- .../bin/tasks/run_without_leak_linux.dart | 14 + .../bin/tasks/run_without_leak_mac.dart | 14 + .../bin/tasks/run_without_leak_win.dart | 14 + dev/devicelab/lib/framework/framework.dart | 37 ++- .../lib/framework/running_processes.dart | 261 ++++++++++++++++++ dev/devicelab/lib/tasks/run_without_leak.dart | 61 ++++ dev/devicelab/manifest.yaml | 21 ++ .../test/running_processes_test.dart | 68 +++++ 8 files changed, 489 insertions(+), 1 deletion(-) create mode 100644 dev/devicelab/bin/tasks/run_without_leak_linux.dart create mode 100644 dev/devicelab/bin/tasks/run_without_leak_mac.dart create mode 100644 dev/devicelab/bin/tasks/run_without_leak_win.dart create mode 100644 dev/devicelab/lib/framework/running_processes.dart create mode 100644 dev/devicelab/lib/tasks/run_without_leak.dart create mode 100644 dev/devicelab/test/running_processes_test.dart diff --git a/dev/devicelab/bin/tasks/run_without_leak_linux.dart b/dev/devicelab/bin/tasks/run_without_leak_linux.dart new file mode 100644 index 0000000000..614ea70229 --- /dev/null +++ b/dev/devicelab/bin/tasks/run_without_leak_linux.dart @@ -0,0 +1,14 @@ +// Copyright (c) 2019 The Flutter Authors. All rights reserved. +// 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_devicelab/framework/utils.dart'; +import 'package:flutter_devicelab/tasks/run_without_leak.dart'; +import 'package:flutter_devicelab/framework/framework.dart'; +import 'package:path/path.dart' as path; + +Future main() async { + await task(createRunWithoutLeakTest(path.join(flutterDirectory.path, 'examples', 'hello_world'))); +} diff --git a/dev/devicelab/bin/tasks/run_without_leak_mac.dart b/dev/devicelab/bin/tasks/run_without_leak_mac.dart new file mode 100644 index 0000000000..614ea70229 --- /dev/null +++ b/dev/devicelab/bin/tasks/run_without_leak_mac.dart @@ -0,0 +1,14 @@ +// Copyright (c) 2019 The Flutter Authors. All rights reserved. +// 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_devicelab/framework/utils.dart'; +import 'package:flutter_devicelab/tasks/run_without_leak.dart'; +import 'package:flutter_devicelab/framework/framework.dart'; +import 'package:path/path.dart' as path; + +Future main() async { + await task(createRunWithoutLeakTest(path.join(flutterDirectory.path, 'examples', 'hello_world'))); +} diff --git a/dev/devicelab/bin/tasks/run_without_leak_win.dart b/dev/devicelab/bin/tasks/run_without_leak_win.dart new file mode 100644 index 0000000000..614ea70229 --- /dev/null +++ b/dev/devicelab/bin/tasks/run_without_leak_win.dart @@ -0,0 +1,14 @@ +// Copyright (c) 2019 The Flutter Authors. All rights reserved. +// 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_devicelab/framework/utils.dart'; +import 'package:flutter_devicelab/tasks/run_without_leak.dart'; +import 'package:flutter_devicelab/framework/framework.dart'; +import 'package:path/path.dart' as path; + +Future main() async { + await task(createRunWithoutLeakTest(path.join(flutterDirectory.path, 'examples', 'hello_world'))); +} diff --git a/dev/devicelab/lib/framework/framework.dart b/dev/devicelab/lib/framework/framework.dart index b3063a54e6..c7372a7f55 100644 --- a/dev/devicelab/lib/framework/framework.dart +++ b/dev/devicelab/lib/framework/framework.dart @@ -11,6 +11,7 @@ import 'dart:isolate'; import 'package:logging/logging.dart'; import 'package:stack_trace/stack_trace.dart'; +import 'running_processes.dart'; import 'utils.dart'; /// Maximum amount of time a single task is allowed to take to run. @@ -82,7 +83,37 @@ class _TaskRunner { try { _taskStarted = true; print('Running task.'); - final TaskResult result = await _performTask().timeout(taskTimeout); + final String exe = Platform.isWindows ? '.exe' : ''; + section('Checking running Dart$exe processes'); + final Set beforeRunningDartInstances = await getRunningProcesses( + processName: 'dart$exe', + ).toSet(); + beforeRunningDartInstances.forEach(print); + + TaskResult result = await _performTask().timeout(taskTimeout); + + section('Checking running Dart$exe processes after task...'); + final List afterRunningDartInstances = await getRunningProcesses( + processName: 'dart$exe', + ).toList(); + for (final RunningProcessInfo info in afterRunningDartInstances) { + if (!beforeRunningDartInstances.contains(info)) { + print('$info was leaked by this test.'); + // TODO(dnfield): remove this special casing after https://github.com/flutter/flutter/issues/29141 is resolved. + if (result is TaskResultCheckProcesses) { + result = TaskResult.failure('This test leaked dart processes'); + } else { + result = TaskResult.success(null); + } + final bool killed = await killProcess(info.pid); + if (!killed) { + print('Failed to kill process ${info.pid}.'); + } else { + print('Killed process id ${info.pid}.'); + } + } + } + _completer.complete(result); return result; } on TimeoutException catch (_) { @@ -231,3 +262,7 @@ class TaskResult { return json; } } + +class TaskResultCheckProcesses extends TaskResult { + TaskResultCheckProcesses() : super.success(null); +} diff --git a/dev/devicelab/lib/framework/running_processes.dart b/dev/devicelab/lib/framework/running_processes.dart new file mode 100644 index 0000000000..0efb77c4a4 --- /dev/null +++ b/dev/devicelab/lib/framework/running_processes.dart @@ -0,0 +1,261 @@ +// Copyright 2019 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import 'dart:io'; + +import 'package:meta/meta.dart'; +import 'package:process/process.dart'; + +@immutable +class RunningProcessInfo { + const RunningProcessInfo(this.pid, this.creationDate, this.commandLine) + : assert(pid != null), + assert(commandLine != null); + + final String commandLine; + final int pid; + final DateTime creationDate; + + @override + bool operator ==(Object other) { + return other is RunningProcessInfo && + other.pid == pid && + other.commandLine == commandLine && + other.creationDate == creationDate; + } + + @override + int get hashCode { + // TODO(dnfield): Replace this when Object.hashValues lands. + int hash = 17; + if (pid != null) { + hash = hash * 23 + pid.hashCode; + } + if (commandLine != null) { + hash = hash * 23 + commandLine.hashCode; + } + if (creationDate != null) { + hash = hash * 23 + creationDate.hashCode; + } + return hash; + } + + @override + String toString() { + return 'RunningProcesses{pid: $pid, commandLine: $commandLine, creationDate: $creationDate}'; + } +} + +Future killProcess(int pid, {ProcessManager processManager}) async { + assert(pid != null, 'Must specify a pid to kill'); + processManager ??= const LocalProcessManager(); + ProcessResult result; + if (Platform.isWindows) { + result = await processManager.run([ + 'taskkill.exe', + '/pid', + pid.toString(), + '/f', + ]); + } else { + result = await processManager.run([ + 'kill', + '-9', + pid.toString(), + ]); + } + return result.exitCode == 0; +} + +Stream getRunningProcesses({ + String processName, + ProcessManager processManager, +}) { + processManager ??= const LocalProcessManager(); + if (Platform.isWindows) { + return windowsRunningProcesses(processName); + } + return posixRunningProcesses(processName, processManager); +} + +@visibleForTesting +Stream windowsRunningProcesses(String processName) async* { + // PowerShell script to get the command line arguments and create time of + // a process. + // See: https://docs.microsoft.com/en-us/windows/desktop/cimwin32prov/win32-process + final String script = processName != null + ? '"Get-CimInstance Win32_Process -Filter \\\"name=\'$processName\'\\\" | Select-Object ProcessId,CreationDate,CommandLine | Format-Table -AutoSize | Out-String -Width 4096"' + : '"Get-CimInstance Win32_Process | Select-Object ProcessId,CreationDate,CommandLine | Format-Table -AutoSize | Out-String -Width 4096"'; + // Unfortunately, there doesn't seem to be a good way to get ProcessManager to + // run this. May be a bug in Dart. + // TODO(dnfield): fix this when https://github.com/dart-lang/sdk/issues/36175 is resolved. + final ProcessResult result = await Process.run( + 'powershell -command $script', + [], + ); + if (result.exitCode != 0) { + print('Could not list processes!'); + print(result.stderr); + print(result.stdout); + return; + } + for (RunningProcessInfo info in processPowershellOutput(result.stdout)) { + yield info; + } +} + +/// Parses the output of the PowerShell script from [windowsRunningProcesses]. +/// +/// E.g.: +/// ProcessId CreationDate CommandLine +/// --------- ------------ ----------- +/// 2904 3/11/2019 11:01:54 AM "C:\Program Files\Android\Android Studio\jre\bin\java.exe" -Xmx1536M -Dfile.encoding=windows-1252 -Duser.country=US -Duser.language=en -Duser.variant -cp C:\Users\win1\.gradle\wrapper\dists\gradle-4.10.2-all\9fahxiiecdb76a5g3aw9oi8rv\gradle-4.10.2\lib\gradle-launcher-4.10.2.jar org.gradle.launcher.daemon.bootstrap.GradleDaemon 4.10.2 +@visibleForTesting +Iterable processPowershellOutput(String output) sync* { + if (output == null) { + return; + } + + const int processIdHeaderSize = 'ProcessId'.length; + const int creationDateHeaderStart = processIdHeaderSize + 1; + int creationDateHeaderEnd; + int commandLineHeaderStart; + bool inTableBody = false; + for (String line in output.split('\n')) { + if (line.startsWith('ProcessId')) { + commandLineHeaderStart = line.indexOf('CommandLine'); + creationDateHeaderEnd = commandLineHeaderStart - 1; + } + if (line.startsWith('--------- ------------')) { + inTableBody = true; + continue; + } + if (!inTableBody || line.isEmpty) { + continue; + } + if (line.length < commandLineHeaderStart) { + continue; + } + + // 3/11/2019 11:01:54 AM + // 12/11/2019 11:01:54 AM + String rawTime = line.substring( + creationDateHeaderStart, + creationDateHeaderEnd, + ).trim(); + + if (rawTime[1] == '/') { + rawTime = '0$rawTime'; + } + if (rawTime[4] == '/') { + rawTime = rawTime.substring(0, 3) + '0' + rawTime.substring(3); + } + final String year = rawTime.substring(6, 10); + final String month = rawTime.substring(3, 5); + final String day = rawTime.substring(0, 2); + String time = rawTime.substring(11, 19); + if (time[7] == ' ') { + time = '0$time'.trim(); + } + if (rawTime.endsWith('PM')) { + final int hours = int.parse(time.substring(0, 2)); + time = '${hours + 12}${time.substring(2)}'; + } + + final int pid = int.parse(line.substring(0, processIdHeaderSize).trim()); + final DateTime creationDate = DateTime.parse('$year-$month-${day}T$time'); + final String commandLine = line.substring(commandLineHeaderStart).trim(); + yield RunningProcessInfo(pid, creationDate, commandLine); + } +} + +@visibleForTesting +Stream posixRunningProcesses( + String processName, + ProcessManager processManager, +) async* { + // Cirrus is missing this in Linux for some reason. + if (!processManager.canRun('ps')) { + print('Cannot list processes on this system: `ps` not available.'); + return; + } + final ProcessResult result = await processManager.run([ + 'ps', + '-eo', + 'lstart,pid,command', + ]); + if (result.exitCode != 0) { + print('Could not list processes!'); + print(result.stderr); + print(result.stdout); + return; + } + for (RunningProcessInfo info in processPsOutput(result.stdout, processName)) { + yield info; + } +} + +/// Parses the output of the command in [posixRunningProcesses]. +/// +/// E.g.: +/// +/// STARTED PID COMMAND +/// Sat Mar 9 20:12:47 2019 1 /sbin/launchd +/// Sat Mar 9 20:13:00 2019 49 /usr/sbin/syslogd +@visibleForTesting +Iterable processPsOutput( + String output, + String processName, +) sync* { + if (output == null) { + return; + } + bool inTableBody = false; + for (String line in output.split('\n')) { + if (line.trim().startsWith('STARTED')) { + inTableBody = true; + continue; + } + if (!inTableBody || line.isEmpty) { + continue; + } + + if (processName != null && !line.contains(processName)) { + continue; + } + if (line.length < 25) { + continue; + } + + // 'Sat Feb 16 02:29:55 2019' + // 'Sat Mar 9 20:12:47 2019' + const Map months = { + 'Jan': '01', + 'Feb': '02', + 'Mar': '03', + 'Apr': '04', + 'May': '05', + 'Jun': '06', + 'Jul': '07', + 'Aug': '08', + 'Sep': '09', + 'Oct': '10', + 'Nov': '11', + 'Dec': '12', + }; + final String rawTime = line.substring(0, 24); + + final String year = rawTime.substring(20, 24); + final String month = months[rawTime.substring(4, 7)]; + final String day = rawTime.substring(8, 10).replaceFirst(' ', '0'); + final String time = rawTime.substring(11, 19); + + final DateTime creationDate = DateTime.parse('$year-$month-${day}T$time'); + line = line.substring(24).trim(); + final int nextSpace = line.indexOf(' '); + final int pid = int.parse(line.substring(0, nextSpace)); + final String commandLine = line.substring(nextSpace + 1); + yield RunningProcessInfo(pid, creationDate, commandLine); + } +} diff --git a/dev/devicelab/lib/tasks/run_without_leak.dart b/dev/devicelab/lib/tasks/run_without_leak.dart new file mode 100644 index 0000000000..eb9ff50115 --- /dev/null +++ b/dev/devicelab/lib/tasks/run_without_leak.dart @@ -0,0 +1,61 @@ +// Copyright 2019 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import 'dart:async'; +import 'dart:convert'; +import 'dart:io'; + +import 'package:path/path.dart' as path; + +import '../framework/adb.dart'; +import '../framework/framework.dart'; +import '../framework/utils.dart'; + +TaskFunction createRunWithoutLeakTest(dynamic dir) { + return () async { + final Device device = await devices.workingDevice; + await device.unlock(); + final List options = [ + '-d', device.deviceId, '--verbose', + ]; + setLocalEngineOptionIfNecessary(options); + int exitCode; + await inDirectory(dir, () async { + final Process process = await startProcess( + path.join(flutterDirectory.path, 'bin', 'flutter'), + ['run']..addAll(options), + environment: null, + ); + final Completer stdoutDone = Completer(); + final Completer stderrDone = Completer(); + process.stdout + .transform(utf8.decoder) + .transform(const LineSplitter()) + .listen((String line) { + if (line.contains('\] For a more detailed help message, press "h". To detach, press "d"; to quit, press "q"')) { + process.stdin.writeln('q'); + } + print('stdout: $line'); + }, onDone: () { + stdoutDone.complete(); + }); + process.stderr + .transform(utf8.decoder) + .transform(const LineSplitter()) + .listen((String line) { + print('stderr: $line'); + }, onDone: () { + stderrDone.complete(); + }); + + await Future.wait( + >[stdoutDone.future, stderrDone.future]); + exitCode = await process.exitCode; + }); + + return exitCode == 0 + ? TaskResultCheckProcesses() + : TaskResult.failure('Failed to run $dir'); + }; +} diff --git a/dev/devicelab/manifest.yaml b/dev/devicelab/manifest.yaml index 3402f11201..a5db41a1a1 100644 --- a/dev/devicelab/manifest.yaml +++ b/dev/devicelab/manifest.yaml @@ -486,6 +486,13 @@ tasks: stage: devicelab_win required_agent_capabilities: ["windows/android"] + run_without_leak_win: + description: > + Checks that `flutter run` does not leak dart.exe on Windows. + stage: devicelab_win + required_agent_capabilities: ["windows/android"] + flaky: true + # Tests running on Linux hosts hot_mode_dev_cycle_linux__benchmark: @@ -551,6 +558,13 @@ tasks: stage: devicelab required_agent_capabilities: ["linux/android"] + run_without_leak_linux: + description: > + Checks that `flutter run` does not leak dart on Linux. + stage: devicelab + required_agent_capabilities: ["linux/android"] + flaky: true + flutter_gallery_ios32__start_up: description: > Measures the startup time of the Flutter Gallery app on 32-bit iOS. @@ -563,3 +577,10 @@ tasks: 32-bit iOS. stage: devicelab_ios required_agent_capabilities: ["mac/ios32"] + + run_without_leak_mac: + description: > + Checks that `flutter run` does not leak dart on macOS. + stage: devicelab + required_agent_capabilities: ["mac/android"] + flaky: true diff --git a/dev/devicelab/test/running_processes_test.dart b/dev/devicelab/test/running_processes_test.dart new file mode 100644 index 0000000000..395ada4673 --- /dev/null +++ b/dev/devicelab/test/running_processes_test.dart @@ -0,0 +1,68 @@ +// Copyright 2018 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import 'package:flutter_devicelab/framework/running_processes.dart'; +import 'common.dart'; + +void main() { + test('Parse PowerShell result', () { + const String powershellOutput = r''' + +ProcessId CreationDate CommandLine +--------- ------------ ----------- + 6552 3/7/2019 5:00:27 PM "C:\tools\dart-sdk\bin\dart.exe" .\bin\agent.dart ci + 6553 3/7/2019 10:00:27 PM "C:\tools\dart-sdk1\bin\dart.exe" .\bin\agent.dart ci + 6554 3/7/2019 11:00:27 AM "C:\tools\dart-sdk2\bin\dart.exe" .\bin\agent.dart ci + + +'''; + final List results = + processPowershellOutput(powershellOutput).toList(); + expect(results.length, 3); + expect( + results, + equals([ + RunningProcessInfo( + 6552, + DateTime(2019, 7, 3, 17, 0, 27), + r'"C:\tools\dart-sdk\bin\dart.exe" .\bin\agent.dart ci', + ), + RunningProcessInfo( + 6553, + DateTime(2019, 7, 3, 22, 0, 27), + r'"C:\tools\dart-sdk1\bin\dart.exe" .\bin\agent.dart ci', + ), + RunningProcessInfo( + 6554, + DateTime(2019, 7, 3, 11, 0, 27), + r'"C:\tools\dart-sdk2\bin\dart.exe" .\bin\agent.dart ci', + ), + ])); + }); + + test('Parse Posix output', () { + const String psOutput = r'''STARTED PID COMMAND +Sat Mar 9 20:12:47 2019 1 /sbin/launchd +Sat Mar 9 20:13:00 2019 49 /usr/sbin/syslogd +'''; + + final List results = + processPsOutput(psOutput, null).toList(); + expect(results.length, 2); + expect( + results, + equals([ + RunningProcessInfo( + 1, + DateTime(2019, 3, 9, 20, 12, 47), + '/sbin/launchd', + ), + RunningProcessInfo( + 49, + DateTime(2019, 3, 9, 20, 13, 00), + '/usr/sbin/syslogd', + ), + ])); + }); +}