From: "midnight (Sarun R)" <noreply@...>
Date: 2022-05-24T22:27:19+00:00
Subject: [ruby-core:108683] [Ruby master Bug#18791] Unexpected behavior of Socket#connect_nonblock

Issue #18791 has been updated by midnight (Sarun R).


@jeremyevans0
The example is overly simplified, and it does lead to confusion in practice when you put it in context.

Realistically speaking, no one would use a single value in `IO.select`; at least, in the worst-case scenario.  
So the return value of `IO.select` doesn't get discarded like in the example; the `connect_nonblock` would generically run in a loop, not a one-off attempt.

This is the relevant part of an IRL Ruby method implementing a POC version of RFC8305:
~~~Ruby
selectable = []
elapse = 0.0
socket_map = {}
addresses.each do |address|
  socket_domain, resolved_address = address.values_at(0, 3)
  socket = ::Socket.new(socket_domain, :STREAM, 0)
  sock_address = ::Socket.pack_sockaddr_in(port, resolved_address)
  socket_map[socket] = sock_address
  begin
    selectable << socket
    socket.connect_nonblock(sock_address)
  rescue ::IO::WaitWritable
    ready_connections = ::IO.select(nil, selectable, nil, @connection_attempt_delay)
    Array(ready_connections).flatten(1).compact.each do |test_connection|
      test_address = socket_map[test_connection]
      raise AddressResolutionError unless test_address
      test_connection.connect_nonblock(test_address)
    rescue Errno::EISCONN
      return connected_handler.call(test_connection)
    end
    elapse += @connection_attempt_delay
    raise ConnectionTimeoutError if @connection_timeout < elapse
  end
end
~~~

It is not runnable by itself but you'll get the idea.  
The code surprised me by always making at least two connection attempts without a connection issue on any of the `addresses`.

To be clear the example did nothing wrong in isolating but does exploit something unrealistic in practice.  
The example discards the return value of `IO.select` and lets the code fall through to reach `socket.write`, disregarding the returned value.  
When we don't let the code fall through, it does appear to be incorrect to me.

----------------------------------------
Bug #18791: Unexpected behavior of Socket#connect_nonblock
https://bugs.ruby-lang.org/issues/18791#change-97724

* Author: midnight (Sarun R)
* Status: Rejected
* Priority: Normal
* ruby -v: ruby 3.1.1p18 (2022-02-18 revision 53f5fc4236) [x86_64-linux]
* Backport: 2.7: UNKNOWN, 3.0: UNKNOWN, 3.1: UNKNOWN
----------------------------------------
I followed an example of [Socket#connect_nonblock](https://ruby-doc.org/stdlib-3.1.2/libdoc/socket/rdoc/Socket.html#method-i-connect_nonblock) on the document.
Waiting for multiple Socket connections at once.

~~~Ruby
# Pull down Google's web page
require 'socket'
include Socket::Constants
socket = Socket.new(AF_INET, SOCK_STREAM, 0)
sockaddr = Socket.sockaddr_in(80, 'www.google.com')
begin # emulate blocking connect
  socket.connect_nonblock(sockaddr)
rescue IO::WaitWritable
  IO.select(nil, [socket]) # wait 3-way handshake completion
  begin
    # ****************** The problem is here ********************
    socket.connect_nonblock(sockaddr) # check connection failure
  rescue Errno::EISCONN
  end
end
socket.write("GET / HTTP/1.0\r\n\r\n")
results = socket.read
~~~

The first call to `connect_nonblock` raises `IO::WaitWritable` as expected.
But the confirmation call did not raise `Errno::EISCONN`; instead, it returned `0`.

Upon source code inspection, 0 was returned from `connect` and is supposed to mean success.

~~~C
static VALUE
sock_connect_nonblock(VALUE sock, VALUE addr, VALUE ex)
{
    VALUE rai;
    rb_io_t *fptr;
    int n;

    SockAddrStringValueWithAddrinfo(addr, rai);
    addr = rb_str_new4(addr);
    GetOpenFile(sock, fptr);
    rb_io_set_nonblock(fptr);
    n = connect(fptr->fd, (struct sockaddr*)RSTRING_PTR(addr), RSTRING_SOCKLEN(addr));
    if (n < 0) {
	int e = errno;
	if (e == EINPROGRESS) {
            if (ex == Qfalse) {
                return sym_wait_writable;
            }
            rb_readwrite_syserr_fail(RB_IO_WAIT_WRITABLE, e, "connect(2) would block");
	}
	if (e == EISCONN) {
            if (ex == Qfalse) {
                return INT2FIX(0);
            }
	}
	rsock_syserr_fail_raddrinfo_or_sockaddr(e, "connect(2)", addr, rai);
    }

    return INT2FIX(n);
}
~~~

I made sure `ex` is `true`, so it is not `return INT2FIX(0);` that get returned but the last statement `return INT2FIX(n);`.
Is this the intended behavior? It does surprise me and clearly doesn't explain very well in the document.
The example shown implied that the only way to confirm the connection is `Errno::EISCONN`; never mention the return code.



-- 
https://bugs.ruby-lang.org/

Unsubscribe: <mailto:ruby-core-request@ruby-lang.org?subject=unsubscribe>
<http://lists.ruby-lang.org/cgi-bin/mailman/options/ruby-core>