[flutter_tools] Update background isolates when performing hot reload/restart (#52149)

When performing a hot restart, collect isolates without an attached flutter view and send a kill signal. These must have been spawned by running main, so restarting without removing them leads to isolate duplication.

When performing a hot reload, ensure that we send a reloadSources command to every isolate and not just uiIsolates.

packages/flutter_tools/lib/src/resident_runner.dart

@@ -283,8 +283,8 @@ class FlutterDevice {
     final Uri deviceEntryUri = devFS.baseUri.resolveUri(globals.fs.path.toUri(entryPath));
     final Uri devicePackagesUri = devFS.baseUri.resolve('.packages');
     return <Future<Map<String, dynamic>>>[
-      for (final FlutterView view in views)
-        view.uiIsolate.reloadSources(
+      for (final Isolate isolate in vmService.vm.isolates)
+        isolate.reloadSources(
           pause: pause,
           rootLibUri: deviceEntryUri,
           packagesUri: devicePackagesUri,

packages/flutter_tools/lib/src/run_hot.dart

@@ -93,9 +93,9 @@ class HotRunner extends ResidentRunner {
   bool _didAttach = false;
 
   final Map<String, List<int>> benchmarkData = <String, List<int>>{};
-  // The initial launch is from a snapshot.
-  bool _runningFromSnapshot = true;
+
   DateTime firstBuildTime;
+  bool _shouldResetAssetDirectory = true;
 
   void _addBenchmarkData(String name, int value) {
     benchmarkData[name] ??= <int>[];
@@ -520,14 +520,16 @@ class HotRunner extends ResidentRunner {
       }
     }
     // Check if the isolate is paused and resume it.
-    final List<Future<void>> futures = <Future<void>>[];
+    final List<Future<void>> operations = <Future<void>>[];
     for (final FlutterDevice device in flutterDevices) {
+      final Set<Isolate> uiIsolates = <Isolate>{};
       for (final FlutterView view in device.views) {
         if (view.uiIsolate == null) {
           continue;
         }
+        uiIsolates.add(view.uiIsolate);
         // Reload the isolate.
-        futures.add(view.uiIsolate.reload().then((ServiceObject _) {
+        operations.add(view.uiIsolate.reload().then((ServiceObject _) {
           final ServiceEvent pauseEvent = view.uiIsolate.pauseEvent;
           if ((pauseEvent != null) && pauseEvent.isPauseEvent) {
             // Resume the isolate so that it can be killed by the embedder.
@@ -536,16 +538,22 @@ class HotRunner extends ResidentRunner {
           return null;
         }));
       }
+      // The engine handles killing and recreating isolates that it has spawned
+      // ("uiIsolates"). The isolates that were spawned from these uiIsolates
+      // will not be restared, and so they must be manually killed.
+      for (final Isolate isolate in device?.vmService?.vm?.isolates ?? <Isolate>[]) {
+        if (!uiIsolates.contains(isolate)) {
+          operations.add(isolate.invokeRpcRaw('kill', params: <String, dynamic>{
+            'isolateId': isolate.id,
+          }));
+        }
+      }
     }
-    await Future.wait(futures);
+    await Future.wait(operations);
 
-    // We are now running from source.
-    _runningFromSnapshot = false;
     await _launchFromDevFS(mainPath + '.dill');
     restartTimer.stop();
     globals.printTrace('Hot restart performed in ${getElapsedAsMilliseconds(restartTimer.elapsed)}.');
-    // We are now running from sources.
-    _runningFromSnapshot = false;
     _addBenchmarkData('hotRestartMillisecondsToFrame',
         restartTimer.elapsed.inMilliseconds);
 
@@ -734,10 +742,8 @@ class HotRunner extends ResidentRunner {
     String reason,
     bool pause,
   }) async {
-    final bool reloadOnTopOfSnapshot = _runningFromSnapshot;
-    final String progressPrefix = reloadOnTopOfSnapshot ? 'Initializing' : 'Performing';
     Status status = globals.logger.startProgress(
-      '$progressPrefix hot reload...',
+      'Performing hot reload...',
       timeout: timeoutConfiguration.fastOperation,
       progressId: 'hot.reload',
     );
@@ -788,13 +794,6 @@ class HotRunner extends ResidentRunner {
       }
     }
 
-    // The initial launch is from a script snapshot. When we reload from source
-    // on top of a script snapshot, the first reload will be a worst case reload
-    // because all of the sources will end up being dirty (library paths will
-    // change from host path to a device path). Subsequent reloads will
-    // not be affected, so we resume reporting reload times on the second
-    // reload.
-    bool shouldReportReloadTime = !_runningFromSnapshot;
     final Stopwatch reloadTimer = Stopwatch()..start();
 
     if (!_isPaused()) {
@@ -805,6 +804,7 @@ class HotRunner extends ResidentRunner {
     final Stopwatch devFSTimer = Stopwatch()..start();
     final UpdateFSReport updatedDevFS = await _updateDevFS();
     // Record time it took to synchronize to DevFS.
+    bool shouldReportReloadTime = true;
     _addBenchmarkData('hotReloadDevFSSyncMilliseconds', devFSTimer.elapsed.inMilliseconds);
     if (!updatedDevFS.success) {
       return OperationResult(1, 'DevFS synchronization failed');
@@ -819,10 +819,11 @@ class HotRunner extends ResidentRunner {
       );
       final List<Future<DeviceReloadReport>> allReportsFutures = <Future<DeviceReloadReport>>[];
       for (final FlutterDevice device in flutterDevices) {
-        if (_runningFromSnapshot) {
+        if (_shouldResetAssetDirectory) {
           // Asset directory has to be set only once when we switch from
-          // running from snapshot to running from uploaded files.
+          // running from bundle to uploaded files.
           await device.resetAssetDirectory();
+          _shouldResetAssetDirectory = false;
         }
         final List<Future<Map<String, dynamic>>> reportFutures = device.reloadSources(
           entryPath, pause: pause,
@@ -905,8 +906,6 @@ class HotRunner extends ResidentRunner {
     }
     await Future.wait(allDevices);
 
-    // We are now running from source.
-    _runningFromSnapshot = false;
     // Check if any isolates are paused.
     final List<FlutterView> reassembleViews = <FlutterView>[];
     String serviceEventKind;

packages/flutter_tools/test/general.shard/resident_runner_test.dart

@@ -92,6 +92,9 @@ void main() {
     ]);
     when(mockFlutterDevice.device).thenReturn(mockDevice);
     when(mockFlutterView.uiIsolate).thenReturn(mockIsolate);
+    final MockVM mockVM = MockVM();
+    when(mockVMService.vm).thenReturn(mockVM);
+    when(mockVM.isolates).thenReturn(<Isolate>[mockIsolate]);
     when(mockFlutterView.runFromSource(any, any, any)).thenAnswer((Invocation invocation) async {});
     when(mockFlutterDevice.stopEchoingDeviceLog()).thenAnswer((Invocation invocation) async { });
     when(mockFlutterDevice.observatoryUris).thenAnswer((_) => Stream<Uri>.value(testUri));
@@ -744,6 +747,7 @@ class MockDevicePortForwarder extends Mock implements DevicePortForwarder {}
 class MockUsage extends Mock implements Usage {}
 class MockProcessManager extends Mock implements ProcessManager {}
 class MockServiceEvent extends Mock implements ServiceEvent {}
+class MockVM extends Mock implements VM {}
 class TestFlutterDevice extends FlutterDevice {
   TestFlutterDevice(Device device, this.views, { Stream<Uri> observatoryUris })
     : super(device, buildInfo: BuildInfo.debug) {

packages/flutter_tools/test/integration.shard/background_isolate_test.dart

@@ -0,0 +1,88 @@
+// Copyright 2014 The Flutter 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 'package:file/file.dart';
+import 'package:flutter_tools/src/base/file_system.dart';
+
+import '../src/common.dart';
+import 'test_data/background_project.dart';
+import 'test_driver.dart';
+import 'test_utils.dart';
+
+void main() {
+  Directory tempDir;
+
+  setUp(() async {
+    tempDir = createResolvedTempDirectorySync('hot_reload_test.');
+  });
+
+  tearDown(() async {
+    tryToDelete(tempDir);
+  });
+
+  test('Hot restart kills background isolates', () async {
+    final BackgroundProject project = BackgroundProject();
+    await project.setUpIn(tempDir);
+    final FlutterRunTestDriver flutter = FlutterRunTestDriver(tempDir);
+
+    const String newBackgroundMessage = 'New Background';
+    final Completer<void> sawForgroundMessage = Completer<void>.sync();
+    final Completer<void> sawBackgroundMessage = Completer<void>.sync();
+    final Completer<void> sawNewBackgroundMessage = Completer<void>.sync();
+    final StreamSubscription<String> subscription = flutter.stdout.listen((String line) {
+        print('[LOG]:"$line"');
+        if (line.contains('Main thread') && !sawForgroundMessage.isCompleted) {
+          sawForgroundMessage.complete();
+        }
+        if (line.contains('Isolate thread')) {
+          sawBackgroundMessage.complete();
+        }
+        if (line.contains(newBackgroundMessage)) {
+          sawNewBackgroundMessage.complete();
+        }
+      },
+    );
+    await flutter.run();
+    await sawForgroundMessage.future;
+    await sawBackgroundMessage.future;
+
+    project.updateTestIsolatePhrase(newBackgroundMessage);
+    await flutter.hotRestart();
+    await sawBackgroundMessage.future;
+    // Wait a tiny amount of time in case we did not kill the background isolate.
+    await Future<void>.delayed(const Duration(milliseconds: 10));
+    await subscription.cancel();
+    await flutter?.stop();
+  });
+
+  test('Hot reload updates background isolates', () async {
+    final RepeatingBackgroundProject project = RepeatingBackgroundProject();
+    await project.setUpIn(tempDir);
+    final FlutterRunTestDriver flutter = FlutterRunTestDriver(tempDir);
+
+    const String newBackgroundMessage = 'New Background';
+    final Completer<void> sawBackgroundMessage = Completer<void>.sync();
+    final Completer<void> sawNewBackgroundMessage = Completer<void>.sync();
+    final StreamSubscription<String> subscription = flutter.stdout.listen((String line) {
+        print('[LOG]:"$line"');
+        if (line.contains('Isolate thread') && !sawBackgroundMessage.isCompleted) {
+          sawBackgroundMessage.complete();
+        }
+        if (line.contains(newBackgroundMessage) && !sawNewBackgroundMessage.isCompleted) {
+          sawNewBackgroundMessage.complete();
+        }
+      },
+    );
+    await flutter.run();
+    await sawBackgroundMessage.future;
+
+    project.updateTestIsolatePhrase(newBackgroundMessage);
+    await flutter.hotReload();
+    await sawNewBackgroundMessage.future;
+    await subscription.cancel();
+    await flutter?.stop();
+  });
+}

packages/flutter_tools/test/integration.shard/test_data/background_project.dart

@@ -0,0 +1,108 @@
+// Copyright 2014 The Flutter 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 'package:flutter_tools/src/globals.dart' as globals;
+
+import '../test_utils.dart';
+import 'project.dart';
+
+/// Spawns a background isolate that prints a debug message.
+class BackgroundProject extends Project {
+
+  @override
+  final String pubspec = '''
+  name: test
+  environment:
+    sdk: ">=2.0.0-dev.68.0 <3.0.0"
+
+  dependencies:
+    flutter:
+      sdk: flutter
+  ''';
+
+  @override
+  final String main = r'''
+  import 'dart:async';
+  import 'dart:isolate';
+
+  import 'package:flutter/widgets.dart';
+  import 'package:flutter/material.dart';
+
+  void main() {
+    Isolate.spawn<void>(background, null, debugName: 'background');
+    TestMain();
+  }
+
+  void background(void message) {
+    TestIsolate();
+  }
+
+  class TestMain {
+    TestMain() {
+      debugPrint('Main thread');
+    }
+  }
+
+  class TestIsolate {
+    TestIsolate() {
+      debugPrint('Isolate thread');
+    }
+  }
+  ''';
+
+  void updateTestIsolatePhrase(String message) {
+    final String newMainContents = main.replaceFirst('Isolate thread', message);
+    writeFile(globals.fs.path.join(dir.path, 'lib', 'main.dart'), newMainContents);
+  }
+}
+
+// Spawns a background isolate that repeats a message.
+class RepeatingBackgroundProject extends Project {
+
+  @override
+  final String pubspec = '''
+  name: test
+  environment:
+    sdk: ">=2.0.0-dev.68.0 <3.0.0"
+
+  dependencies:
+    flutter:
+      sdk: flutter
+  ''';
+
+  @override
+  final String main = r'''
+  import 'dart:async';
+  import 'dart:isolate';
+
+  import 'package:flutter/widgets.dart';
+  import 'package:flutter/material.dart';
+
+  void main() {
+    Isolate.spawn<void>(background, null, debugName: 'background');
+    TestMain();
+  }
+
+  void background(void message) {
+    Timer.periodic(const Duration(milliseconds: 500), (Timer timer) => TestIsolate());
+  }
+
+  class TestMain {
+    TestMain() {
+      debugPrint('Main thread');
+    }
+  }
+
+  class TestIsolate {
+    TestIsolate() {
+      debugPrint('Isolate thread');
+    }
+  }
+  ''';
+
+  void updateTestIsolatePhrase(String message) {
+    final String newMainContents = main.replaceFirst('Isolate thread', message);
+    writeFile(globals.fs.path.join(dir.path, 'lib', 'main.dart'), newMainContents);
+  }
+}