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

Tracking Issue for unix_sigpipe #97889

Open
11 of 23 tasks
Enselic opened this issue Jun 8, 2022 · 13 comments
Open
11 of 23 tasks

Tracking Issue for unix_sigpipe #97889

Enselic opened this issue Jun 8, 2022 · 13 comments
Labels
C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. T-libs Relevant to the library team, which will review and decide on the PR/issue.

Comments

@Enselic
Copy link
Member

Enselic commented Jun 8, 2022

NOTE

The ui changed from an attribute to a compiler flag: #124480. I will update this description later.


The feature gate for the issue is #![feature(unix_sigpipe)].
It enables a new fn main() attribute #[unix_sigpipe = "..."].

Usage

Any simple Rust program that writes a sizeable amount of data to stdout will panic if its output is limited via pipes.

fn main() {
    loop {
        println!("hello world");
    }
}
% ./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 backtrac

To prevent panicking we can use the new attribute:

#![feature(unix_sigpipe)]

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

More Info

Please refer to the unstable book section for more details. In short:

#[unix_sigpipe = "..."] Behaviour
sig_ign Set SIGPIPE handler to SIG_IGN before invoking fn main(). Default behaviour since 2014.
sig_dfl Set SIGPIPE handler to SIG_DFL before invoking fn main().
inherit Leave SIGPIPE handler untounched before entering fn main().

The problem with the current SIGPIPE code in libstd as well as several other aspects of this problem is discussed extensively at these places:

Naming convention

The naming follows the convention used by #![windows_subsystem = "windows|console"] where the values "windows" and "console" have the same names as the actual linker flags: /SUBSYSTEM:WINDOWS and /SUBSYSTEM:CONSOLE.

The names sig_ign and sig_dfl comes from the signal handler names SIG_IGN and SIG_DFL.

Steps

Unresolved Questions That Blocks Stabilisation

  • How can we make it easy to put fn lang_start() in an external crate that can be compiled with stable?
  • We should rename the attribute and attribute values to things that reflect what they do rather than how they do it.
  • #[unix_sigpipe = "sig_dfl"]
    • None
  • #[unix_sigpipe = "sig_ign"]
  • #[unix_sigpipe = "inherit"]
    • Is the name clear enough? Maybe rename to unchanged?

Unresolved Questions That Does Not Block Stabilisation

Because these questions can be resolved incrementally after stabilization.

