Fix a race condition in vmservice_test.dart (#23835)
Fixes https://github.com/flutter/flutter/issues/19273
diff --git a/packages/flutter_tools/lib/src/resident_runner.dart b/packages/flutter_tools/lib/src/resident_runner.dart
index 4fdee10..05c9562 100644
--- a/packages/flutter_tools/lib/src/resident_runner.dart
+++ b/packages/flutter_tools/lib/src/resident_runner.dart
@@ -82,15 +82,13 @@
vmServices = localVmServices;
}
- Future<void> refreshViews() {
+ Future<void> refreshViews() async {
if (vmServices == null || vmServices.isEmpty)
return Future<void>.value(null);
final List<Future<void>> futures = <Future<void>>[];
for (VMService service in vmServices)
- futures.add(service.vm.refreshViews());
- final Completer<void> completer = Completer<void>();
- Future.wait(futures).whenComplete(() => completer.complete(null)); // ignore: unawaited_futures
- return completer.future;
+ futures.add(service.vm.refreshViews(waitForViews: true));
+ await Future.wait(futures);
}
List<FlutterView> get views {
@@ -490,8 +488,10 @@
}
Future<void> refreshViews() async {
+ final List<Future<void>> futures = <Future<void>>[];
for (FlutterDevice device in flutterDevices)
- await device.refreshViews();
+ futures.add(device.refreshViews());
+ await Future.wait(futures);
}
Future<void> _debugDumpApp() async {
diff --git a/packages/flutter_tools/lib/src/run_hot.dart b/packages/flutter_tools/lib/src/run_hot.dart
index 5491808..b0de03a 100644
--- a/packages/flutter_tools/lib/src/run_hot.dart
+++ b/packages/flutter_tools/lib/src/run_hot.dart
@@ -163,7 +163,10 @@
}) async {
_didAttach = true;
try {
- await connectToServiceProtocol(reloadSources: _reloadSourcesService, compileExpression: _compileExpressionService);
+ await connectToServiceProtocol(
+ reloadSources: _reloadSourcesService,
+ compileExpression: _compileExpressionService,
+ );
} catch (error) {
printError('Error connecting to the service protocol: $error');
return 2;
@@ -189,8 +192,10 @@
}
final Stopwatch initialUpdateDevFSsTimer = Stopwatch()..start();
final bool devfsResult = await _updateDevFS(fullRestart: true);
- _addBenchmarkData('hotReloadInitialDevFSSyncMilliseconds',
- initialUpdateDevFSsTimer.elapsed.inMilliseconds);
+ _addBenchmarkData(
+ 'hotReloadInitialDevFSSyncMilliseconds',
+ initialUpdateDevFSsTimer.elapsed.inMilliseconds,
+ );
if (!devfsResult)
return 3;
@@ -276,7 +281,7 @@
return attach(
connectionInfoCompleter: connectionInfoCompleter,
- appStartedCompleter: appStartedCompleter
+ appStartedCompleter: appStartedCompleter,
);
}
diff --git a/packages/flutter_tools/lib/src/vmservice.dart b/packages/flutter_tools/lib/src/vmservice.dart
index 54e5c30..633fc9d 100644
--- a/packages/flutter_tools/lib/src/vmservice.dart
+++ b/packages/flutter_tools/lib/src/vmservice.dart
@@ -105,7 +105,7 @@
// TODO(flutter/flutter#23031): Test this.
/// A connection to the Dart VM Service.
class VMService {
- VMService._(
+ VMService(
this._peer,
this.httpAddress,
this.wsAddress,
@@ -236,7 +236,7 @@
final Uri wsUri = httpUri.replace(scheme: 'ws', path: fs.path.join(httpUri.path, 'ws'));
final StreamChannel<String> channel = await _openChannel(wsUri);
final rpc.Peer peer = rpc.Peer.withoutJson(jsonDocument.bind(channel));
- final VMService service = VMService._(peer, httpUri, wsUri, requestTimeout, reloadSources, compileExpression);
+ final VMService service = VMService(peer, httpUri, wsUri, requestTimeout, reloadSources, compileExpression);
// This call is to ensure we are able to establish a connection instead of
// keeping on trucking and failing farther down the process.
await service._sendRequest('getVersion', const <String, dynamic>{});
@@ -311,7 +311,7 @@
final Map<String, dynamic> eventIsolate = eventData['isolate'];
// Log event information.
- printTrace(data.toString());
+ printTrace('Notification from VM: $data');
ServiceEvent event;
if (eventIsolate != null) {
@@ -341,15 +341,9 @@
}
/// Reloads the VM.
- Future<VM> getVM() async {
- return await _vm.reload();
- }
+ Future<void> getVM() async => await vm.reload();
- Future<void> refreshViews() async {
- if (!vm.isFlutterEngine)
- return;
- await vm.refreshViews();
- }
+ Future<void> refreshViews({ bool waitForViews = false }) => vm.refreshViews(waitForViews: waitForViews);
}
/// An error that is thrown when constructing/updating a service object.
@@ -371,9 +365,8 @@
/// to return a cached / canonicalized object.
void _upgradeCollection(dynamic collection,
ServiceObjectOwner owner) {
- if (collection is ServiceMap) {
+ if (collection is ServiceMap)
return;
- }
if (collection is Map<String, dynamic>) {
_upgradeMap(collection, owner);
} else if (collection is List) {
@@ -382,7 +375,7 @@
}
void _upgradeMap(Map<String, dynamic> map, ServiceObjectOwner owner) {
- map.forEach((String k, dynamic v) {
+ map.forEach((String k, Object v) {
if ((v is Map<String, dynamic>) && _isServiceMap(v)) {
map[k] = owner.getFromMap(v);
} else if (v is List) {
@@ -394,8 +387,8 @@
}
void _upgradeList(List<dynamic> list, ServiceObjectOwner owner) {
- for (int i = 0; i < list.length; i++) {
- final dynamic v = list[i];
+ for (int i = 0; i < list.length; i += 1) {
+ final Object v = list[i];
if ((v is Map<String, dynamic>) && _isServiceMap(v)) {
list[i] = owner.getFromMap(v);
} else if (v is List) {
@@ -477,9 +470,8 @@
/// If this is not already loaded, load it. Otherwise reload.
Future<ServiceObject> load() async {
- if (loaded) {
+ if (loaded)
return this;
- }
return reload();
}
@@ -499,15 +491,12 @@
// We should always reload the VM.
// We can't reload objects without an id.
// We shouldn't reload an immutable and already loaded object.
- final bool skipLoad = !isVM && (!hasId || (immutable && loaded));
- if (skipLoad) {
+ if (!isVM && (!hasId || (immutable && loaded)))
return this;
- }
if (_inProgressReload == null) {
final Completer<ServiceObject> completer = Completer<ServiceObject>();
_inProgressReload = completer.future;
-
try {
final Map<String, dynamic> response = await _fetchDirect();
if (_stripRef(response['type']) == 'Sentinel') {
@@ -661,9 +650,7 @@
VM get vm => this;
@override
- Future<Map<String, dynamic>> _fetchDirect() async {
- return invokeRpcRaw('getVM');
- }
+ Future<Map<String, dynamic>> _fetchDirect() => invokeRpcRaw('getVM');
@override
void _update(Map<String, dynamic> map, bool mapIsRef) {
@@ -675,11 +662,9 @@
_upgradeCollection(map, this);
_loaded = true;
- // TODO(johnmccutchan): Extract any properties we care about here.
_pid = map['pid'];
- if (map['_heapAllocatedMemoryUsage'] != null) {
+ if (map['_heapAllocatedMemoryUsage'] != null)
_heapAllocatedMemoryUsage = map['_heapAllocatedMemoryUsage'];
- }
_maxRSS = map['_maxRSS'];
_embedder = map['_embedder'];
@@ -818,6 +803,13 @@
return Future<Isolate>.value(_isolateCache[isolateId]);
}
+ static String _truncate(String message, int width, String ellipsis) {
+ assert(ellipsis.length < width);
+ if (message.length <= width)
+ return message;
+ return message.substring(0, width - ellipsis.length) + ellipsis;
+ }
+
/// Invoke the RPC and return the raw response.
///
/// If `timeoutFatal` is false, then a timeout will result in a null return
@@ -827,14 +819,14 @@
Duration timeout,
bool timeoutFatal = true,
}) async {
- printTrace('$method: $params');
-
+ printTrace('Sending to VM service: $method($params)');
assert(params != null);
timeout ??= _vmService._requestTimeout;
try {
final Map<String, dynamic> result = await _vmService
._sendRequest(method, params)
.timeout(timeout);
+ printTrace('Result: ${_truncate(result.toString(), 250, '...')}');
return result;
} on TimeoutException {
printTrace('Request to Dart VM Service timed out: $method($params)');
@@ -949,12 +941,29 @@
return invokeRpcRaw('_getVMTimeline', timeout: kLongRequestTimeout);
}
- Future<void> refreshViews() {
+ Future<void> refreshViews({ bool waitForViews = false }) async {
+ assert(waitForViews != null);
+ assert(loaded);
if (!isFlutterEngine)
- return Future<void>.value();
- _viewCache.clear();
- // Send one per-application request that refreshes all views in the app.
- return vmService.vm.invokeRpc<ServiceObject>('_flutter.listViews', timeout: kLongRequestTimeout);
+ return;
+ int failCount = 0;
+ while (true) {
+ _viewCache.clear();
+ // When the future returned by invokeRpc() below returns,
+ // the _viewCache will have been updated.
+ // This message updates all the views of every isolate.
+ await vmService.vm.invokeRpc<ServiceObject>(
+ '_flutter.listViews',
+ timeout: kLongRequestTimeout,
+ );
+ if (_viewCache.values.isNotEmpty || !waitForViews)
+ return;
+ failCount += 1;
+ if (failCount == 5) // waited 200ms
+ printStatus('Flutter is taking longer than expected to report its views. Still trying...');
+ await Future<void>.delayed(const Duration(milliseconds: 50));
+ await reload();
+ }
}
Iterable<FlutterView> get views => _viewCache.values;
@@ -1080,9 +1089,7 @@
}
@override
- Future<Map<String, dynamic>> _fetchDirect() {
- return invokeRpcRaw('getIsolate');
- }
+ Future<Map<String, dynamic>> _fetchDirect() => invokeRpcRaw('getIsolate');
/// Invoke the RPC and return the raw response.
Future<Map<String, dynamic>> invokeRpcRaw(String method, {
@@ -1427,7 +1434,7 @@
packagesUri,
assetsDirectoryUri);
await completer.future;
- await owner.vm.refreshViews();
+ await owner.vm.refreshViews(waitForViews: true);
await subscription.cancel();
}
diff --git a/packages/flutter_tools/test/vmservice_test.dart b/packages/flutter_tools/test/vmservice_test.dart
index c96e839..6f056ce 100644
--- a/packages/flutter_tools/test/vmservice_test.dart
+++ b/packages/flutter_tools/test/vmservice_test.dart
@@ -2,12 +2,158 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
+import 'dart:async';
+
+import 'package:flutter_tools/src/base/io.dart';
+import 'package:flutter_tools/src/base/logger.dart';
import 'package:flutter_tools/src/vmservice.dart';
+import 'package:json_rpc_2/json_rpc_2.dart' as rpc;
+import 'package:quiver/testing/async.dart';
import 'src/common.dart';
import 'src/context.dart';
+import 'src/mocks.dart';
+
+class MockPeer implements rpc.Peer {
+ @override
+ Future<dynamic> get done async {
+ throw 'unexpected call to done';
+ }
+
+ @override
+ bool get isClosed {
+ throw 'unexpected call to isClosed';
+ }
+
+ @override
+ Future<dynamic> close() async {
+ throw 'unexpected call to close()';
+ }
+
+ @override
+ Future<dynamic> listen() async {
+ // this does get called
+ }
+
+ @override
+ void registerFallback(dynamic callback(rpc.Parameters parameters)) {
+ throw 'unexpected call to registerFallback';
+ }
+
+ @override
+ void registerMethod(String name, Function callback) {
+ // this does get called
+ }
+
+ @override
+ void sendNotification(String method, [ dynamic parameters ]) {
+ throw 'unexpected call to sendNotification';
+ }
+
+ bool isolatesEnabled = false;
+
+ Future<void> _getVMLatch;
+ Completer<void> _currentGetVMLatchCompleter;
+
+ void tripGetVMLatch() {
+ final Completer<void> lastCompleter = _currentGetVMLatchCompleter;
+ _currentGetVMLatchCompleter = Completer<void>();
+ _getVMLatch = _currentGetVMLatchCompleter.future;
+ lastCompleter?.complete();
+ }
+
+ int returnedFromSendRequest = 0;
+
+ @override
+ Future<dynamic> sendRequest(String method, [ dynamic parameters ]) async {
+ if (method == 'getVM')
+ await _getVMLatch;
+ await Future<void>.delayed(Duration.zero);
+ returnedFromSendRequest += 1;
+ if (method == 'getVM') {
+ return <String, dynamic>{
+ 'type': 'VM',
+ 'name': 'vm',
+ 'architectureBits': 64,
+ 'targetCPU': 'x64',
+ 'hostCPU': ' Intel(R) Xeon(R) CPU E5-1650 v2 @ 3.50GHz',
+ 'version': '2.1.0-dev.7.1.flutter-45f9462398 (Fri Oct 19 19:27:56 2018 +0000) on "linux_x64"',
+ '_profilerMode': 'Dart',
+ '_nativeZoneMemoryUsage': 0,
+ 'pid': 103707,
+ 'startTime': 1540426121876,
+ '_embedder': 'Flutter',
+ '_maxRSS': 312614912,
+ '_currentRSS': 33091584,
+ 'isolates': isolatesEnabled ? <dynamic>[
+ <String, dynamic>{
+ 'type': '@Isolate',
+ 'fixedId': true,
+ 'id': 'isolates/242098474',
+ 'name': 'main.dart:main()',
+ 'number': 242098474,
+ },
+ ] : <dynamic>[],
+ };
+ }
+ if (method == 'getIsolate') {
+ return <String, dynamic>{
+ 'type': 'Isolate',
+ 'fixedId': true,
+ 'id': 'isolates/242098474',
+ 'name': 'main.dart:main()',
+ 'number': 242098474,
+ '_originNumber': 242098474,
+ 'startTime': 1540488745340,
+ '_heaps': <String, dynamic>{
+ 'new': <String, dynamic>{
+ 'used': 0,
+ 'capacity': 0,
+ 'external': 0,
+ 'collections': 0,
+ 'time': 0.0,
+ 'avgCollectionPeriodMillis': 0.0,
+ },
+ 'old': <String, dynamic>{
+ 'used': 0,
+ 'capacity': 0,
+ 'external': 0,
+ 'collections': 0,
+ 'time': 0.0,
+ 'avgCollectionPeriodMillis': 0.0,
+ },
+ },
+ };
+ }
+ if (method == '_flutter.listViews') {
+ return <String, dynamic>{
+ 'type': 'FlutterViewList',
+ 'views': isolatesEnabled ? <dynamic>[
+ <String, dynamic>{
+ 'type': 'FlutterView',
+ 'id': '_flutterView/0x4a4c1f8',
+ 'isolate': <String, dynamic>{
+ 'type': '@Isolate',
+ 'fixedId': true,
+ 'id': 'isolates/242098474',
+ 'name': 'main.dart:main()',
+ 'number': 242098474,
+ },
+ },
+ ] : <dynamic>[],
+ };
+ }
+ return null;
+ }
+
+ @override
+ dynamic withBatch(dynamic callback()) {
+ throw 'unexpected call to withBatch';
+ }
+}
void main() {
+ final MockStdio mockStdio = MockStdio();
group('VMService', () {
testUsingContext('fails connection eagerly in the connect() method', () async {
expect(
@@ -15,5 +161,73 @@
throwsToolExit(),
);
});
+
+ testUsingContext('refreshViews', () {
+ FakeAsync().run((FakeAsync time) {
+ bool done = false;
+ final MockPeer mockPeer = MockPeer();
+ expect(mockPeer.returnedFromSendRequest, 0);
+ final VMService vmService = VMService(mockPeer, null, null, const Duration(seconds: 1), null, null);
+ vmService.getVM().then((void value) { done = true; });
+ expect(done, isFalse);
+ expect(mockPeer.returnedFromSendRequest, 0);
+ time.elapse(Duration.zero);
+ expect(done, isTrue);
+ expect(mockPeer.returnedFromSendRequest, 1);
+
+ done = false;
+ mockPeer.tripGetVMLatch(); // this blocks the upcoming getVM call
+ final Future<void> ready = vmService.refreshViews(waitForViews: true);
+ ready.then((void value) { done = true; });
+ expect(mockPeer.returnedFromSendRequest, 1);
+ time.elapse(Duration.zero); // this unblocks the listViews call which returns nothing
+ expect(mockPeer.returnedFromSendRequest, 2);
+ time.elapse(const Duration(milliseconds: 50)); // the last listViews had no views, so it waits 50ms, then calls getVM
+ expect(done, isFalse);
+ expect(mockPeer.returnedFromSendRequest, 2);
+ mockPeer.tripGetVMLatch(); // this unblocks the getVM call
+ expect(mockPeer.returnedFromSendRequest, 2);
+ time.elapse(Duration.zero); // here getVM returns with no isolates and listViews returns no views
+ expect(mockPeer.returnedFromSendRequest, 4);
+ time.elapse(const Duration(milliseconds: 50)); // so refreshViews waits another 50ms
+ expect(done, isFalse);
+ expect(mockPeer.returnedFromSendRequest, 4);
+ mockPeer.tripGetVMLatch(); // this unblocks the getVM call
+ expect(mockPeer.returnedFromSendRequest, 4);
+ time.elapse(Duration.zero); // here getVM returns with no isolates and listViews returns no views
+ expect(mockPeer.returnedFromSendRequest, 6);
+ time.elapse(const Duration(milliseconds: 50)); // so refreshViews waits another 50ms
+ expect(done, isFalse);
+ expect(mockPeer.returnedFromSendRequest, 6);
+ mockPeer.tripGetVMLatch(); // this unblocks the getVM call
+ expect(mockPeer.returnedFromSendRequest, 6);
+ time.elapse(Duration.zero); // here getVM returns with no isolates and listViews returns no views
+ expect(mockPeer.returnedFromSendRequest, 8);
+ time.elapse(const Duration(milliseconds: 50)); // so refreshViews waits another 50ms
+ expect(done, isFalse);
+ expect(mockPeer.returnedFromSendRequest, 8);
+ mockPeer.tripGetVMLatch(); // this unblocks the getVM call
+ expect(mockPeer.returnedFromSendRequest, 8);
+ time.elapse(Duration.zero); // here getVM returns with no isolates and listViews returns no views
+ expect(mockPeer.returnedFromSendRequest, 10);
+ const String message = 'Flutter is taking longer than expected to report its views. Still trying...\n';
+ expect(mockStdio.writtenToStdout.join(''), message);
+ expect(mockStdio.writtenToStderr.join(''), '');
+ time.elapse(const Duration(milliseconds: 50)); // so refreshViews waits another 50ms
+ expect(done, isFalse);
+ expect(mockPeer.returnedFromSendRequest, 10);
+ mockPeer.isolatesEnabled = true;
+ mockPeer.tripGetVMLatch(); // this unblocks the getVM call
+ expect(mockPeer.returnedFromSendRequest, 10);
+ time.elapse(Duration.zero); // now it returns an isolate and the listViews call returns views
+ expect(mockPeer.returnedFromSendRequest, 13);
+ expect(done, isTrue);
+ expect(mockStdio.writtenToStdout.join(''), message);
+ expect(mockStdio.writtenToStderr.join(''), '');
+ });
+ }, overrides: <Type, Generator>{
+ Logger: () => StdoutLogger(),
+ Stdio: () => mockStdio,
+ });
});
}
