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: reintroduce getter/setter for SO_LINGER to tokio::net::tcp::stream #3143

Merged
merged 5 commits into from
Nov 17, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion tokio/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ bytes = { version = "0.6.0", optional = true }
futures-core = { version = "0.3.0", optional = true }
lazy_static = { version = "1.4.0", optional = true }
memchr = { version = "2.2", optional = true }
mio = { version = "0.7.5", optional = true }
mio = { version = "0.7.6", optional = true }
num_cpus = { version = "1.8.0", optional = true }
parking_lot = { version = "0.11.0", optional = true }
slab = { version = "0.4.1", optional = true }
Expand Down
72 changes: 72 additions & 0 deletions tokio/src/net/tcp/stream.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ use std::io;
use std::net::{Shutdown, SocketAddr};
use std::pin::Pin;
use std::task::{Context, Poll};
use std::time::Duration;

cfg_net! {
/// A TCP stream between a local and a remote socket.
Expand Down Expand Up @@ -664,6 +665,65 @@ impl TcpStream {
self.io.set_nodelay(nodelay)
}

/// Reads the linger duration for this socket by getting the `SO_LINGER`
/// option.
///
/// For more information about this option, see [`set_linger`].
///
/// [`set_linger`]: TcpStream::set_linger
///
/// # Examples
///
/// ```no_run
/// use tokio::net::TcpStream;
///
/// # async fn dox() -> Result<(), Box<dyn std::error::Error>> {
/// let stream = TcpStream::connect("127.0.0.1:8080").await?;
///
/// println!("{:?}", stream.linger()?);
/// # Ok(())
/// # }
/// ```
pub fn linger(&self) -> io::Result<Option<Duration>> {
let mio_socket = mio::net::TcpSocket::from(self);
Copy link
Member

Choose a reason for hiding this comment

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

It should be possible to access self.io.get_linger(), similar to how nodelay does it above.

Copy link
Contributor Author

@zekisherif zekisherif Nov 16, 2020

Choose a reason for hiding this comment

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

There are no linger options in either Mio's TcpStream which self.io refers to nor its inner which is std::net::tcp

Copy link
Member

Choose a reason for hiding this comment

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

@seanmonstar I actually don't think that's right; the methods for getting and setting SO_LINGER are defined on mio's TcpSocket type, not on its TcpStream, and self.io is a TcpStream.

That does bring up a different question, though: is there any reason it's necessary to expose these methods on Tokio's TcpStream type, rather than on its TcpSocket type? tokio::net::TcpSocket wraps mio's TcpSocket...

Copy link
Member

Choose a reason for hiding this comment

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

Oh, I'm sorry, I misread that as TcpStream::from instead of TcpSocket.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@hawkw In our use case (pre tokio 0.3) we set the linger option from a stream object not TcpSocket.

I could also update this PR to also expose this on tokio::net::TcpSocket.

Copy link
Member

Choose a reason for hiding this comment

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

I am a bit confused here. set_linger is on the mio socket and PollEvented holds the mio socket? PollEvented also derefs to the mio socket, so why does self.io.get_linger() not work?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

PolledEvented holds Mio TcpStream

Copy link
Member

Choose a reason for hiding this comment

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

Ah yes, that is terribly confusing 😆


let result = mio_socket.get_linger();

std::mem::forget(mio_socket);
zekisherif marked this conversation as resolved.
Show resolved Hide resolved

result
}

/// Sets the linger duration of this socket by setting the SO_LINGER option.
///
/// This option controls the action taken when a stream has unsent messages and the stream is
/// closed. If SO_LINGER is set, the system shall block the process until it can transmit the
/// data or until the time expires.
///
/// If SO_LINGER is not specified, and the stream is closed, the system handles the call in a
/// way that allows the process to continue as quickly as possible.
///
/// # Examples
///
/// ```no_run
/// use tokio::net::TcpStream;
///
/// # async fn dox() -> Result<(), Box<dyn std::error::Error>> {
/// let stream = TcpStream::connect("127.0.0.1:8080").await?;
///
/// stream.set_linger(None)?;
/// # Ok(())
/// # }
/// ```
pub fn set_linger(&self, dur: Option<Duration>) -> io::Result<()> {
let mio_socket = mio::net::TcpSocket::from(self);

let result = mio_socket.set_linger(dur);

std::mem::forget(mio_socket);
result
}

/// Gets the value of the `IP_TTL` option for this socket.
///
/// For more information about this option, see [`set_ttl`].
Expand Down Expand Up @@ -826,6 +886,12 @@ mod sys {
self.io.as_raw_fd()
}
}

impl From<&TcpStream> for mio::net::TcpSocket {
zekisherif marked this conversation as resolved.
Show resolved Hide resolved
fn from(stream: &TcpStream) -> Self {
unsafe { mio::net::TcpSocket::from_raw_fd(stream.as_raw_fd()) }
}
}
}

#[cfg(windows)]
Expand All @@ -838,4 +904,10 @@ mod sys {
self.io.as_raw_socket()
}
}

impl From<&TcpStream> for mio::net::TcpSocket {
fn from(stream: &TcpStream) -> Self {
unsafe { mio::net::TcpSocket::from_raw_socket(stream.as_raw_socket()) }
}
}
}
16 changes: 16 additions & 0 deletions tokio/tests/tcp_stream.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,25 @@ use tokio_test::{assert_ok, assert_pending, assert_ready_ok};

use std::io;
use std::task::Poll;
use std::time::Duration;

use futures::future::poll_fn;

#[tokio::test]
async fn set_linger() {
let listener = TcpListener::bind("127.0.0.1:0").await.unwrap();

let stream = TcpStream::connect(listener.local_addr().unwrap())
.await
.unwrap();

assert_ok!(stream.set_linger(Some(Duration::from_secs(1))));
assert_eq!(stream.linger().unwrap().unwrap().as_secs(), 1);

assert_ok!(stream.set_linger(None));
assert!(stream.linger().unwrap().is_none());
}

#[tokio::test]
async fn try_read_write() {
const DATA: &[u8] = b"this is some data to write to the socket";
Expand Down