driver: add support for registering file descriptors with user-specified flags #6089
Conversation
…ied epoll flags WIP fix for #6084. This currently only adds support for TcpListener.
Noah-Kennedy
changed the title
Co-authored-by: Alice Ryhl <[email protected]>
tokio/src/io/async_fd.rs
Outdated
| /// Create a new AsyncFd with the provided raw epoll flags for registration. | ||
| /// | ||
| /// These flags replace any epoll flags would normally set when registering the fd. | ||
| /// | ||
| /// **Note**: This is an [unstable API][unstable]. The public API of this may break in 1.x | ||
| /// releases. | ||
| /// See [the documentation on unstable features][unstable] for details. | ||
| /// | ||
| /// [unstable]: crate#unstable-features |
There was a problem hiding this comment.
Please add to the documentation that EPOLLONESHOT must not be used, and that EPOLLET must be set. And please add debug asserts for this.
…OLLET, and add debug assert to check for EPOLLONESHOT
There was a problem hiding this comment.
Thanks for getting this started. I left some comments inline. I also left a higher-level question in the original issue asking about how edge-triggered vs. level-triggered impacts this.
| #[track_caller] | ||
| #[cfg(all(target_os = "linux", tokio_unstable))] | ||
| #[cfg_attr(docsrs, doc(cfg(all(tokio_unstable, target_os = "linux"))))] | ||
| pub fn from_std_with_epoll_flags( |
There was a problem hiding this comment.
I would rather not have these methods on TcpListener. Consider adding them to TcpSocket instead as that is the socket builder API.
| #[track_caller] | ||
| #[cfg(all(target_os = "linux", tokio_unstable))] | ||
| #[cfg_attr(docsrs, doc(cfg(all(tokio_unstable, target_os = "linux"))))] | ||
| pub fn from_std_with_epoll_flags( |
There was a problem hiding this comment.
If the TCP method is added to TcpSocket, this will need to be removed and either skipped initially or we will need a UnixSocket API.
|
|
||
| // TODO: if this returns an err, the `ScheduledIo` leaks... | ||
| let res = unsafe { | ||
| libc::epoll_ctl( |
There was a problem hiding this comment.
We can't call epoll_* methods from Tokio as there is no guarantee that Mio uses epoll under the hood. Either we should have mio expose more APIs and call those, or we need to use epoll directly in Tokio (obviously, the first option is preferable).
There was a problem hiding this comment.
What guarantees are made by Mio around it's fd?
There was a problem hiding this comment.
I know that this behavior is not guaranteed, but we don't really say what behavior is.
There was a problem hiding this comment.
Which also I suppose begs the question "if it isn't here so people can do this as an escape hatch, why is it here at all?"
There was a problem hiding this comment.
That being said, there's a deeper question here, which is "do we need Interest::CUSTOM() in Mio, and how would that work?"
Personally, I think if we're going to continue to use Mio within tokio, we probably need something like this.
|
@carllerche since it's unstable, is it fine to initially put this on the listeners while we bikeshed UnixSocket, as it seems like we exposed another question about our AF_UNIX APIs here? |
|
Picking this back up again |
This change adds unstable support for custom user-specified epoll flags when registering file descriptors to our
TcpListener,UnixListener, andAsyncFdAPIs.It also adds a simple unit test to demonstrate basic functionality with EPOLLEXCLUSIVE.
This is the initial implementation of #6084.