[flutter_tools] pin frontend_server_client because upgrading from 3.2.0 -> 4.0.0 regresses test

[christopherfujino]

Example failing build: https://ci.chromium.org/p/flutter/builders/try/Windows%20tool_integration_tests_2_6/26698?

From attempted roll: #145959

Upstream tracking issue: dart-lang/webdev#2377

[christopherfujino]

I ran this locally and added debug prints on the stdout and stderr of the dart.bat invocation:

[stderr] Checking Dart SDK version...

[stderr] Downloading Dart SDK from Flutter engine ...

[stderr] Rename-Item : Access to the path 'D:\git\flutter\bin\cache\dart-sdk' is denied.
At D:\git\flutter\bin\internal\update_dart_sdk.ps1:74 char:9
+         Rename-Item $dartSdkPath "$oldDartSdkPrefix$oldDartSdkSuffix"
+         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : WriteError: (D:\git\flutter\bin\cache\dart-sdk:String) [Rename-Item], IOException

[stderr]     + FullyQualifiedErrorId : RenameItemIOError,Microsoft.PowerShell.Commands.RenameItemCommand

[stderr]

[stderr] Error: Unable to update Dart SDK. Retrying...

[stderr] ERROR: Input redirection is not supported, exiting the process immediately.

[stderr] Checking Dart SDK version...

[stderr] Downloading Dart SDK from Flutter engine ...

[derekxu16]

The timeout was happening for me locally because batch_entrypoint_test.dart tries to rename frontend_server_aot.dart.snapshot, but this happens when frontend_server_aot.dart.snapshot is actively in use, because it is used by the test runner, which makes Rename-Item fail. The changes in #146229 make the test pass locally, but the CI check is still timing out, so there must be something else going wrong.

[derekxu16]

I added some logging statements to #146229 and observed that the test times out on CI because a file is not getting copied successfully. Excerpt from these logs:

../../../../t/flutter_batch_entrypoint_test.d8ce6ed5/flutter/packages/flutter_tools/bin/flutter_tools.dart:5:8: Error: Error when reading '../../../../t/flutter_batch_entrypoint_test.d8ce6ed5/flutter/packages/flutter_tools/lib/executable.dart': The system cannot find the path specified.


import 'package:flutter_tools/executable.dart' as executable;
       ^
../../../../t/flutter_batch_entrypoint_test.d8ce6ed5/flutter/packages/flutter_tools/bin/flutter_tools.dart:8:14: Error: Method not found: 'main'.
  executable.main(args);
             ^^^^

Error: Unable to create dart snapshot for flutter tool.

I updated the PR with the following additional expectation to confirm this:

flutter/packages/flutter_tools/test/integration.shard/batch_entrypoint_test.dart

Lines 68 to 76 in 2bd2ef9

	expect(
	flutterRootInTempDir
	.childDirectory('packages')
	.childDirectory('flutter_tools')
	.childDirectory('lib')
	.childFile('executable.dart')
	.existsSync(),
	true,
	);

Excerpt from these logs:

22:45 +11 ~2 -1: test/integration.shard/batch_entrypoint_test.dart: flutter/bin/dart updates the Dart SDK without hanging [E]
  Expected: <true>
    Actual: <false>
  
  package:matcher                                         expect
  test\integration.shard\batch_entrypoint_test.dart 68:5  main.<fn>

One solution is to make batch_entrypoint_test.dart do a clone of the flutter/flutter repo into the temp folder instead of trying to copy it there. Is that acceptable, @christopherfujino? Or do you have another idea?

[christopherfujino]

I added some logging statements to #146229 and observed that the test times out on CI because a file is not getting copied successfully. Excerpt from these logs:

../../../../t/flutter_batch_entrypoint_test.d8ce6ed5/flutter/packages/flutter_tools/bin/flutter_tools.dart:5:8: Error: Error when reading '../../../../t/flutter_batch_entrypoint_test.d8ce6ed5/flutter/packages/flutter_tools/lib/executable.dart': The system cannot find the path specified.


import 'package:flutter_tools/executable.dart' as executable;
       ^
../../../../t/flutter_batch_entrypoint_test.d8ce6ed5/flutter/packages/flutter_tools/bin/flutter_tools.dart:8:14: Error: Method not found: 'main'.
  executable.main(args);
             ^^^^

Error: Unable to create dart snapshot for flutter tool.

I updated the PR with the following additional expectation to confirm this:

flutter/packages/flutter_tools/test/integration.shard/batch_entrypoint_test.dart

Lines 68 to 76 in 2bd2ef9

	expect(
	flutterRootInTempDir
	.childDirectory('packages')
	.childDirectory('flutter_tools')
	.childDirectory('lib')
	.childFile('executable.dart')
	.existsSync(),
	true,
	);

Excerpt from these logs:

22:45 +11 ~2 -1: test/integration.shard/batch_entrypoint_test.dart: flutter/bin/dart updates the Dart SDK without hanging [E]
  Expected: <true>
    Actual: <false>
  
  package:matcher                                         expect
  test\integration.shard\batch_entrypoint_test.dart 68:5  main.<fn>

One solution is to make batch_entrypoint_test.dart do a clone of the flutter/flutter repo into the temp folder instead of trying to copy it there. Is that acceptable, @christopherfujino? Or do you have another idea?

I'm taking a look at this now...

[christopherfujino]

So after investigating this issue and the proposed fix in #146229, here are my conclusions:

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.

It seems like the immediately implications of rolling frontend_server_client forward will be that--on windows--any user using flutter upgrade while a dart process spawned via frontend_server_client is running will experience a crash (of the flutter upgrade process), and likely not be able to download the new Dart SDK until the other dart process exits.

@elliette & @derekxu16 do you know if anything else in Flutter is using frontend_server_client other than testing?

[christopherfujino]

For context, I filed a more general tracking issue for how this will affect Flutter users: #146375

[derekxu16]

@elliette & @derekxu16 do you know if anything else in Flutter is using frontend_server_client other than testing?

I don't know of anything else in Flutter that uses package:frontend_server_client. This commit (1a44710) added some sites where flutter_tools launches processes that run the frontend_server AOT snapshot though. These processes currently never outlive the run of the flutter tool that starts them.

[derekxu16]

Fixed by #146650

[christopherfujino]

Fixed by #146650

Thanks!

[github-actions]

This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.