Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

"flutter pub get" does not terminate when killed #61357

Open
Tracked by #130635
DanTup opened this issue Jul 13, 2020 · 5 comments
Open
Tracked by #130635

"flutter pub get" does not terminate when killed #61357

DanTup opened this issue Jul 13, 2020 · 5 comments
Labels
P2 Important issues not at the top of the work list team-tool Owned by Flutter Tool team tool Affects the "flutter" command-line tool. See also t: labels. triaged-tool Triaged by Flutter Tool team

Comments

@DanTup
Copy link
Contributor

DanTup commented Jul 13, 2020

This came up in Dart-Code/Dart-Code#2612. If you run flutter pub get and try to kill it programatically (similar to how Dart-Code would if you clicked the Cancel button on the dialog), the process doesn't terminate.

To repro, you can use this script which creates a pubspec.yaml with an invalid package name, which puts Flutter into an endless loop of retries (which is discussed in dart-lang/pub#2242), and then tries to kill it - however it does not stop.

I repro'd this using latest Flutter master today.

import 'dart:io';

import 'package:path/path.dart' as path;

Future<void> main() async {
  // Create a temp project
  final projectFolder = Directory(path.join(Directory.systemTemp.path, 'flutter_pub_test'))
        ..createSync(recursive: true);

  // Write a pubspec with a bad package name
  File(path.join(projectFolder.path, 'pubspec.yaml'))
    ..writeAsStringSync('name: sample\n'
        "environment:\n  sdk: '>=2.10.0 <3.0.0'\n"
        'dependencies:\n  not_a_real_package:');

  // Run "flutter pub get" and print any stdout/stderr output
  final proc = await Process.start(
      'flutter', ['pub', 'get'],
      workingDirectory: projectFolder.path,
  );
  proc.stdout.listen((s) => print(String.fromCharCodes(s)));
  proc.stderr.listen((s) => print(String.fromCharCodes(s)));

  // Kill the process and wait for it to end
  await Future.delayed(Duration(seconds: 2));
  print('Killing process!');
  proc.kill();

  print('Waiting for exit!');
  await proc.exitCode;
}

Output:

Running "flutter pub get" in flutter_pub_test...

Because sample depends on not_a_real_package any which doesn't exist (could not find package not_a_real_package at https://pub.dartlang.org), version solving failed.

pub get failed (server unavailable) -- attempting retry 1 in 1 second...
Killing process!
Waiting for exit!

Because sample depends on not_a_real_package any which doesn't exist (could not find package not_a_real_package at https://pub.dartlang.org), version solving failed.

pub get failed (server unavailable) -- attempting retry 2 in 2 seconds...


Because sample depends on not_a_real_package any which doesn't exist (could not find package not_a_real_package at https://pub.dartlang.org), version solving failed.

pub get failed (server unavailable) -- attempting retry 3 in 4 seconds...

Because sample depends on not_a_real_package any which doesn't exist (could not find package not_a_real_package at https://pub.dartlang.org), version solving failed.

pub get failed (server unavailable) -- attempting retry 4 in 8 seconds...

// etc.

This results in orphaned processes being left around forever if the user ever saves pubspec with an invalid package name. I thought it might have been related to dart-lang/sdk#42092, however that seems to have been included in this version of Flutter but it's still not fixed.

(it's very possible this is a similar issue to we have elsewhere, where sending kill signals to a shell-executed process is unreliable.. we get around this in may places by emitting some JSON with the pid and adding that to the list of things to terminate - however pub get doesn't have a machine mode to add that too).

@DanTup DanTup added the tool Affects the "flutter" command-line tool. See also t: labels. label Jul 13, 2020
@jonahwilliams jonahwilliams added this to Awaiting triage in Tools - Dart and pub review via automation Jul 13, 2020
@jonahwilliams jonahwilliams moved this from Awaiting triage to Engineer reviewed in Tools - Dart and pub review Jul 13, 2020
@jason-simmons
Copy link
Member

The proc.kill() call will kill the bin/flutter shell script process but not the dart flutter_tools.snapshot pub get process that it spawned.

One option would be changing the bin/flutter script to trap the SIGTERM sent by proc.kill and kill the underlying dart process.

@DanTup
Copy link
Contributor Author

DanTup commented Jul 14, 2020

One option would be changing the bin/flutter script to trap the SIGTERM sent by proc.kill and kill the underlying dart process.

We'd probably need to ensure we can do this on Windows with flutter.bat too?

Another option could be to have --machine flag that just outputs a JSON banner at the top with the pid in it. We already do this in many places (for ex. flutter run --machine) but perhaps this could be more general. (It's possible we could achieve the same with --pid-file, though the pids in startup JSON is fairly consistent across Flutter/package:test/etc. now).

@DanTup
Copy link
Contributor Author

DanTup commented Aug 4, 2020

Looks like there isn't a --pid-file option we can use here anyway. So as things are, I don't think there's any workaround, so no way an editor can cancel a pub get at all. This is quite bad given how easily you can put it in an infinite loop:

Because my_ads_test depends on notreal any which doesn't exist (could not find package notreal at https://pub.dartlang.org), version solving failed.
pub get failed (server unavailable) -- attempting retry 1 in 1 second...
Because my_ads_test depends on notreal any which doesn't exist (could not find package notreal at https://pub.dartlang.org), version solving failed.
pub get failed (server unavailable) -- attempting retry 2 in 2 seconds...
Because my_ads_test depends on notreal any which doesn't exist (could not find package notreal at https://pub.dartlang.org), version solving failed.
pub get failed (server unavailable) -- attempting retry 3 in 4 seconds...
Because my_ads_test depends on notreal any which doesn't exist (could not find package notreal at https://pub.dartlang.org), version solving failed.
pub get failed (server unavailable) -- attempting retry 4 in 8 seconds...
Because my_ads_test depends on notreal any which doesn't exist (could not find package notreal at https://pub.dartlang.org), version solving failed.
pub get failed (server unavailable) -- attempting retry 5 in 16 seconds...
Because my_ads_test depends on notreal any which doesn't exist (could not find package notreal at https://pub.dartlang.org), version solving failed.
pub get failed (server unavailable) -- attempting retry 6 in 32 seconds...

Some other possible ideas:

  • Limit the number of retries (editors could pass something low like 3?)
  • Accept JSON on stdin to let editors send commands to cancel (perhaps gated on a --machine mode flag)

@maheshmnj
Copy link
Member

Hi @DanTup, I tried running your code and I see the process does get killed. But the output seems to give a misleading message though.

You should edit pubspec.yaml to contain an SDK constraint:

when my pubspec has a constraint

environment:
  sdk: ">=2.16.2 <3.0.0"

logs

Running "flutter pub get" in flutter_pub_test...                


pubspec.yaml has no lower-bound SDK constraint.

You should edit pubspec.yaml to contain an SDK constraint:


environment:
  sdk: '>=2.10.0 <3.0.0'

See https://dart.dev/go/sdk-constraint

pub get failed (65; See https://dart.dev/go/sdk-constraint)

Killing process!
Waiting for exit!
flutter doctor -v (mac)
[✓] Flutter (Channel stable, 3.0.5, on macOS 12.4 21F79 darwin-arm, locale en-IN)
    • Flutter version 3.0.5 at /Users/mahesh/Documents/flutter
    • Upstream repository https://github.com/flutter/flutter.git
    • Framework revision f1875d570e (5 days ago), 2022-07-13 11:24:16 -0700
    • Engine revision e85ea0e79c
    • Dart version 2.17.6
    • DevTools version 2.12.2

[✓] Android toolchain - develop for Android devices (Android SDK version 33.0.0-rc4)
    • Android SDK at /Users/mahesh/Library/Android/sdk
    • Platform android-32, build-tools 33.0.0-rc4
    • ANDROID_HOME = /Users/mahesh/Library/Android/sdk
    • Java binary at: /Applications/Android Studio.app/Contents/jre/Contents/Home/bin/java
    • Java version OpenJDK Runtime Environment (build 11.0.12+0-b1504.28-7817840)
    • All Android licenses accepted.

[✓] Xcode - develop for iOS and macOS (Xcode 13.2.1)
    • Xcode at /Applications/Xcode.app/Contents/Developer
    • CocoaPods version 1.11.2

[✓] Chrome - develop for the web
    • Chrome at /Applications/Google Chrome.app/Contents/MacOS/Google Chrome

[✓] Android Studio (version 2021.2)
    • Android Studio at /Applications/Android Studio.app/Contents
    • Flutter plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/9212-flutter
    • Dart plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/6351-dart
    • Java version OpenJDK Runtime Environment (build 11.0.12+0-b1504.28-7817840)

[✓] IntelliJ IDEA Community Edition (version 2021.2.1)
    • IntelliJ at /Applications/IntelliJ IDEA CE.app
    • Flutter plugin version 61.2.4
    • Dart plugin version 212.5080.8

[✓] VS Code (version 1.67.2)
    • VS Code at /Applications/Visual Studio Code.app/Contents
    • Flutter extension version 3.44.0

[✓] Connected device (3 available)
    • sdk gphone arm64 (mobile) • emulator-5554 • android-arm64  • Android 11 (API 30) (emulator)
    • macOS (desktop)           • macos         • darwin-arm64   • macOS 12.4 21F79 darwin-arm
    • Chrome (web)              • chrome        • web-javascript • Google Chrome 103.0.5060.114

[✓] HTTP Host Availability
    • All required HTTP hosts are available

• No issues found!
[✓] Flutter (Channel master, 3.1.0-0.0.pre.1868, on macOS 12.4 21F79 darwin-arm, locale en-IN)
    • Flutter version 3.1.0-0.0.pre.1868 on channel master at /Users/mahesh/Documents/flutter_master
    • Upstream repository https://github.com/flutter/flutter.git
    • Framework revision 925bee92e7 (3 days ago), 2022-07-26 04:49:06 -0400
    • Engine revision 5dcaeae656
    • Dart version 2.19.0 (build 2.19.0-34.0.dev)
    • DevTools version 2.15.0

[✓] Android toolchain - develop for Android devices (Android SDK version 33.0.0-rc4)
    • Android SDK at /Users/mahesh/Library/Android/sdk
    • Platform android-32, build-tools 33.0.0-rc4
    • ANDROID_HOME = /Users/mahesh/Library/Android/sdk
    • Java binary at: /Applications/Android Studio.app/Contents/jre/Contents/Home/bin/java
    • Java version OpenJDK Runtime Environment (build 11.0.12+0-b1504.28-7817840)
    • All Android licenses accepted.

[✓] Xcode - develop for iOS and macOS (Xcode 13.2.1)
    • Xcode at /Applications/Xcode.app/Contents/Developer
    • Build 13C100
    • CocoaPods version 1.11.2

[✓] Chrome - develop for the web
    • Chrome at /Applications/Google Chrome.app/Contents/MacOS/Google Chrome

[✓] Android Studio (version 2021.2)
    • Android Studio at /Applications/Android Studio.app/Contents
    • Flutter plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/9212-flutter
    • Dart plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/6351-dart
    • Java version OpenJDK Runtime Environment (build 11.0.12+0-b1504.28-7817840)

[✓] IntelliJ IDEA Community Edition (version 2021.2.1)
    • IntelliJ at /Applications/IntelliJ IDEA CE.app
    • Flutter plugin version 61.2.4
    • Dart plugin version 212.5080.8

[✓] VS Code (version 1.69.2)
    • VS Code at /Applications/Visual Studio Code.app/Contents
    • Flutter extension version 3.44.0

[✓] Connected device (2 available)
    • macOS (desktop) • macos  • darwin-arm64   • macOS 12.4 21F79 darwin-arm
    • Chrome (web)    • chrome • web-javascript • Google Chrome 103.0.5060.134

[✓] HTTP Host Availability
    • All required HTTP hosts are available

• No issues found!

@DanTup
Copy link
Contributor Author

DanTup commented Jul 30, 2022

@maheshmnj I also can't repro on macOS any more. However, I can still repro on Windows. You need to change 'flutter' to 'flutter.bat' in the Process.spawn line (I've also tweaked the code to include an environment field in the pubspec or it would fail to start fetching the packages).

image

Edit: Confirmed same behaviour on both stable and master.

@flutter-triage-bot flutter-triage-bot bot added the team-tool Owned by Flutter Tool team label Jul 8, 2023
@eliasyishak eliasyishak added P2 Important issues not at the top of the work list triaged-tool Triaged by Flutter Tool team labels Oct 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2 Important issues not at the top of the work list team-tool Owned by Flutter Tool team tool Affects the "flutter" command-line tool. See also t: labels. triaged-tool Triaged by Flutter Tool team
Projects
Tools - Dart and pub review
  
Engineer reviewed
Development

No branches or pull requests

6 participants