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

make do_io() public to allow customized I/O operations #1550

Closed
masa-koz opened this issue Feb 18, 2022 · 7 comments · Fixed by #1551
Closed

make do_io() public to allow customized I/O operations #1550

masa-koz opened this issue Feb 18, 2022 · 7 comments · Fixed by #1551

Comments

@masa-koz
Copy link
Contributor

I'm trying to use WSARecvMsg() on mio::net::UDPSocket. And I find that I should call WSARecvMsg() in do_io() like UDPSocket#recv(). But I cannot do because do_io() is not public.

So, I think that do_io() should be callable by the user of mio::net::UDPSocket.
Is this ok?

Currently, I'm working in https://github.com/masa-koz/mio/blob/6dd2162e526cc74bdca183b7c1ec46565aecad83/src/net/udp.rs#L329

@Thomasdezeeuw
Copy link
Collaborator

I'm afraid do_io is an implementation detail and we won't make it public. But if you're looking for vectored I/O on UDP sockets we can add that.

@masa-koz
Copy link
Contributor Author

masa-koz commented Feb 20, 2022

I'm trying to use WSARecvMsg to retrieve some ancillary datas such as in_pktinfo. So I would like to use recvmsg itself like https://docs.rs/nix/0.23.1/nix/sys/socket/fn.recvmsg.html

@Thomasdezeeuw
Copy link
Collaborator

On Unix you could just use the call since do_io doesn't do anything, see

mio/src/sys/unix/mod.rs

Lines 46 to 54 in e8aed7b

pub fn do_io<T, F, R>(&self, f: F, io: &T) -> io::Result<R>
where
F: FnOnce(&T) -> io::Result<R>,
{
// We don't hold state, so we can just call the function and
// return.
f(io)
}
}
. However on Windows our internals don't allow for that I'm afraid, see
pub fn do_io<T, F, R>(&self, f: F, io: &T) -> io::Result<R>
where
F: FnOnce(&T) -> io::Result<R>,
{
let result = f(io);
if let Err(ref e) = result {
if e.kind() == io::ErrorKind::WouldBlock {
self.inner.as_ref().map_or(Ok(()), |state| {
state
.selector
.reregister(state.sock_state.clone(), state.token, state.interests)
})?;
}
}
result
}
.

@masa-koz
Copy link
Contributor Author

Can you add do_rawio (or do_rawapi) which calls do_io internally? do_io is still private, so you can change the detail without breaking api.

If this is also unacceptable, I will rewrite my code using miow.

@Thomasdezeeuw
Copy link
Collaborator

@masa-koz we've agreed that this addition would be acceptable.

It should be added to the following types:

  • net::TcpStream
  • net::UnixStream
  • net::UdpSocket
  • net::UnixDatagram
  • unix::pipe::Sender
  • unix::pipe::Receiver

I think the something like the following should work.

impl TcpStream {
    /// Execute an I/O operation ensuring that the socket receives more events
    /// if it hits a [`WouldBlock`] error.
    ///
    /// # Notes
    ///
    /// This method is required to be called for **all** I/O operations to
    /// ensure the user will receive events once the socket is ready again after
    /// returning a [`WouldBlock`] error.
    ///
    /// [`WouldBlock`]: io::ErrorKind::WouldBlock
    ///
    /// # Examples
    ///
    /// ```
    /// # use std::error::Error;
    /// #
    /// # fn main() -> Result<(), Box<dyn Error>> {
    /// use std::io;
    /// use std::os::unix::io::AsRawFd;
    /// use mio::net::TcpStream;
    ///
    /// let address = "127.0.0.1:8080".parse().unwrap();
    /// let stream = TcpStream::connect(address)?;
    ///
    /// // Wait until the steram is readable...
    ///
    /// // Read from the stream using a direct libc call, of course the
    /// // `io::Read` implementation would be easier to use.
    /// let mut buf = [0; 512];
    /// let n = stream.try_io(|| {
    ///     let buf_ptr = &mut buf as *mut _ as *mut _;
    ///     let res = unsafe { libc::recv(stream.as_raw_fd(), buf_ptr, buf.len(), 0) };
    ///     if res != -1 {
    ///         Ok(res as usize)
    ///     } else {
    ///         Err(io::Error::last_os_error())
    ///     }
    /// })?;
    /// eprintln!("read {} bytes", n);
    /// # Ok(())
    /// }
    /// ```
    pub fn try_io<F, T>(&self, f: F) -> io::Result<T>
    where
        F: FnOnce() -> io::Result<T>,
    {
        self.inner.do_io(|_| f())
    }
}

@masa-koz
Copy link
Contributor Author

It sounds great. Can I create a PR?

@Thomasdezeeuw
Copy link
Collaborator

It sounds great. Can I create a PR?

@masa-koz yes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants