[tool] Guard more write/writeln calls on Process.stdin (#151146)

Contributes to fixing https://github.com/flutter/flutter/issues/137184.

This PR guards write calls in non-test files. This PR excludes
* packages/flutter_tools/lib/src/dart/analysis.dart due to a test timeout I would like to figure out in a separate PR and
* packages/flutter_tools/lib/src/compile.dart due to https://github.com/flutter/flutter/issues/151255
This commit is contained in:
Andrew Kolos
2024-07-23 12:27:52 -07:00
committed by GitHub
parent ed587e6da0
commit 91813060c0
6 changed files with 74 additions and 26 deletions

View File

@@ -11,6 +11,7 @@ import '../base/context.dart';
import '../base/io.dart';
import '../base/logger.dart';
import '../base/platform.dart';
import '../base/process.dart';
import '../base/user_messages.dart';
import '../base/version.dart';
import '../convert.dart';
@@ -334,7 +335,7 @@ class AndroidLicenseValidator extends DoctorValidator {
<String>[_androidSdk!.sdkManagerPath!, '--licenses'],
environment: _java?.environment,
);
process.stdin.write('n\n');
await ProcessUtils.writelnToStdinUnsafe(stdin: process.stdin, line: 'n');
// We expect logcat streams to occasionally contain invalid utf-8,
// see: https://github.com/flutter/flutter/pull/8864.
final Future<void> output = process.stdout
@@ -349,7 +350,7 @@ class AndroidLicenseValidator extends DoctorValidator {
.asFuture<void>();
await Future.wait<void>(<Future<void>>[output, errors]);
return status ?? LicensesAccepted.unknown;
} on ProcessException catch (e) {
} on IOException catch (e) {
_logger.printTrace('Failed to run Android sdk manager: $e');
return LicensesAccepted.unknown;
}

View File

@@ -278,21 +278,50 @@ abstract class ProcessUtils {
);
}
static Future<void> writelnToStdinUnsafe({
required IOSink stdin,
required String line,
}) async {
await _writeToStdinUnsafe(
stdin: stdin,
content: line,
isLine: true,
);
}
static Future<void> writeToStdinUnsafe({
required IOSink stdin,
required String content,
}) async {
await _writeToStdinUnsafe(
stdin: stdin,
content: content,
isLine: false,
);
}
static Future<void> _writeToStdinGuarded({
required IOSink stdin,
required String content,
required void Function(Object, StackTrace) onError,
required bool isLine,
}) async {
try {
await _writeToStdinUnsafe(stdin: stdin, content: content, isLine: isLine);
} on Exception catch (error, stackTrace) {
onError(error, stackTrace);
}
}
static Future<void> _writeToStdinUnsafe({
required IOSink stdin,
required String content,
required bool isLine,
}) {
final Completer<void> completer = Completer<void>();
void handleError(Object error, StackTrace stackTrace) {
try {
onError(error, stackTrace);
completer.complete();
} on Exception catch (e) {
completer.completeError(e);
}
completer.completeError(error, stackTrace);
}
void writeFlushAndComplete() {
@@ -309,17 +338,7 @@ abstract class ProcessUtils {
);
}
runZonedGuarded(
writeFlushAndComplete,
(Object error, StackTrace stackTrace) {
handleError(error, stackTrace);
// We may have already completed with an error in `handleError`.
if (!completer.isCompleted) {
completer.complete();
}
},
);
runZonedGuarded(writeFlushAndComplete, handleError);
return completer.future;
}

View File

