rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
98.23k stars 12.7k forks source link

Spurious "broken pipe" error messages, when used in typical UNIX shell pipelines #46016

Open infinity0 opened 6 years ago

infinity0 commented 6 years ago
$ cat yes.rs 
fn main() { loop { println!("y"); } }
$ rustc yes.rs && ./yes | head -n1
y
thread 'main' panicked at 'failed printing to stdout: Broken pipe (os error 32)', src/libstd/io/stdio.rs:692:8
note: Run with `RUST_BACKTRACE=1` for a backtrace.
$ yes | head -n1
y

This was originally filed here but @sfackler determined the cause:

This is due to println! panicking on errors: https://github.com/rust-lang/rust/blob/f1ea23e2cc72cafad1dc25a06c09ec2de8e323eb/src/libstd/io/stdio.rs#L671.

C-based programs typically just get killed off with a SIGPIPE, but Rust ignores that signal.

Note that to see the backtrace, the data being piped has to be large enough to overflow the kernel pipe buffer.

sfackler commented 6 years ago

We could provide a function in std::io to unignore SIGPIPE so applications could more easily opt-in to acting like a "standard" command line program.

infinity0 commented 6 years ago

Perhaps only the error message should be suppressed, it looks like the "traditional" programs do fail as a result of a broken pipe:

$ ./yes | head -n1
y
thread 'main' panicked at 'failed printing to stdout: Broken pipe (os error 32)', src/libstd/io/stdio.rs:692:8
note: Run with `RUST_BACKTRACE=1` for a backtrace.
$ echo "${PIPESTATUS[@]}"
101 0

$ yes | head -n1
y
$ echo "${PIPESTATUS[@]}"
141 0

$ find / | head -n1
/
$ echo "${PIPESTATUS[@]}"
141 0

141 seems to be the traditional exit code for a broken pipe.

sfackler commented 6 years ago

141 is the exit code set by the kernel after it has terminated a process due to a SIGPIPE.

coriolinus commented 6 years ago

We could provide a function in std::io to unignore SIGPIPE so applications could more easily opt-in to acting like a "standard" command line program.

I'm not sure what that API would look like: call a magic unignore_sigpipe() function and then your program just terminates on broken pipe, or a variant of the println!() family of macros, or what?

The former feels like it's just setting a global variable, which has a pretty bad smell. The latter means that unless you switch to using the new SIGPIPE-respecting macros throughout, your code might still generate the error.

What's not obvious to me is why Rust ignores that signal in the first place. I see that there's a test in place designed to ensure that the process shouldn't just crash, but at the same time the whole point of SIGPIPE is to terminate the receiving process silently. My intuition of correct behavior from Rust would be for it to do the same thing it does on SIGTERM: immediately, cleanly, and quietly shut itself down.

sfackler commented 6 years ago

call a magic unignore_sigpipe() function and then your program just terminates on broken pipe

That's what it would be presumably.

The former feels like it's just setting a global variable, which has a pretty bad smell.

Signal disposition is a process-global setting. Feel free to complain to the POSIX standards commitee about the smell of their global variables.

What's not obvious to me is why Rust ignores that signal in the first place.

SIGPIPE is a kind of hacky thing that only really makes sense when writing command line applications designed to be used in pipelines that only poke at their standard inputs and outputs. If you are writing anything more complex then it's something you need to turn off. Imagine a web server that crashed any time a client hung up, or a command line application that talks to the internet and crashed every time the server hung up.

coriolinus commented 6 years ago

Signal disposition is a process-global setting. Feel free to complain to the POSIX standards commitee about the smell of their global variables.

Haha, fair enough. I also do appreciate the explanation of the reasoning of turning it off by default. My own Rust applications tend to be unixy command-line applications which only ever really poke at their standard inputs and outputs, so that's the lens through which I view this issue, but I couldn't argue against the assertion that ignoring SIGPIPE is a more useful default.

In that case, I'd say that having an unignore_sigpipe function in the standard library somewhere would be an improvement on the current situation. Any idea how hard such a thing would be to implement?

sfackler commented 6 years ago

It'd just run this code: https://github.com/rust-lang/rust/blob/c284f8807eb3a1d728242bb6a767b0306d6f6bd5/src/libstd/sys/unix/process/process_unix.rs#L199-L222.

arthurprs commented 6 years ago

Found this out today when piping stdout to head.

Any ideas how to fix it nicely? Maybe we can take inspiration from other languages implementations.

coriolinus commented 6 years ago

Until someone implements and merges unignore_sigpipe(), your best bet will be to use the write!() macro instead of the print*!() family of macros, and then handle errors appropriately.

richardwhiuk commented 6 years ago

In the short term you can do:

extern crate libc;

...

    unsafe {
        libc::signal(libc::SIGPIPE, libc::SIG_DFL);
    }
jarcane commented 6 years ago

I seem to have run into this as well (see above issue), and I am finding the various linked and recommended solutions a bit vague. It's not clear to me how to assemble the bits and pieces to go about using it with write!, and I'd certainly rather avoid resorting to unsafe libc calls.