Resolved Questions

  • We don't want to change to a -Z unix_sigpipe flag instead, see https://rust-lang.zulipchat.com/#narrow/stream/219381-t-libs/topic/Proposal.3A.20First.20step.20towards.20solving.20the.20SIGPIPE.20problem/near/285499895, at least not initially.
  • Should we only have the third sigpipe: u8 argument to fn lang_start() on Unix platform via cfg?
    Answer: No, this is not allowed, see top level comment in https://github.com/rust-lang/rust/blob/master/src/tools/tidy/src/pal.rs
  • Should we stabilize sig_dfl or is inherit and sig_ign sufficient?
    Answer: There are noteworthy examples of real projects that has opted to use SIG_DFL to solve the BrokenPipe problem. Notably rustc itself. So if we don't stabilize sig_dfl, such projects can't make use of our new attribute. Therefore, we also need to stabilize sig_dfl.
  • Should the attribute go on fn main() or on the top-level module (#![unix_sigpipe="..."])?
    Answer: It makes a lot of semantic sense to have the attribute on fn main(), because it is a way to configure what the Rust runtime should do before fn main() is invoked. For libraries, no entry point code that modifies SIGPIPE is generated, so allowing the attribute in these situations does not make much sense. See Change process spawning to inherit the parent's signal mask by default #101077 (comment) for small-scale discussion.
  • Can we write the code in a way that allows lto to remove the _signal stub code completely? With a bool it works (see Support #[unix_sigpipe = "inherit|sig_dfl"] on fn main() to prevent ignoring SIGPIPE #97802 (comment)), but with the current u8 we might need to do some tweaks. Answer: There are currently 4 values and I see no feasible way to reduce it to 2.
  • Can and should we alter the BrokenPipe error message and make it suggest to use the new attribute? Answer: No, because that would mean we would end up giving developer advice to users that can't act on the advice.
  • Does this have any impact on defining a stable ABI? Answer: No, because ABI discussion are about enabling things such as calling Rust functions in binary artifacts produced by an older Rust compiler than the current one. That we changed the ABI of fn lang_start() is not relevant. And a stable Rust ABI is not even close (see Define a Rust ABI rfcs#600).
  • Can we use MSG_NOSIGNAL with send() etc instead of setting SIGPIPE globally? Answer: No, because there is no equivalent for write(), and it would incur an extra syscall for each write-operation, which is likely to have significant performance drawbacks.

Disclaimer: I have taken the liberty to mark some questions resolved that I find unlikely to be controversial. If you would like me to create a proper discussion ticket for any of the resolved or unresolved questions, please let me know!

About tracking issues

Tracking issues are used to record the overall progress of implementation. They are also used as hubs connecting to other relevant issues, e.g., bugs or open design questions. A tracking issue is however not meant for large scale discussion, questions, or bug reports about a feature. Instead, open a dedicated issue for the specific matter and add the relevant feature gate label.

@rustbot label +T-libs

@Enselic Enselic added the C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. label Jun 8, 2022
@rustbot rustbot added the T-libs Relevant to the library team, which will review and decide on the PR/issue. label Jun 8, 2022
@Enselic Enselic changed the title Tracking Issue for no_ignore_sigpipe Tracking Issue for #![sigpipe_handler(sig_ign|unchanged)] Jun 10, 2022
@Enselic Enselic changed the title Tracking Issue for #![sigpipe_handler(sig_ign|unchanged)] Tracking Issue for pipeable Jun 13, 2022
@Enselic Enselic changed the title Tracking Issue for pipeable Tracking Issue for sigpipe Jun 16, 2022
@Enselic Enselic changed the title Tracking Issue for sigpipe Tracking Issue for unix_sigpipe Jun 19, 2022
bors added a commit to rust-lang-ci/rust that referenced this issue Sep 2, 2022
…, r=joshtriplett

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#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#97889
bjorn3 pushed a commit to rust-lang/rustc_codegen_cranelift that referenced this issue Sep 4, 2022
…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
workingjubilee pushed a commit to tcdi/postgrestd that referenced this issue Sep 15, 2022
…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
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue Oct 24, 2022
…h726

rustc: Use `unix_sigpipe` instead of `rustc_driver::set_sigpipe_handler`

This is the first (known) step towards starting to use `unix_sigpipe` in the wild. Eventually, `rustc_driver::set_sigpipe_handler` can be removed and all clients can use `unix_sigpipe` instead.

For now we just start using `unix_sigpipe` in one place: `rustc` itself.

It is easy to manually verify this change. If you remove `#[unix_sigpipe = "sig_dfl"]` and run `./x.py build` you will get an ICE when you do `./build/x86_64-unknown-linux-gnu/stage1/bin/rustc --help | false`. Add back `#[unix_sigpipe = "sig_dfl"]` and the ICE disappears again.

PR that added `set_sigpipe_handler`: rust-lang#49606

Tracking issue for `unix_sigpipe`: rust-lang#97889

Not sure exactly how to label this PR. Going with T-libs for now since this is a T-libs feature.

`@rustdoc` labels +T-libs
JohnTitor pushed a commit to JohnTitor/rust that referenced this issue Oct 24, 2022
…triddle

rustdoc: Use `unix_sigpipe` instead of `rustc_driver::set_sigpipe_handler`

Do what was already done for `rustc` in rust-lang#102587, namely start using `unix_sigpipe` instead of `rustc_driver::set_sigpipe_handler`.

After this has been merged, we can completely remove `rustc_driver::set_sigpipe_handler`.

PR that added `set_sigpipe_handler`: rust-lang#49606

Tracking issue for `unix_sigpipe`: rust-lang#97889

Verification of this change
---------------------------

1. Remove `#[unix_sigpipe = "sig_dfl"]`
1. Run `./x.py build`
1. Run `./build/aarch64-apple-darwin/stage1/bin/rustdoc --help | false`
1. Observe ICE
1. Add back `#[unix_sigpipe = "sig_dfl"]`
1. Run `./x.py build`
1. Run `./build/aarch64-apple-darwin/stage1/bin/rustdoc --help | false`
1. Observe ICE fixed

`@rustbot` labels +T-rustdoc
JohnTitor pushed a commit to JohnTitor/rust that referenced this issue Oct 24, 2022
…triddle

rustdoc: Use `unix_sigpipe` instead of `rustc_driver::set_sigpipe_handler`

Do what was already done for `rustc` in rust-lang#102587, namely start using `unix_sigpipe` instead of `rustc_driver::set_sigpipe_handler`.

After this has been merged, we can completely remove `rustc_driver::set_sigpipe_handler`.

PR that added `set_sigpipe_handler`: rust-lang#49606

Tracking issue for `unix_sigpipe`: rust-lang#97889

Verification of this change
---------------------------

1. Remove `#[unix_sigpipe = "sig_dfl"]`
1. Run `./x.py build`
1. Run `./build/aarch64-apple-darwin/stage1/bin/rustdoc --help | false`
1. Observe ICE
1. Add back `#[unix_sigpipe = "sig_dfl"]`
1. Run `./x.py build`
1. Run `./build/aarch64-apple-darwin/stage1/bin/rustdoc --help | false`
1. Observe ICE fixed

``@rustbot`` labels +T-rustdoc
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue Oct 25, 2022
…h726

rustc: Use `unix_sigpipe` instead of `rustc_driver::set_sigpipe_handler`

This is the first (known) step towards starting to use `unix_sigpipe` in the wild. Eventually, `rustc_driver::set_sigpipe_handler` can be removed and all clients can use `unix_sigpipe` instead.

For now we just start using `unix_sigpipe` in one place: `rustc` itself.

It is easy to manually verify this change. If you remove `#[unix_sigpipe = "sig_dfl"]` and run `./x.py build` you will get an ICE when you do `./build/x86_64-unknown-linux-gnu/stage1/bin/rustc --help | false`. Add back `#[unix_sigpipe = "sig_dfl"]` and the ICE disappears again.

PR that added `set_sigpipe_handler`: rust-lang#49606

Tracking issue for `unix_sigpipe`: rust-lang#97889

Not sure exactly how to label this PR. Going with T-libs for now since this is a T-libs feature.

``@rustdoc`` labels +T-libs
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue Oct 25, 2022
…h726

rustc: Use `unix_sigpipe` instead of `rustc_driver::set_sigpipe_handler`

This is the first (known) step towards starting to use `unix_sigpipe` in the wild. Eventually, `rustc_driver::set_sigpipe_handler` can be removed and all clients can use `unix_sigpipe` instead.

For now we just start using `unix_sigpipe` in one place: `rustc` itself.

It is easy to manually verify this change. If you remove `#[unix_sigpipe = "sig_dfl"]` and run `./x.py build` you will get an ICE when you do `./build/x86_64-unknown-linux-gnu/stage1/bin/rustc --help | false`. Add back `#[unix_sigpipe = "sig_dfl"]` and the ICE disappears again.

PR that added `set_sigpipe_handler`: rust-lang#49606

Tracking issue for `unix_sigpipe`: rust-lang#97889

Not sure exactly how to label this PR. Going with T-libs for now since this is a T-libs feature.

```@rustdoc``` labels +T-libs
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue Oct 25, 2022
…h726

rustc: Use `unix_sigpipe` instead of `rustc_driver::set_sigpipe_handler`

This is the first (known) step towards starting to use `unix_sigpipe` in the wild. Eventually, `rustc_driver::set_sigpipe_handler` can be removed and all clients can use `unix_sigpipe` instead.

For now we just start using `unix_sigpipe` in one place: `rustc` itself.

It is easy to manually verify this change. If you remove `#[unix_sigpipe = "sig_dfl"]` and run `./x.py build` you will get an ICE when you do `./build/x86_64-unknown-linux-gnu/stage1/bin/rustc --help | false`. Add back `#[unix_sigpipe = "sig_dfl"]` and the ICE disappears again.

PR that added `set_sigpipe_handler`: rust-lang#49606

Tracking issue for `unix_sigpipe`: rust-lang#97889

Not sure exactly how to label this PR. Going with T-libs for now since this is a T-libs feature.

````@rustdoc```` labels +T-libs
compiler-errors added a commit to compiler-errors/rust that referenced this issue Oct 25, 2022
…, r=tmiasko

Remove `rustc_driver::set_sigpipe_handler()`

Its usage was removed in rust-lang#102587 and rust-lang#103495, so we do not need to keep it around any longer. According to [preliminary input](https://rust-lang.zulipchat.com/#narrow/stream/182449-t-compiler.2Fhelp/topic/Find.20.60rustc_driver.60.20dependent.20projects.3F/near/304490764), we do not need to worry about any deprecation cycle for this explicitly unstable API, and can just straight up remove it.

PR that added `set_sigpipe_handler`: rust-lang#49606

Tracking issue for `unix_sigpipe`: rust-lang#97889

Migration instructions for any remaining clients
---

Change from

```rust
#![feature(rustc_private)]

extern crate rustc_driver;

fn main() {
    rustc_driver::set_sigpipe_handler();
    // ...
```

to

```rust
#![feature(unix_sigpipe)]

#[unix_sigpipe = "sig_dfl"]
fn main() {
    // ...
```

`@rustbot` labels +T-compiler
@jstarks
Copy link

jstarks commented Mar 20, 2024

Modified idea: instead of specifying SIG_IGN, std could register a SIGPIPE handler that does something like received_sigpipe = true. When inherit_sigpipe is called, it would switch back to the original disposition. After this, if received_sigpipe is true, then it would call raise(SIGPIPE). That way, the signal is not lost, just delayed, which is allowed by the specification.

This has the added advantage that we no longer need to remember to revert SIGPIPE to SIG_DFL when execing a process, since this happens automatically for caught signals. This would be an observable change for programs calling exec without using std::process, though, since they get SIG_IGN today.

@Enselic
Copy link
Member Author

Enselic commented Mar 22, 2024

fn lang_start() and sigpipe: u8

I fully support the long-term goal of allowing any crate to define fn lang_start(). And I agree we can't have the sigpipe: u8 argument on fn lang_start() if we stabilize its signature for that purpose.

As long as there is only one fn lang_start() to choose from, I think the best approach is to parameterize its SIGPIPE behavior.

However, if one of many fn lang_start() can be chosen, we don't need any sigpipe: u8 parameter any longer. Instead, different implementations can be chosen. I imagine it would work similar to how choosing panic strategy works, where we currently have both
libpanic_abort-84c045833028a0b3.rlib and libpanic_unwind-5c5363659220970d.rlib in the sysroot, and which one that is chosen is controlled by the codegen flag -Cpanic=....

Similarly, I think we could add support for something like -Cruntime=sigpipe_kills which would link against liblangstart_sigpipe_sig_dfl-625a57f.rlib. With such a mechanism, we would not need any sigpipe: u8 parameter in the fn lang_start() signature.

Edit: After thinking some more on the next steps, I think the natural next step is to get rid of the sigpipe: u8 parameter to fn lang_start() already now. I will look into it.

The UI

Separately from above is the question of the UI for this. Should we use an attribute on fn main() or only a compiler flag?

When this was discussed previously the preference was to have an attribute so that the source code on its own defines the behavior of the program as much as possible.

But if you prefer a compiler flag instead @m-ou-se I would be happy to change to that. Would you prefer me to change it?

@jstarks
Copy link

jstarks commented Mar 23, 2024

Could you have the Unix lang_start call some function rust_setup_sigpipe, defined with weak linkage to call signal(SIGPIPE, SIG_IGN)? And then have the attribute redefine it with strong linkage to be a no-op?

If it works, it would:

  1. Give us the attribute-based UI (which I think is acceptable to people).
  2. Eliminate the parameter from lang_start immediately.
  3. Have optimal performance (maybe with one additional call if LTO can't see through weak references).

(By the way, here I'm assuming that we only stabilize an attribute that skips all SIGPIPE configuration altogether ("inherit"), rather than stabilize a SIG_DFL option. Anyone asking for SIG_DFL really wants "inherit". And if I'm wrong about that, well, you they always get SIG_DFL on their own if they have "inherit". But the reverse is not true--if Rust doesn't offer "inherit", then there's no way to get it after the fact.)

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Mar 30, 2024
…ark-Simulacrum

unix_sigpipe: Add test for SIGPIPE disposition in child processes

To make it clearer what the impact would be to stop using `SIG_IGN` and instead use a noop handler, like suggested [here](rust-lang#62569 (comment)) and implemented [here](rust-lang#121578).

Part of rust-lang#97889
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Mar 30, 2024
…ark-Simulacrum

unix_sigpipe: Add test for SIGPIPE disposition in child processes

To make it clearer what the impact would be to stop using `SIG_IGN` and instead use a noop handler, like suggested [here](rust-lang#62569 (comment)) and implemented [here](rust-lang#121578).

Part of rust-lang#97889
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Mar 30, 2024
Rollup merge of rust-lang#121573 - Enselic:sigpipe-child-process, r=Mark-Simulacrum

unix_sigpipe: Add test for SIGPIPE disposition in child processes

To make it clearer what the impact would be to stop using `SIG_IGN` and instead use a noop handler, like suggested [here](rust-lang#62569 (comment)) and implemented [here](rust-lang#121578).

Part of rust-lang#97889
RalfJung pushed a commit to RalfJung/rust-analyzer that referenced this issue Apr 20, 2024
…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
fmease added a commit to fmease/rust that referenced this issue Apr 24, 2024
…r=fmease

Test `#[unix_sigpipe = "inherit"]` with both `SIG_DFL` and `SIG_IGN`

Extend our `#[unix_sigpipe = "inherit"]` test so that it detects if  `SIGPIPE` wrongly ends up being `SIG_DFL` when the parent has `SIG_IGN`. We have no current test for this particular case.

Tracking issue: rust-lang#97889
fmease added a commit to fmease/rust that referenced this issue Apr 24, 2024
…r=fmease

Test `#[unix_sigpipe = "inherit"]` with both `SIG_DFL` and `SIG_IGN`

Extend our `#[unix_sigpipe = "inherit"]` test so that it detects if  `SIGPIPE` wrongly ends up being `SIG_DFL` when the parent has `SIG_IGN`. We have no current test for this particular case.

Tracking issue: rust-lang#97889
fmease added a commit to fmease/rust that referenced this issue Apr 24, 2024
…r=fmease

Test `#[unix_sigpipe = "inherit"]` with both `SIG_DFL` and `SIG_IGN`

Extend our `#[unix_sigpipe = "inherit"]` test so that it detects if  `SIGPIPE` wrongly ends up being `SIG_DFL` when the parent has `SIG_IGN`. We have no current test for this particular case.

Tracking issue: rust-lang#97889
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Apr 24, 2024
Rollup merge of rust-lang#123316 - Enselic:sigpipe-inherit-variants, r=fmease

Test `#[unix_sigpipe = "inherit"]` with both `SIG_DFL` and `SIG_IGN`

Extend our `#[unix_sigpipe = "inherit"]` test so that it detects if  `SIGPIPE` wrongly ends up being `SIG_DFL` when the parent has `SIG_IGN`. We have no current test for this particular case.

Tracking issue: rust-lang#97889
github-actions bot pushed a commit to rust-lang/miri that referenced this issue Apr 25, 2024
Test `#[unix_sigpipe = "inherit"]` with both `SIG_DFL` and `SIG_IGN`

Extend our `#[unix_sigpipe = "inherit"]` test so that it detects if  `SIGPIPE` wrongly ends up being `SIG_DFL` when the parent has `SIG_IGN`. We have no current test for this particular case.

Tracking issue: rust-lang/rust#97889
RalfJung pushed a commit to RalfJung/rust-analyzer that referenced this issue Apr 27, 2024
…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
compiler-errors added a commit to compiler-errors/rust that referenced this issue May 4, 2024
Change `SIGPIPE` ui from `#[unix_sigpipe = "..."]` to `-Zon-broken-pipe=...`

In the stabilization [attempt](rust-lang#120832) of `#[unix_sigpipe = "sig_dfl"]`, a concern was [raised ](rust-lang#120832 (comment)) related to using a language attribute for the feature: Long term, we want `fn lang_start()` to be definable by any crate, not just libstd. Having a special language attribute in that case becomes awkward.

So as a first step towards the next stabilization attempt, this PR changes the `#[unix_sigpipe = "..."]` attribute to a compiler flag `-Zon-broken-pipe=...` to remove that concern, since now the language is not "contaminated" by this feature.

Another point was [also raised](rust-lang#120832 (comment)), namely that the ui should not leak **how** it does things, but rather what the **end effect** is. The new flag uses the proposed naming. This is of course something that can be iterated on further before stabilization.

Tracking issue: rust-lang#97889
compiler-errors added a commit to compiler-errors/rust that referenced this issue May 4, 2024
Change `SIGPIPE` ui from `#[unix_sigpipe = "..."]` to `-Zon-broken-pipe=...`

In the stabilization [attempt](rust-lang#120832) of `#[unix_sigpipe = "sig_dfl"]`, a concern was [raised ](rust-lang#120832 (comment)) related to using a language attribute for the feature: Long term, we want `fn lang_start()` to be definable by any crate, not just libstd. Having a special language attribute in that case becomes awkward.

So as a first step towards the next stabilization attempt, this PR changes the `#[unix_sigpipe = "..."]` attribute to a compiler flag `-Zon-broken-pipe=...` to remove that concern, since now the language is not "contaminated" by this feature.

Another point was [also raised](rust-lang#120832 (comment)), namely that the ui should not leak **how** it does things, but rather what the **end effect** is. The new flag uses the proposed naming. This is of course something that can be iterated on further before stabilization.

Tracking issue: rust-lang#97889
compiler-errors added a commit to compiler-errors/rust that referenced this issue May 4, 2024
Change `SIGPIPE` ui from `#[unix_sigpipe = "..."]` to `-Zon-broken-pipe=...`

In the stabilization [attempt](rust-lang#120832) of `#[unix_sigpipe = "sig_dfl"]`, a concern was [raised ](rust-lang#120832 (comment)) related to using a language attribute for the feature: Long term, we want `fn lang_start()` to be definable by any crate, not just libstd. Having a special language attribute in that case becomes awkward.

So as a first step towards the next stabilization attempt, this PR changes the `#[unix_sigpipe = "..."]` attribute to a compiler flag `-Zon-broken-pipe=...` to remove that concern, since now the language is not "contaminated" by this feature.

Another point was [also raised](rust-lang#120832 (comment)), namely that the ui should not leak **how** it does things, but rather what the **end effect** is. The new flag uses the proposed naming. This is of course something that can be iterated on further before stabilization.

Tracking issue: rust-lang#97889
rust-timer added a commit to rust-lang-ci/rust that referenced this issue May 4, 2024
Rollup merge of rust-lang#124480 - Enselic:on-broken-pipe, r=jieyouxu

Change `SIGPIPE` ui from `#[unix_sigpipe = "..."]` to `-Zon-broken-pipe=...`

In the stabilization [attempt](rust-lang#120832) of `#[unix_sigpipe = "sig_dfl"]`, a concern was [raised ](rust-lang#120832 (comment)) related to using a language attribute for the feature: Long term, we want `fn lang_start()` to be definable by any crate, not just libstd. Having a special language attribute in that case becomes awkward.

So as a first step towards the next stabilization attempt, this PR changes the `#[unix_sigpipe = "..."]` attribute to a compiler flag `-Zon-broken-pipe=...` to remove that concern, since now the language is not "contaminated" by this feature.

Another point was [also raised](rust-lang#120832 (comment)), namely that the ui should not leak **how** it does things, but rather what the **end effect** is. The new flag uses the proposed naming. This is of course something that can be iterated on further before stabilization.

Tracking issue: rust-lang#97889
RalfJung pushed a commit to RalfJung/miri that referenced this issue May 4, 2024
Change `SIGPIPE` ui from `#[unix_sigpipe = "..."]` to `-Zon-broken-pipe=...`

In the stabilization [attempt](rust-lang/rust#120832) of `#[unix_sigpipe = "sig_dfl"]`, a concern was [raised ](rust-lang/rust#120832 (comment)) related to using a language attribute for the feature: Long term, we want `fn lang_start()` to be definable by any crate, not just libstd. Having a special language attribute in that case becomes awkward.

So as a first step towards the next stabilization attempt, this PR changes the `#[unix_sigpipe = "..."]` attribute to a compiler flag `-Zon-broken-pipe=...` to remove that concern, since now the language is not "contaminated" by this feature.

Another point was [also raised](rust-lang/rust#120832 (comment)), namely that the ui should not leak **how** it does things, but rather what the **end effect** is. The new flag uses the proposed naming. This is of course something that can be iterated on further before stabilization.

Tracking issue: rust-lang/rust#97889
wcampbell0x2a added a commit to wcampbell0x2a/scuba that referenced this issue May 8, 2024
Rust ignores SIGPIPE by default, this patch overrides that behaviour to fix
this by *not* panic'ing on Broken pipes and restore old scubainit behavior.

In short, the following fails:
> image: debian:latest
>
> aliases:
>  test: yes '' | echo "test"

$ scuba test:
> test
> yes: standard output: Broken pipe

* Use nightly on-broken-pipe="inherit" to inherit the behavior from the parent process,
  Instead of killing our process. See docs here:
  https://github.com/rust-lang/rust/blob/master/src/doc/unstable-book/src/compiler-flags/on-broken-pipe.md
  This nightly fix is definitely unstable(with very recent API changes),
  but hopefully they keep this interface as a compiler option the same
  until stabilization.

* Add test to verify this doesn't break in the future

* Add rust-toolchain.toml to define the locked rust version since this requires a
  nightly option. I specifically didn't think nightly was an issue, since you
  were looking into using -Zbuild-std for scubainit size minimization
  (which has a long path to stabilization)

This has been a longstanding issue for the Rust language:
- rust-lang/rust#62569
- rust-lang/rust#97889
wcampbell0x2a added a commit to wcampbell0x2a/scuba that referenced this issue May 8, 2024
Rust ignores SIGPIPE by default, this patch overrides that behaviour to fix
this by *not* panic'ing on Broken pipes and restore old scubainit behavior.

In short, the following fails:
> image: debian:latest
>
> aliases:
>  test: yes '' | echo "test"

$ scuba test:
> test
> yes: standard output: Broken pipe

* Use nightly on-broken-pipe="inherit" to inherit the behavior from the parent process,
  Instead of killing our process. See docs here:
  https://github.com/rust-lang/rust/blob/master/src/doc/unstable-book/src/compiler-flags/on-broken-pipe.md
  This nightly fix is definitely unstable(with very recent API changes),
  but hopefully they keep this interface as a compiler option the same
  until stabilization.

* Add test to verify this doesn't break in the future

* Add rust-toolchain.toml to define the locked rust version since this requires a
  nightly option. I specifically didn't think nightly was an issue, since you
  were looking into using -Zbuild-std for scubainit size minimization
  (which has a long path to stabilization)

This has been a longstanding issue for the Rust language:
- rust-lang/rust#62569
- rust-lang/rust#97889
lnicola pushed a commit to lnicola/rust-analyzer that referenced this issue May 18, 2024
Change `SIGPIPE` ui from `#[unix_sigpipe = "..."]` to `-Zon-broken-pipe=...`

In the stabilization [attempt](rust-lang/rust#120832) of `#[unix_sigpipe = "sig_dfl"]`, a concern was [raised ](rust-lang/rust#120832 (comment)) related to using a language attribute for the feature: Long term, we want `fn lang_start()` to be definable by any crate, not just libstd. Having a special language attribute in that case becomes awkward.

So as a first step towards the next stabilization attempt, this PR changes the `#[unix_sigpipe = "..."]` attribute to a compiler flag `-Zon-broken-pipe=...` to remove that concern, since now the language is not "contaminated" by this feature.

Another point was [also raised](rust-lang/rust#120832 (comment)), namely that the ui should not leak **how** it does things, but rather what the **end effect** is. The new flag uses the proposed naming. This is of course something that can be iterated on further before stabilization.

Tracking issue: rust-lang/rust#97889
JonathonReinhart added a commit to JonathonReinhart/scuba that referenced this issue May 20, 2024
Rust pre-main code may change the SIGPIPE disposition to ignore:
* rust-lang/rust#62569
* rust-lang/rust#97889

We could use the nightly compiler flag -Zon-broken-pipe=inherit to
disable this behavior. Instead, we take the simpler route and restore
the default disposition ourselves.

Fixes #254
JonathonReinhart added a commit to JonathonReinhart/scuba that referenced this issue May 20, 2024
Rust pre-main code may change the SIGPIPE disposition to ignore:
* rust-lang/rust#62569
* rust-lang/rust#97889

We could use the nightly compiler flag -Zon-broken-pipe=inherit to
disable this behavior. Instead, we take the simpler route and restore
the default disposition ourselves.

Fixes #254
JonathonReinhart added a commit to JonathonReinhart/scuba that referenced this issue May 22, 2024
Rust pre-main code may change the SIGPIPE disposition to ignore:
* rust-lang/rust#62569
* rust-lang/rust#97889

We could use the nightly compiler flag -Zon-broken-pipe=inherit to
disable this behavior. Instead, we take the simpler route and restore
the default disposition ourselves.

Fixes #254
@jpmckinney
Copy link

For anyone else seeing this on nightly:

error: cannot find attribute `unix_sigpipe` in this scope
   --> src/main.rs:137:3
    |
137 | #[unix_sigpipe = "sig_dfl"]
    |   ^^^^^^^^^^^^

error[E0635]: unknown feature `unix_sigpipe`
 --> src/main.rs:1:12
  |
1 | #![feature(unix_sigpipe)]
  |            ^^^^^^^^^^^^

It seems like this feature is gone and is replaced with a compiler flag: #124480

Note that when doing a web search for these language features or compiler flags, it's not obvious which version of the Unstable Book you're looking at.

Nightly: https://doc.rust-lang.org/nightly/unstable-book/compiler-flags/on-broken-pipe.html
Beta: https://doc.rust-lang.org/beta/unstable-book/language-features/unix-sigpipe.html

JonathonReinhart added a commit to JonathonReinhart/scuba that referenced this issue May 29, 2024
Rust pre-main code may change the SIGPIPE disposition to ignore:
* rust-lang/rust#62569
* rust-lang/rust#97889

We could use the nightly compiler flag -Zon-broken-pipe=inherit to
disable this behavior. Instead, we take the simpler route and restore
the default disposition ourselves.

Fixes #254
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

7 participants