From 86f1d55b744bc35af9cf104a38012845aeac6d97 Mon Sep 17 00:00:00 2001 From: Anna Gringauze Date: Wed, 14 Apr 2021 16:59:10 -0700 Subject: [PATCH 1/4] Initiaze new isolate after restart - reset initialization completer on `destroyIsolate` so correct signal is set after `createIsolate` - make all supported VM API wait for isolate initizaliation before proceeding. Towards:https://github.com/flutter/flutter/issues/74903 --- .../src/services/chrome_proxy_service.dart | 62 ++++++++++++++----- dwds/test/reload_test.dart | 59 +++++++++++++++++- 2 files changed, 102 insertions(+), 19 deletions(-) diff --git a/dwds/lib/src/services/chrome_proxy_service.dart b/dwds/lib/src/services/chrome_proxy_service.dart index b4589a89f..49b16c637 100644 --- a/dwds/lib/src/services/chrome_proxy_service.dart +++ b/dwds/lib/src/services/chrome_proxy_service.dart @@ -52,7 +52,7 @@ class ChromeProxyService implements VmServiceInterface { /// are dynamic and roughly map to chrome tabs. final VM _vm; - final _initializedCompleter = Completer(); + Completer _initializedCompleter = Completer(); Future get isInitialized => _initializedCompleter.future; @@ -165,10 +165,10 @@ class ChromeProxyService implements VmServiceInterface { _skipLists.initialize(); // We do not need to wait for compiler dependencies to be udpated as the // [ExpressionEvaluator] is robust to evaluation requests during updates. - unawaited(updateCompilerDependencies(entrypoint)); + unawaited(_updateCompilerDependencies(entrypoint)); } - Future updateCompilerDependencies(String entrypoint) async { + Future _updateCompilerDependencies(String entrypoint) async { var metadataProvider = globalLoadStrategy.metadataProviderFor(entrypoint); var moduleFormat = globalLoadStrategy.moduleFormat; var soundNullSafety = await metadataProvider.soundNullSafety; @@ -273,6 +273,7 @@ class ChromeProxyService implements VmServiceInterface { void destroyIsolate() { var isolate = _inspector?.isolate; if (isolate == null) return; + _initializedCompleter = Completer(); _streamNotify( 'Isolate', Event( @@ -299,9 +300,11 @@ class ChromeProxyService implements VmServiceInterface { @override Future addBreakpoint(String isolateId, String scriptId, int line, - {int column}) async => - (await _debugger) - .addBreakpoint(isolateId, scriptId, line, column: column); + {int column}) async { + await isInitialized; + return (await _debugger) + .addBreakpoint(isolateId, scriptId, line, column: column); + } @override Future addBreakpointAtEntry(String isolateId, String functionId) { @@ -312,6 +315,7 @@ class ChromeProxyService implements VmServiceInterface { Future addBreakpointWithScriptUri( String isolateId, String scriptUri, int line, {int column}) async { + await isInitialized; var dartUri = DartUri(scriptUri, uri); var ref = await _inspector.scriptRefFor(dartUri.serverPath); return (await _debugger) @@ -321,6 +325,7 @@ class ChromeProxyService implements VmServiceInterface { @override Future callServiceExtension(String method, {String isolateId, Map args}) async { + await isInitialized; // Validate the isolate id is correct, _getIsolate throws if not. if (isolateId != null) _getIsolate(isolateId); args ??= {}; @@ -414,6 +419,7 @@ ${globalLoadStrategy.loadModuleSnippet}("dart_sdk").developer.invokeExtension( try { if (_expressionEvaluator != null) { + await isInitialized; _validateIsolateId(isolateId); var library = await _inspector?.getLibrary(isolateId, targetId); @@ -454,6 +460,7 @@ ${globalLoadStrategy.loadModuleSnippet}("dart_sdk").developer.invokeExtension( try { if (_expressionEvaluator != null) { + await isInitialized; _validateIsolateId(isolateId); var result = await _getEvaluationResult( @@ -516,25 +523,38 @@ ${globalLoadStrategy.loadModuleSnippet}("dart_sdk").developer.invokeExtension( } @override - Future getIsolate(String isolateId) async => _getIsolate(isolateId); + Future getIsolate(String isolateId) async { + await isInitialized; + return _getIsolate(isolateId); + } @override - Future getMemoryUsage(String isolateId) { + Future getMemoryUsage(String isolateId) async { + await isInitialized; return _inspector.getMemoryUsage(isolateId); } @override Future getObject(String isolateId, String objectId, - {int offset, int count}) => - _inspector?.getObject(isolateId, objectId, offset: offset, count: count); + {int offset, int count}) async { + await isInitialized; + return _inspector?.getObject(isolateId, objectId, + offset: offset, count: count); + } @override - Future getScripts(String isolateId) => - _inspector?.getScripts(isolateId); + Future getScripts(String isolateId) async { + await isInitialized; + return _inspector?.getScripts(isolateId); + } @override Future getSourceReport(String isolateId, List reports, - {String scriptId, int tokenPos, int endTokenPos, bool forceCompile}) { + {String scriptId, + int tokenPos, + int endTokenPos, + bool forceCompile}) async { + await isInitialized; return _inspector?.getSourceReport(isolateId, reports, scriptId: scriptId, tokenPos: tokenPos, @@ -548,8 +568,10 @@ ${globalLoadStrategy.loadModuleSnippet}("dart_sdk").developer.invokeExtension( /// /// The returned stack will contain up to [limit] frames if provided. @override - Future getStack(String isolateId, {int limit}) async => - (await _debugger).getStack(isolateId, limit: limit); + Future getStack(String isolateId, {int limit}) async { + await isInitialized; + return (await _debugger).getStack(isolateId, limit: limit); + } @override Future getVM() async { @@ -577,6 +599,7 @@ ${globalLoadStrategy.loadModuleSnippet}("dart_sdk").developer.invokeExtension( Future invoke( String isolateId, String targetId, String selector, List argumentIds, {bool disableBreakpoints}) async { + await isInitialized; // TODO(798) - respect disableBreakpoints. var remote = await _inspector?.invoke(isolateId, targetId, selector, argumentIds); @@ -633,7 +656,10 @@ ${globalLoadStrategy.loadModuleSnippet}("dart_sdk").developer.invokeExtension( } @override - Future pause(String isolateId) async => (await _debugger).pause(); + Future pause(String isolateId) async { + await isInitialized; + return (await _debugger).pause(); + } @override Future registerService(String service, String alias) async { @@ -653,6 +679,7 @@ ${globalLoadStrategy.loadModuleSnippet}("dart_sdk").developer.invokeExtension( @override Future removeBreakpoint( String isolateId, String breakpointId) async { + await isInitialized; _disabledBreakpoints .removeWhere((breakpoint) => breakpoint.id == breakpointId); return (await _debugger).removeBreakpoint(isolateId, breakpointId); @@ -664,6 +691,7 @@ ${globalLoadStrategy.loadModuleSnippet}("dart_sdk").developer.invokeExtension( if (_inspector == null) throw StateError('No running isolate.'); if (_inspector.appConnection.isStarted) { var stopwatch = Stopwatch()..start(); + await isInitialized; var result = await (await _debugger) .resume(isolateId, step: step, frameIndex: frameIndex); emitEvent(DwdsEvent('RESUME', { @@ -679,6 +707,7 @@ ${globalLoadStrategy.loadModuleSnippet}("dart_sdk").developer.invokeExtension( @override Future setExceptionPauseMode(String isolateId, String mode) async { + await isInitialized; return (await _debugger).setExceptionPauseMode(isolateId, mode); } @@ -695,6 +724,7 @@ ${globalLoadStrategy.loadModuleSnippet}("dart_sdk").developer.invokeExtension( @override Future setName(String isolateId, String name) async { + await isInitialized; var isolate = _getIsolate(isolateId); isolate.name = name; return Success(); diff --git a/dwds/test/reload_test.dart b/dwds/test/reload_test.dart index 415099f24..346731576 100644 --- a/dwds/test/reload_test.dart +++ b/dwds/test/reload_test.dart @@ -90,7 +90,7 @@ void main() { group('Injected client', () { setUp(() async { - await context.setUp(); + await context.setUp(enableExpressionEvaluation: true); }); tearDown(() async { @@ -204,13 +204,66 @@ void main() { expect(source.contains('Hello World!'), isTrue); expect(source.contains('Gary is awesome!'), isTrue); - // Should not be paused. vm = await client.getVM(); isolateId = vm.isolates.first.id; var isolate = await client.getIsolate(isolateId); - expect(isolate.pauseEvent.kind, EventKind.kResume); + // Previous breakpoint should still exist. expect(isolate.breakpoints.isNotEmpty, isTrue); + var bp = isolate.breakpoints.first; + + // Should pause eventually. + await stream + .firstWhere((event) => event.kind == EventKind.kPauseBreakpoint); + + expect(await client.removeBreakpoint(isolate.id, bp.id), isA()); + expect(await client.resume(isolate.id), isA()); + }); + + test('can evaluate expressions after hot restart ', () async { + var client = context.debugConnection.vmService; + var vm = await client.getVM(); + var isolateId = vm.isolates.first.id; + await client.streamListen('Debug'); + var stream = client.onEvent('Debug'); + var scriptList = await client.getScripts(isolateId); + var main = scriptList.scripts + .firstWhere((script) => script.uri.contains('main.dart')); + var bpLine = + await context.findBreakpointLine('printCount', isolateId, main); + await client.addBreakpoint(isolateId, main.id, bpLine); + await stream + .firstWhere((event) => event.kind == EventKind.kPauseBreakpoint); + + await client.callServiceExtension('hotRestart'); + + vm = await client.getVM(); + isolateId = vm.isolates.first.id; + var isolate = await client.getIsolate(isolateId); + var library = isolate.rootLib.uri; + var bp = isolate.breakpoints.first; + + // Should pause eventually. + var event = await stream + .firstWhere((event) => event.kind == EventKind.kPauseBreakpoint); + + // Expression evaluation while paused on a breakpoint should work. + var result = await client.evaluateInFrame( + isolate.id, event.topFrame.index, 'count'); + expect( + result, + isA().having((instance) => instance.valueAsString, + 'valueAsString', greaterThanOrEqualTo('0'))); + + await client.removeBreakpoint(isolateId, bp.id); + await client.resume(isolateId); + + // Expression evaluation while running should work. + result = await client.evaluate(isolateId, library, 'true'); + expect( + result, + isA().having( + (instance) => instance.valueAsString, 'valueAsString', 'true')); }); }); From f05fbbb0c93eba15fe6a8ea8ad070892a798efcb Mon Sep 17 00:00:00 2001 From: Anna Gringauze Date: Wed, 14 Apr 2021 18:22:33 -0700 Subject: [PATCH 2/4] Update changelog --- dwds/CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/dwds/CHANGELOG.md b/dwds/CHANGELOG.md index 2db505d3a..6ac6e55e1 100644 --- a/dwds/CHANGELOG.md +++ b/dwds/CHANGELOG.md @@ -5,6 +5,7 @@ - Fix incorrect `rootLib` returned by `ChromeProxyService`. - Fix not working breakpoints in library part files. - Fix data race in calculating locations for a module. +- Fix uninitialized isolate after hot restart. ## 10.0.1 From 283ae748ecf8cb69975ef21b68048a2856836283 Mon Sep 17 00:00:00 2001 From: Anna Gringauze Date: Thu, 15 Apr 2021 11:29:32 -0700 Subject: [PATCH 3/4] Fix evaluate completing before update dependencies --- dwds/CHANGELOG.md | 2 ++ .../lib/src/services/expression_compiler_service.dart | 11 ++++++----- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/dwds/CHANGELOG.md b/dwds/CHANGELOG.md index 6ac6e55e1..6b60fb1f6 100644 --- a/dwds/CHANGELOG.md +++ b/dwds/CHANGELOG.md @@ -6,6 +6,8 @@ - Fix not working breakpoints in library part files. - Fix data race in calculating locations for a module. - Fix uninitialized isolate after hot restart. +- Fix intermittent failure caused by evaluation not waiting for dependencies + to be updated. ## 10.0.1 diff --git a/dwds/lib/src/services/expression_compiler_service.dart b/dwds/lib/src/services/expression_compiler_service.dart index 8c53de8df..8b69d8e73 100644 --- a/dwds/lib/src/services/expression_compiler_service.dart +++ b/dwds/lib/src/services/expression_compiler_service.dart @@ -24,7 +24,8 @@ class _Compiler { final ReceivePort _receivePort; final SendPort _sendPort; - Future _dependencyUpdate; + Completer _updateDependenciesCompleter = Completer(); + Future get dependenciesUpdated => _updateDependenciesCompleter.future; _Compiler._( this._worker, @@ -132,8 +133,8 @@ class _Compiler { if (_worker == null) { throw StateError('Expression compilation service has stopped'); } - var updateCompleter = Completer(); - _dependencyUpdate = updateCompleter.future; + + _updateDependenciesCompleter = Completer(); _logger.info('Updating dependencies...'); _logger.finest('Dependencies: $modules'); @@ -158,7 +159,7 @@ class _Compiler { var s = response['stackTrace']; _logger.severe('Failed to update dependencies: $e:$s'); } - updateCompleter.complete(); + _updateDependenciesCompleter.complete(); return result; } @@ -177,7 +178,7 @@ class _Compiler { } _logger.finest('Waiting for dependencies to update'); - await _dependencyUpdate; + await dependenciesUpdated; _logger.finest('Compiling "$expression" at $libraryUri:$line'); From 732e973aebff14396520f3f8a20cdcd3cc3dc122 Mon Sep 17 00:00:00 2001 From: Anna Gringauze Date: Thu, 15 Apr 2021 18:31:11 -0700 Subject: [PATCH 4/4] Actually fix expression evaluation before updateDependencies --- dwds/lib/src/services/chrome_proxy_service.dart | 15 ++++++++++++--- .../src/services/expression_compiler_service.dart | 11 +++++------ 2 files changed, 17 insertions(+), 9 deletions(-) diff --git a/dwds/lib/src/services/chrome_proxy_service.dart b/dwds/lib/src/services/chrome_proxy_service.dart index 49b16c637..f2b76704c 100644 --- a/dwds/lib/src/services/chrome_proxy_service.dart +++ b/dwds/lib/src/services/chrome_proxy_service.dart @@ -52,10 +52,14 @@ class ChromeProxyService implements VmServiceInterface { /// are dynamic and roughly map to chrome tabs. final VM _vm; + /// Signals when isolate is intialized. Completer _initializedCompleter = Completer(); - Future get isInitialized => _initializedCompleter.future; + /// Signals when expression compiler is ready to evaluate. + Completer _compilerCompleter = Completer(); + Future get isCompilerInitialized => _compilerCompleter.future; + /// The root URI at which we're serving. final String uri; @@ -183,6 +187,8 @@ class ChromeProxyService implements VmServiceInterface { await globalLoadStrategy.moduleInfoForEntrypoint(entrypoint); var stopwatch = Stopwatch()..start(); await _compiler.updateDependencies(dependencies); + // Expression evaluation is ready after dependencies are updated. + if (!_compilerCompleter.isCompleted) _compilerCompleter.complete(); emitEvent(DwdsEvent('COMPILER_UPDATE_DEPENDENCIES', { 'entrypoint': entrypoint, 'elapsedMilliseconds': stopwatch.elapsedMilliseconds, @@ -274,6 +280,7 @@ class ChromeProxyService implements VmServiceInterface { var isolate = _inspector?.isolate; if (isolate == null) return; _initializedCompleter = Completer(); + _compilerCompleter = Completer(); _streamNotify( 'Isolate', Event( @@ -418,8 +425,9 @@ ${globalLoadStrategy.loadModuleSnippet}("dart_sdk").developer.invokeExtension( dynamic error; try { + await isInitialized; if (_expressionEvaluator != null) { - await isInitialized; + await isCompilerInitialized; _validateIsolateId(isolateId); var library = await _inspector?.getLibrary(isolateId, targetId); @@ -459,8 +467,9 @@ ${globalLoadStrategy.loadModuleSnippet}("dart_sdk").developer.invokeExtension( dynamic error; try { + await isInitialized; if (_expressionEvaluator != null) { - await isInitialized; + await isCompilerInitialized; _validateIsolateId(isolateId); var result = await _getEvaluationResult( diff --git a/dwds/lib/src/services/expression_compiler_service.dart b/dwds/lib/src/services/expression_compiler_service.dart index 8b69d8e73..8c53de8df 100644 --- a/dwds/lib/src/services/expression_compiler_service.dart +++ b/dwds/lib/src/services/expression_compiler_service.dart @@ -24,8 +24,7 @@ class _Compiler { final ReceivePort _receivePort; final SendPort _sendPort; - Completer _updateDependenciesCompleter = Completer(); - Future get dependenciesUpdated => _updateDependenciesCompleter.future; + Future _dependencyUpdate; _Compiler._( this._worker, @@ -133,8 +132,8 @@ class _Compiler { if (_worker == null) { throw StateError('Expression compilation service has stopped'); } - - _updateDependenciesCompleter = Completer(); + var updateCompleter = Completer(); + _dependencyUpdate = updateCompleter.future; _logger.info('Updating dependencies...'); _logger.finest('Dependencies: $modules'); @@ -159,7 +158,7 @@ class _Compiler { var s = response['stackTrace']; _logger.severe('Failed to update dependencies: $e:$s'); } - _updateDependenciesCompleter.complete(); + updateCompleter.complete(); return result; } @@ -178,7 +177,7 @@ class _Compiler { } _logger.finest('Waiting for dependencies to update'); - await dependenciesUpdated; + await _dependencyUpdate; _logger.finest('Compiling "$expression" at $libraryUri:$line');