I was able to get a simple solution working with the try_print crate but using it on large streams brings back some old performance regressions I ran into on a previous issue, which was caused by excessive string allocations.

I must say it does seem a bit strange for my program to crash because of what another program downstream does or doesn't do with its inputs, but I would welcome any input on what a clear drop-in solution is that doesn't introduce any performance cost.

coriolinus commented 6 years ago

write! isn't hard: see here for one example of how to use it in production. Note that literally the only difference as far as the app is concerned is the gratuitous use of ? to handle potential errors.

jarcane commented 6 years ago

I was having trouble sorting out how to use write! with stdout, but I think your linked code should give me the hints I need. Thanks. :)

jyn514 commented 5 years ago

This affects the compiler itself:

$ cargo +nightly rustc -- -Zunpretty=hir-tree | head > /dev/null 
  Compiling project v0.1.0 (/home/joshua/Documents/Programming/rust/project)
thread 'main' panicked at 'failed printing to stdout: Broken pipe (os error 32)', src/libstd/io/stdio.rs:792:9
note: Run with `RUST_BACKTRACE=1` environment variable to display a backtrace.
pscott commented 4 years ago

We just disabled this in substrate. Instead of using libc we used nix.

For those in need for some help:

  1. Add this to your Cargo.toml file:
    [target.'cfg(target_family = "unix")'.dependencies]
    nix = "0.17.0"
  2. Create the reset function:

    /// This should be called before calling any cli method or printing any output.
    pub fn reset_signal_pipe_handler() -> Result<()> {
    #[cfg(target_family = "unix")]
    {
        use nix::sys::signal;
    
        unsafe {
            signal::signal(signal::Signal::SIGPIPE, signal::SigHandler::SigDfl)
                .map_err(|e| Error::Other(e.to_string()))?;
        }
    }
    
    Ok(())
    }
  3. Call this function at the start of your program.
mirao commented 3 years ago

Just got the same issue in this code, when playing with infinity in iterators:

fn main() {
    for number in 0.. {
        println!("{}", number);
    }
}

If you run cargo run|less and press "q", it fails:

thread 'main' panicked at 'failed printing to stdout: Broken pipe (os error 32)', library/std/src/io/stdio.rs:935:9
stack backtrace:
   0: rust_begin_unwind
             at /rustc/53cb7b09b00cbea8754ffb78e7e3cb521cb8af4b/library/std/src/panicking.rs:493:5
   1: std::panicking::begin_panic_fmt
             at /rustc/53cb7b09b00cbea8754ffb78e7e3cb521cb8af4b/library/std/src/panicking.rs:435:5
   2: std::io::stdio::print_to
             at /rustc/53cb7b09b00cbea8754ffb78e7e3cb521cb8af4b/library/std/src/io/stdio.rs:935:9
   3: std::io::stdio::_print
             at /rustc/53cb7b09b00cbea8754ffb78e7e3cb521cb8af4b/library/std/src/io/stdio.rs:947:5
   4: closures::main
             at ./src/main.rs:3:9
   5: core::ops::function::FnOnce::call_once
             at /home/mirao/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ops/function.rs:227:5

Used SW:

kolbma commented 2 years ago

Lol. A print which crashes your process, since 2017 known and documented wrongly, because it could write to stdout all the time and panics nevertheless.

Enselic commented 2 years ago

With nightly-2022-09-04 or later you can put #[unix_sigpipe = "sig_dfl"] on fn main() to prevent the panic:

#![feature(unix_sigpipe)]

#[unix_sigpipe = "sig_dfl"]
fn main() {
    loop {
        println!("hello world");
    }
}
% ./main | head -n 1
hello world

Tracking issue for unix_sigpipe: https://github.com/rust-lang/rust/issues/97889

Also see: https://github.com/rust-lang/rust/issues/62569

juancampa commented 2 years ago

The way I've worked around this is by replacing the print/println/eprint/etc macros with fallible versions. I just put this at the top of main.rs

// These macros are needed because the normal ones panic when there's a broken pipe.
// This is especially problematic for CLI tools that are frequently piped into `head` or `grep -q`
macro_rules! println {
  () => (print!("\n"));
  ($fmt:expr) => ({
    writeln!(std::io::stdout(), $fmt)
  });
  ($fmt:expr, $($arg:tt)*) => ({
    writeln!(std::io::stdout(), $fmt, $($arg)*)
  })
}

macro_rules! print {
  () => (print!("\n"));
  ($fmt:expr) => ({
    write!(std::io::stdout(), $fmt)
  });
  ($fmt:expr, $($arg:tt)*) => ({
    write!(std::io::stdout(), $fmt, $($arg)*)
  })
}

And then handle the std::io::Error appropriately:

fn main() -> ExitCode {
  match run() {
    Err(Error::IOError(err)) if err.kind() == io::ErrorKind::BrokenPipe => {
      // Okay, this happens when the output is piped to a program like `head`
      ExitCode::SUCCESS
    }
    Err(err) => {
      eprintln!("{}", err).ok();
      ExitCode::FAILURE
    }
    Ok(_) => ExitCode::SUCCESS,
  }
}