connect_timeout incorrectly succeeds on illumos and probably other systems

Summary

I tried this code:

use std::net::SocketAddr;
use std::net::TcpStream;
use std::time::Duration;

fn main() {
    let sockaddr_s = std::env::args()
        .skip(1)
        .next()
        .expect("expected at least one argument (sockaddr to connect to");
    let sockaddr: SocketAddr = sockaddr_s
        .parse()
        .expect("argument was not a valid socket address");

    match TcpStream::connect_timeout(&sockaddr, Duration::from_secs(1)) {
        Ok(_) => {
            println!("successfully connected to {sockaddr}");
        }
        Err(error) => {
            println!("failed to connect to {sockaddr}: {error:#}");
        }
    }
}

I expected to see this happen: when I provide this program with an IP address and remote port on which there is no server listening, this should print "failed to connect to [address]: Connection refused".

Instead, it sometimes prints "successfully connected to [address]". The result depends on whether the OS reports the error synchronously with the first attempted to call to connect (the syscall). If so, you get the correct behavior. If not, the call always succeeds, even if the underlying connect attempt fails.

To be more specific, on my network, my machine is 172.20.2.70 and I have another machine at 172.20.2.80. Neither is running a server on port 12345. If I try the local system, I get the expected error:

$ ./target/debug/connfail 172.20.2.70:12345
failed to connect to 172.20.2.70:12345: Connection refused (os error 146)

but if I try the other machine, it incorrectly succeeds:

$ ./target/debug/connfail 172.20.2.80:12345
successfully connected to 172.20.2.80:12345

Details

The illumos manual page for connect(3SOCKET) explains:

   When a socket is set to be non-blocking, a
   call to connect initiates an asynchronous connection. If the connection
   cannot be completed without blocking, such as when making a TCP
   connection to a remote server, then  the connection attempt is made in
   the background and connect returns -1 and errno is set to EINPROGRESS.

   Applications can obtain the state of this connection attempt by polling
   the socket's file descriptor for POLLOUT.

...

   When an asynchronous connection has completed, the application must call
   getsockopt(3SOCKET) using the macro SOL_SOCKET as the level argument and
   the macro SO_ERROR as the value of the option argument.  If the value of
   the SO_ERROR socket option is zero, then the connect was successfully
   established. Otherwise, the connection could not be established and the
   value is the corresponding error code that would be commonly found in
   errno.

The exact problem I'm reporting here was previously reported for vxworks under #127018 and fixed for that platform under #127300. It looks to me like the fix for that platform exactly matches what the illumos manual pages say to do, which makes me think this is probably a common pattern on various Unix-like systems.

It was discussed under 127300 whether to apply the fix more generally and the decision was not to do this. At the time, @tgross35 mentioned:

The [Linux] manpage doesn't say anything about setting an error to SO_ERROR. I would assume this is always true, but it does seem safer to have the fallback in place.

This behavior does appear documented on Linux under connect:

   EINPROGRESS
          The socket is nonblocking and the connection cannot be
          completed immediately.  (UNIX domain sockets failed with
          EAGAIN instead.)  It is possible to select(2) or poll(2)
          for completion by selecting the socket for writing.  After
          select(2) indicates writability, use getsockopt(2) to read
          the SO_ERROR option at level SOL_SOCKET to determine
          whether connect() completed successfully (SO_ERROR is zero)
          or unsuccessfully (SO_ERROR is one of the usual error codes
          listed here, explaining the reason for the failure).

It seems to me that the fix for vxworks (which postdates a different fix for Linux) ought to work for Linux and illumos too and maybe should just be the behavior for all Unix-like systems? But I haven't done more digging than this.

Also, I imagine there's no test for this behavior or the test isn't working correctly.

Start at the TcpStream::connect_timeout entry point and compare its Unix-like handling with the VxWorks fix from issue #127300 and pull request #127300. Reproduce the illumos case described here, then verify that an asynchronous connection failure is reported rather than treated as success; the existing behavior should be covered by a working test.