Reland "Make sure all isolates start during flutter driver tests" (#64432)
diff --git a/packages/flutter_driver/lib/src/driver/vmservice_driver.dart b/packages/flutter_driver/lib/src/driver/vmservice_driver.dart index b85c79c..5409461 100644 --- a/packages/flutter_driver/lib/src/driver/vmservice_driver.dart +++ b/packages/flutter_driver/lib/src/driver/vmservice_driver.dart
@@ -37,8 +37,21 @@ bool logCommunicationToFile = true, }) : _printCommunication = printCommunication, _logCommunicationToFile = logCommunicationToFile, + _isolateSubscription = _initIsolateHandler(_serviceClient), _driverId = _nextDriverId++; + // The Dart VM may be running with --pause-isolates-on-start. + // Set a listener to unpause new isolates as they are ready to run, + // otherwise they'll hang indefinitely. + static StreamSubscription<VMIsolateRef> _initIsolateHandler( + VMServiceClient serviceClient) { + return serviceClient.onIsolateRunnable.listen( + (VMIsolateRef isolateRef) async { + await _resumeLeniently(isolateRef); + } + ); + } + /// Connects to a Flutter application. /// /// See [FlutterDriver.connect] for more documentation. @@ -100,9 +113,12 @@ null ? vm.isolates.first : vm.isolates.firstWhere( (VMIsolateRef isolate) => isolate.number == isolateNumber); - _log('Isolate found with number: ${isolateRef.number}'); + _log('Isolate found with number: ${isolateRef.number}'); + // There is a race condition here, the isolate may have been destroyed before + // we get around to loading it. VMIsolate isolate = await isolateRef.loadRunnable(); + _log('Loaded isolate: ${isolate.number}'); // TODO(yjbanov): vm_service_client does not support "None" pause event yet. // It is currently reported as null, but we cannot rely on it because @@ -117,9 +133,22 @@ isolate.pauseEvent is! VMPauseExceptionEvent && isolate.pauseEvent is! VMPauseInterruptedEvent && isolate.pauseEvent is! VMResumeEvent) { + _log('Reloading first isolate.'); isolate = await isolateRef.loadRunnable(); + _log('Reloaded first isolate.'); } + // Tells the Dart VM Service to notify us about "Isolate" events. + // + // This is a workaround for an issue in package:vm_service_client, which + // subscribes to the "Isolate" stream lazily upon subscription, which + // results in lost events. + // + // Details: https://github.com/dart-lang/vm_service_client/issues/17 + await connection.peer.sendRequest('streamListen', <String, String>{ + 'streamId': 'Isolate', + }); + final VMServiceFlutterDriver driver = VMServiceFlutterDriver.connectedTo( client, connection.peer, isolate, printCommunication: printCommunication, @@ -128,27 +157,6 @@ driver._dartVmReconnectUrl = dartVmServiceUrl; - // Attempts to resume the isolate, but does not crash if it fails because - // the isolate is already resumed. There could be a race with other tools, - // such as a debugger, any of which could have resumed the isolate. - Future<dynamic> resumeLeniently() { - _log('Attempting to resume isolate'); - return isolate.resume().catchError((dynamic e) { - const int vmMustBePausedCode = 101; - if (e is rpc.RpcException && e.code == vmMustBePausedCode) { - // No biggie; something else must have resumed the isolate - _log( - 'Attempted to resume an already resumed isolate. This may happen ' - 'when we lose a race with another tool (usually a debugger) that ' - 'is connected to the same isolate.' - ); - } else { - // Failed to resume due to another reason. Fail hard. - throw e; - } - }); - } - /// Waits for a signal from the VM service that the extension is registered. /// /// Looks at the list of loaded extensions for the current [isolateRef], as @@ -183,42 +191,8 @@ ]); } - /// Tells the Dart VM Service to notify us about "Isolate" events. - /// - /// This is a workaround for an issue in package:vm_service_client, which - /// subscribes to the "Isolate" stream lazily upon subscription, which - /// results in lost events. - /// - /// Details: https://github.com/dart-lang/vm_service_client/issues/17 - Future<void> enableIsolateStreams() async { - await connection.peer.sendRequest('streamListen', <String, String>{ - 'streamId': 'Isolate', - }); - } - // Attempt to resume isolate if it was paused - if (isolate.pauseEvent is VMPauseStartEvent) { - _log('Isolate is paused at start.'); - - await resumeLeniently(); - } else if (isolate.pauseEvent is VMPauseExitEvent || - isolate.pauseEvent is VMPauseBreakpointEvent || - isolate.pauseEvent is VMPauseExceptionEvent || - isolate.pauseEvent is VMPauseInterruptedEvent) { - // If the isolate is paused for any other reason, assume the extension is - // already there. - _log('Isolate is paused mid-flight.'); - await resumeLeniently(); - } else if (isolate.pauseEvent is VMResumeEvent) { - _log('Isolate is not paused. Assuming application is ready.'); - } else { - _log( - 'Unknown pause event type ${isolate.pauseEvent.runtimeType}. ' - 'Assuming application is ready.' - ); - } - - await enableIsolateStreams(); + await _resumeLeniently(isolate); // We will never receive the extension event if the user does not register // it. If that happens, show a message but continue waiting. @@ -241,6 +215,58 @@ return driver; } + static bool _alreadyRunning(VMIsolate isolate) { + // Expected pause events. + if (isolate.pauseEvent is VMPauseStartEvent || + isolate.pauseEvent is VMPauseExitEvent || + isolate.pauseEvent is VMPauseBreakpointEvent || + isolate.pauseEvent is VMPauseExceptionEvent || + isolate.pauseEvent is VMPauseInterruptedEvent || + isolate.pauseEvent is VMNoneEvent) { + return false; + } + // Already running. + if (isolate.pauseEvent is VMResumeEvent) { + _log('Isolate is not paused. Assuming application is ready.'); + return true; + } + _log('Unknown pause event type ${isolate.pauseEvent.runtimeType}. ' + 'Assuming application is ready.'); + return true; + } + + /// Attempts to resume the isolate, but does not crash if it fails because + /// the isolate is already resumed. There could be a race with other tools, + /// such as a debugger, any of which could have resumed the isolate. + /// Returns the isolate if it is runnable, null otherwise. + static Future<void> _resumeLeniently(VMIsolateRef isolateRef) async { + try { + _log('New runnable isolate ${isolateRef.number}'); + final VMIsolate isolate = await isolateRef.load(); + if (_alreadyRunning(isolate)) { + return; + } + _log('Attempting to resume isolate ${isolate.number}'); + await isolate.resume(); + _log('Resumed isolate ${isolate.number}'); + } on VMSentinelException { + // Probably OK, the isolate vanished before we got to it. + _log('Failed to load isolate, proceeding.'); + } on rpc.RpcException catch (e) { + const int vmMustBePausedCode = 101; + if (e.code == vmMustBePausedCode) { + // No biggie; something else must have resumed the isolate + _log('Attempted to resume an already resumed isolate. This may happen ' + 'when we lose a race with another tool (usually a debugger) that ' + 'is connected to the same isolate.' + ); + } else { + // Failed to resume due to another reason. Fail hard. + rethrow; + } + } + } + static int _nextDriverId = 0; static const String _flutterExtensionMethodName = 'ext.flutter.driver'; @@ -275,6 +301,9 @@ /// would like to instrument. final VMServiceClient _serviceClient; + /// Subscription to isolate events happening in the app. + final StreamSubscription<VMIsolateRef> _isolateSubscription; + /// JSON-RPC client useful for sending raw JSON requests. rpc.Peer _peer; @@ -552,6 +581,7 @@ @override Future<void> close() async { // Don't leak vm_service_client-specific objects, if any + await _isolateSubscription.cancel(); await _serviceClient.close(); await _peer.close(); }
diff --git a/packages/flutter_driver/test/flutter_driver_test.dart b/packages/flutter_driver/test/flutter_driver_test.dart index 3487fa3..4ff4e77 100644 --- a/packages/flutter_driver/test/flutter_driver_test.dart +++ b/packages/flutter_driver/test/flutter_driver_test.dart
@@ -35,6 +35,7 @@ MockVM mockVM; MockIsolate mockIsolate; MockPeer mockPeer; + MockIsolate otherIsolate; void expectLogContains(String message) { expect(log, anyElement(contains(message))); @@ -45,10 +46,15 @@ mockClient = MockVMServiceClient(); mockVM = MockVM(); mockIsolate = MockIsolate(); + otherIsolate = MockIsolate(); mockPeer = MockPeer(); when(mockClient.getVM()).thenAnswer((_) => Future<MockVM>.value(mockVM)); + when(mockClient.onIsolateRunnable).thenAnswer((Invocation invocation) { + return Stream<VMIsolateRef>.fromIterable(<VMIsolateRef>[otherIsolate]); + }); when(mockVM.isolates).thenReturn(<VMRunnableIsolate>[mockIsolate]); when(mockIsolate.loadRunnable()).thenAnswer((_) => Future<MockIsolate>.value(mockIsolate)); + when(mockIsolate.load()).thenAnswer((_) => Future<MockIsolate>.value(mockIsolate)); when(mockIsolate.extensionRpcs).thenReturn(<String>[]); when(mockIsolate.onExtensionAdded).thenAnswer((Invocation invocation) { return Stream<String>.fromIterable(<String>['ext.flutter.driver']); @@ -60,6 +66,10 @@ VMServiceClientConnection(mockClient, mockPeer) ); }; + when(otherIsolate.load()).thenAnswer((_) => Future<MockIsolate>.value(otherIsolate)); + when(otherIsolate.resume()).thenAnswer((Invocation invocation) { + return Future<dynamic>.value(null); + }); }); tearDown(() async { @@ -77,15 +87,20 @@ connectionLog.add('resume'); return Future<dynamic>.value(null); }); + when(otherIsolate.pauseEvent).thenReturn(MockVMPauseStartEvent()); when(mockIsolate.onExtensionAdded).thenAnswer((Invocation invocation) { connectionLog.add('onExtensionAdded'); return Stream<String>.fromIterable(<String>['ext.flutter.driver']); }); + when(otherIsolate.resume()).thenAnswer((Invocation invocation) { + connectionLog.add('other-resume'); + return Future<dynamic>.value(null); + }); final FlutterDriver driver = await FlutterDriver.connect(dartVmServiceUrl: ''); expect(driver, isNotNull); - expectLogContains('Isolate is paused at start'); - expect(connectionLog, <String>['resume', 'streamListen', 'onExtensionAdded']); + expectLogContains('Attempting to resume isolate'); + expect(connectionLog, <String>['streamListen', 'resume', 'other-resume', 'onExtensionAdded']); }); test('connects to isolate paused mid-flight', () async { @@ -94,7 +109,7 @@ final FlutterDriver driver = await FlutterDriver.connect(dartVmServiceUrl: ''); expect(driver, isNotNull); - expectLogContains('Isolate is paused mid-flight'); + expectLogContains('Attempting to resume isolate'); }); // This test simulates a situation when we believe that the isolate is @@ -160,6 +175,9 @@ mockClient = MockVMServiceClient(); mockPeer = MockPeer(); mockIsolate = MockIsolate(); + when(mockClient.onIsolateRunnable).thenAnswer((Invocation invocation) { + return Stream<VMIsolateRef>.fromIterable(<VMIsolateRef>[]); + }); driver = VMServiceFlutterDriver.connectedTo(mockClient, mockPeer, mockIsolate); }); @@ -790,6 +808,9 @@ mockClient = MockVMServiceClient(); mockPeer = MockPeer(); mockIsolate = MockIsolate(); + when(mockClient.onIsolateRunnable).thenAnswer((Invocation invocation) { + return Stream<VMIsolateRef>.fromIterable(<VMIsolateRef>[]); + }); driver = VMServiceFlutterDriver.connectedTo(mockClient, mockPeer, mockIsolate); });