From 9f420ffb3ec1f0bb33de8d8859ae66ffad181716 Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Fri, 12 Mar 2021 16:21:14 -0800 Subject: [PATCH] [flutter_tools] io cleanups to simplify null safety migration (#77955) --- packages/flutter_tools/lib/src/base/io.dart | 41 ++++++++++--------- .../flutter_tools/lib/src/base/signals.dart | 4 +- .../lib/src/commands/attach.dart | 2 +- .../flutter_tools/lib/src/commands/logs.dart | 4 +- .../flutter_tools/lib/src/commands/run.dart | 3 +- .../flutter_tools/lib/src/context_runner.dart | 2 +- packages/flutter_tools/lib/src/globals.dart | 2 + .../lib/src/reporting/events.dart | 2 +- .../lib/src/resident_runner.dart | 10 ++--- .../lib/src/runner/flutter_command.dart | 4 +- .../test/general.shard/base/io_test.dart | 15 ++++++- .../runner/flutter_command_test.dart | 2 +- .../test/integration.shard/test_driver.dart | 2 +- 13 files changed, 54 insertions(+), 39 deletions(-) diff --git a/packages/flutter_tools/lib/src/base/io.dart b/packages/flutter_tools/lib/src/base/io.dart index 7722d51a13..28045cbc8b 100644 --- a/packages/flutter_tools/lib/src/base/io.dart +++ b/packages/flutter_tools/lib/src/base/io.dart @@ -48,11 +48,10 @@ import 'dart:io' as io stdout; import 'package:meta/meta.dart'; +import 'package:file/file.dart'; -import '../globals.dart' as globals; import 'async_guard.dart'; -import 'context.dart'; -import 'file_system.dart'; +import 'platform.dart'; import 'process.dart'; export 'dart:io' @@ -165,16 +164,18 @@ void restoreExitFunction() { /// [ProcessSignal] instances are available on this class (e.g. "send"). class ProcessSignal { @visibleForTesting - const ProcessSignal(this._delegate); + const ProcessSignal(this._delegate, {@visibleForTesting Platform platform = const LocalPlatform()}) + : _platform = platform; - static const ProcessSignal SIGWINCH = _PosixProcessSignal._(io.ProcessSignal.sigwinch); - static const ProcessSignal SIGTERM = _PosixProcessSignal._(io.ProcessSignal.sigterm); - static const ProcessSignal SIGUSR1 = _PosixProcessSignal._(io.ProcessSignal.sigusr1); - static const ProcessSignal SIGUSR2 = _PosixProcessSignal._(io.ProcessSignal.sigusr2); - static const ProcessSignal SIGINT = ProcessSignal(io.ProcessSignal.sigint); - static const ProcessSignal SIGKILL = ProcessSignal(io.ProcessSignal.sigkill); + static const ProcessSignal sigwinch = PosixProcessSignal(io.ProcessSignal.sigwinch); + static const ProcessSignal sigterm = PosixProcessSignal(io.ProcessSignal.sigterm); + static const ProcessSignal sigusr1 = PosixProcessSignal(io.ProcessSignal.sigusr1); + static const ProcessSignal sigusr2 = PosixProcessSignal(io.ProcessSignal.sigusr2); + static const ProcessSignal sigint = ProcessSignal(io.ProcessSignal.sigint); + static const ProcessSignal sigkill = ProcessSignal(io.ProcessSignal.sigkill); final io.ProcessSignal _delegate; + final Platform _platform; Stream watch() { return _delegate.watch().map((io.ProcessSignal signal) => this); @@ -184,12 +185,12 @@ class ProcessSignal { /// /// Returns true if the signal was delivered, false otherwise. /// - /// On Windows, this can only be used with [ProcessSignal.SIGTERM], which + /// On Windows, this can only be used with [ProcessSignal.sigterm], which /// terminates the process. /// /// This is implemented by sending the signal using [Process.killPid]. bool send(int pid) { - assert(!globals.platform.isWindows || this == ProcessSignal.SIGTERM); + assert(!_platform.isWindows || this == ProcessSignal.sigterm); return io.Process.killPid(pid, _delegate); } @@ -200,13 +201,16 @@ class ProcessSignal { /// A [ProcessSignal] that is only available on Posix platforms. /// /// Listening to a [_PosixProcessSignal] is a no-op on Windows. -class _PosixProcessSignal extends ProcessSignal { +@visibleForTesting +class PosixProcessSignal extends ProcessSignal { - const _PosixProcessSignal._(io.ProcessSignal wrappedSignal) : super(wrappedSignal); + const PosixProcessSignal(io.ProcessSignal wrappedSignal, {@visibleForTesting Platform platform = const LocalPlatform()}) + : super(wrappedSignal, platform: platform); @override Stream watch() { - if (globals.platform.isWindows) { + // This uses the real platform since it invokes dart:io functionality directly. + if (_platform.isWindows) { return const Stream.empty(); } return super.watch(); @@ -367,10 +371,9 @@ class Stdio { /// An overridable version of io.ProcessInfo. abstract class ProcessInfo { - factory ProcessInfo() => _DefaultProcessInfo(globals.fs); - factory ProcessInfo.test(FileSystem fs) => _TestProcessInfo(fs); + factory ProcessInfo(FileSystem fs) => _DefaultProcessInfo(fs); - static ProcessInfo get instance => context.get(); + factory ProcessInfo.test(FileSystem fs) => _TestProcessInfo(fs); int get currentRss; @@ -379,8 +382,6 @@ abstract class ProcessInfo { 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); diff --git a/packages/flutter_tools/lib/src/base/signals.dart b/packages/flutter_tools/lib/src/base/signals.dart index 1496930af8..38f187b2a4 100644 --- a/packages/flutter_tools/lib/src/base/signals.dart +++ b/packages/flutter_tools/lib/src/base/signals.dart @@ -24,8 +24,8 @@ abstract class Signals { // The default list of signals that should cause the process to exit. static const List defaultExitSignals = [ - ProcessSignal.SIGTERM, - ProcessSignal.SIGINT, + ProcessSignal.sigterm, + ProcessSignal.sigint, ]; /// Adds a signal handler to run on receipt of signal. diff --git a/packages/flutter_tools/lib/src/commands/attach.dart b/packages/flutter_tools/lib/src/commands/attach.dart index 6658c4a9c0..f62aac11f1 100644 --- a/packages/flutter_tools/lib/src/commands/attach.dart +++ b/packages/flutter_tools/lib/src/commands/attach.dart @@ -368,7 +368,7 @@ 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, + processInfo: globals.processInfo, reportReady: boolArg('report-ready'), pidFile: stringArg('pid-file'), ) diff --git a/packages/flutter_tools/lib/src/commands/logs.dart b/packages/flutter_tools/lib/src/commands/logs.dart index 731eb30974..c65397318f 100644 --- a/packages/flutter_tools/lib/src/commands/logs.dart +++ b/packages/flutter_tools/lib/src/commands/logs.dart @@ -67,12 +67,12 @@ class LogsCommand extends FlutterCommand { ); // When terminating, close down the log reader. - ProcessSignal.SIGINT.watch().listen((ProcessSignal signal) { + ProcessSignal.sigint.watch().listen((ProcessSignal signal) { subscription.cancel(); globals.printStatus(''); exitCompleter.complete(0); }); - ProcessSignal.SIGTERM.watch().listen((ProcessSignal signal) { + ProcessSignal.sigterm.watch().listen((ProcessSignal signal) { subscription.cancel(); exitCompleter.complete(0); }); diff --git a/packages/flutter_tools/lib/src/commands/run.dart b/packages/flutter_tools/lib/src/commands/run.dart index 1928a03145..b5fff7293f 100644 --- a/packages/flutter_tools/lib/src/commands/run.dart +++ b/packages/flutter_tools/lib/src/commands/run.dart @@ -12,7 +12,6 @@ import 'package:vm_service/vm_service.dart'; import '../android/android_device.dart'; import '../base/common.dart'; import '../base/file_system.dart'; -import '../base/io.dart'; import '../base/utils.dart'; import '../build_info.dart'; import '../device.dart'; @@ -639,7 +638,7 @@ class RunCommand extends RunCommandBase { logger: globals.logger, terminal: globals.terminal, signals: globals.signals, - processInfo: processInfo, + processInfo: globals.processInfo, reportReady: boolArg('report-ready'), pidFile: stringArg('pid-file'), ) diff --git a/packages/flutter_tools/lib/src/context_runner.dart b/packages/flutter_tools/lib/src/context_runner.dart index 6130318f7e..fbef360f94 100644 --- a/packages/flutter_tools/lib/src/context_runner.dart +++ b/packages/flutter_tools/lib/src/context_runner.dart @@ -286,7 +286,7 @@ Future runInContext( logger: globals.logger, platform: globals.platform, ), - ProcessInfo: () => ProcessInfo(), + ProcessInfo: () => ProcessInfo(globals.fs), ProcessManager: () => ErrorHandlingProcessManager( delegate: const LocalProcessManager(), platform: globals.platform, diff --git a/packages/flutter_tools/lib/src/globals.dart b/packages/flutter_tools/lib/src/globals.dart index 998e0504d8..41aac35fa1 100644 --- a/packages/flutter_tools/lib/src/globals.dart +++ b/packages/flutter_tools/lib/src/globals.dart @@ -126,6 +126,8 @@ Future get isRunningOnBot => botDetector.isRunningOnBot; /// The current system clock instance. SystemClock get systemClock => context.get(); +ProcessInfo get processInfo => context.get(); + /// Display an error level message to the user. Commands should use this if they /// fail in some way. /// diff --git a/packages/flutter_tools/lib/src/reporting/events.dart b/packages/flutter_tools/lib/src/reporting/events.dart index a668511d66..205ed22247 100644 --- a/packages/flutter_tools/lib/src/reporting/events.dart +++ b/packages/flutter_tools/lib/src/reporting/events.dart @@ -209,7 +209,7 @@ class CommandResultEvent extends UsageEvent { // so that we can get the command result even if trying to grab maxRss // throws an exception. try { - final int maxRss = processInfo.maxRss; + final int maxRss = globals.processInfo.maxRss; flutterUsage.sendEvent( 'tool-command-max-rss', category, diff --git a/packages/flutter_tools/lib/src/resident_runner.dart b/packages/flutter_tools/lib/src/resident_runner.dart index 304bbc30e2..c2d474c869 100644 --- a/packages/flutter_tools/lib/src/resident_runner.dart +++ b/packages/flutter_tools/lib/src/resident_runner.dart @@ -1501,11 +1501,11 @@ class TerminalHandler { void registerSignalHandlers() { assert(residentRunner.stayResident); - _addSignalHandler(io.ProcessSignal.SIGINT, _cleanUp); - _addSignalHandler(io.ProcessSignal.SIGTERM, _cleanUp); + _addSignalHandler(io.ProcessSignal.sigint, _cleanUp); + _addSignalHandler(io.ProcessSignal.sigterm, _cleanUp); if (residentRunner.supportsServiceProtocol && residentRunner.supportsRestart) { - _addSignalHandler(io.ProcessSignal.SIGUSR1, _handleSignal); - _addSignalHandler(io.ProcessSignal.SIGUSR2, _handleSignal); + _addSignalHandler(io.ProcessSignal.sigusr1, _handleSignal); + _addSignalHandler(io.ProcessSignal.sigusr2, _handleSignal); if (_pidFile != null) { _logger.printTrace('Writing pid to: $_pidFile'); _actualPidFile = _processInfo.writePidFile(_pidFile); @@ -1663,7 +1663,7 @@ class TerminalHandler { } _processingUserRequest = true; - final bool fullRestart = signal == io.ProcessSignal.SIGUSR2; + final bool fullRestart = signal == io.ProcessSignal.sigusr2; try { await residentRunner.restart(fullRestart: fullRestart); diff --git a/packages/flutter_tools/lib/src/runner/flutter_command.dart b/packages/flutter_tools/lib/src/runner/flutter_command.dart index 8742ba05c3..fc350d06fc 100644 --- a/packages/flutter_tools/lib/src/runner/flutter_command.dart +++ b/packages/flutter_tools/lib/src/runner/flutter_command.dart @@ -1075,8 +1075,8 @@ abstract class FlutterCommand extends Command { globals.systemClock.now(), ); } - globals.signals.addHandler(io.ProcessSignal.SIGTERM, handler); - globals.signals.addHandler(io.ProcessSignal.SIGINT, handler); + globals.signals.addHandler(io.ProcessSignal.sigterm, handler); + globals.signals.addHandler(io.ProcessSignal.sigint, handler); } /// Logs data about this command. diff --git a/packages/flutter_tools/test/general.shard/base/io_test.dart b/packages/flutter_tools/test/general.shard/base/io_test.dart index 6185bc4411..91c13d08ff 100644 --- a/packages/flutter_tools/test/general.shard/base/io_test.dart +++ b/packages/flutter_tools/test/general.shard/base/io_test.dart @@ -9,6 +9,7 @@ import 'dart:io' as io; import 'package:file/memory.dart'; import 'package:flutter_tools/src/base/io.dart'; +import 'package:flutter_tools/src/base/platform.dart'; import 'package:test/fake.dart'; import '../../src/common.dart'; @@ -66,7 +67,7 @@ void main() { }); testUsingContext('ProcessSignal toString() works', () async { - expect(io.ProcessSignal.sigint.toString(), ProcessSignal.SIGINT.toString()); + expect(io.ProcessSignal.sigint.toString(), ProcessSignal.sigint.toString()); }); test('exit throws a StateError if called without being overriden', () { @@ -100,6 +101,18 @@ void main() { resetNetworkInterfaceLister(); }); + + testWithoutContext('Does not listen to Posix process signals on windows', () async { + final FakePlatform windows = FakePlatform(operatingSystem: 'windows'); + final FakePlatform linux = FakePlatform(operatingSystem: 'linux'); + final FakeProcessSignal fakeSignalA = FakeProcessSignal(); + final FakeProcessSignal fakeSignalB = FakeProcessSignal(); + fakeSignalA.controller.add(fakeSignalA); + fakeSignalB.controller.add(fakeSignalB); + + expect(await PosixProcessSignal(fakeSignalA, platform: windows).watch().isEmpty, true); + expect(await PosixProcessSignal(fakeSignalB, platform: linux).watch().first, isNotNull); + }); } class FakeProcessSignal extends Fake implements io.ProcessSignal { diff --git a/packages/flutter_tools/test/general.shard/runner/flutter_command_test.dart b/packages/flutter_tools/test/general.shard/runner/flutter_command_test.dart index aa92c0c7da..2c1646ded6 100644 --- a/packages/flutter_tools/test/general.shard/runner/flutter_command_test.dart +++ b/packages/flutter_tools/test/general.shard/runner/flutter_command_test.dart @@ -637,7 +637,7 @@ class FakeSignals implements Signals { @override Object addHandler(ProcessSignal signal, SignalHandler handler) { - if (signal == ProcessSignal.SIGTERM) { + if (signal == ProcessSignal.sigterm) { return delegate.addHandler(subForSigTerm, handler); } return delegate.addHandler(signal, handler); diff --git a/packages/flutter_tools/test/integration.shard/test_driver.dart b/packages/flutter_tools/test/integration.shard/test_driver.dart index 911745ad2d..634961951a 100644 --- a/packages/flutter_tools/test/integration.shard/test_driver.dart +++ b/packages/flutter_tools/test/integration.shard/test_driver.dart @@ -204,7 +204,7 @@ abstract class FlutterTestDriver { Future _killForcefully() { _debugPrint('Sending SIGKILL to $_processPid..'); - ProcessSignal.SIGKILL.send(_processPid); + ProcessSignal.sigkill.send(_processPid); return _process.exitCode; }