Unverified Commit b1cc4874 authored by Ian Hickson's avatar Ian Hickson Committed by GitHub

Only write the pid-file while listening to SIGUSR signals. (#74533)

parent 1b441333
......@@ -2,19 +2,11 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
import 'package:flutter_driver/driver_extension.dart';
import 'package:flutter/material.dart';
import 'package:flutter/rendering.dart';
String log = '';
void main() {
enableFlutterDriverExtension(handler: (String message) async {
log = 'log:';
await WidgetsBinding.instance.reassembleApplication();
return log;
});
print('called main');
runApp(const MaterialApp(home: Test()));
}
......@@ -22,9 +14,7 @@ class Test extends SingleChildRenderObjectWidget {
const Test({ Key key }) : super(key: key);
@override
RenderTest createRenderObject(BuildContext context) {
return RenderTest();
}
RenderTest createRenderObject(BuildContext context) => RenderTest();
}
class RenderTest extends RenderProxyBox {
......@@ -33,11 +23,17 @@ class RenderTest extends RenderProxyBox {
@override
void debugPaintSize(PaintingContext context, Offset offset) {
super.debugPaintSize(context, offset);
log += ' debugPaintSize';
print('called debugPaintSize');
}
@override
void paint(PaintingContext context, Offset offset) {
log += ' paint';
print('called paint');
}
@override
void reassemble() {
print('called reassemble');
super.reassemble();
}
}
// Copyright 2014 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 'package:flutter/material.dart';
void main() {
runApp(const MaterialApp(home: Test()));
}
class Test extends StatefulWidget {
const Test({ Key key }) : super(key: key);
@override
State<Test> createState() => _TestState();
}
class _TestState extends State<Test> {
bool _triggered = false;
@override
void reassemble() {
_triggered = true;
super.reassemble();
}
@override
Widget build(BuildContext context) {
if (!_triggered)
return const SizedBox.shrink();
return Row(children: const <Widget>[
SizedBox(width: 10000.0),
SizedBox(width: 10000.0),
SizedBox(width: 10000.0),
SizedBox(width: 10000.0),
SizedBox(width: 10000.0),
SizedBox(width: 10000.0),
SizedBox(width: 10000.0),
]);
}
}
......@@ -52,6 +52,7 @@ import 'package:meta/meta.dart';
import '../globals.dart' as globals;
import 'async_guard.dart';
import 'context.dart';
import 'file_system.dart';
import 'process.dart';
export 'dart:io'
......@@ -364,34 +365,60 @@ class Stdio {
Future<void> addStderrStream(Stream<List<int>> stream) => stderr.addStream(stream);
}
// TODO(zra): Move pid and writePidFile into `ProcessInfo`.
void writePidFile(String pidFile) {
if (pidFile != null) {
// Write our pid to the file.
globals.fs.file(pidFile).writeAsStringSync(io.pid.toString());
}
}
/// An overridable version of io.ProcessInfo.
abstract class ProcessInfo {
factory ProcessInfo() => _DefaultProcessInfo();
factory ProcessInfo() => _DefaultProcessInfo(globals.fs);
factory ProcessInfo.test(FileSystem fs) => _TestProcessInfo(fs);
static ProcessInfo get instance => context.get<ProcessInfo>();
int get currentRss;
int get maxRss;
File writePidFile(String pidFile);
}
ProcessInfo get processInfo => ProcessInfo.instance;
/// The default implementation of [ProcessInfo], which uses [io.ProcessInfo].
class _DefaultProcessInfo implements ProcessInfo {
_DefaultProcessInfo(this._fileSystem);
final FileSystem _fileSystem;
@override
int get currentRss => io.ProcessInfo.currentRss;
@override
int get maxRss => io.ProcessInfo.maxRss;
@override
File writePidFile(String pidFile) {
assert(pidFile != null);
return _fileSystem.file(pidFile)
..writeAsStringSync(io.pid.toString());
}
}
/// The test version of [ProcessInfo].
class _TestProcessInfo implements ProcessInfo {
_TestProcessInfo(this._fileSystem);
final FileSystem _fileSystem;
@override
int currentRss = 1000;
@override
int maxRss = 2000;
@override
File writePidFile(String pidFile) {
assert(pidFile != null);
return _fileSystem.file(pidFile)
..writeAsStringSync('12345');
}
}
/// The return type for [listNetworkInterfaces].
......
......@@ -90,18 +90,26 @@ class AttachCommand extends FlutterCommand {
'This parameter is case-insensitive.',
)..addOption(
'pid-file',
help: 'Specify a file to write the process id to. '
help: 'Specify a file to write the process ID to. '
'You can send SIGUSR1 to trigger a hot reload '
'and SIGUSR2 to trigger a hot restart.',
'and SIGUSR2 to trigger a hot restart. '
'The file is created when the signal handlers '
'are hooked and deleted when they are removed.',
)..addFlag(
'report-ready',
help: 'Print "ready" to the console after handling a keyboard command.\n'
'This is primarily useful for tests and other automation, but consider '
'using --machine instead.',
hide: !verboseHelp,
)..addOption(
'project-root',
hide: !verboseHelp,
help: 'Normally used only in run target',
help: 'Normally used only in run target.',
)..addFlag('machine',
hide: !verboseHelp,
negatable: false,
help: 'Handle machine structured JSON command input and provide output '
'and progress in machine friendly format.',
'and progress in machine-friendly format.',
);
usesTrackWidgetCreation(verboseHelp: verboseHelp);
addDdsOptions(verboseHelp: verboseHelp);
......@@ -200,8 +208,6 @@ known, it can be explicitly provided to attach via the command-line, e.g.
Future<FlutterCommandResult> runCommand() async {
await _validateArguments();
writePidFile(stringArg('pid-file'));
final Device device = await findTargetDevice();
final Artifacts overrideArtifacts = device.artifactOverrides ?? globals.artifacts;
......@@ -362,9 +368,12 @@ known, it can be explicitly provided to attach via the command-line, e.g.
logger: globals.logger,
terminal: globals.terminal,
signals: globals.signals,
processInfo: processInfo,
reportReady: boolArg('report-ready'),
pidFile: stringArg('pid-file'),
)
..setupTerminal()
..registerSignalHandlers();
..registerSignalHandlers()
..setupTerminal();
}));
result = await runner.attach(
appStartedCompleter: onAppStart,
......
......@@ -289,11 +289,18 @@ class RunCommand extends RunCommandBase {
help: 'Stay resident after launching the application. Not available with "--trace-startup".',
)
..addOption('pid-file',
help: 'Specify a file to write the process id to. '
help: 'Specify a file to write the process ID to. '
'You can send SIGUSR1 to trigger a hot reload '
'and SIGUSR2 to trigger a hot restart.',
)
..addFlag('benchmark',
'and SIGUSR2 to trigger a hot restart. '
'The file is created when the signal handlers '
'are hooked and deleted when they are removed.',
)..addFlag(
'report-ready',
help: 'Print "ready" to the console after handling a keyboard command.\n'
'This is primarily useful for tests and other automation, but consider '
'using --machine instead.',
hide: !verboseHelp,
)..addFlag('benchmark',
negatable: false,
hide: !verboseHelp,
help: 'Enable a benchmarking mode. This will run the given application, '
......@@ -514,8 +521,6 @@ class RunCommand extends RunCommandBase {
final bool hotMode = shouldUseHotMode(buildInfo);
final String applicationBinaryPath = stringArg('use-application-binary');
writePidFile(stringArg('pid-file'));
if (boolArg('machine')) {
if (devices.length > 1) {
throwToolExit('--machine does not support -d all.');
......@@ -620,34 +625,39 @@ class RunCommand extends RunCommandBase {
//
// Do not add more operations to the future.
final Completer<void> appStartedTimeRecorder = Completer<void>.sync();
TerminalHandler handler;
// This callback can't throw.
unawaited(appStartedTimeRecorder.future.then<void>(
(_) {
appStartedTime = globals.systemClock.now();
if (stayResident) {
TerminalHandler(
handler = TerminalHandler(
runner,
logger: globals.logger,
terminal: globals.terminal,
signals: globals.signals,
processInfo: processInfo,
reportReady: boolArg('report-ready'),
pidFile: stringArg('pid-file'),
)
..setupTerminal()
..registerSignalHandlers();
..registerSignalHandlers()
..setupTerminal();
}
}
));
try {
final int result = await runner.run(
appStartedCompleter: appStartedTimeRecorder,
enableDevTools: stayResident && boolArg(FlutterCommand.kEnableDevTools),
route: route,
);
handler?.stop();
if (result != 0) {
throwToolExit(null, exitCode: result);
}
} on RPCError catch (err) {
if (err.code == RPCErrorCodes.kServiceDisappeared) {
} on RPCError catch (error) {
if (error.code == RPCErrorCodes.kServiceDisappeared) {
throwToolExit('Lost connection to device.');
}
rethrow;
......
......@@ -177,7 +177,9 @@ class DevtoolsServerLauncher extends DevtoolsLauncher {
@override
Future<void> close() async {
devToolsUrl = null;
if (devToolsUrl != null) {
devToolsUrl = null;
}
if (_devToolsProcess != null) {
_devToolsProcess.kill();
await _devToolsProcess.exitCode;
......
......@@ -969,7 +969,7 @@ abstract class ResidentRunner {
await residentDevtoolsHandler.shutdown();
await stopEchoingDeviceLog();
await preExit();
await exitApp();
await exitApp(); // calls appFinished
await shutdownDartDevelopmentService();
}
......@@ -1452,17 +1452,27 @@ class TerminalHandler {
@required Logger logger,
@required Terminal terminal,
@required Signals signals,
@required io.ProcessInfo processInfo,
@required bool reportReady,
String pidFile,
}) : _logger = logger,
_terminal = terminal,
_signals = signals;
_signals = signals,
_processInfo = processInfo,
_reportReady = reportReady,
_pidFile = pidFile;
final Logger _logger;
final Terminal _terminal;
final Signals _signals;
final io.ProcessInfo _processInfo;
final bool _reportReady;
final String _pidFile;
final ResidentRunner residentRunner;
bool _processingUserRequest = false;
StreamSubscription<void> subscription;
File _actualPidFile;
@visibleForTesting
String lastReceivedCommand;
......@@ -1476,7 +1486,6 @@ class TerminalHandler {
subscription = _terminal.keystrokes.listen(processTerminalInput);
}
final Map<io.ProcessSignal, Object> _signalTokens = <io.ProcessSignal, Object>{};
void _addSignalHandler(io.ProcessSignal signal, SignalHandler handler) {
......@@ -1485,19 +1494,30 @@ class TerminalHandler {
void registerSignalHandlers() {
assert(residentRunner.stayResident);
_addSignalHandler(io.ProcessSignal.SIGINT, _cleanUp);
_addSignalHandler(io.ProcessSignal.SIGTERM, _cleanUp);
if (!residentRunner.supportsServiceProtocol || !residentRunner.supportsRestart) {
return;
if (residentRunner.supportsServiceProtocol && residentRunner.supportsRestart) {
_addSignalHandler(io.ProcessSignal.SIGUSR1, _handleSignal);
_addSignalHandler(io.ProcessSignal.SIGUSR2, _handleSignal);
if (_pidFile != null) {
_logger.printTrace('Writing pid to: $_pidFile');
_actualPidFile = _processInfo.writePidFile(_pidFile);
}
}
_addSignalHandler(io.ProcessSignal.SIGUSR1, _handleSignal);
_addSignalHandler(io.ProcessSignal.SIGUSR2, _handleSignal);
}
/// Unregisters terminal signal and keystroke handlers.
void stop() {
assert(residentRunner.stayResident);
if (_actualPidFile != null) {
try {
_logger.printTrace('Deleting pid file (${_actualPidFile.path}).');
_actualPidFile.deleteSync();
} on FileSystemException catch (error) {
_logger.printError('Failed to delete pid file (${_actualPidFile.path}): ${error.message}');
}
_actualPidFile = null;
}
for (final MapEntry<io.ProcessSignal, Object> entry in _signalTokens.entries) {
_signals.removeHandler(entry.key, entry.value);
}
......@@ -1623,6 +1643,9 @@ class TerminalHandler {
rethrow;
} finally {
_processingUserRequest = false;
if (_reportReady) {
_logger.printStatus('ready');
}
}
}
......
......@@ -6,6 +6,8 @@
import 'dart:async';
import 'package:file/memory.dart';
import 'package:flutter_tools/src/base/io.dart';
import 'package:flutter_tools/src/base/logger.dart';
import 'package:flutter_tools/src/base/signals.dart';
import 'package:flutter_tools/src/base/terminal.dart';
......@@ -22,11 +24,15 @@ void main() {
final Logger logger = BufferLogger.test();
final Signals signals = Signals.test();
final Terminal terminal = Terminal.test();
final MemoryFileSystem fs = MemoryFileSystem.test();
final ProcessInfo processInfo = ProcessInfo.test(fs);
final TerminalHandler terminalHandler = TerminalHandler(
testRunner,
logger: logger,
signals: signals,
terminal: terminal,
processInfo: processInfo,
reportReady: false,
);
expect(testRunner.hasHelpBeenPrinted, false);
......@@ -39,11 +45,15 @@ void main() {
final Logger logger = BufferLogger.test();
final Signals signals = Signals.test();
final Terminal terminal = Terminal.test();
final MemoryFileSystem fs = MemoryFileSystem.test();
final ProcessInfo processInfo = ProcessInfo.test(fs);
final TerminalHandler terminalHandler = TerminalHandler(
testRunner,
logger: logger,
signals: signals,
terminal: terminal,
processInfo: processInfo,
reportReady: false,
);
expect(testRunner.hasHelpBeenPrinted, false);
......@@ -60,12 +70,16 @@ void main() {
testLogger = BufferLogger.test();
final Signals signals = Signals.test();
final Terminal terminal = Terminal.test();
final MemoryFileSystem fs = MemoryFileSystem.test();
final ProcessInfo processInfo = ProcessInfo.test(fs);
mockResidentRunner = MockResidentRunner();
terminalHandler = TerminalHandler(
mockResidentRunner,
logger: testLogger,
signals: signals,
terminal: terminal,
processInfo: processInfo,
reportReady: false,
);
when(mockResidentRunner.supportsServiceProtocol).thenReturn(true);
});
......@@ -310,6 +324,34 @@ void main() {
verify(mockResidentRunner.debugToggleDebugCheckElevationsEnabled()).called(2);
});
});
testWithoutContext('pidfile creation', () {
final BufferLogger testLogger = BufferLogger.test();
final Signals signals = _TestSignals(Signals.defaultExitSignals);
final Terminal terminal = Terminal.test();
final MemoryFileSystem fs = MemoryFileSystem.test();
final ProcessInfo processInfo = ProcessInfo.test(fs);
final ResidentRunner mockResidentRunner = MockResidentRunner();
when(mockResidentRunner.stayResident).thenReturn(true);
when(mockResidentRunner.supportsServiceProtocol).thenReturn(true);
when(mockResidentRunner.supportsRestart).thenReturn(true);
const String filename = 'test.pid';
final TerminalHandler terminalHandler = TerminalHandler(
mockResidentRunner,
logger: testLogger,
signals: signals,
terminal: terminal,
processInfo: processInfo,
reportReady: false,
pidFile: filename,
);
expect(fs.file(filename).existsSync(), isFalse);
terminalHandler.setupTerminal();
terminalHandler.registerSignalHandlers();
expect(fs.file(filename).existsSync(), isTrue);
terminalHandler.stop();
expect(fs.file(filename).existsSync(), isFalse);
});
}
class MockDevice extends Mock implements Device {
......@@ -353,3 +395,35 @@ class TestRunner extends Mock implements ResidentRunner {
bool enableDevTools = false,
}) async => null;
}
class _TestSignals implements Signals {
_TestSignals(this.exitSignals);
final List<ProcessSignal> exitSignals;
final Map<ProcessSignal, Map<Object, SignalHandler>> _handlersTable =
<ProcessSignal, Map<Object, SignalHandler>>{};
@override
Object addHandler(ProcessSignal signal, SignalHandler handler) {
final Object token = Object();
_handlersTable.putIfAbsent(signal, () => <Object, SignalHandler>{})[token] = handler;
return token;
}
@override
Future<bool> removeHandler(ProcessSignal signal, Object token) async {
if (!_handlersTable.containsKey(signal)) {
return false;
}
if (!_handlersTable[signal].containsKey(token)) {
return false;
}
_handlersTable[signal].remove(token);
return true;
}
@override
Stream<Object> get errors => _errors.stream;
final StreamController<Object> _errors = StreamController<Object>();
}
......@@ -39,16 +39,6 @@ void main() {
tryToDelete(tempDir);
});
testWithoutContext('writes pid-file', () async {
final File pidFile = tempDir.childFile('test.pid');
await _flutterRun.run(withDebugger: true);
await _flutterAttach.attach(
_flutterRun.vmServicePort,
pidFile: pidFile,
);
expect(pidFile.existsSync(), isTrue);
});
testWithoutContext('can hot reload', () async {
await _flutterRun.run(withDebugger: true);
await _flutterAttach.attach(_flutterRun.vmServicePort);
......
......@@ -51,12 +51,6 @@ void main() {
}
});
testWithoutContext('flutter run writes pid-file', () async {
final File pidFile = tempDir.childFile('test.pid');
await _flutter.run(pidFile: pidFile);
expect(pidFile.existsSync(), isTrue);
});
testWithoutContext('sets activeDevToolsServerAddress extension', () async {
await _flutter.run(
startPaused: true,
......
......@@ -103,36 +103,4 @@ void main() {
expect(stdout.toString(), isNot(contains(_exceptionStart)));
});
testWithoutContext('flutter run for web reports an early error in an application', () async {
final StringBuffer stdout = StringBuffer();
await _flutter.run(
startPaused: true,
withDebugger: true,
structuredErrors: true,
chrome: true,
machine: false,
);
await _flutter.resume();
final Completer<void> completer = Completer<void>();
bool lineFound = false;
await Future<void>(() async {
_flutter.stdout.listen((String line) {
stdout.writeln(line);
if (line.startsWith('Another exception was thrown') && !lineFound) {
lineFound = true;
completer.complete();
}
});
await completer.future;
}).timeout(const Duration(seconds: 15), onTimeout: () {
// Complete anyway in case we don't see the 'Another exception' line.
completer.complete();
});
expect(stdout.toString(), contains(_exceptionStart));
await _flutter.stop();
}, skip: 'Running in cirrus environment causes premature exit');
}
......@@ -86,7 +86,6 @@ abstract class FlutterTestDriver {
List<String> arguments, {
String script,
bool withDebugger = false,
File pidFile,
bool singleWidgetReloads = false,
}) async {
final String flutterBin = fileSystem.path.join(getFlutterRoot(), 'bin', 'flutter');
......@@ -96,9 +95,6 @@ abstract class FlutterTestDriver {
if (_printDebugOutputToStdOut) {
arguments.add('--verbose');
}
if (pidFile != null) {
arguments.addAll(<String>['--pid-file', pidFile.path]);
}
if (script != null) {
arguments.add(script);
}
......@@ -467,9 +463,7 @@ class FlutterRunTestDriver extends FlutterTestDriver {
bool chrome = false,
bool expressionEvaluation = true,
bool structuredErrors = false,
bool machine = true,
bool singleWidgetReloads = false,
File pidFile,
String script,
List<String> additionalCommandArgs,
}) async {
......@@ -478,7 +472,7 @@ class FlutterRunTestDriver extends FlutterTestDriver {
'run',
if (!chrome)
'--disable-service-auth-codes',
if (machine) '--machine',
'--machine',
if (!spawnDdsInstance) '--disable-dds',
...getLocalEngineArguments(),
'-d',
......@@ -497,7 +491,6 @@ class FlutterRunTestDriver extends FlutterTestDriver {
withDebugger: withDebugger,
startPaused: startPaused,
pauseOnExceptions: pauseOnExceptions,
pidFile: pidFile,
script: script,
singleWidgetReloads: singleWidgetReloads,
);
......@@ -508,7 +501,6 @@ class FlutterRunTestDriver extends FlutterTestDriver {
bool withDebugger = false,
bool startPaused = false,
bool pauseOnExceptions = false,
File pidFile,
bool singleWidgetReloads = false,
List<String> additionalCommandArgs,
}) async {
......@@ -529,7 +521,6 @@ class FlutterRunTestDriver extends FlutterTestDriver {
withDebugger: withDebugger,
startPaused: startPaused,
pauseOnExceptions: pauseOnExceptions,
pidFile: pidFile,
singleWidgetReloads: singleWidgetReloads,
attachPort: port,
);
......@@ -543,7 +534,6 @@ class FlutterRunTestDriver extends FlutterTestDriver {
bool startPaused = false,
bool pauseOnExceptions = false,
bool singleWidgetReloads = false,
File pidFile,
int attachPort,
}) async {
assert(!startPaused || withDebugger);
......@@ -551,7 +541,6 @@ class FlutterRunTestDriver extends FlutterTestDriver {
args,
script: script,
withDebugger: withDebugger,
pidFile: pidFile,
singleWidgetReloads: singleWidgetReloads,
);
......@@ -738,7 +727,6 @@ class FlutterTestTestDriver extends FlutterTestDriver {
bool withDebugger = false,
bool pauseOnExceptions = false,
bool coverage = false,
File pidFile,
Future<void> Function() beforeStart,
}) async {
await _setupProcess(<String>[
......@@ -748,7 +736,7 @@ class FlutterTestTestDriver extends FlutterTestDriver {
'--machine',
if (coverage)
'--coverage',
], script: testFile, withDebugger: withDebugger, pauseOnExceptions: pauseOnExceptions, pidFile: pidFile, beforeStart: beforeStart);
], script: testFile, withDebugger: withDebugger, pauseOnExceptions: pauseOnExceptions, beforeStart: beforeStart);
}
@override
......@@ -757,7 +745,6 @@ class FlutterTestTestDriver extends FlutterTestDriver {
String script,
bool withDebugger = false,
bool pauseOnExceptions = false,
File pidFile,
Future<void> Function() beforeStart,
bool singleWidgetReloads = false,
}) async {
......@@ -765,7 +752,6 @@ class FlutterTestTestDriver extends FlutterTestDriver {
args,
script: script,
withDebugger: withDebugger,
pidFile: pidFile,
singleWidgetReloads: singleWidgetReloads,
);
......
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