[dwds] Fix bug where no-op hot restart doesn't cancel a subscription and publish 25.0.1

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.

dwds/lib/src/dwds_vm_client.dart

@@ -611,20 +611,20 @@ Future<Map<String, dynamic>> _hotRestart(
         globalToolConfiguration.loadStrategy is DdcLibraryBundleStrategy;
     final computedReloadedSrcs = Completer<void>();
     final reloadedSrcs = <String>{};
+    late StreamSubscription<String> 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<void>();
       final debugger = await chromeProxyService.debuggerFuture;
-      late StreamSubscription<String> 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<Map<String, dynamic>> _hotRestart(
         chromeProxyService.allowedToCreateIsolate.complete();
       }
       computedReloadedSrcs.complete();
+      await chromeProxyService.allowedToCreateIsolate.future;
+      await parsedScriptsSubscription.cancel();
     } else {
       assert(remoteObject.value == null);
     }

dwds/lib/src/services/chrome_proxy_service.dart

@@ -1089,18 +1089,15 @@ class ChromeProxyService extends ProxyService {
     final parsedAllReloadedSrcs = Completer<void>();
     // Wait until all the reloaded scripts are parsed before we reinitialize
     // metadata below.
-    late StreamSubscription<String> 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.

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

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(

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<void> recompile() async {
+    await context.recompile(fullRestart: false);
+  }
+
   Future<void> 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<void> waitForEvaluateToExecute() async {
+    final completer = Completer<void>();
+    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);

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() {