Skip to content

Commit

Permalink
Auto merge of #97802 - Enselic:add-no_ignore_sigkill-feature, r=josht…
Browse files Browse the repository at this point in the history
…riplett

Support `#[unix_sigpipe = "inherit|sig_dfl"]` on `fn main()` to prevent ignoring `SIGPIPE`

When enabled, programs don't have to explicitly handle `ErrorKind::BrokenPipe` any longer. Currently, the program

```rust
fn main() { loop { println!("hello world"); } }
```

will print an error if used with a short-lived pipe, e.g.

    % ./main | head -n 1
    hello world
    thread 'main' panicked at 'failed printing to stdout: Broken pipe (os error 32)', library/std/src/io/stdio.rs:1016:9
    note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

by enabling `#[unix_sigpipe = "sig_dfl"]` like this

```rust
#![feature(unix_sigpipe)]
#[unix_sigpipe = "sig_dfl"]
fn main() { loop { println!("hello world"); } }
```

there is no error, because `SIGPIPE` will not be ignored and thus the program will be killed appropriately:

    % ./main | head -n 1
    hello world

The current libstd behaviour of ignoring `SIGPIPE` before `fn main()` can be explicitly requested by using `#[unix_sigpipe = "sig_ign"]`.

With `#[unix_sigpipe = "inherit"]`, no change at all is made to `SIGPIPE`, which typically means the behaviour will be the same as `#[unix_sigpipe = "sig_dfl"]`.

See rust-lang/rust#62569 and referenced issues for discussions regarding the `SIGPIPE` problem itself

