Unverified Commit 45f3c8d0 authored by Zachary Anderson's avatar Zachary Anderson Committed by GitHub

[flutter_tool] Report to analytics when the tool is killed by a signal (#41493)

* [flutter_tool] Report to analytics when the tool is killed by a signal

* Fix analyzer lint
parent 7e73cd74
......@@ -12,11 +12,20 @@ typedef SignalHandler = FutureOr<void> Function(ProcessSignal signal);
Signals get signals => Signals.instance;
// The default list of signals that should cause the process to exit.
const List<ProcessSignal> _defaultExitSignals = <ProcessSignal>[
ProcessSignal.SIGTERM,
ProcessSignal.SIGINT,
ProcessSignal.SIGKILL,
];
/// A class that manages signal handlers
///
/// Signal handlers are run in the order that they were added.
abstract class Signals {
factory Signals() => _DefaultSignals._();
factory Signals({
List<ProcessSignal> exitSignals = _defaultExitSignals,
}) => _DefaultSignals._(exitSignals);
static Signals get instance => context.get<Signals>();
......@@ -40,7 +49,9 @@ abstract class Signals {
}
class _DefaultSignals implements Signals {
_DefaultSignals._();
_DefaultSignals._(this.exitSignals);
final List<ProcessSignal> exitSignals;
// A table mapping (signal, token) -> signal handler.
final Map<ProcessSignal, Map<Object, SignalHandler>> _handlersTable =
......@@ -119,12 +130,5 @@ class _DefaultSignals implements Signals {
}
}
// The list of signals that should cause the process to exit.
static const List<ProcessSignal> _exitingSignals = <ProcessSignal>[
ProcessSignal.SIGTERM,
ProcessSignal.SIGINT,
ProcessSignal.SIGKILL,
];
bool _shouldExitFor(ProcessSignal signal) => _exitingSignals.contains(signal);
bool _shouldExitFor(ProcessSignal signal) => exitSignals.contains(signal);
}
......@@ -14,6 +14,7 @@ import '../base/common.dart';
import '../base/context.dart';
import '../base/file_system.dart';
import '../base/io.dart' as io;
import '../base/signals.dart';
import '../base/terminal.dart';
import '../base/time.dart';
import '../base/user_messages.dart';
......@@ -37,6 +38,7 @@ enum ExitStatus {
success,
warning,
fail,
killed,
}
/// [FlutterCommand]s' subclasses' [FlutterCommand.runCommand] can optionally
......@@ -73,6 +75,8 @@ class FlutterCommandResult {
return 'warning';
case ExitStatus.fail:
return 'fail';
case ExitStatus.killed:
return 'killed';
default:
assert(false);
return null;
......@@ -447,6 +451,7 @@ abstract class FlutterCommand extends Command<void> {
flutterUsage.printWelcome();
}
final String commandPath = await usagePath;
_registerSignalHandlers(commandPath, startTime);
FlutterCommandResult commandResult;
try {
commandResult = await verifyThenRunCommand(commandPath);
......@@ -462,6 +467,19 @@ abstract class FlutterCommand extends Command<void> {
);
}
void _registerSignalHandlers(String commandPath, DateTime startTime) {
final SignalHandler handler = (io.ProcessSignal s) {
_sendPostUsage(
commandPath,
const FlutterCommandResult(ExitStatus.killed),
startTime,
systemClock.now(),
);
};
signals.addHandler(io.ProcessSignal.SIGTERM, handler);
signals.addHandler(io.ProcessSignal.SIGINT, handler);
}
/// Logs data about this command.
///
/// For example, the command path (e.g. `build/apk`) and the result,
......
......@@ -34,6 +34,8 @@ void main() {
controller.add(mockSignal);
await completer.future;
}, overrides: <Type, Generator>{
Signals: () => Signals(),
});
testUsingContext('signal handlers run in order', () async {
......@@ -54,6 +56,8 @@ void main() {
controller.add(mockSignal);
await completer.future;
}, overrides: <Type, Generator>{
Signals: () => Signals(),
});
testUsingContext('signal handler error goes on error stream', () async {
......@@ -72,6 +76,8 @@ void main() {
await completer.future;
await errSub.cancel();
expect(errList, <Object>['Error']);
}, overrides: <Type, Generator>{
Signals: () => Signals(),
});
testUsingContext('removed signal handler does not run', () async {
......@@ -90,6 +96,8 @@ void main() {
await errSub.cancel();
expect(errList, isEmpty);
}, overrides: <Type, Generator>{
Signals: () => Signals(),
});
testUsingContext('non-removed signal handler still runs', () async {
......@@ -113,6 +121,8 @@ void main() {
await completer.future;
await errSub.cancel();
expect(errList, isEmpty);
}, overrides: <Type, Generator>{
Signals: () => Signals(),
});
testUsingContext('only handlers for the correct signal run', () async {
......@@ -141,6 +151,42 @@ void main() {
await completer.future;
await errSub.cancel();
expect(errList, isEmpty);
}, overrides: <Type, Generator>{
Signals: () => Signals(),
});
testUsingContext('all handlers for exiting signals are run before exit', () async {
final Completer<void> completer = Completer<void>();
bool first = false;
bool second = false;
setExitFunctionForTests((int exitCode) {
// Both handlers have run before exit is called.
expect(first, isTrue);
expect(second, isTrue);
expect(exitCode, 0);
restoreExitFunction();
completer.complete();
});
signals.addHandler(signalUnderTest, (ProcessSignal s) {
expect(s, signalUnderTest);
expect(first, isFalse);
expect(second, isFalse);
first = true;
});
signals.addHandler(signalUnderTest, (ProcessSignal s) {
expect(s, signalUnderTest);
expect(first, isTrue);
expect(second, isFalse);
second = true;
});
controller.add(mockSignal);
await completer.future;
}, overrides: <Type, Generator>{
Signals: () => Signals(exitSignals: <ProcessSignal>[signalUnderTest]),
});
});
}
......
......@@ -39,6 +39,7 @@ void main() {
fuchsiaArtifactsNoCompiler = MockFuchsiaArtifacts();
when(linuxPlatform.isLinux).thenReturn(true);
when(linuxPlatform.isWindows).thenReturn(false);
when(windowsPlatform.isWindows).thenReturn(true);
when(windowsPlatform.isLinux).thenReturn(false);
when(windowsPlatform.isMacOS).thenReturn(false);
......
......@@ -49,7 +49,9 @@ void main() {
return const Stream<List<int>>.empty();
});
when(linuxPlatform.isLinux).thenReturn(true);
when(linuxPlatform.isWindows).thenReturn(false);
when(notLinuxPlatform.isLinux).thenReturn(false);
when(notLinuxPlatform.isWindows).thenReturn(false);
});
testUsingContext('Linux build fails when there is no linux project', () async {
......
......@@ -61,7 +61,9 @@ void main() {
return const Stream<List<int>>.empty();
});
when(macosPlatform.isMacOS).thenReturn(true);
when(macosPlatform.isWindows).thenReturn(false);
when(notMacosPlatform.isMacOS).thenReturn(false);
when(notMacosPlatform.isWindows).thenReturn(false);
});
// Sets up the minimal mock project files necessary for macOS builds to succeed.
......
......@@ -2,8 +2,12 @@
// 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:io' as io;
import 'package:flutter_tools/src/base/common.dart';
import 'package:flutter_tools/src/base/io.dart';
import 'package:flutter_tools/src/base/signals.dart';
import 'package:flutter_tools/src/base/time.dart';
import 'package:flutter_tools/src/cache.dart';
import 'package:flutter_tools/src/reporting/reporting.dart';
......@@ -130,6 +134,54 @@ void main() {
}
});
group('signals tests', () {
MockIoProcessSignal mockSignal;
ProcessSignal signalUnderTest;
StreamController<io.ProcessSignal> signalController;
setUp(() {
mockSignal = MockIoProcessSignal();
signalUnderTest = ProcessSignal(mockSignal);
signalController = StreamController<io.ProcessSignal>();
when(mockSignal.watch()).thenAnswer((Invocation invocation) => signalController.stream);
});
testUsingContext('reports command that is killed', () async {
// Crash if called a third time which is unexpected.
mockTimes = <int>[1000, 2000];
final Completer<void> completer = Completer<void>();
setExitFunctionForTests((int exitCode) {
expect(exitCode, 0);
restoreExitFunction();
completer.complete();
});
final DummyFlutterCommand flutterCommand = DummyFlutterCommand(
commandFunction: () async {
final Completer<void> c = Completer<void>();
await c.future;
return null; // unreachable
}
);
unawaited(flutterCommand.run());
signalController.add(mockSignal);
await completer.future;
verify(usage.sendCommand(any, parameters: anyNamed('parameters')));
verify(usage.sendEvent(any, 'killed', parameters: anyNamed('parameters')));
}, overrides: <Type, Generator>{
ProcessInfo: () => mockProcessInfo,
Signals: () => FakeSignals(
subForSigTerm: signalUnderTest,
exitSignals: <ProcessSignal>[signalUnderTest],
),
SystemClock: () => clock,
Usage: () => usage,
});
});
testUsingCommandContext('reports maxRss', () async {
// Crash if called a third time which is unexpected.
mockTimes = <int>[1000, 2000];
......@@ -264,3 +316,29 @@ class FakeCommand extends FlutterCommand {
class MockVersion extends Mock implements FlutterVersion {}
class MockProcessInfo extends Mock implements ProcessInfo {}
class MockIoProcessSignal extends Mock implements io.ProcessSignal {}
class FakeSignals implements Signals {
FakeSignals({
this.subForSigTerm,
List<ProcessSignal> exitSignals,
}) : delegate = Signals(exitSignals: exitSignals);
final ProcessSignal subForSigTerm;
final Signals delegate;
@override
Object addHandler(ProcessSignal signal, SignalHandler handler) {
if (signal == ProcessSignal.SIGTERM) {
return delegate.addHandler(subForSigTerm, handler);
}
return delegate.addHandler(signal, handler);
}
@override
Future<bool> removeHandler(ProcessSignal signal, Object token) =>
delegate.removeHandler(signal, token);
@override
Stream<Object> get errors => delegate.errors;
}
......@@ -12,6 +12,7 @@ import 'package:flutter_tools/src/base/file_system.dart';
import 'package:flutter_tools/src/base/io.dart';
import 'package:flutter_tools/src/base/logger.dart';
import 'package:flutter_tools/src/base/os.dart';
import 'package:flutter_tools/src/base/signals.dart';
import 'package:flutter_tools/src/base/terminal.dart';
import 'package:flutter_tools/src/base/time.dart';
import 'package:flutter_tools/src/cache.dart';
......@@ -90,6 +91,7 @@ void testUsingContext(
FileSystem: () => const LocalFileSystemBlockingSetCurrentDirectory(),
TimeoutConfiguration: () => const TimeoutConfiguration(),
PlistParser: () => FakePlistParser(),
Signals: () => FakeSignals(),
},
body: () {
final String flutterRoot = getFlutterRoot();
......@@ -387,3 +389,18 @@ class LocalFileSystemBlockingSetCurrentDirectory extends LocalFileSystem {
'code to not require setting fs.currentDirectory.';
}
}
class FakeSignals implements Signals {
@override
Object addHandler(ProcessSignal signal, SignalHandler handler) {
return null;
}
@override
Future<bool> removeHandler(ProcessSignal signal, Object token) async {
return true;
}
@override
Stream<Object> get errors => const Stream<Object>.empty();
}
......@@ -14,6 +14,7 @@ import 'package:flutter_tools/src/base/io.dart';
import 'package:flutter_tools/src/base/logger.dart';
import 'package:flutter_tools/src/base/os.dart';
import 'package:flutter_tools/src/base/platform.dart';
import 'package:flutter_tools/src/base/signals.dart';
import 'package:flutter_tools/src/base/terminal.dart';
import 'package:flutter_tools/src/cache.dart';
import 'package:flutter_tools/src/context_runner.dart';
......@@ -35,6 +36,7 @@ final Map<Type, Generator> _testbedDefaults = <Type, Generator>{
OutputPreferences: () => OutputPreferences(showColor: false), // configures BufferLogger to avoid color codes.
Usage: () => NoOpUsage(), // prevent addition of analytics from burdening test mocks
FlutterVersion: () => FakeFlutterVersion(), // prevent requirement to mock git for test runner.
Signals: () => FakeSignals(), // prevent registering actual signal handlers.
};
/// Manages interaction with the tool injection and runner system.
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment