Skip to content

rustup probabilistically panics with "Broken pipe" when used in a shell pipeline #1730

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

Closed
ssokolow opened this issue Mar 28, 2019 · 3 comments · Fixed by #1765
Closed

rustup probabilistically panics with "Broken pipe" when used in a shell pipeline #1730

ssokolow opened this issue Mar 28, 2019 · 3 comments · Fixed by #1765
Labels
Milestone

Comments

@ssokolow
Copy link

Problem

When I run the install-rustup-deps task in the justfile for my rust-cli-boilerplate repo, there's about a 50/50 chance that one of the several calls to rustup that it makes will panic with the following output:

thread 'main' panicked at 'failed printing to stdout: Broken pipe (os error 32)', src/libstd/io/stdio.rs:690:9
stack backtrace:
   0: std::sys::unix::backtrace::tracing::imp::unwind_backtrace
             at src/libstd/sys/unix/backtrace/tracing/gcc_s.rs:39
   1: std::panicking::default_hook::{{closure}}
             at src/libstd/sys_common/backtrace.rs:70
             at src/libstd/sys_common/backtrace.rs:58
             at src/libstd/panicking.rs:200
   2: std::panicking::rust_panic_with_hook
             at src/libstd/panicking.rs:215
             at src/libstd/panicking.rs:478
   3: std::panicking::continue_panic_fmt
             at src/libstd/panicking.rs:385
   4: std::panicking::begin_panic_fmt
             at src/libstd/panicking.rs:340
   5: std::io::stdio::_print
             at src/libstd/io/stdio.rs:690
             at src/libstd/io/stdio.rs:699
   6: rustup_init::rustup_mode::main
   7: rustup_init::run_rustup
   8: rustup_init::main
   9: std::rt::lang_start::{{closure}}
  10: main
  11: __libc_start_main
  12: <unknown>

I've been experimenting with porting the repo to cargo-make, so I've confirmed that the issue also occurs under cargo-make, and I pared it down to a small test shell script which also exhibits the bug.

Steps

  1. Save the following shell script and mark it executable
  2. Run it a few times
#!/bin/sh
rustup toolchain list | grep -q stable || rustup toolchain install stable
rustup toolchain list | grep -q nightly || rustup toolchain install nightly
rustup target list | grep -q 'i686-unknown-linux-musl (' || rustup target add 'i686-unknown-linux-musl'
rustup component list | grep -q 'clippy-\S* (' || rustup component add clippy
rustup component list --toolchain nightly | grep -q 'rustfmt-\S* (' || rustup component add rustfmt --toolchain nightly

You should notice that the command which panics varies from run to run and, sometimes, it completes without panicking at all.

Possible Solution(s)

In the case of a broken pipe, exit without an error message (like other shell commands) and with a success exit code (because I haven't run across any other tools which cause cargo-make to abort with spurious failures when used in this way.)

Notes

Output of rustup --version: rustup 1.17.0 (069c88ed6 2019-03-05)
Output of rustup show:

Default host: x86_64-unknown-linux-gnu

installed toolchains
--------------------

stable-i686-pc-windows-gnu
stable-x86_64-pc-windows-gnu
stable-x86_64-unknown-linux-gnu (default)
nightly-2017-12-20-x86_64-unknown-linux-gnu
nightly-x86_64-unknown-linux-gnu

installed targets for active toolchain
--------------------------------------

arm-unknown-linux-gnueabi
i686-pc-windows-gnu
i686-unknown-linux-musl
x86_64-pc-windows-gnu
x86_64-unknown-linux-gnu
x86_64-unknown-linux-musl

active toolchain
----------------

stable-x86_64-unknown-linux-gnu (default)
rustc 1.33.0 (2aa4c46cf 2019-02-28)
@ssokolow ssokolow added the bug label Mar 28, 2019
@kinnison
Copy link
Contributor

Hi, we have started to address this issue - so far we've merged #1622 which deals with this for rustup show I believe. Would you be able to provide a PR to extend that change to the other commands you are trying to process?

As a word of warning though, we do not make any guarantees as to the long-term reliability of such parsing, though we will be aiming to introduce a JSON dump of rustup state at some point as a start of plumbing commands.

@ssokolow
Copy link
Author

Would you be able to provide a PR to extend that change to the other commands you are trying to process?

Unfortunately, I have enough already on my plate that I don't really have time to get up to speed with other people's codebases right now.

As a word of warning though, we do not make any guarantees as to the long-term reliability of such parsing, though we will be aiming to introduce a JSON dump of rustup state at some point as a start of plumbing commands.

I didn't expect otherwise. It's for that exact reason that I've tried to make as few assumptions as possible and, if the assumptions I do make are broken, the failure state is annoying but should be harmless.

@kinnison
Copy link
Contributor

Short term, you could write the output to a temporary file which you then parse. That'd enable you to continue until we have something more supportable for plumbing.

@kinnison kinnison added this to the 1.18.0 milestone Apr 14, 2019
kinnison added a commit to kinnison/rustup that referenced this issue Apr 14, 2019
In order to be easier for scripts to use us, until we define a
proper CLI API for that, handle EPIPE situations on all the list
type subcommands as well as the show subcommands.

Fixes rust-lang#1730

Signed-off-by: Daniel Silverstone <[email protected]>
fnichol added a commit to fnichol/workstation that referenced this issue Apr 23, 2019
When checking for an installing plugins, there isn't a need to call
cargo for each plugin to check. While this is just-as-good as previous
behavior, it also limits any issues when calling a Cargo subcommand that
might fail.

Presently, there are times where a race exists and either `STDOUT` or
`STDERR` might disppear when Rustup (or potentially Cargo) is used in a
shell pipeline, leading to a program panic with `failed printing to
stdout: Broken pipe (os error 32)`. This change tries to minimize the
probability that this occurs until a more robust upstream fix has
landed.

References: rust-lang/rustup#1730
References: rust-lang/rustup#1765

Signed-off-by: Fletcher Nichol <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants