Adding package http makes dart run really slow #55289
Comments
|
Reproducible with local build (3.4.0-266.0.dev) but only when the package was ran with 255.0 (current flutter main) before. |
|
So this is quite weird. I have the same SDK revision (567a19a), when built locally as part of flutter engine (profile/arm64) I don't get the problem. But when using prebuilt SDK shipped with Flutter (same revision), I get the issue. |
|
Actually, I get the issue when I copy (copy, not just symlink) the |
|
Here's complete stacktrace Seems to be stuck here for about 15 seconds. I don't quite understand why this only happens when I copy the entire sdk to |
|
Is there any logging that can be obtained when starting |
|
So DartDev seems to be sending this almost instantly to the VM: |
|
So the main thread is stuck at It seems like the monitor gets notified immediately but |
|
So dartdev main method finishes, but there are still some events pending which prevent the runloop from stopping. I'm guessing related to network I/O These are the messages received by RunLoop after the delay (assuming a timeout somewhere). |
|
Okay, so the problem is a 15 second idle timeout applied because of keepAlive connection in pub http client. dartdev should probably close the To confirm the above modifying import 'package:pub/src/http.dart';
/// The entry point for dartdev.
Future<void> main(List<String> args, SendPort? port) async {
await runDartdev(args, port);
globalHttpClient.close();
}
... |
|
Impressive sleuthing! Sounds very probable. I've had similar unexpected "hangs" at exit of my own scripts that use If it's only the script itself that uses an http client, then the script should close its own client. (Check if that stoned your problem, if it's your own program which keeps the Dart process alive after both it and dartdev have ended.) If the script using an http client makes dartdev wait, then we should worry about whether it can be affected by the rubbing program in other ways. Meaning to be - very aware of any process-wide properties that are used after the user program has run. |
lrhn
added
the
area-dart-cli
|
To elaborate,
Not sure why this manifests now, maybe the client before had Also maybe it's not clear from the description. This all happens before any of my code even starts getting executed. It holds the |
|
I'll second lrhn's assessment: excellent debugging! I'm able to reproduce, albeit in a different fashion:
Resolving dependencies in /private/tmp/test_1...
Because test_1 depends on http 10.0.0 which doesn't match any versions, version solving failed.
You can try the following suggestion to make the pubspec resolve:
* Consider downgrading your constraint on http: dart pub add http:^1.2.1
real 0m15.955s
user 0m1.088s
sys 0m0.227sI can confirm that adding the patch from #55289 (comment) fixes the issue. @sigurdm @jonasfj, are we okay with reaching into |
|
Ohh what an oversight. I think we should better close the client inside pub - here: I'll make a PR. |
|
Thanks for the fix! I think this can be closed. |
|
I'll close it when it is rolled to the sdk |
|
Roll is here: https://dart-review.googlesource.com/c/sdk/+/359480 |
|
Thanks for the quick fix @sigurdm! |
Changes: ``` > git log --format="%C(auto) %h %s" 4a0cd04..3f0df78 https://dart.googlesource.com/pub.git/+/3f0df784 Close the global http client after `ensurePubspecResolved` (4199) https://dart.googlesource.com/pub.git/+/9532f0a8 Let add and remove act upon the work pubspec (4196) https://dart.googlesource.com/pub.git/+/2ce3da14 Remove stray lockfiles and packageconfigs from workspace (4194) https://dart.googlesource.com/pub.git/+/a44e2e51 Tests of workspace error handling (4195) https://dart.googlesource.com/pub.git/+/2179b765 Resolve workspace root and workPackage when invoking pub from any sub-directory (4186) https://dart.googlesource.com/pub.git/+/7a668d12 Update repository specification with info about the 'pub_display_url' field (4193) https://dart.googlesource.com/pub.git/+/cf9ba6c5 Use local pub in binstubs when testing (4192) https://dart.googlesource.com/pub.git/+/0b8a261e Import package:path `as p` everywhere (4187) https://dart.googlesource.com/pub.git/+/2a7c0e92 Update repository specification with info about the 'affected[].versions' fields in advisories (4191) ``` Diff: https://dart.googlesource.com/pub.git/+/4a0cd0403f70382feca9e17ae8854ffbef0fee98..3f0df78417f7c112b933fbcdc1c5c87bde680cb1/ Bug: #55289 Change-Id: Ifc2e6280cfbb83b463dd197b463d6841f0528e3e Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/359480 Reviewed-by: Jonas Jensen <[email protected]> Commit-Queue: Sigurd Meldgaard <[email protected]>
|
Seems to be rolled in main. Should this be cherry-picked to stable? |
|
I can still reproduce this with |
|
@sigurdm, @jonasfj, it doesn't seem like |
Oh - my mistake Fixing the wrong place. I think @jonasfj is right in. We should in general not really rely on a global http client, but instead pass it in in the public api and close it outside. Will try to make a proper fix. |
|
Once we get a fix landed we should consider requesting a cherry pick so this goes out in the next beta/stable. |
|
Sorry - didn't get to this today. Hopefully tomorrow. |
|
Discussed this with @jonasfj, and we decided doing it in DartDev as #55289 (comment) is an ok hack for now. Also easier for creating a limited cherry pick. Opened an issue for doing a refactor later. Will create a PR. |
|
https://dart-review.googlesource.com/c/sdk/+/361364 Will create a cherry pick to beta after the weekend. |
Bug: #55289 Change-Id: I61548d0b223167b769a27ac6ab956c4fd42a7be6 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/361364 Reviewed-by: Ben Konyi <[email protected]> Reviewed-by: Jonas Jensen <[email protected]> Commit-Queue: Sigurd Meldgaard <[email protected]>
|
Cherry pick landed |
Bug: #55289 Cherry-pick: https://dart-review.googlesource.com/c/sdk/+/361364 Cherry-pick: https://dart-review.googlesource.com/c/sdk/+/362360 Cherry-pick-request: #55402 Change-Id: I61548d0b223167b769a27ac6ab956c4fd42a7be6 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/361562 Reviewed-by: Ben Konyi <[email protected]> Commit-Queue: Sigurd Meldgaard <[email protected]>
I'm not sure if this is the right project to report this.
Dart SDK version: 3.4.0-265.0.dev (dev) (Sat Mar 23 01:03:42 2024 -0700) on "macos_arm64"
Steps to reproduce
outputs
add package
httpoutputs
Not sure why this takes 15 seconds. It seems that the main thread is waiting for something
The dart build shipped with Flutter has stripped symbols so it doesn't help much. I'll try running this with custom engine build.
The text was updated successfully, but these errors were encountered: