-
Notifications
You must be signed in to change notification settings - Fork 666
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
feat: I/O safety for 'sys/signal' #1936
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apart from the doc nit, this LGTM. However, it may cause borrow checker issues upstack. The change works for me in mio-aio, but it could cause difficulty if Tokio adopts I/O Safety. I think we should probably merge it, but work with Tokio to ensure that all of the pieces will fit together before we release.
0ddc42f
to
9a3760c
Compare
macOS CI failure: network issue
|
Hi @asomers, I think we can merge this now, looks like Tokio won't adopt I/O safety. |
9a3760c
to
866789d
Compare
/// variants. | ||
#[doc(hidden)] | ||
#[cfg(not(freebsdlike))] | ||
_Unreachable(&'fd std::convert::Infallible), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I changed the associated type of this variant to &'fd Infallible
, since Infallible
is an enum with 0 variants, this variant can now never be constructed. (As a contrast, PhantomData
is very easy to create)
@@ -1,6 +1,3 @@ | |||
// Portions of this file are Copyright 2014 The Rust Project Developers. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove this header, looks like it is the only header we have.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just checked again. There's no way to make this work with Tokio except by using BorrowedFd::borrow_raw
, which defeats the whole point of I/O safety. But we can merge it if there are other users who can benefit.
Ah, right, since Tokio does not adopt I/O safety (in its implementation), all its internal interfaces are using RawFd, so it has to do the tedious conversion when interacting with I/O-safe interfaces. |
What does this PR do
sys/signal
.Checklist:
CONTRIBUTING.md