DRAFT: Use a noop SIGPIPE handler instead of SIG_IGN#121578
Closed
Enselic wants to merge 2 commits intorust-lang:masterfrom
Closed
DRAFT: Use a noop SIGPIPE handler instead of SIG_IGN#121578Enselic wants to merge 2 commits intorust-lang:masterfrom
SIGPIPE handler instead of SIG_IGN#121578Enselic wants to merge 2 commits intorust-lang:masterfrom
Conversation
Collaborator
|
r? @ChrisDenton rustbot has assigned @ChrisDenton. Use r? to explicitly pick a reviewer |
rustbot
added
O-unix
Enselic
changed the title
SIGPIPE handler instead of SIG_IGNSIGPIPE handler instead of SIG_IGN
Collaborator
|
The job Click to see the possible cause of the failure (guessed by this bot) |
bjorn3
reviewed
Feb 26, 2024
| target_os = "fuchsia", | ||
| target_os = "horizon", | ||
| )))] | ||
| extern "C" fn _rustc_sigaction_noop(_: libc::c_int) {} |
Member
There was a problem hiding this comment.
Maybe put this next to the sa_sigaction assignment?
Collaborator
|
☔ The latest upstream changes (presumably #121627) made this pull request unmergeable. Please resolve the merge conflicts. |
This was referenced Mar 9, 2024
matthiaskrgr
added a commit
to matthiaskrgr/rust
that referenced
this pull request
Mar 10, 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
jhpratt
added a commit
to jhpratt/rust
that referenced
this pull request
Mar 11, 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 pull request
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 pull request
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 pull request
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
Member
Author
|
I will close this for now as this is not something we will merge in the near future. I have an updated local branch that I probably will resume work on at a later point. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Here is an implementation of the proposal to use a noop handler instead of SIG_IGN.
Note that I have based this PR on #121573 which is not merged yet.
I have two questions:
SIGPIPEis ignored?The
oldactreturn value ofsigaction()will now point to a hidden function that will be tricky to interpret by others instead of a well-known handler constant.SIG_DFLif the Rust runtime has set it toSIG_IGN?Personally I currently think the answer is "yes", but in the current implementation, the answer is explicitly and by design "no". I have added this as an open unresolved question for
#[unix_sigpipe = "sig_ign"]to the tracking issue.