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

try_io() of UDPSocket doesn't work on Windows #4510

Closed
masa-koz opened this issue Feb 16, 2022 · 9 comments · Fixed by #4582
Closed

try_io() of UDPSocket doesn't work on Windows #4510

masa-koz opened this issue Feb 16, 2022 · 9 comments · Fixed by #4582
Assignees
Labels
A-tokio Area: The main tokio crate C-bug Category: This is a bug. M-io Module: tokio/io

Comments

@masa-koz
Copy link
Contributor

masa-koz commented Feb 16, 2022

Version
└── tokio v1.17.0
└── tokio-macros v1.7.0 (proc-macro)

Platform
Windows 10 64bit

Description
I'm trying to use try_io() of UDPSocket for using WSARecvMsg() with tokio. But the following code doesn't work properly on Windows.
On Linux, it works as expected.

I tried this code:

use std::time::Duration;

#[cfg(target_family = "unix")]
use std::os::unix::io::AsRawFd;
#[cfg(target_family = "windows")]
use std::os::windows::prelude::AsRawSocket;

#[tokio::main]
async fn main() {
    let task_recv = tokio::spawn(async move {
        let udp = tokio::net::UdpSocket::bind("127.0.0.1:3456").await.unwrap();
        let mut buffer = vec![0; 1500];

        loop {
            let _ = udp.readable().await;
            let res = udp.try_io(tokio::io::Interest::READABLE, || {
                let res = unsafe {
                    #[cfg(target_family = "unix")]
                    let sock = udp.as_raw_fd();
                    #[cfg(target_family = "unix")]
                    let buf = buffer.as_mut_ptr() as *mut libc::c_void;
                    #[cfg(target_family = "unix")]
                    let buf_len = buffer.len();
                    
                    #[cfg(target_family = "windows")]
                    let sock =  udp.as_raw_socket() as usize;
                    #[cfg(target_family = "windows")]
                    let buf = buffer.as_mut_ptr() as *mut i8;
                    #[cfg(target_family = "windows")]
                    let buf_len = buffer.len() as i32;
        

                    libc::recvfrom(
                        sock,
                        buf,
                        buf_len,
                        0,
                        std::ptr::null_mut(),
                        std::ptr::null_mut()
                    )
                };
                if res < 0 {
                    Err(std::io::Error::last_os_error())
                } else {
                    Ok(res as usize)
                }
            });
            println!("res: {:?}", res);
        }
    });
    let task_send = tokio::spawn(async move {
        let udp = tokio::net::UdpSocket::bind("127.0.0.1:0").await.unwrap();
        loop {
            udp.send_to(&[0; 10], "127.0.0.1:3456")
                .await
                .expect("couldn't send data");
            tokio::time::sleep(Duration::from_secs(1)).await;
        }
    });

    let _ = tokio::join!(task_recv, task_send);
}

I expected to see this happen:

res: Ok(10)
res: Err(Kind(WouldBlock))
res: Ok(10)
res: Err(Kind(WouldBlock))
res: Ok(10)
res: Err(Kind(WouldBlock))
res: Ok(10)
res: Err(Kind(WouldBlock))
^C⏎

Instead, this happened:

res: Ok(10)
res: Err(Kind(WouldBlock))
error: process didn't exit successfully: `target\debug\check-try_io.exe` (exit code: 0xc000013a, STATUS_CONTROL_C_EXIT)

It seems that readable() never returns even though many udp packets should arrive.

@masa-koz masa-koz added A-tokio Area: The main tokio crate C-bug Category: This is a bug. labels Feb 16, 2022
@Darksonn Darksonn added the M-io Module: tokio/io label Feb 16, 2022
@Noah-Kennedy Noah-Kennedy self-assigned this Feb 16, 2022
@Noah-Kennedy
Copy link
Contributor

@masa-koz did this previously work on 1.16.1?

@masa-koz
Copy link
Contributor Author

@Noah-Kennedy I've just tried this on 1.16.1, but it does'n work, too.
On Linux (5.10.60.1-microsoft-standard-WSL2) both versions work.

@masa-koz
Copy link
Contributor Author

I find that I should call libc::recvfrom() in do_io() on Windows. Now, I'm suggesting to add do_io() in mio::net::UDPSocket.

tokio-rs/mio#1550

@Noah-Kennedy
Copy link
Contributor

@masa-koz so that fixed your issue?

@masa-koz
Copy link
Contributor Author

@Noah-Kennedy Yes. my test code works expectedly after I change try_io as below.

pub fn try_io<R>(
    &self,
    interest: Interest,
    f: impl FnOnce() -> io::Result<R>,
) -> io::Result<R> {
    self.io.registration().try_io(interest, || self.io.do_io(f))
}

@Noah-Kennedy
Copy link
Contributor

@masa-koz can we close this again or is there something here that you think is actionable in tokio?

@masa-koz
Copy link
Contributor Author

I don't mind closing this now. If there's any progress in mio, I would like to reopen this.

@masa-koz
Copy link
Contributor Author

Since mio 0.8.1, there are try_io methods on TCPStream, UDPSocket, UnixStream, and UnixDatagram. So, try_io methods on tokio should call these methods internally, I think. If acceptable, I will create PR.

masa-koz@1ed65a6

@masa-koz masa-koz reopened this Mar 23, 2022
@Darksonn
Copy link
Contributor

Yes, please open a PR for that.

masa-koz added a commit to masa-koz/tokio that referenced this issue Mar 26, 2022
A user defined I/O closure should be called by mio's try_io to ensure
that the I/O receives more events if it hits a WouldBlock error.

Fixes: tokio-rs#4510
masa-koz added a commit to masa-koz/tokio that referenced this issue Mar 26, 2022
A user defined I/O closure should be called by mio's try_io to ensure
that the I/O receives more events if it hits a WouldBlock error.

Fixes: tokio-rs#4510
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tokio Area: The main tokio crate C-bug Category: This is a bug. M-io Module: tokio/io
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants