Rust: Should Rust still ignore SIGPIPE by default?

Created on 10 Jul 2019  路  18Comments  路  Source: rust-lang/rust

Back in 2014, the Rust startup code (which runs before main) started ignoring SIGPIPE by default: https://github.com/rust-lang/rust/pull/13158

This dates back to when Rust had a runtime and a green-threads implementation, and more magic handling of I/O. The pull request mentions that it "brings the behavior inline with libgreen".

This doesn't seem to be documented anywhere, as far as I can tell; at a minimum, this needs documentation.

But I'd like to question whether we should still do this by default. See https://github.com/rust-lang/rust/issues/46016 for some recent discussion of this. This changes the default behavior of pipes from what people might expect when writing UNIX applications. Rust currently resets the signal mask in a child process, but that's only if you use Rust to launch the child process rather than calling a library to do so. And in addition to that, signal handlers are process-wide, and people might not expect Rust to change process-wide state like this. This also means that Rust libraries will not have the same behavior as Rust applications.

Either way, this is something we need to document far better. Some people will want one behavior, and some people will want the other, so whichever we use as the default needs careful documentation and a documented procedure to change it.

EDIT (based on a suggestion from @Centril): Perhaps we could control this with an attribute on main or on the top-level module?

A-runtime T-lang T-libs

Most helpful comment

This seems like a problem that could be fixed with the next edition? That would justify the use of an attribute to disable/enable this feature, in that upgrading binary crates to the new edition would need to add the attribute to re-enable this feature.

All 18 comments

I'm not sure if the @rust-lang/libs team is the right team for this; suggestions welcome if another team would be the right owner for "behavior of Rust at application start-up".

If we stop ignoring SIGPIPE by default, we will break back compat for every socket server written in Rust, right?

Perhaps we should have a build-time option for this to make it easy to disable/enable in an application?

Why this and e.g. not an attribute on fn main() {? (As a general rule, I think having program semantics defined as much as possible by the source code is preferable to flags since it makes things more standalone.)

@sfackler Right now, we have inconsistent behavior, where your socket server will also break if you ever write main() in any language other than Rust, and turn your code into a library. That flies in the face of one of Rust's greatest strengths: "we don't have a runtime". I discovered this when looking over the pre-main startup code.

At the very least, we need to document that we ignore SIGPIPE, and we need to provide an easy way to disable this behavior. I'd also fully expect that any change to the default itself would require a sizable transition period (or even an edition).

@Centril Good point! An attribute within the source code (on main or the top-level module) does indeed seem preferable, rather than a build-time option.

I agree that we should do something about this. I don't think we can change the current default behavior because of backcompat concerns, as @sfackler mentioned, but the current status quo is that nominal command line applications start out of the gate as broken, and it's a fairly subtle issue to fix. Firstly, if you're using println! and have enough output to exceed stdout's default buffer size, then it's likely that running, for example, command | head -n1 will result in a panic. Secondly, if you're using writeln!(...)? instead, and are setting an error code when writeln!(...) fails, then it's likely that command | head -n1 will stop with an unexpected exit code.

For example, normally a process will exit with a 141 exit code upon receipt of a pipe error:

$ seq 1 10000 | head -n1
1
$ echo ${pipestatus[1]}
141

So to fix this, you firstly need to know enough to use writeln!(...), and then you either need to assume all errors produced by writeln!(...) are broken pipe errors (and emit the correct exit code), or explicitly check errors for io::ErrorKind::BrokenPipe and respond accordingly. The former is, strictly speaking, incorrect, even though it's likely the only error you'll encounter when writing to stdout is a broken pipe error (but the code using write! may not _know_ that it is writing to stdout). The latter works, but it's tedious, and even I get it wrong:

$ rg . rust/ | head -n1
rust/bstr/rustfmt.toml:disable_all_formatting = true
$ echo ${pipestatus[1]}
0

This, presumably, should be 141. But at least it doesn't panic.

So yeah, on the one hand, ignoring SIGPIPE by default means _pure Rust_ networked applications probably have the right behavior by default, but _not_ ignoring SIGPIPE by default means command line applications probably have the wrong behavior by default. Both of these groups seem fairly sizable to me.

I don't mind the idea of some incantation one can put in the source code to fix this. An alternative might be to just provide a function in std, if it's possible, that un-ignores SIGPIPE and is intended to run at the beginning of main. If that's workable, then that might be preferrable since it adds fewer directives to the language itself. The other issue here is documentation and discoverability. i.e., How can we help folks writing CLI tools discover, in the first place, that they need to un-ignore SIGPIPE?

(Although, I guess adding a function to std and requiring users to call it doesn't fix @joshtriplett's case, where Rust is a library. Am I understanding that right?)

I personally agree with @sfackler that I don't think we can really change any defaults due to backwards-compatibility reasons. AFAIK this really only affects one thing which is unix-y CLI applications. Any portable CLI application to Windows cannot rely on the existence of SIGPIPE and almost all non-CLI applications I imagine want to receive errors instead of SIGPIPE to handle. That feels, to me, niche enough that we made the defaults the right way and there should be documentation/crates for authors writing unix-y CLI applications (aka ripgrep and such).

@alexcrichton I agree that we have to handle backwards-compatibility, but that wouldn't stop us from having a top-level attribute to opt-out of it.

If the proposal ends up boiling down to adding a new top-level attribute to opt-out, then I think a blocker in my mind for the proposal would be to justify why a change to the compiler/standard library is necessary when otherwise a crate on crates.io would only require two lines to add to a project (e.g. a dependency directive in Cargo.toml and a one-liner in src/main.rs to enable)

@alexcrichton Because there's a difference between "do some setup to ignore SIGPIPE, then un-ignore it" and "never ignore SIGPIPE in the first place". Because people want to build smaller, faster executables. Because libraries and applications shouldn't have subtle differences in behavior, especially undocumented differences. Because you can do this in C and Rust makes it difficult. Because not every project should need to have a dependency to do this. Because it's a pain to add a platform-specific dependency and a platform-specific one-liner for an application that could otherwise be portable, and #![no_ignore_sigpipe] would be easier. Because it should be possible to write Rust code that doesn't have any startup code at all, and with some care we could potentially eliminate all of the current startup code in the future.

Er sorry that sort of feels like a particularly antagonistic reponse to me? Many of your "Because ..." can be pretty trivially fixed (the crate doesn't have to be platform specific, difficulty seems like it's a function of the crate, etc) and seem like they're maybe blowing the problem out of proportion (we're not slowing down executables or bloating them by adding an extra syscall at the beginning)?

