Refactor connecting of sockets

This commit makes the following changes:

1. We remove the InProgress constant from the VM.

2. When we detect EISCONN as the result of a connect(2), we ignore it.

3. We properly read SO_ERROR from a socket when connecting in order to
   determine the actual result of connect(2).

4. We fix cloning of registered sockets.

== Removal of InProgress

Using the InProgress variant meant that the result register of
SocketConnect would not be set when a connect is in progress. This could
lead to unexpected results if one were to assume it was actually set.

== Using SO_ERROR

Socket::connect() in the VM now checks the value of SO_ERROR when a
connect(2) is reported as blocking. On at least Windows, the result of
connect(2) always appears to be EAGAIN/WSAEWOULDBLOCK, with the actual
error being stored in SO_ERROR. Not checking this value could result in
a connect getting stuck in the loop of: performing the connect ->
reported as blocking -> retrying later on -> repeat.

== Socket cloning

We also fix the cloning of sockets to make sure that the "registered"
flag is not cloned, as doing this could lead to errors when trying to
register a socket with a network poller.
parent 0631b684
Pipeline #61336587 passed with stages
in 25 minutes and 11 seconds
......@@ -94,7 +94,7 @@ test.group('std::net::socket::Socket.connect') do (g) {
let listener = try! Socket.new(domain: IPV4, kind: STREAM)
let stream = try! Socket.new(domain: IPV4, kind: STREAM)
try! listener.bind(ip: '0.0.0.0', port: 0)
try! listener.bind(ip: '127.0.0.1', port: 0)
try! listener.listen
let addr = try! listener.local_address
......@@ -722,7 +722,7 @@ test.group('std::net::socket::TcpStream.new') do (g) {
assert.no_panic {
let listener = try! Socket.new(domain: IPV4, kind: STREAM)
try! listener.bind(ip: '0.0.0.0', port: 0)
try! listener.bind(ip: '127.0.0.1', port: 0)
try! listener.listen
let listener_addr = try! listener.local_address
......
......@@ -17,18 +17,12 @@ pub enum RuntimeError {
/// A non-blocking operation would block, and should be retried at a later
/// point in time.
WouldBlock,
/// A non-blocking operation is still in progress and should not be retried.
/// Instead, the process should be suspended until the operation is done,
/// after which it should start off at the _next_ instruction.
InProgress,
}
impl RuntimeError {
pub fn should_poll(&self) -> bool {
match self {
RuntimeError::WouldBlock => true,
RuntimeError::InProgress => true,
_ => false,
}
}
......
......@@ -17,10 +17,12 @@ use std::net::{IpAddr, SocketAddr};
use std::slice;
#[cfg(unix)]
use libc::EINPROGRESS;
use libc::{EINPROGRESS, EISCONN};
#[cfg(windows)]
use winapi::shared::winerror::WSAEINPROGRESS as EINPROGRESS;
use winapi::shared::winerror::{
WSAEINPROGRESS as EINPROGRESS, WSAEISCONN as EISCONN,
};
macro_rules! socket_setter {
($setter:ident, $type:ty) => {
......@@ -227,17 +229,26 @@ impl Socket {
match self.socket.connect(&sockaddr) {
Ok(_) => {}
#[cfg(windows)]
Err(ref e) if e.kind() == io::ErrorKind::WouldBlock => {
Err(ref e)
if e.kind() == io::ErrorKind::WouldBlock
|| e.raw_os_error() == Some(EINPROGRESS as i32) =>
{
if let Ok(Some(err)) = self.socket.take_error() {
// When performing a connect(), the error returned may be
// WouldBlock, with the actual error being stored in
// SO_ERROR on the socket. Windows in particular seems to
// take this approach.
return Err(err.into());
}
// On Windows a connect(2) might throw WSAEWOULDBLOCK, the
// Windows equivalent of EAGAIN/EWOULDBLOCK. When this happens
// we should not retry the connect(), as that may then fail with
// WSAEISCONN. Instead, we signal that the network poller should
// just wait until the socket is ready for writing.
return Err(RuntimeError::InProgress);
// Windows equivalent of EAGAIN/EWOULDBLOCK. Other platforms may
// also produce some error that Rust will report as WouldBlock.
return Err(RuntimeError::WouldBlock);
}
Err(ref e) if e.raw_os_error() == Some(EINPROGRESS as i32) => {
return Err(RuntimeError::InProgress);
Err(ref e) if e.raw_os_error() == Some(EISCONN as i32) => {
// We may run into an EISCONN if a previous connect(2) attempt
// would block. In this case we can just continue.
}
Err(e) => {
return Err(e.into());
......@@ -452,8 +463,25 @@ impl Clone for Socket {
.socket
.try_clone()
.expect("Failed to clone the socket"),
registered: self.registered,
registered: false,
unix: self.unix,
}
}
}
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn test_clone() {
let mut socket1 = Socket::new(0, 0).unwrap();
socket1.registered = true;
let socket2 = socket1.clone();
assert_eq!(socket2.registered, false);
assert_eq!(socket2.unix, false);
}
}
......@@ -132,11 +132,6 @@ macro_rules! try_runtime_error {
// bail out.
return Ok(());
}
RuntimeError::InProgress => {
$context.instruction_index = $index;
return Ok(());
}
};
}
}
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment