diff --git a/dwds/CHANGELOG.md b/dwds/CHANGELOG.md index b32fe60af..2add9e7f8 100644 --- a/dwds/CHANGELOG.md +++ b/dwds/CHANGELOG.md @@ -1,3 +1,10 @@ +## 25.0.1 + +### Bug Fixes: + +- Fix issue in hot restart where a hot restart with no changes followed by one + with changes, a `Future` was completed again, resulting in a crash. + ## 25.0.0 - Implemented hot restart over websockets with multi window support. diff --git a/dwds/lib/src/dwds_vm_client.dart b/dwds/lib/src/dwds_vm_client.dart index 73c3aff72..37f24bf3d 100644 --- a/dwds/lib/src/dwds_vm_client.dart +++ b/dwds/lib/src/dwds_vm_client.dart @@ -611,20 +611,20 @@ Future> _hotRestart( globalToolConfiguration.loadStrategy is DdcLibraryBundleStrategy; final computedReloadedSrcs = Completer(); final reloadedSrcs = {}; + late StreamSubscription parsedScriptsSubscription; if (isDdcLibraryBundle) { // Injected client should send a request to recreate the isolate after the // hot restart. The creation of the isolate should in turn wait until all // scripts are parsed. chromeProxyService.allowedToCreateIsolate = Completer(); final debugger = await chromeProxyService.debuggerFuture; - late StreamSubscription parsedScriptsSubscription; parsedScriptsSubscription = debugger.parsedScriptsController.stream .listen((url) { computedReloadedSrcs.future.then((_) async { reloadedSrcs.remove(Uri.parse(url).normalizePath().path); - if (reloadedSrcs.isEmpty) { + if (reloadedSrcs.isEmpty && + !chromeProxyService.allowedToCreateIsolate.isCompleted) { chromeProxyService.allowedToCreateIsolate.complete(); - await parsedScriptsSubscription.cancel(); } }); }); @@ -648,6 +648,8 @@ Future> _hotRestart( chromeProxyService.allowedToCreateIsolate.complete(); } computedReloadedSrcs.complete(); + await chromeProxyService.allowedToCreateIsolate.future; + await parsedScriptsSubscription.cancel(); } else { assert(remoteObject.value == null); } diff --git a/dwds/lib/src/services/chrome_proxy_service.dart b/dwds/lib/src/services/chrome_proxy_service.dart index 288e1ee6c..198e54155 100644 --- a/dwds/lib/src/services/chrome_proxy_service.dart +++ b/dwds/lib/src/services/chrome_proxy_service.dart @@ -1089,18 +1089,15 @@ class ChromeProxyService extends ProxyService { final parsedAllReloadedSrcs = Completer(); // Wait until all the reloaded scripts are parsed before we reinitialize // metadata below. - late StreamSubscription parsedScriptsSubscription; - parsedScriptsSubscription = debugger.parsedScriptsController.stream.listen(( - url, - ) { - computedReloadedSrcs.future.then((_) async { - reloadedSrcs.remove(Uri.parse(url).normalizePath().path); - if (reloadedSrcs.isEmpty) { - parsedAllReloadedSrcs.complete(); - await parsedScriptsSubscription.cancel(); - } - }); - }); + final parsedScriptsSubscription = debugger.parsedScriptsController.stream + .listen((url) { + computedReloadedSrcs.future.then((_) { + reloadedSrcs.remove(Uri.parse(url).normalizePath().path); + if (reloadedSrcs.isEmpty && !parsedAllReloadedSrcs.isCompleted) { + parsedAllReloadedSrcs.complete(); + } + }); + }); // Initiate a hot reload. _logger.info('Issuing \$dartHotReloadStartDwds request'); @@ -1121,6 +1118,7 @@ class ChromeProxyService extends ProxyService { } computedReloadedSrcs.complete(); if (reloadedSrcs.isNotEmpty) await parsedAllReloadedSrcs.future; + await parsedScriptsSubscription.cancel(); if (!pauseIsolatesOnStart) { // Finish hot reload immediately. diff --git a/dwds/lib/src/version.dart b/dwds/lib/src/version.dart index b40640984..fa092f758 100644 --- a/dwds/lib/src/version.dart +++ b/dwds/lib/src/version.dart @@ -1,2 +1,2 @@ // Generated code. Do not modify. -const packageVersion = '25.0.0'; +const packageVersion = '25.0.1'; diff --git a/dwds/pubspec.yaml b/dwds/pubspec.yaml index e3ae323a4..3b9e52480 100644 --- a/dwds/pubspec.yaml +++ b/dwds/pubspec.yaml @@ -1,6 +1,6 @@ name: dwds # Every time this changes you need to run `dart run build_runner build`. -version: 25.0.0 +version: 25.0.1 description: >- A service that proxies between the Chrome debug protocol and the Dart VM diff --git a/dwds/test/common/hot_restart_common.dart b/dwds/test/common/hot_restart_common.dart index 504704a15..5cea54d1e 100644 --- a/dwds/test/common/hot_restart_common.dart +++ b/dwds/test/common/hot_restart_common.dart @@ -470,6 +470,42 @@ void runTests({ ), ); }); + + test('can hot restart with no changes, hot restart with changes, and ' + 'hot restart again with no changes', () async { + // Empty hot restart. + var mainDone = waitForMainToExecute(); + await recompile(); + final hotRestart = context.getRegisteredServiceExtension('hotRestart'); + await fakeClient.callServiceExtension(hotRestart!); + + await mainDone; + var source = await context.webDriver.pageSource; + expect(source.contains(originalString), isTrue); + expect(source.contains(newString), isFalse); + + // Hot restart. + mainDone = waitForMainToExecute(); + await makeEditAndRecompile(); + await fakeClient.callServiceExtension(hotRestart); + + await mainDone; + source = await context.webDriver.pageSource; + // Main is re-invoked which shouldn't clear the state. + expect(source.contains(originalString), isTrue); + expect(source.contains(newString), isTrue); + + // Empty hot restart. + mainDone = waitForMainToExecute(); + await recompile(); + await fakeClient.callServiceExtension(hotRestart); + + await mainDone; + source = await context.webDriver.pageSource; + expect(source.contains(originalString), isTrue); + // `newString` should now exist twice in the source. + expect(source.contains(RegExp('$newString.*$newString')), isTrue); + }); }, timeout: Timeout.factor(2)); group( diff --git a/dwds/test/hot_reload_test.dart b/dwds/test/hot_reload_test.dart index d9eec22f9..27c48fad4 100644 --- a/dwds/test/hot_reload_test.dart +++ b/dwds/test/hot_reload_test.dart @@ -7,6 +7,8 @@ @Timeout(Duration(minutes: 5)) library; +import 'dart:async'; + import 'package:dwds/expression_compiler.dart'; import 'package:test/test.dart'; import 'package:test_common/logging.dart'; @@ -33,13 +35,31 @@ void main() { tearDownAll(provider.dispose); + Future recompile() async { + await context.recompile(fullRestart: false); + } + Future makeEditAndRecompile() async { context.makeEditToDartLibFile( libFileName: 'library1.dart', toReplace: originalString, replaceWith: newString, ); - await context.recompile(fullRestart: false); + await recompile(); + } + + /// Wait for `evaluate` to finish executing before checking expectations by + /// checking for a log output. + Future waitForEvaluateToExecute() async { + final completer = Completer(); + final expectedString = 'evaluate executed'; + final subscription = context.webkitDebugger.onConsoleAPICalled.listen((e) { + if (e.args.first.value == expectedString) { + completer.complete(); + } + }); + await completer.future; + await subscription.cancel(); } group('Injected client', () { @@ -64,11 +84,10 @@ void main() { test('can hot reload', () async { final client = context.debugConnection.vmService; - await makeEditAndRecompile(); + await makeEditAndRecompile(); final vm = await client.getVM(); final isolate = await client.getIsolate(vm.isolates!.first.id!); - final report = await fakeClient.reloadSources(isolate.id!); expect(report.success, true); @@ -78,8 +97,54 @@ void main() { expect(source, contains(originalString)); expect(source.contains(newString), false); + final evaluateDone = waitForEvaluateToExecute(); final rootLib = isolate.rootLib; await client.evaluate(isolate.id!, rootLib!.id!, 'evaluate()'); + await evaluateDone; + source = await context.webDriver.pageSource; + expect(source, contains(newString)); + expect(source.contains(originalString), false); + }); + + test('can hot reload with no changes, hot reload with changes, and ' + 'hot reload again with no changes', () async { + final client = context.debugConnection.vmService; + + // Empty hot reload, + await recompile(); + final vm = await client.getVM(); + final isolate = await client.getIsolate(vm.isolates!.first.id!); + var report = await fakeClient.reloadSources(isolate.id!); + expect(report.success, true); + + var evaluateDone = waitForEvaluateToExecute(); + final rootLib = isolate.rootLib; + await client.evaluate(isolate.id!, rootLib!.id!, 'evaluate()'); + await evaluateDone; + var source = await context.webDriver.pageSource; + expect(source, contains(originalString)); + expect(source.contains(newString), false); + + // Hot reload. + await makeEditAndRecompile(); + report = await fakeClient.reloadSources(isolate.id!); + expect(report.success, true); + + evaluateDone = waitForEvaluateToExecute(); + await client.evaluate(isolate.id!, rootLib.id!, 'evaluate()'); + await evaluateDone; + source = await context.webDriver.pageSource; + expect(source, contains(newString)); + expect(source.contains(originalString), false); + + // Empty hot reload. + await recompile(); + report = await fakeClient.reloadSources(isolate.id!); + expect(report.success, true); + + evaluateDone = waitForEvaluateToExecute(); + await client.evaluate(isolate.id!, rootLib.id!, 'evaluate()'); + await evaluateDone; source = await context.webDriver.pageSource; expect(source, contains(newString)); expect(source.contains(originalString), false); diff --git a/fixtures/_test_hot_reload/web/main.dart b/fixtures/_test_hot_reload/web/main.dart index fb4229367..e1b1f02f8 100644 --- a/fixtures/_test_hot_reload/web/main.dart +++ b/fixtures/_test_hot_reload/web/main.dart @@ -7,11 +7,17 @@ import 'dart:js_interop'; import 'package:_test_hot_reload/library1.dart'; +@JS('console.log') +external void log(String _); + @JS('document.body.innerHTML') external set innerHtml(String html); void evaluate() { innerHtml = 'Program is running!\n $reloadValue}\n'; + + // Wait for this print statement so that we know evaluate is done executing. + log('evaluate executed'); } void main() {