I think one of the main points you're bringing up is the difference in library/binary behavior today, but from what @sfackler mentioned above and I agree we unfortunately can't change the binary behavior and library authors largely just need to be aware of this if they run into it (which thankfully isn't all the time). We can of course also have crates to smooth this over one way or another.

My point (which I don't think you really addressed?) was that I don't understand why a compiler solution is needed. It seems that a suitable crate can be added to crates.io to solve almost all of these issues.

My point (which I don't think you really addressed?) was that I don't understand why a compiler solution is needed. It seems that a suitable crate can be added to crates.io to solve almost all of these issues.

It's possible to make a crate that causes start to never register a sigpipe handler at all? I thought that was pretty baked in?

If this can be reasonably supported as a library function then that seems like the best solution since it is low cost. At any rate if people absolutely don't want to take on that dependency then libstd is still lower cost than a built in attribute. All in all I think @alexcrichton makes fine points.

@alexcrichton My apologies, I certainly didn't intend to come across as antagonistic.

My most specific point is that a library crate can only undo what std does, it can't stop std from doing it in the first place. I'd like a solution that provides more control over what Rust's startup code is doing in the first place.

(Also, as one more reason, every unnecessary syscall also represents an additional syscall needed in seccomp filters to lock down a program.)

@OvermindDL1

It's possible to make a crate that causes start to never register a sigpipe handler at all? I thought that was pretty baked in?

Exactly. That's the main reason I'm asking after having built-in support.

Exactly. That's the main reason I'm asking after having built-in support.

I thought as much, it really should be in the compiler then. An attribute on main seems sufficient, or even a compiler flag (exposed via something in the Cargo.toml file perhaps as well?).

This seems like a problem that could be fixed with the next edition? That would justify the use of an attribute to disable/enable this feature, in that upgrading binary crates to the new edition would need to add the attribute to re-enable this feature.

Changing any signal disposition by default in the startup code is inherently broken, because dispositions are inherited across fork/exec. The same goes for signal masking. IMO this behavior should just be removed.

If you really really don't want SIGPIPE to happen, without applications having to take their own measures to stop it, there is a right way that does not clobber any state. For each write or equivalent (note: send and sendto can just use sendmsg as their backend with MSG_NOSIGNAL flag), do the following:

  1. Block SIGPIPE.
  2. Perform the write.
  3. If it failed with EPIPE, callsigwaitinfoto consume theSIGPIPE` while it's still blocked.
  4. Restore the old mask.

This procedure is entirely portable and thread-safe, because signal masks are thread-local and SIGPIPE is generated synchronously for the thread, not for the process.

an alternate solution:
println! calls std::process::exit(141) instead of panic!() if it fails on EPIPE

that way, both servers and cli tools work by default

The issue of igoring SIGPIPE is related to, but separate from, the issue of what println! does on write failure. Changing signal dispositions behind the application's back in a way it has no way to fix up before exec'ing something else (AFAICT the original disposition is lost) is a much bigger problem than what println! does, since applications can just refrain from using println!.

With that said, I agree std::process::exit(141) would be less bad than panic!(). The latter should be for programming errors, not for completely valid runtime conditions.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

nikomatsakis picture nikomatsakis  路  236Comments

nikomatsakis picture nikomatsakis  路  331Comments

withoutboats picture withoutboats  路  213Comments

cramertj picture cramertj  路  512Comments

Leo1003 picture Leo1003  路  898Comments