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

Implemented the From trait for converting mio socket types into std #1767

Merged
merged 5 commits into from
Mar 29, 2024

Conversation

tglane
Copy link
Contributor

@tglane tglane commented Mar 22, 2024

Implemented the following traits for conversion between the mio socket types and the std socket types:
From<mio::net::UdpSocket> for std::net::UdpSocket;
From<mio::net::TcpListener> for std::net::TcpListener;
From<mio::net::TcpStream> for std::net::TcpStream;

This allows the conversion without using the unsafe keyword in the user code. Inside of the trait function we use the unsafe keyword to access the raw underlying socket of the mic sockets. But since the mid socket types are in a well-defined state after construction, the use of unsafe here should be safe.

Closes #1754

socket types without using the unsafe keyword
Copy link
Collaborator

@Thomasdezeeuw Thomasdezeeuw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two small suggestions, if you can apply those to all implementations this LGTM.

Could you also add From implementations for the Unix-only UDS types in src/net/uds?

src/net/tcp/listener.rs Outdated Show resolved Hide resolved
src/net/tcp/listener.rs Outdated Show resolved Hide resolved
- Return temporaries instead of bound variables.
- Removed unnecessary documentation for impl of std types.
@tglane
Copy link
Contributor Author

tglane commented Mar 24, 2024

Two small suggestions, if you can apply those to all implementations this LGTM.

Could you also add From implementations for the Unix-only UDS types in src/net/uds?

I added the trait implementation for UnixDatagram, UnixStream and UnixListener as well.

// mio::net::TcpListener which ensures that we actually pass in a valid file
// descriptor/socket
unsafe {
#[cfg(any(unix, target_os = "hermit"))]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This currently fails to build on WASI. Can you try the following, to see if that fixes it? You might need to add use std::os::wasi::io::{FromRawFd, IntoRawFd}; to some files.

Suggested change
#[cfg(any(unix, target_os = "hermit"))]
#[cfg(any(unix, target_os = "hermit", target_os = "wasi"))]

Copy link
Contributor Author

@tglane tglane Mar 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure. With that addition the lib can be built for the target wasm32-wasi on my machine.

Edit: We still get failing CI tasks but they do not seem to be related to the changes made in this PR since the same tasks fail in other MR as well.

@Thomasdezeeuw Thomasdezeeuw merged commit a09e036 into tokio-rs:master Mar 29, 2024
29 of 31 checks passed
@Thomasdezeeuw
Copy link
Collaborator

Thanks @tglane

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 this pull request may close these issues.

why is it possible to create a mio TcpStream from std TcpStream but going the other way around is unsafe?
2 participants