Skip to content

Commit

Permalink
Fix ttl tests on Windows
Browse files Browse the repository at this point in the history
On Windows calling setsockopt on an unconnected socket is undefined behaviour. More details can be found in issue #1117.

This registers the socket and waits for an event before attempting to use the set/get ttl methods.

Also adds documentation to TcpStream.

Closes #1117
  • Loading branch information
dtacalau authored and Thomasdezeeuw committed Nov 5, 2019
1 parent 34dd84c commit 60bf56b
Show file tree
Hide file tree
Showing 4 changed files with 139 additions and 4 deletions.
24 changes: 24 additions & 0 deletions src/net/tcp/stream.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,12 @@ impl TcpStream {
/// small amount of data. When not set, data is buffered until there is a
/// sufficient amount to send out, thereby avoiding the frequent sending of
/// small packets.
///
/// # Notes
///
/// On Windows make sure the stream is connected before calling this method,
/// by receiving an (writable) event. Trying to set `nodelay` on an
/// unconnected `TcpStream` is undefined behavior.
pub fn set_nodelay(&self, nodelay: bool) -> io::Result<()> {
self.sys.set_nodelay(nodelay)
}
Expand All @@ -114,6 +120,12 @@ impl TcpStream {
/// For more information about this option, see [`set_nodelay`][link].
///
/// [link]: #method.set_nodelay
///
/// # Notes
///
/// On Windows make sure the stream is connected before calling this method,
/// by receiving an (writable) event. Trying to get `nodelay` on an
/// unconnected `TcpStream` is undefined behavior.
pub fn nodelay(&self) -> io::Result<bool> {
self.sys.nodelay()
}
Expand All @@ -122,6 +134,12 @@ impl TcpStream {
///
/// This value sets the time-to-live field that is used in every packet sent
/// from this socket.
///
/// # Notes
///
/// On Windows make sure the stream is connected before calling this method,
/// by receiving an (writable) event. Trying to set `ttl` on an
/// unconnected `TcpStream` is undefined behavior.
pub fn set_ttl(&self, ttl: u32) -> io::Result<()> {
self.sys.set_ttl(ttl)
}
Expand All @@ -130,6 +148,12 @@ impl TcpStream {
///
/// For more information about this option, see [`set_ttl`][link].
///
/// # Notes
///
/// On Windows make sure the stream is connected before calling this method,
/// by receiving an (writable) event. Trying to get `ttl` on an
/// unconnected `TcpStream` is undefined behavior.
///
/// [link]: #method.set_ttl
pub fn ttl(&self) -> io::Result<u32> {
self.sys.ttl()
Expand Down
14 changes: 13 additions & 1 deletion tests/tcp_listener.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,17 +73,29 @@ fn smoke_test_tcp_listener(addr: SocketAddr) {
}

#[test]
fn ttl() {
fn set_get_ttl() {
init();

let listener = TcpListener::bind(any_local_address()).unwrap();

// set TTL, get TTL, make sure it has the expected value
const TTL: u32 = 10;
listener.set_ttl(TTL).unwrap();
assert_eq!(listener.ttl().unwrap(), TTL);
assert!(listener.take_error().unwrap().is_none());
}

#[test]
fn get_ttl_without_previous_set() {
init();

let listener = TcpListener::bind(any_local_address()).unwrap();

// expect a get TTL to work w/o any previous set_ttl
listener.ttl().expect("unable to get TTL for TCP listener");
assert!(listener.take_error().unwrap().is_none());
}

#[test]
#[cfg(unix)]
fn raw_fd() {
Expand Down
86 changes: 83 additions & 3 deletions tests/tcp_stream.rs
Original file line number Diff line number Diff line change
Expand Up @@ -167,14 +167,28 @@ fn try_clone() {
}

#[test]
fn ttl() {
init();
fn set_get_ttl() {
let (mut poll, mut events) = init_with_poll();

let barrier = Arc::new(Barrier::new(2));
let (thread_handle, address) = start_listener(1, Some(barrier.clone()), false);

let stream = TcpStream::connect(address).unwrap();

// on Windows: the stream must be connected before setting the ttl, otherwise
// it is undefined behavior, register and expect a WRITABLE here to make sure
// the stream is connected
poll.registry()
.register(&stream, ID1, Interests::WRITABLE)
.expect("unable to register TCP stream");

expect_events(
&mut poll,
&mut events,
vec![ExpectEvent::new(ID1, Interests::WRITABLE)],
);

// set TTL, get TTL, make sure it has the expected value
const TTL: u32 = 10;
stream.set_ttl(TTL).unwrap();
assert_eq!(stream.ttl().unwrap(), TTL);
Expand All @@ -185,14 +199,47 @@ fn ttl() {
}

#[test]
fn nodelay() {
fn get_ttl_without_previous_set() {
let (mut poll, mut events) = init_with_poll();

let barrier = Arc::new(Barrier::new(2));
let (thread_handle, address) = start_listener(1, Some(barrier.clone()), false);

let stream = TcpStream::connect(address).unwrap();

// on Windows: the stream must be connected before getting the ttl, otherwise
// it is undefined behavior, register and expect a WRITABLE here to make sure
// the stream is connected
poll.registry()
.register(&stream, ID1, Interests::WRITABLE)
.expect("unable to register TCP stream");

expect_events(
&mut poll,
&mut events,
vec![ExpectEvent::new(ID1, Interests::WRITABLE)],
);

// expect a get TTL to work w/o any previous set_ttl
stream.ttl().expect("unable to get TTL for TCP stream");
assert!(stream.take_error().unwrap().is_none());

barrier.wait();
thread_handle.join().expect("unable to join thread");
}

#[test]
fn set_get_nodelay() {
let (mut poll, mut events) = init_with_poll();

let barrier = Arc::new(Barrier::new(2));
let (thread_handle, address) = start_listener(1, Some(barrier.clone()), false);

let stream = TcpStream::connect(address).unwrap();

// on Windows: the stream must be connected before setting the nodelay, otherwise
// it is undefined behavior, register and expect a WRITABLE here to make sure
// the stream is connected
poll.registry()
.register(&stream, ID1, Interests::WRITABLE)
.expect("unable to register TCP stream");
Expand All @@ -203,6 +250,7 @@ fn nodelay() {
vec![ExpectEvent::new(ID1, Interests::WRITABLE)],
);

// set nodelay, get nodelay, make sure it has the expected value
const NO_DELAY: bool = true;
stream.set_nodelay(NO_DELAY).unwrap();
assert_eq!(stream.nodelay().unwrap(), NO_DELAY);
Expand All @@ -212,6 +260,38 @@ fn nodelay() {
thread_handle.join().expect("unable to join thread");
}

#[test]
fn get_nodelay_without_previous_set() {
let (mut poll, mut events) = init_with_poll();

let barrier = Arc::new(Barrier::new(2));
let (thread_handle, address) = start_listener(1, Some(barrier.clone()), false);

let stream = TcpStream::connect(address).unwrap();

// on Windows: the stream must be connected before setting the nodelay, otherwise
// it is undefined behavior, register and expect a WRITABLE here to make sure
// the stream is connected
poll.registry()
.register(&stream, ID1, Interests::WRITABLE)
.expect("unable to register TCP stream");

expect_events(
&mut poll,
&mut events,
vec![ExpectEvent::new(ID1, Interests::WRITABLE)],
);

// expect a get nodelay to work w/o any previous set nodelay
stream
.nodelay()
.expect("Unable to get nodelay for TCP stream");
assert!(stream.take_error().unwrap().is_none());

barrier.wait();
thread_handle.join().expect("unable to join thread");
}

#[test]
fn shutdown_read() {
let (mut poll, mut events) = init_with_poll();
Expand Down
19 changes: 19 additions & 0 deletions tests/udp_socket.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,25 @@ fn smoke_test_unconnected_udp_socket(socket1: UdpSocket, socket2: UdpSocket) {
assert!(socket2.take_error().unwrap().is_none());
}

#[test]
fn set_get_ttl() {
let socket1 = UdpSocket::bind(any_local_address()).unwrap();

// set TTL, get TTL, make sure it has the expected value
const TTL: u32 = 10;
socket1.set_ttl(TTL).unwrap();
assert_eq!(socket1.ttl().unwrap(), TTL);
assert!(socket1.take_error().unwrap().is_none());
}

#[test]
fn get_ttl_without_previous_set() {
let socket1 = UdpSocket::bind(any_local_address()).unwrap();

// expect a get TTL to work w/o any previous set_ttl
socket1.ttl().expect("unable to get TTL for UDP socket");
}

#[test]
fn connected_udp_socket_ipv4() {
let socket1 = UdpSocket::bind(any_local_address()).unwrap();
Expand Down

0 comments on commit 60bf56b

Please sign in to comment.