Repository navigation
[flutter_tools] pin frontend_server_client because upgrading from 3.2.0 -> 4.0.0 regresses test #146164
Description
Activity
- addedP1High-priority issues at the top of the work listHigh-priority issues at the top of the work list
on Apr 2, 2024 christopherfujino commented
on Apr 2, 2024 ContributorAuthorMore actionsI ran this locally and added debug prints on the stdout and stderr of the
dart.batinvocation:[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 ...The timeout was happening for me locally because
batch_entrypoint_test.darttries to renamefrontend_server_aot.dart.snapshot, but this happens whenfrontend_server_aot.dart.snapshotis actively in use, because it is used by the test runner, which makesRename-Itemfail. 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.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.dartdo 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 commented
on Apr 5, 2024 ContributorAuthorMore actionsI 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.dartdo 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 commented
on Apr 6, 2024 ContributorAuthorMore actionsSo after investigating this issue and the proposed fix in #146229, here are my conclusions:
- The disk i/o implementations of
dart.exe foo.appjit.snapshotanddartaotruntime.exe foo.aot.snapshotare 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 callingRename-Itemon the snapshot file--the rename succeeded on the appjit snapshot but failed on aot snapshot withRename-Item : The process cannot access the file because it is being used by another process.. - version 4.0.0 of
frontend_server_clientis 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. - 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-Itemon the existing (or old) Dart SDK to move it to a path withdart-sdk.oldin the name. I believe we do this because just deleting the old one while the tool is running would fail. - 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.
- 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 upgradewould 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_clientforward will be that--on windows--any user usingflutter upgradewhile a dart process spawned viafrontend_server_clientis 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?
- The disk i/o implementations of
christopherfujino commented
on Apr 6, 2024 ContributorAuthorMore actionsFor context, I filed a more general tracking issue for how this will affect Flutter users: #146375
@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 whereflutter_toolslaunches processes that run thefrontend_serverAOT snapshot though. These processes currently never outlive the run of thefluttertool that starts them.Fixed by #146650
christopherfujino commented
on Apr 17, 2024 ContributorAuthorMore actionsFixed by #146650
Thanks!
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 -vand a minimal reproduction of the issue.- locked as resolved and limited conversation to collaborators
on May 1, 2024
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