diff --git a/packages/devtools/lib/src/vm_service_wrapper.dart b/packages/devtools/lib/src/vm_service_wrapper.dart index f26b5de4e74..0138f58f97c 100644 --- a/packages/devtools/lib/src/vm_service_wrapper.dart +++ b/packages/devtools/lib/src/vm_service_wrapper.dart @@ -7,24 +7,33 @@ import 'dart:async'; import 'package:vm_service_lib/vm_service_lib.dart'; class VmServiceWrapper implements VmService { - VmServiceWrapper(this._vmService); + VmServiceWrapper( + this._vmService, { + this.trackFutures = false, + }); VmServiceWrapper.fromNewVmService( Stream /*String|List*/ inStream, void writeMessage(String message), { Log log, DisposeHandler disposeHandler, + this.trackFutures = false, }) { _vmService = VmService(inStream, writeMessage, log: log, disposeHandler: disposeHandler); } VmService _vmService; + final bool trackFutures; final Map> _activeStreams = {}; - // ignore: prefer_collection_literals - final Set> _activeFutures = Set(); - Completer allFuturesCompleted = Completer(); + final Set> activeFutures = {}; + Completer _allFuturesCompleter = Completer() + // Mark the future as completed by default so if we don't track any + // futures but someone tries to wait on [allFuturesCompleted] they don't + // hang. The first tracked future will replace this with a new completer. + ..complete(true); + Future get allFuturesCompleted => _allFuturesCompleter.future; @override Future addBreakpoint( @@ -33,13 +42,14 @@ class VmServiceWrapper implements VmService { int line, { int column, }) { - return _trackFuture( + return _trackFuture('addBreakpoint', _vmService.addBreakpoint(isolateId, scriptId, line, column: column)); } @override Future addBreakpointAtEntry(String isolateId, String functionId) { - return _trackFuture(_vmService.addBreakpointAtEntry(isolateId, functionId)); + return _trackFuture('addBreakpointAtEntry', + _vmService.addBreakpointAtEntry(isolateId, functionId)); } @override @@ -49,17 +59,19 @@ class VmServiceWrapper implements VmService { int line, { int column, }) { - return _trackFuture(_vmService.addBreakpointWithScriptUri( - isolateId, - scriptUri, - line, - column: column, - )); + return _trackFuture( + 'addBreakpointWithScriptUri', + _vmService.addBreakpointWithScriptUri( + isolateId, + scriptUri, + line, + column: column, + )); } @override Future callMethod(String method, {String isolateId, Map args}) { - return _trackFuture( + return _trackFuture('callMethod $method', _vmService.callMethod(method, isolateId: isolateId, args: args)); } @@ -69,26 +81,30 @@ class VmServiceWrapper implements VmService { String isolateId, Map args, }) { - return _trackFuture(_vmService.callServiceExtension( - method, - isolateId: isolateId, - args: args, - )); + return _trackFuture( + 'callServiceExtension $method', + _vmService.callServiceExtension( + method, + isolateId: isolateId, + args: args, + )); } @override Future clearCpuProfile(String isolateId) { - return _trackFuture(_vmService.clearCpuProfile(isolateId)); + return _trackFuture( + 'clearCpuProfile', _vmService.clearCpuProfile(isolateId)); } @override Future clearVMTimeline() { - return _trackFuture(_vmService.clearVMTimeline()); + return _trackFuture('clearVMTimeline', _vmService.clearVMTimeline()); } @override Future collectAllGarbage(String isolateId) { - return _trackFuture(_vmService.collectAllGarbage(isolateId)); + return _trackFuture( + 'collectAllGarbage', _vmService.collectAllGarbage(isolateId)); } @override @@ -102,13 +118,15 @@ class VmServiceWrapper implements VmService { Map scope, bool disableBreakpoints, }) { - return _trackFuture(_vmService.evaluate( - isolateId, - targetId, - expression, - scope: scope, - disableBreakpoints: disableBreakpoints, - )); + return _trackFuture( + 'evaluate $expression', + _vmService.evaluate( + isolateId, + targetId, + expression, + scope: scope, + disableBreakpoints: disableBreakpoints, + )); } @override @@ -119,13 +137,15 @@ class VmServiceWrapper implements VmService { Map scope, bool disableBreakpoints, }) { - return _trackFuture(_vmService.evaluateInFrame( - isolateId, - frameIndex, - expression, - scope: scope, - disableBreakpoints: disableBreakpoints, - )); + return _trackFuture( + 'evaluateInFrame $expression', + _vmService.evaluateInFrame( + isolateId, + frameIndex, + expression, + scope: scope, + disableBreakpoints: disableBreakpoints, + )); } @override @@ -134,40 +154,46 @@ class VmServiceWrapper implements VmService { String gc, bool reset, }) { - return _trackFuture(_vmService.getAllocationProfile(isolateId)); + return _trackFuture( + 'getAllocationProfile', _vmService.getAllocationProfile(isolateId)); } @override Future getCpuProfile(String isolateId, String tags) { - return _trackFuture(_vmService.getCpuProfile(isolateId, tags)); + return _trackFuture( + 'getCpuProfile', _vmService.getCpuProfile(isolateId, tags)); } // TODO(kenzie): keep track of all private methods we are currently using to // share with the VM team and request that they be made public. Future getCpuProfileTimeline( String isolateId, int origin, int extent) async { - return _trackFuture(callMethod( - '_getCpuProfileTimeline', - isolateId: isolateId, - args: { - 'tags': 'None', - 'timeOriginMicros': origin, - 'timeExtentMicros': extent, - }, - )); + return _trackFuture( + 'getCpuProfileTimeline', + callMethod( + '_getCpuProfileTimeline', + isolateId: isolateId, + args: { + 'tags': 'None', + 'timeOriginMicros': origin, + 'timeExtentMicros': extent, + }, + )); } @override - Future getFlagList() => _trackFuture(_vmService.getFlagList()); + Future getFlagList() => + _trackFuture('getFlagList', _vmService.getFlagList()); @override Future getInstances(String isolateId, String classId, int limit) { - return _trackFuture(_vmService.getInstances(isolateId, classId, limit)); + return _trackFuture( + 'getInstances', _vmService.getInstances(isolateId, classId, limit)); } @override Future getIsolate(String isolateId) { - return _trackFuture(_vmService.getIsolate(isolateId)); + return _trackFuture('getIsolate', _vmService.getIsolate(isolateId)); } @override @@ -177,12 +203,12 @@ class VmServiceWrapper implements VmService { int offset, int count, }) { - return _trackFuture(_vmService.getObject(isolateId, objectId)); + return _trackFuture('getObject', _vmService.getObject(isolateId, objectId)); } @override Future getScripts(String isolateId) { - return _trackFuture(_vmService.getScripts(isolateId)); + return _trackFuture('getScripts', _vmService.getScripts(isolateId)); } @override @@ -194,29 +220,33 @@ class VmServiceWrapper implements VmService { int endTokenPos, bool forceCompile, }) { - return _trackFuture(_vmService.getSourceReport( - isolateId, - reports, - scriptId: scriptId, - tokenPos: tokenPos, - endTokenPos: endTokenPos, - forceCompile: forceCompile, - )); + return _trackFuture( + 'getSourceReport', + _vmService.getSourceReport( + isolateId, + reports, + scriptId: scriptId, + tokenPos: tokenPos, + endTokenPos: endTokenPos, + forceCompile: forceCompile, + )); } @override Future getStack(String isolateId) { - return _trackFuture(_vmService.getStack(isolateId)); + return _trackFuture('getStack', _vmService.getStack(isolateId)); } @override - Future getVM() => _trackFuture(_vmService.getVM()); + Future getVM() => _trackFuture('getVM', _vmService.getVM()); @override - Future getVMTimeline() => _trackFuture(_vmService.getVMTimeline()); + Future getVMTimeline() => + _trackFuture('getVMTimeline', _vmService.getVMTimeline()); @override - Future getVersion() => _trackFuture(_vmService.getVersion()); + Future getVersion() => + _trackFuture('getVersion', _vmService.getVersion()); @override Future invoke( @@ -226,18 +256,20 @@ class VmServiceWrapper implements VmService { List argumentIds, { bool disableBreakpoints, }) { - return _trackFuture(_vmService.invoke( - isolateId, - targetId, - selector, - argumentIds, - disableBreakpoints: disableBreakpoints, - )); + return _trackFuture( + 'invoke $selector', + _vmService.invoke( + isolateId, + targetId, + selector, + argumentIds, + disableBreakpoints: disableBreakpoints, + )); } @override Future kill(String isolateId) { - return _trackFuture(_vmService.kill(isolateId)); + return _trackFuture('kill', _vmService.kill(isolateId)); } @override @@ -278,12 +310,13 @@ class VmServiceWrapper implements VmService { @override Future pause(String isolateId) { - return _trackFuture(_vmService.pause(isolateId)); + return _trackFuture('pause', _vmService.pause(isolateId)); } @override Future registerService(String service, String alias) { - return _trackFuture(_vmService.registerService(service, alias)); + return _trackFuture( + 'registerService $service', _vmService.registerService(service, alias)); } @override @@ -299,18 +332,21 @@ class VmServiceWrapper implements VmService { String rootLibUri, String packagesUri, }) { - return _trackFuture(_vmService.reloadSources( - isolateId, - force: force, - pause: pause, - rootLibUri: rootLibUri, - packagesUri: packagesUri, - )); + return _trackFuture( + 'reloadSources', + _vmService.reloadSources( + isolateId, + force: force, + pause: pause, + rootLibUri: rootLibUri, + packagesUri: packagesUri, + )); } @override Future removeBreakpoint(String isolateId, String breakpointId) { - return _trackFuture(_vmService.removeBreakpoint(isolateId, breakpointId)); + return _trackFuture('removeBreakpoint', + _vmService.removeBreakpoint(isolateId, breakpointId)); } @override @@ -319,24 +355,25 @@ class VmServiceWrapper implements VmService { String roots, bool collectGarbage, ) { - return _trackFuture( + return _trackFuture('requestHeapSnapshot', _vmService.requestHeapSnapshot(isolateId, roots, collectGarbage)); } @override Future resume(String isolateId, {String step, int frameIndex}) { - return _trackFuture( + return _trackFuture('resume', _vmService.resume(isolateId, step: step, frameIndex: frameIndex)); } @override Future setExceptionPauseMode(String isolateId, String mode) { - return _trackFuture(_vmService.setExceptionPauseMode(isolateId, mode)); + return _trackFuture('setExceptionPauseMode', + _vmService.setExceptionPauseMode(isolateId, mode)); } @override Future setFlag(String name, String value) { - return _trackFuture(_vmService.setFlag(name, value)); + return _trackFuture('setFlag', _vmService.setFlag(name, value)); } @override @@ -345,29 +382,30 @@ class VmServiceWrapper implements VmService { String libraryId, bool isDebuggable, ) { - return _trackFuture( + return _trackFuture('setLibraryDebuggable', _vmService.setLibraryDebuggable(isolateId, libraryId, isDebuggable)); } @override Future setName(String isolateId, String name) { - return _trackFuture(_vmService.setName(isolateId, name)); + return _trackFuture('setName', _vmService.setName(isolateId, name)); } @override Future setVMName(String name) { - return _trackFuture(_vmService.setVMName(name)); + return _trackFuture('setVMName', _vmService.setVMName(name)); } @override Future setVMTimelineFlags(List recordedStreams) { - return _trackFuture(_vmService.setVMTimelineFlags(recordedStreams)); + return _trackFuture( + 'setVMTimelineFlags', _vmService.setVMTimelineFlags(recordedStreams)); } @override Future streamCancel(String streamId) { _activeStreams.remove(streamId); - return _trackFuture(_vmService.streamCancel(streamId)); + return _trackFuture('streamCancel', _vmService.streamCancel(streamId)); } // We tweaked this method so that we do not try to listen to the same stream @@ -377,7 +415,7 @@ class VmServiceWrapper implements VmService { Future streamListen(String streamId) { if (!_activeStreams.containsKey(streamId)) { final Future future = - _trackFuture(_vmService.streamListen(streamId)); + _trackFuture('streamListen', _vmService.streamListen(streamId)); _activeStreams[streamId] = future; return future; } else { @@ -393,21 +431,25 @@ class VmServiceWrapper implements VmService { /// used after a hot restart to avoid bugs where we have zombie futures lying /// around causing tests to flake. void doNotWaitForPendingFuturesBeforeExit() { - allFuturesCompleted = Completer(); - allFuturesCompleted.complete(true); - _activeFutures.clear(); + _allFuturesCompleter = Completer(); + _allFuturesCompleter.complete(true); + activeFutures.clear(); } - Future _trackFuture(Future future) { - if (allFuturesCompleted.isCompleted) { - allFuturesCompleted = Completer(); + Future _trackFuture(String name, Future future) { + if (!trackFutures) { + return future; + } + final trackedFuture = new TrackedFuture(name, future); + if (_allFuturesCompleter.isCompleted) { + _allFuturesCompleter = Completer(); } - _activeFutures.add(future); + activeFutures.add(trackedFuture); void futureComplete() { - _activeFutures.remove(future); - if (_activeFutures.isEmpty && !allFuturesCompleted.isCompleted) { - allFuturesCompleted.complete(true); + activeFutures.remove(trackedFuture); + if (activeFutures.isEmpty && !_allFuturesCompleter.isCompleted) { + _allFuturesCompleter.complete(true); } } @@ -418,3 +460,10 @@ class VmServiceWrapper implements VmService { return future; } } + +class TrackedFuture { + TrackedFuture(this.name, this.future); + + final String name; + final Future future; +} diff --git a/packages/devtools/pubspec.yaml b/packages/devtools/pubspec.yaml index e2d793f8962..c76013893a9 100644 --- a/packages/devtools/pubspec.yaml +++ b/packages/devtools/pubspec.yaml @@ -9,7 +9,7 @@ author: Dart Team homepage: https://github.com/flutter/devtools environment: - sdk: '>=2.1.0-dev <3.0.0' + sdk: '>=2.2.0-dev <3.0.0' dependencies: args: ^1.5.1 diff --git a/packages/devtools/test/service_manager_test.dart b/packages/devtools/test/service_manager_test.dart index b3a5822428f..1c5421f51dc 100644 --- a/packages/devtools/test/service_manager_test.dart +++ b/packages/devtools/test/service_manager_test.dart @@ -281,7 +281,7 @@ void main() { }); tearDown(() async { - await service.allFuturesCompleted.future; + await service.allFuturesCompleted; await _flutter.stop(); }); diff --git a/packages/devtools/test/support/flutter_test_driver.dart b/packages/devtools/test/support/flutter_test_driver.dart index fbd6398faab..448020ecc8d 100644 --- a/packages/devtools/test/support/flutter_test_driver.dart +++ b/packages/devtools/test/support/flutter_test_driver.dart @@ -327,7 +327,9 @@ class FlutterRunTestDriver extends FlutterTestDriver { _vmServiceWsUri = getVmServiceUriFromObservatoryUri(_vmServiceWsUri); vmService = VmServiceWrapper( - await vmServiceConnectUri(_vmServiceWsUri.toString())); + await vmServiceConnectUri(_vmServiceWsUri.toString()), + trackFutures: true, + ); vmService.onSend.listen((String s) => _debugPrint('==> $s')); vmService.onReceive.listen((String s) => _debugPrint('<== $s')); await Future.wait(>[ diff --git a/packages/devtools/test/support/flutter_test_environment.dart b/packages/devtools/test/support/flutter_test_environment.dart index c3995abec65..72a20d98177 100644 --- a/packages/devtools/test/support/flutter_test_environment.dart +++ b/packages/devtools/test/support/flutter_test_environment.dart @@ -89,7 +89,12 @@ class FlutterTestEnvironment { } if (_beforeTearDown != null) await _beforeTearDown(); - await _service.allFuturesCompleted.future; + await _service.allFuturesCompleted.timeout(const Duration(seconds: 20), + onTimeout: () { + throw 'Timed out waiting for futures to complete during teardown. ' + '${_service.activeFutures.length} futures remained:\n\n' + ' ${_service.activeFutures.map((tf) => tf.name).join('\n ')}'; + }); await _flutter.stop(); _flutter = null;