[flutter_tools] On Windows, running a dart process with an aot snapshot vendored as part of the Dart SDK will cause flutter upgrade to crash

[christopherfujino]

On Windows, running a dart process with an aot snapshot vendored as part of the Dart SDK will cause flutter upgrade to crash. Context for this issue was originally in #146164 (comment), my summary of which I have copy pasted here:

1. The disk i/o implementations of dart.exe foo.appjit.snapshot and dartaotruntime.exe foo.aot.snapshot are different in that the latter will keep the snapshot file open for the duration of VM execution while the former will (presumably) close it after having read the data. I verified this with running a minimal dart program that runs indefinitely from both appjit and aot snapshots, and from another powershell session tried calling Rename-Item on the snapshot file--the rename succeeded on the appjit snapshot but failed on aot snapshot with Rename-Item : The process cannot access the file because it is being used by another process..
2. version 4.0.0 of frontend_server_client is changing the package:test runner to use an aot snapshot of the frontend_server rather than an appjit snapshot: Make FrontendServerClient start the frontend server from AOT snapshot by default dart-lang/webdev#2263.
3. The tool's batch_entrypoint_test is validating the logic in Flutter's update_dart_sdk.ps1 powershell script. Of interest to this issue is that at line 74, before downloading a new version of the Dart SDK, we call Rename-Item on the existing (or old) Dart SDK to move it to a path with dart-sdk.old in the name. I believe we do this because just deleting the old one while the tool is running would fail.
4. The fact that this test is failing after this change to the frontend_server_client depended on by the test runner is incidental to the specific behavior the test is running, and thus Derek's proposed test changes would make sense there.
5. The fact that this test is failing inadvertently revealed what I believe would be a real customer regression, in that when calling flutter upgrade, if there is any running dart process that is using an aot snapshot from the Dart SDK within the Flutter SDK's cache, flutter upgrade would likely fail at the step where it tries to download the new version of the Dart SDK. Re-running the flutter CLI tool would likely repeatedly fail as long as said Dart process is running.

At HEAD, the contents of //flutter/bin/cache/dart-sdk/bin/snapshots is:

analysis_server.dart.snapshot
dart2js.dart.snapshot
dart2wasm_product.snapshot
dartdevc.dart.snapshot
dartdev.dart.snapshot
dart_tooling_daemon.dart.snapshot
dds.dart.snapshot
frontend_server_aot.dart.snapshot
frontend_server.dart.snapshot
gen_kernel_aot.dart.snapshot
kernel-service.dart.snapshot
kernel_worker.dart.snapshot

It is my understanding that all of the appjit snapshots above will imminently be migrated to aot snapshots, thus increasing the likelihood that one of them is actively running when a user executes flutter upgrade.

FYI @a-siva @zanderso @derekxu16 @jacob314 @anderdobo

[christopherfujino]

For context, I am fairly confident that this will not affect macOS and Linux users.

With the following main.dart program:

import 'dart:async';

void main(List<String> args) {
  if (args.length == 1) { // training
    return;
  }

  int x = 0;
  Timer.periodic(const Duration(seconds: 1), (_) => print(x++));
}

And this Makefile next to it:

.PHONY: test-aot
test-aot: aot.snapshot
        test -n "$(DART_SDK)" # please set env var DART_SDK to point to your Dart SDK
        $(DART_SDK)/bin/dartaotruntime aot.snapshot &
        sleep 1
        mv aot.snapshot aot.snapshot.backup

.PHONY: test-appjit
test-appjit: appjit.snapshot
        dart run appjit.snapshot &
        sleep 1 # without this we hit error "Could not find file `appjit.snapshot`"
        mv appjit.snapshot appjit.snapshot.backup

appjit.snapshot: main.dart
        dart compile jit-snapshot -o appjit.snapshot main.dart --train

aot.snapshot: main.dart
        dart compile aot-snapshot -o aot.snapshot main.dart

The test-aot target verifies that we can successfully call mv on an aot snapshot that is currently being run in a Dart process.

However, when I ran this same program on Windows, and then called Rename-Item aot.snapshot aot.snapshot.backup I received:

PS C:\Users\chris\git\tmp\Rename-Item-test> Rename-Item aot.snapshot aot.snapshot.backup
Rename-Item : The process cannot access the file because it is being used by another process.
At line:1 char:1
+ Rename-Item aot.snapshot aot.snapshot.backup
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : WriteError: (C:\Users\chris\...st\aot.snapshot:String) [Rename-Item], IOException
    + FullyQualifiedErrorId : RenameItemIOError,Microsoft.PowerShell.Commands.RenameItemCommand

Running from appjit snapshot and then calling Remove-Item succeeded as expected.

[christopherfujino]

After meeting about this with the VM team, we came up with a proposed solution:

1. When downloading a Dart SDK, rather than always extracting it to //flutter/bin/cache/dart-sdk, instead namespace it by the engine revision (which today we already use as //flutter/bin/cache/engine-dart-sdk.stamp). We do NOT attempt to rename or delete the previous Dart SDK.
2. From the //flutter/bin/dart.{bat,sh} shell entrypoints, invoke the dart.exe binary via the namespaced path.
3. In the flutter tool, we check for stale Dart SDKs (that is, any Dart SDK directory with a namespace not matching the current engine version), and attempt to delete them. If the delete fails with a permissions error, recover silently.

[aam]

https://dart-review.googlesource.com/c/sdk/+/361985 should fix aot snapshot file locking behavior on Windows.

[a-siva]

@christopherfujino can the 'dependency:dart' label be removed on this issue.