From efb88a03807c2043ac05a0b8001fb902c487693f Mon Sep 17 00:00:00 2001 From: Danny Tuppeny Date: Wed, 7 Feb 2018 16:58:23 +0000 Subject: [PATCH] Reject requests for hot reload if a hot reload is already in progress. (#14494) * Reject requests for hot reload if a hot reload is already in progress. Fixes #14184 * Implement TODO, verifying further hot reloads complete sucessfully. * Fix year on new file. * Add missing type annotations to fix lints * Add run_machine_concurrent_hot_reload to manifest for CI * Reformat document ... but undo things that cause lints (like single-line ifs) * Extract std stream transformations * Make inProgressHotReload private * Disallow all types of reload while hot reload in progress * Simplify code handling in-progress hot reloads --- .../run_machine_concurrent_hot_reload.dart | 155 ++++++++++++++++++ dev/devicelab/manifest.yaml | 6 + .../lib/src/commands/daemon.dart | 10 +- 3 files changed, 170 insertions(+), 1 deletion(-) create mode 100644 dev/devicelab/bin/tasks/run_machine_concurrent_hot_reload.dart diff --git a/dev/devicelab/bin/tasks/run_machine_concurrent_hot_reload.dart b/dev/devicelab/bin/tasks/run_machine_concurrent_hot_reload.dart new file mode 100644 index 0000000000..ce4b89a78b --- /dev/null +++ b/dev/devicelab/bin/tasks/run_machine_concurrent_hot_reload.dart @@ -0,0 +1,155 @@ +// Copyright (c) 2018 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import 'dart:async'; +import 'dart:convert'; +import 'dart:io'; + +import 'package:path/path.dart' as path; +import 'package:vm_service_client/vm_service_client.dart'; + +import 'package:flutter_devicelab/framework/adb.dart'; +import 'package:flutter_devicelab/framework/framework.dart'; +import 'package:flutter_devicelab/framework/utils.dart'; + +void main() { + Map parseFlutterResponse(String line) { + if (line.startsWith('[') && line.endsWith(']')) { + try { + return JSON.decode(line)[0]; + } catch (e) { + // Not valid JSON, so likely some other output that was surrounded by [brackets] + return null; + } + } + return null; + } + + Stream transformToLines(Stream> byteStream) { + return byteStream.transform(UTF8.decoder).transform(const LineSplitter()); + } + + task(() async { + int vmServicePort; + String appId; + + final Device device = await devices.workingDevice; + await device.unlock(); + final Directory appDir = + dir(path.join(flutterDirectory.path, 'dev/integration_tests/ui')); + await inDirectory(appDir, () async { + final Completer ready = new Completer(); + bool ok; + print('run: starting...'); + final Process run = await startProcess( + path.join(flutterDirectory.path, 'bin', 'flutter'), + [ + 'run', + '--machine', + '--verbose', + '-d', + device.deviceId, + 'lib/commands.dart' + ], + ); + final StreamController stdout = + new StreamController.broadcast(); + transformToLines(run.stdout).listen((String line) { + print('run:stdout: $line'); + stdout.add(line); + final dynamic json = parseFlutterResponse(line); + if (json != null && json['event'] == 'app.debugPort') { + vmServicePort = Uri.parse(json['params']['wsUri']).port; + print('service protocol connection available at port $vmServicePort'); + } else if (json != null && json['event'] == 'app.started') { + appId = json['params']['appId']; + print('run: ready!'); + ready.complete(); + ok ??= true; + } + }); + transformToLines(run.stderr).listen((String line) { + stderr.writeln('run:stderr: $line'); + }); + run.exitCode.then((int exitCode) { + ok = false; + }); + await Future.any(>[ready.future, run.exitCode]); + if (!ok) + throw 'Failed to run test app.'; + + final VMServiceClient client = + new VMServiceClient.connect('ws://localhost:$vmServicePort/ws'); + + int id = 1; + Future> sendRequest( + String method, dynamic params) async { + final int requestId = id++; + final Completer> response = + new Completer>(); + final StreamSubscription responseSubscription = + stdout.stream.listen((String line) { + final Map json = parseFlutterResponse(line); + if (json != null && json['id'] == requestId) + response.complete(json); + }); + final Map req = { + 'id': requestId, + 'method': method, + 'params': params + }; + final String json = JSON.encode(>[req]); + print('run:stdin: $json'); + run.stdin.writeln(json); + final Map result = await response.future; + responseSubscription.cancel(); + return result; + } + + print('test: sending two hot reloads...'); + final Future hotReload1 = sendRequest('app.restart', + {'appId': appId, 'fullRestart': false}); + final Future hotReload2 = sendRequest('app.restart', + {'appId': appId, 'fullRestart': false}); + final Future> reloadRequests = + Future.wait(>[hotReload1, hotReload2]); + final dynamic results = await Future + .any(>[run.exitCode, reloadRequests]); + + if (!ok) + throw 'App crashed during hot reloads.'; + + final List responses = results; + final List errorResponses = + responses.where((dynamic r) => r['error'] != null).toList(); + final List successResponses = responses + .where((dynamic r) => + r['error'] == null && + r['result'] != null && + r['result']['code'] == 0) + .toList(); + + if (errorResponses.length != 1) + throw 'Did not receive the expected (exactly one) hot reload error response.'; + final String errorMessage = errorResponses.first['error']; + if (!errorMessage.contains('in progress')) + throw 'Error response was not that hot reload was in progress.'; + if (successResponses.length != 1) + throw 'Did not receive the expected (exactly one) successful hot reload response.'; + + final dynamic hotReload3 = await sendRequest('app.restart', + {'appId': appId, 'fullRestart': false}); + if (hotReload3['error'] != null) + throw 'Received an error response from a hot reload after all other hot reloads had completed.'; + + sendRequest('app.stop', {'appId': appId}); + final int result = await run.exitCode; + if (result != 0) + throw 'Received unexpected exit code $result from run process.'; + print('test: validating that the app has in fact closed...'); + await client.done.timeout(const Duration(seconds: 5)); + }); + return new TaskResult.success(null); + }); +} diff --git a/dev/devicelab/manifest.yaml b/dev/devicelab/manifest.yaml index 566dd6166a..13c8326a9e 100644 --- a/dev/devicelab/manifest.yaml +++ b/dev/devicelab/manifest.yaml @@ -200,6 +200,12 @@ tasks: stage: devicelab required_agent_capabilities: ["mac/android"] + run_machine_concurrent_hot_reload: + description: > + Runs tests of concurrent hot reload commands via flutter run --machine. + stage: devicelab + required_agent_capabilities: ["mac/android"] + service_extensions_test: description: > Validates our service protocol extensions. diff --git a/packages/flutter_tools/lib/src/commands/daemon.dart b/packages/flutter_tools/lib/src/commands/daemon.dart index 2a55e489e6..b81abd231e 100644 --- a/packages/flutter_tools/lib/src/commands/daemon.dart +++ b/packages/flutter_tools/lib/src/commands/daemon.dart @@ -447,6 +447,8 @@ class AppDomain extends Domain { bool isRestartSupported(bool enableHotReload, Device device) => enableHotReload && device.supportsHotMode; + Future _inProgressHotReload; + Future restart(Map args) async { final String appId = _getStringArg(args, 'appId', required: true); final bool fullRestart = _getBoolArg(args, 'fullRestart') ?? false; @@ -456,9 +458,15 @@ class AppDomain extends Domain { if (app == null) throw "app '$appId' not found"; - return app._runInZone(this, () { + if (_inProgressHotReload != null) + throw 'hot restart already in progress'; + + _inProgressHotReload = app._runInZone(this, () { return app.restart(fullRestart: fullRestart, pauseAfterRestart: pauseAfterRestart); }); + return _inProgressHotReload.whenComplete(() { + _inProgressHotReload = null; + }); } /// Returns an error, or the service extension result (a map with two fixed