dart-lang / sdk

The Dart SDK, including the VM, JS and Wasm compilers, analysis, core libraries, and more.
https://dart.dev
BSD 3-Clause "New" or "Revised" License
10.09k stars 1.56k forks source link

Dart CLI commands hang on a slow/unstable internet connection due to analytics #56106

Open matanlurey opened 2 months ago

matanlurey commented 2 months ago

To reproduce, intentionally make your internet suck or be unstable. Try a subway or something :)

Then, any command that sends analytics, i.e. dart run or dart test hangs for a period of time.

To "fix", disable analytics (which ironically itself hangs for that same period):

dart --disable-analytics

Analytics collection should run "out of band", that is, it should not be blocking (I would not use await). I might go an additional step further and say that perhaps the actual mechanism that sends the analytics to a server should happen elsewhere, i.e. I should never have this happen:

dart run empty_file_with_void_main.dart
# ... script clearly executes, but 'dart' appears to hang for 10-30s
matanlurey commented 2 months ago

I might check with the flutter CLI folks to see how they handle it.

dart-github-bot commented 2 months ago

Summary: Dart CLI commands, like dart run and dart test, hang when internet connectivity is slow or unstable due to analytics collection blocking the execution flow. This issue can be resolved by disabling analytics, but the process itself is also affected by the network latency.

devoncarew commented 2 months ago

In our previous implementation of analytics it was possible to get a future for the last item sent, wait on that for a brief time w/ a 100ms-200ms timeout, and exit the process if there was a delay in sending (to not hang the VM exit waiting for analytics). We've since re-written the analytics implementation; I'm not sure who's responsible for the integration into the dart cli, but @bkonyi would be the best person to start with.

bkonyi commented 2 months ago

In our previous implementation of analytics it was possible to get a future for the last item sent, wait on that for a brief time w/ a 100ms-200ms timeout, and exit the process if there was a delay in sending (to not hang the VM exit waiting for analytics). We've since re-written the analytics implementation; I'm not sure who's responsible for the integration into the dart cli, but @bkonyi would be the best person to start with.

Yeah, I'm responsible for the CLI. I thought Elias had done some work to account for this (I know the legacy analytics timeout behavior was brought up), but maybe this never ended up happening.

christopherfujino commented 2 months ago

So this call: https://github.com/dart-lang/sdk/blob/main/pkg/dartdev/lib/dartdev.dart#L262

Should default to timing out after 250ms: https://github.com/dart-lang/tools/blob/main/pkgs/unified_analytics/lib/src/analytics.dart#L548

I wonder if there is some other code path by which we don't run this finally? Or some other async call to unified_analytics is hanging, before we get to the finally?

christopherfujino commented 2 months ago

Hmm, when we timeout, we call _gaClient.close();, but I'm thinking maybe that doesn't cancel our pending requests?

https://github.com/dart-lang/tools/blob/main/pkgs/unified_analytics/lib/src/analytics.dart#L554

matanlurey commented 2 months ago

If I trace the code, it eventually gets to <IOClient>.close(), which does a .close(force: true), which should cancel pending requests.

christopherfujino commented 2 months ago

If I trace the code, it eventually gets to <IOClient>.close(), which does a .close(force: true), which should cancel pending requests.

Ok, that makes sense. Also, assuming the flutter cli tool is not susceptible to this bug, then presumably Analytics.close() is closing these connections.

zer09 commented 1 month ago

It also happen to me on a newly created project, it consistently freeze for 20 seconds before exiting when analytics is enable.

image

CC @DanTup ref: https://github.com/Dart-Code/Dart-Code/issues/3270