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

net: add SO_LINGER option for TcpStream #1402

Closed
wants to merge 2 commits into from

Conversation

zekisherif
Copy link
Contributor

It was requested in a tokio PR that Mio TcpStream support setting linger option. This is so that the logic for converting a mio TcpStream into a TcpSocket stays within mio.

/// When set, the closing/shutting down of the socket will not return
/// until all queued messages for the socket have been successfully
/// sent or the linger timeout has been reached.
pub fn set_linger(&self, dur: Option<Duration>) -> io::Result<()> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we should stick to the same API as std lib.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So for tokio::net TcpStream @carllerche mentioned to me the plan was to follow std lib's api but it was abandoned.
I assumed we want mio's TcpStream to be similar to tokio's.

If we only add From traits here then we'd still have to have deal with ManuallyDrop at tokio TcpStream layer.

Copy link
Member

Choose a reason for hiding this comment

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

@Thomasdezeeuw what is the std lib API? I don't see one? Could you clarify what API you mean?

Copy link
Collaborator

Choose a reason for hiding this comment

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

@carllerche that is my point. Not having set_linger on TcpStream.

src/net/tcp/stream.rs Outdated Show resolved Hide resolved
/// When set, the closing/shutting down of the socket will not return
/// until all queued messages for the socket have been successfully
/// sent or the linger timeout has been reached.
pub fn set_linger(&self, dur: Option<Duration>) -> io::Result<()> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

@carllerche that is my point. Not having set_linger on TcpStream.

@@ -334,6 +334,20 @@ impl Drop for TcpSocket {
}
}

impl From<&TcpStream> for TcpSocket {
fn from(stream: &TcpStream) -> TcpSocket {
Copy link
Collaborator

Choose a reason for hiding this comment

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

If drop the TcpSocket now it will close the TcpStream's file descriptor. Can't use From<&TcpStream>.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This could work with something like socket2's SockRef: https://github.com/rust-lang/socket2/blob/9c4a404d5e44df1ecc56d62d35ee22109a886b5f/src/sockref.rs#L59. Maybe it's best to wait for #1381, which I'll work on (likely) next year.

@Thomasdezeeuw
Copy link
Collaborator

Closing this as we're going to follow the standard library API. We can revisited this once TcpStream in the standard library supports linger: rust-lang/rust#88494.

In the mean time the socket2 crate can be used for this.

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.

None yet

3 participants