@@ -11,6 +11,7 @@ import '../../base/common.dart';
import '../../base/file_system.dart';
import '../../base/io.dart';
import '../../base/logger.dart';
import '../../base/process.dart';
import '../../build_info.dart';
import '../../convert.dart';
import '../../devfs.dart';
@@ -205,7 +206,10 @@ class IconTreeShaker {
'using codepoints $codePointsString');
final Process fontSubsetProcess = await _processManager.start(cmd);
try {
fontSubsetProcess.stdin.writeln(codePointsString);
await ProcessUtils.writelnToStdinUnsafe(
stdin: fontSubsetProcess.stdin,
line: codePointsString,
);
await fontSubsetProcess.stdin.flush();
await fontSubsetProcess.stdin.close();
} on Exception {

View File

@@ -9,6 +9,7 @@ import 'package:dds/dap.dart' hide PidTracker;
import 'package:vm_service/vm_service.dart' as vm;
import '../base/io.dart';
import '../base/process.dart';
import '../cache.dart';
import '../convert.dart';
import '../globals.dart' as globals show fs;
@@ -328,7 +329,7 @@ class FlutterDebugAdapter extends FlutterBaseDebugAdapter with VmServiceInfoFile
final int id = _flutterRequestId++;
_flutterRequestCompleters[id] = completer;
sendFlutterMessage(<String, Object?>{
await sendFlutterMessage(<String, Object?>{
'id': id,
'method': method,
'params': params,
@@ -340,7 +341,7 @@ class FlutterDebugAdapter extends FlutterBaseDebugAdapter with VmServiceInfoFile
/// Sends a message to the Flutter run daemon.
///
/// Throws `DebugAdapterException` if a Flutter process is not yet running.
void sendFlutterMessage(Map<String, Object?> message) {
Future<void> sendFlutterMessage(Map<String, Object?> message) async {
final Process? process = this.process;
if (process == null) {
throw DebugAdapterException('Flutter process has not yet started');
@@ -350,7 +351,7 @@ class FlutterDebugAdapter extends FlutterBaseDebugAdapter with VmServiceInfoFile
// Flutter requests are always wrapped in brackets as an array.
final String payload = '[$messageString]\n';
_logTraffic('==> [Flutter] $payload');
process.stdin.writeln(payload);
await ProcessUtils.writelnToStdinUnsafe(stdin: process.stdin, line: payload);
}
/// Called by [terminateRequest] to request that we gracefully shut down the app being run (or in the case of an attach, disconnect).
@@ -534,8 +535,8 @@ class FlutterDebugAdapter extends FlutterBaseDebugAdapter with VmServiceInfoFile
Map<String, Object?>? params,
) {
/// A helper to send a client response to Flutter.
void sendResponseToFlutter(Object? id, Object? value, { bool error = false }) {
sendFlutterMessage(<String, Object?>{
Future<void> sendResponseToFlutter(Object? id, Object? value, { bool error = false }) async {
await sendFlutterMessage(<String, Object?>{
'id': id,
if (error)
'error': value

View File

@@ -152,6 +152,29 @@ void main() {
expect(licenseStatus, LicensesAccepted.unknown);
});
testWithoutContext('licensesAccepted returns LicensesAccepted.unknown if cannot write to sdkmanager', () async {
sdk.sdkManagerPath = '/foo/bar/sdkmanager';
processManager.addCommand(
FakeCommand(
command: <String>[sdk.sdkManagerPath!, '--licenses'],
stdin: IOSink(ClosedStdinController()),
),
);
final AndroidLicenseValidator licenseValidator = AndroidLicenseValidator(
java: FakeJava(),
androidSdk: sdk,
processManager: processManager,
platform: FakePlatform(environment: <String, String>{'HOME': '/home/me'}),
stdio: stdio,
logger: BufferLogger.test(),
userMessages: UserMessages(),
);
final LicensesAccepted licenseStatus = await licenseValidator.licensesAccepted;
expect(licenseStatus, LicensesAccepted.unknown);
expect(processManager, hasNoRemainingExpectations);
});
testWithoutContext('licensesAccepted handles garbage/no output', () async {
sdk.sdkManagerPath = '/foo/bar/sdkmanager';
processManager.addCommand(const FakeCommand(

View File

@@ -204,7 +204,7 @@ class MockFlutterDebugAdapter extends FlutterDebugAdapter {
}
@override
void sendFlutterMessage(Map<String, Object?> message) {
Future<void> sendFlutterMessage(Map<String, Object?> message) async {
dapToFlutterMessages.add(message);
// Don't call super because it will try to write to the process that we
// didn't actually spawn.