See the [this](https://rust-lang.zulipchat.com/#narrow/stream/219381-t-libs/topic/Proposal.3A.20First.20step.20towards.20solving.20the.20SIGPIPE.20problem) Zulip topic for more discussions, including about this PR.

Tracking issue: rust-lang/rust#97889
  • Loading branch information
bors committed Sep 2, 2022
2 parents 24fc95c + a86df17 commit ea6e6dc
Show file tree
Hide file tree
Showing 7 changed files with 63 additions and 14 deletions.
31 changes: 28 additions & 3 deletions std/src/rt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,10 +72,29 @@ macro_rules! rtunwrap {
// Runs before `main`.
// SAFETY: must be called only once during runtime initialization.
// NOTE: this is not guaranteed to run, for example when Rust code is called externally.
//
// # The `sigpipe` parameter
//
// Since 2014, the Rust runtime on Unix has set the `SIGPIPE` handler to
// `SIG_IGN`. Applications have good reasons to want a different behavior
// though, so there is a `#[unix_sigpipe = "..."]` attribute on `fn main()` that
// can be used to select how `SIGPIPE` shall be setup (if changed at all) before
// `fn main()` is called. See <https://github.com/rust-lang/rust/issues/97889>
// for more info.
//
// The `sigpipe` parameter to this function gets its value via the code that
// rustc generates to invoke `fn lang_start()`. The reason we have `sigpipe` for
// all platforms and not only Unix, is because std is not allowed to have `cfg`
// directives as this high level. See the module docs in
// `src/tools/tidy/src/pal.rs` for more info. On all other platforms, `sigpipe`
// has a value, but its value is ignored.
//
// Even though it is an `u8`, it only ever has 3 values. These are documented in
// `compiler/rustc_session/src/config/sigpipe.rs`.
#[cfg_attr(test, allow(dead_code))]
unsafe fn init(argc: isize, argv: *const *const u8) {
unsafe fn init(argc: isize, argv: *const *const u8, sigpipe: u8) {
unsafe {
sys::init(argc, argv);
sys::init(argc, argv, sigpipe);

let main_guard = sys::thread::guard::init();
// Next, set up the current Thread with the guard information we just
Expand Down Expand Up @@ -107,6 +126,7 @@ fn lang_start_internal(
main: &(dyn Fn() -> i32 + Sync + crate::panic::RefUnwindSafe),
argc: isize,
argv: *const *const u8,
sigpipe: u8,
) -> Result<isize, !> {
use crate::{mem, panic};
let rt_abort = move |e| {
Expand All @@ -124,7 +144,7 @@ fn lang_start_internal(
// prevent libstd from accidentally introducing a panic to these functions. Another is from
// user code from `main` or, more nefariously, as described in e.g. issue #86030.
// SAFETY: Only called once during runtime initialization.
panic::catch_unwind(move || unsafe { init(argc, argv) }).map_err(rt_abort)?;
panic::catch_unwind(move || unsafe { init(argc, argv, sigpipe) }).map_err(rt_abort)?;
let ret_code = panic::catch_unwind(move || panic::catch_unwind(main).unwrap_or(101) as isize)
.map_err(move |e| {
mem::forget(e);
Expand All @@ -140,11 +160,16 @@ fn lang_start<T: crate::process::Termination + 'static>(
main: fn() -> T,
argc: isize,
argv: *const *const u8,
#[cfg(not(bootstrap))] sigpipe: u8,
) -> isize {
let Ok(v) = lang_start_internal(
&move || crate::sys_common::backtrace::__rust_begin_short_backtrace(main).report().to_i32(),
argc,
argv,
#[cfg(bootstrap)]
2, // Temporary inlining of sigpipe::DEFAULT until bootstrap stops being special
#[cfg(not(bootstrap))]
sigpipe,
);
v
}
2 changes: 1 addition & 1 deletion std/src/sys/hermit/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ pub extern "C" fn __rust_abort() {

// SAFETY: must be called only once during runtime initialization.
// NOTE: this is not guaranteed to run, for example when Rust code is called externally.
pub unsafe fn init(argc: isize, argv: *const *const u8) {
pub unsafe fn init(argc: isize, argv: *const *const u8, _sigpipe: u8) {
let _ = net::init();
args::init(argc, argv);
}
Expand Down
2 changes: 1 addition & 1 deletion std/src/sys/sgx/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ pub mod locks {

// SAFETY: must be called only once during runtime initialization.
// NOTE: this is not guaranteed to run, for example when Rust code is called externally.
pub unsafe fn init(argc: isize, argv: *const *const u8) {
pub unsafe fn init(argc: isize, argv: *const *const u8, _sigpipe: u8) {
unsafe {
args::init(argc, argv);
}
Expand Down
2 changes: 1 addition & 1 deletion std/src/sys/solid/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ pub mod locks {

// SAFETY: must be called only once during runtime initialization.
// NOTE: this is not guaranteed to run, for example when Rust code is called externally.
pub unsafe fn init(_argc: isize, _argv: *const *const u8) {}
pub unsafe fn init(_argc: isize, _argv: *const *const u8, _sigpipe: u8) {}

// SAFETY: must be called only once during runtime cleanup.
pub unsafe fn cleanup() {}
Expand Down
36 changes: 30 additions & 6 deletions std/src/sys/unix/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,13 @@ pub mod thread_parker;
pub mod time;

#[cfg(target_os = "espidf")]
pub fn init(argc: isize, argv: *const *const u8) {}
pub fn init(argc: isize, argv: *const *const u8, _sigpipe: u8) {}

#[cfg(not(target_os = "espidf"))]
// SAFETY: must be called only once during runtime initialization.
// NOTE: this is not guaranteed to run, for example when Rust code is called externally.
pub unsafe fn init(argc: isize, argv: *const *const u8) {
// See `fn init()` in `library/std/src/rt.rs` for docs on `sigpipe`.
pub unsafe fn init(argc: isize, argv: *const *const u8, sigpipe: u8) {
// The standard streams might be closed on application startup. To prevent
// std::io::{stdin, stdout,stderr} objects from using other unrelated file
// resources opened later, we reopen standards streams when they are closed.
Expand All @@ -61,8 +62,9 @@ pub unsafe fn init(argc: isize, argv: *const *const u8) {
// want!
//
// Hence, we set SIGPIPE to ignore when the program starts up in order
// to prevent this problem.
reset_sigpipe();
// to prevent this problem. Add `#[unix_sigpipe = "..."]` above `fn main()` to
// alter this behavior.
reset_sigpipe(sigpipe);

stack_overflow::init();
args::init(argc, argv);
Expand Down Expand Up @@ -151,9 +153,31 @@ pub unsafe fn init(argc: isize, argv: *const *const u8) {
}
}

unsafe fn reset_sigpipe() {
unsafe fn reset_sigpipe(#[allow(unused_variables)] sigpipe: u8) {
#[cfg(not(any(target_os = "emscripten", target_os = "fuchsia", target_os = "horizon")))]
rtassert!(signal(libc::SIGPIPE, libc::SIG_IGN) != libc::SIG_ERR);
{
// We don't want to add this as a public type to libstd, nor do we
// want to `include!` a file from the compiler (which would break
// Miri and xargo for example), so we choose to duplicate these
// constants from `compiler/rustc_session/src/config/sigpipe.rs`.
// See the other file for docs. NOTE: Make sure to keep them in
// sync!
mod sigpipe {
pub const INHERIT: u8 = 1;
pub const SIG_IGN: u8 = 2;
pub const SIG_DFL: u8 = 3;
}

let handler = match sigpipe {
sigpipe::INHERIT => None,
sigpipe::SIG_IGN => Some(libc::SIG_IGN),
sigpipe::SIG_DFL => Some(libc::SIG_DFL),
_ => unreachable!(),
};
if let Some(handler) = handler {
rtassert!(signal(libc::SIGPIPE, handler) != libc::SIG_ERR);
}
}
}
}

Expand Down
2 changes: 1 addition & 1 deletion std/src/sys/unsupported/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ pub mod memchr {

// SAFETY: must be called only once during runtime initialization.
// NOTE: this is not guaranteed to run, for example when Rust code is called externally.
pub unsafe fn init(_argc: isize, _argv: *const *const u8) {}
pub unsafe fn init(_argc: isize, _argv: *const *const u8, _sigpipe: u8) {}

// SAFETY: must be called only once during runtime cleanup.
// NOTE: this is not guaranteed to run, for example when the program aborts.
Expand Down
2 changes: 1 addition & 1 deletion std/src/sys/windows/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ cfg_if::cfg_if! {

// SAFETY: must be called only once during runtime initialization.
// NOTE: this is not guaranteed to run, for example when Rust code is called externally.
pub unsafe fn init(_argc: isize, _argv: *const *const u8) {
pub unsafe fn init(_argc: isize, _argv: *const *const u8, _sigpipe: u8) {
stack_overflow::init();

// Normally, `thread::spawn` will call `Thread::set_name` but since this thread already
Expand Down

0 comments on commit ea6e6dc

Please sign in to comment.