-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Set MSG_NOSIGNAL for UnixStream #140005
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
base: master
Are you sure you want to change the base?
Set MSG_NOSIGNAL for UnixStream #140005
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
@rustbot label +I-libs-nominated Hey T-libs! Was there a particular reason for this? I think it's reasonable to say that generating signals doesn't make sense for everything but console I/O. r=me if you agree |
Nominating for libs-api; I think the handling of broken pipes is an API question. |
In general, ignoring SIGPIPE and producing EPIPE is often a good idea. However, I'm not sure if this change is something we can or should do on behalf of the user without knowing what they want, for multiple reasons. The minor reason is that the user might not be expecting EPIPE. However, I think that's less of a problem for things other than stdout/stderr. The larger reason is that switching from I think it's still reasonable to consider this change, but we're going to want to announce it as a potential compatibility issue. |
FTR rust/library/std/src/os/unix/net/datagram.rs Line 518 in 1f76d21
|
@mlowicki Agreed. |
We discussed this in today's @rust-lang/libs-api call. We're fine with doing this, but we'd like to do an FCP to confirm this change, since it's user-visible (albeit not a documented guarantee). @rfcbot merge |
Team member @joshtriplett has proposed to merge this. The next step is review by the rest of the tagged team members: No concerns currently listed. Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! See this document for info about what commands tagged team members can give me. |
Once this gets merged I can create another PR to improve the doc for |
This can be done here, FCP hasn't started yet so there is time. |
Done. Fixed it for |
#139956
Same logic as for
rust/library/std/src/sys/net/connection/socket.rs
Lines 399 to 405 in 1f76d21