Fix race condition in protocol_discovery.dart (#10092)
For some reaosn, when we discovered our URI, we were re-instantiating
the `Completer` instance variable whose future we listen to in `nextUri()`.
This led to a race between a caller calling `nextUri()` and us discovering
the URI. If we happened to discover our URI before a caller called
`nextUri()`, then they would be left waiting on a future from the newly
allocated `Completer` (which would never complete).
Fixes #10064
diff --git a/packages/flutter_tools/lib/src/protocol_discovery.dart b/packages/flutter_tools/lib/src/protocol_discovery.dart
index c4a6e12..d3f3654 100644
--- a/packages/flutter_tools/lib/src/protocol_discovery.dart
+++ b/packages/flutter_tools/lib/src/protocol_discovery.dart
@@ -9,72 +9,62 @@
import 'device.dart';
import 'globals.dart';
-/// Discover service protocol on a device
-/// and forward the service protocol device port to the host.
+/// Discovers a specific service protocol on a device, and forward the service
+/// protocol device port to the host.
class ProtocolDiscovery {
- /// [logReader] - a [DeviceLogReader] to look for service messages in.
- ProtocolDiscovery(DeviceLogReader logReader, String serviceName,
- {this.portForwarder, this.hostPort, this.defaultHostPort})
- : _logReader = logReader, _serviceName = serviceName {
+ ProtocolDiscovery._(
+ DeviceLogReader logReader,
+ String serviceName, {
+ this.portForwarder,
+ this.hostPort,
+ this.defaultHostPort,
+ }) : _logReader = logReader, _serviceName = serviceName {
assert(_logReader != null);
- _subscription = _logReader.logLines.listen(_onLine);
assert(portForwarder == null || defaultHostPort != null);
+ _deviceLogSubscription = _logReader.logLines.listen(_onLine);
}
factory ProtocolDiscovery.observatory(DeviceLogReader logReader,
{DevicePortForwarder portForwarder, int hostPort}) =>
- new ProtocolDiscovery(logReader, kObservatoryService,
+ new ProtocolDiscovery._(logReader, _kObservatoryService,
portForwarder: portForwarder,
hostPort: hostPort,
defaultHostPort: kDefaultObservatoryPort);
factory ProtocolDiscovery.diagnosticService(DeviceLogReader logReader,
{DevicePortForwarder portForwarder, int hostPort}) =>
- new ProtocolDiscovery(logReader, kDiagnosticService,
+ new ProtocolDiscovery._(logReader, _kDiagnosticService,
portForwarder: portForwarder,
hostPort: hostPort,
defaultHostPort: kDefaultDiagnosticPort);
- static const String kObservatoryService = 'Observatory';
- static const String kDiagnosticService = 'Diagnostic server';
+ static const String _kObservatoryService = 'Observatory';
+ static const String _kDiagnosticService = 'Diagnostic server';
final DeviceLogReader _logReader;
final String _serviceName;
final DevicePortForwarder portForwarder;
- int hostPort;
+ final int hostPort;
final int defaultHostPort;
+ final Completer<Uri> _completer = new Completer<Uri>();
- Completer<Uri> _completer = new Completer<Uri>();
- StreamSubscription<String> _subscription;
+ StreamSubscription<String> _deviceLogSubscription;
- /// The [Future] returned by this function will complete when the next service
- /// Uri is found.
- Future<Uri> nextUri() async {
- final Uri deviceUri = await _completer.future.timeout(
- const Duration(seconds: 60), onTimeout: () {
- throwToolExit('Timeout while attempting to retrieve Uri for $_serviceName');
- }
- );
- printTrace('$_serviceName Uri on device: $deviceUri');
- Uri hostUri;
- if (portForwarder != null) {
- final int devicePort = deviceUri.port;
- hostPort ??= await portScanner.findPreferredPort(defaultHostPort);
- hostPort = await portForwarder
- .forward(devicePort, hostPort: hostPort)
- .timeout(const Duration(seconds: 60), onTimeout: () {
- throwToolExit('Timeout while atempting to foward device port $devicePort for $_serviceName');
- });
- printTrace('Forwarded host port $hostPort to device port $devicePort for $_serviceName');
- hostUri = deviceUri.replace(port: hostPort);
- } else {
- hostUri = deviceUri;
- }
- return hostUri;
+ /// The discovered service URI.
+ Future<Uri> get uri {
+ return _completer.future
+ .timeout(const Duration(seconds: 60), onTimeout: () {
+ throwToolExit('Timeout while attempting to retrieve Uri for $_serviceName');
+ }).whenComplete(() {
+ _stopScrapingLogs();
+ });
}
- void cancel() {
- _subscription.cancel();
+ Future<Null> cancel() => _stopScrapingLogs();
+
+ Future<Null> _stopScrapingLogs() async {
+ await _deviceLogSubscription?.cancel();
+ _deviceLogSubscription = null;
}
void _onLine(String line) {
@@ -84,19 +74,35 @@
if (index >= 0) {
try {
uri = Uri.parse(line.substring(index + prefix.length));
- } catch (_) {
- // Ignore errors.
+ } catch (error) {
+ _stopScrapingLogs();
+ _completer.completeError(error);
}
}
- if (uri != null)
- _located(uri);
+
+ if (uri != null) {
+ assert(!_completer.isCompleted);
+ _stopScrapingLogs();
+ _completer.complete(_forwardPort(uri));
+ }
}
- void _located(Uri uri) {
- assert(_completer != null);
- assert(!_completer.isCompleted);
+ Future<Uri> _forwardPort(Uri deviceUri) async {
+ printTrace('$_serviceName Uri on device: $deviceUri');
+ Uri hostUri = deviceUri;
- _completer.complete(uri);
- _completer = new Completer<Uri>();
+ if (portForwarder != null) {
+ final int devicePort = deviceUri.port;
+ int hostPort = this.hostPort ?? await portScanner.findPreferredPort(defaultHostPort);
+ hostPort = await portForwarder
+ .forward(devicePort, hostPort: hostPort)
+ .timeout(const Duration(seconds: 60), onTimeout: () {
+ throwToolExit('Timeout while atempting to foward device port $devicePort for $_serviceName');
+ });
+ printTrace('Forwarded host port $hostPort to device port $devicePort for $_serviceName');
+ hostUri = deviceUri.replace(port: hostPort);
+ }
+
+ return hostUri;
}
}
