Skip to content

Commit

Permalink
Auto merge of rust-lang#116132 - darthunix:connect_poll, r=cuviper
Browse files Browse the repository at this point in the history
Make TCP connect handle EINTR correctly

According to the [POSIX](https://pubs.opengroup.org/onlinepubs/009695399/functions/connect.html) standard, if connect() is interrupted by a signal that is caught while blocked waiting to establish a connection, connect() shall fail and set errno to EINTR, but the connection request shall not be aborted, and the connection shall be established asynchronously. When the connection has been established asynchronously, select() and poll() shall indicate that the file descriptor for the socket is ready for writing.

The previous implementation differs from the recomendation: in a case of the EINTR we tried to reconnect in a loop and sometimes get EISCONN error (this problem was originally detected on MacOS).

1. More details about the problem in an [article](http://www.madore.org/~david/computers/connect-intr.html).
2. The original [issue](https://git.picodata.io/picodata/picodata/tarantool-module/-/issues/157).
  • Loading branch information
bors committed Oct 19, 2023
2 parents a01382d + dfadd17 commit 3fbcfd2
Show file tree
Hide file tree
Showing 5 changed files with 38 additions and 12 deletions.
6 changes: 6 additions & 0 deletions library/std/src/sys/hermit/net.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,12 @@ impl Socket {
unimplemented!()
}

pub fn connect(&self, addr: &SocketAddr) -> io::Result<()> {
let (addr, len) = addr.into_inner();
cvt_r(|| unsafe { netc::connect(self.as_raw_fd(), addr.as_ptr(), len) })?;
Ok(())
}

pub fn connect_timeout(&self, addr: &SocketAddr, timeout: Duration) -> io::Result<()> {
self.set_nonblocking(true)?;
let r = unsafe {
Expand Down
11 changes: 7 additions & 4 deletions library/std/src/sys/solid/net.rs
Original file line number Diff line number Diff line change
Expand Up @@ -233,12 +233,15 @@ impl Socket {
}
}

pub fn connect(&self, addr: &SocketAddr) -> io::Result<()> {
let (addr, len) = addr.into_inner();
cvt(unsafe { netc::connect(self.0.raw(), addr.as_ptr(), len) })?;
Ok(())
}

pub fn connect_timeout(&self, addr: &SocketAddr, timeout: Duration) -> io::Result<()> {
self.set_nonblocking(true)?;
let r = unsafe {
let (addr, len) = addr.into_inner();
cvt(netc::connect(self.0.raw(), addr.as_ptr(), len))
};
let r = self.connect(addr);
self.set_nonblocking(false)?;

match r {
Expand Down
17 changes: 17 additions & 0 deletions library/std/src/sys/unix/net.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use crate::net::{Shutdown, SocketAddr};
use crate::os::unix::io::{AsFd, AsRawFd, BorrowedFd, FromRawFd, IntoRawFd, RawFd};
use crate::str;
use crate::sys::fd::FileDesc;
use crate::sys::unix::IsMinusOne;
use crate::sys_common::net::{getsockopt, setsockopt, sockaddr_to_addr};
use crate::sys_common::{AsInner, FromInner, IntoInner};
use crate::time::{Duration, Instant};
Expand Down Expand Up @@ -140,6 +141,22 @@ impl Socket {
unimplemented!()
}

pub fn connect(&self, addr: &SocketAddr) -> io::Result<()> {
let (addr, len) = addr.into_inner();
loop {
let result = unsafe { libc::connect(self.as_raw_fd(), addr.as_ptr(), len) };
if result.is_minus_one() {
let err = crate::sys::os::errno();
match err {
libc::EINTR => continue,
libc::EISCONN => return Ok(()),
_ => return Err(io::Error::from_raw_os_error(err)),
}
}
return Ok(());
}
}

pub fn connect_timeout(&self, addr: &SocketAddr, timeout: Duration) -> io::Result<()> {
self.set_nonblocking(true)?;
let r = unsafe {
Expand Down
12 changes: 7 additions & 5 deletions library/std/src/sys/windows/net.rs
Original file line number Diff line number Diff line change
Expand Up @@ -140,13 +140,15 @@ impl Socket {
}
}

pub fn connect(&self, addr: &SocketAddr) -> io::Result<()> {
let (addr, len) = addr.into_inner();
let result = unsafe { c::connect(self.as_raw(), addr.as_ptr(), len) };
cvt(result).map(drop)
}

pub fn connect_timeout(&self, addr: &SocketAddr, timeout: Duration) -> io::Result<()> {
self.set_nonblocking(true)?;
let result = {
let (addr, len) = addr.into_inner();
let result = unsafe { c::connect(self.as_raw(), addr.as_ptr(), len) };
cvt(result).map(drop)
};
let result = self.connect(addr);
self.set_nonblocking(false)?;

match result {
Expand Down
4 changes: 1 addition & 3 deletions library/std/src/sys_common/net.rs
Original file line number Diff line number Diff line change
Expand Up @@ -226,9 +226,7 @@ impl TcpStream {
init();

let sock = Socket::new(addr, c::SOCK_STREAM)?;

let (addr, len) = addr.into_inner();
cvt_r(|| unsafe { c::connect(sock.as_raw(), addr.as_ptr(), len) })?;
sock.connect(addr)?;
Ok(TcpStream { inner: sock })
}

Expand Down

0 comments on commit 3fbcfd2

Please sign in to comment.