maybe_new_socket() doesn't play well with a dual IPv4/IPv6 stack

[sandr8]

I'm hitting a somewhat annoying issue for active connections: maybe_new_socket() silently succeeds if the socket already exists but is of a different domain.

37 static int maybe_new_socket(uv_tcp_t* handle, int domain, int flags) { 38 int sockfd; 39 int err; 40 41 if (uv__stream_fd(handle) != -1) 42 return 0;

When one has resolved a hostname via uv_getaddrinfo() with ai_family = AF_UNSPEC in the hints, and the linked list of responses might (and most likely will) contain both IPv4 and IPv6 entries, in an order that is not under the application developer's control but depends instead on the gai.conf and the OS implementation.

The correct behavior is to try all of the addresses received in the answer in that order.

Because of the current behavior of maybe_new_socket(), the first invocation of uv_connect() will create a socket of the address family of the first getaddrinfo result. As soon as we attempt to connect to the first result of a different address family, we will get an EINVAL error, because of the address family mismatch.

It seems to me that the correct behavior would be for maybe_new_socket() to close the old socket and open a new one when the requested address family (or "domain" parameter) doesn't match the address family of the existing socket.

I haven't digged much further in the code, but I assume that for now the required edit might simply ignore the case of a listening socket.

[saghul]

If uv_connect fails you need to destroy and create a new handle, you shouldn't reuse it.

[sandr8]

Thanks! It seemed to work fine so long as the address family stayed the same...

[saghul]

I'll revisit the code, maybe we can fail more gracefully. Closing the socket is not an option, the user may have manually bound it to a given IP and port.

[sandr8]

saghul: do you happen to be on IRC?

[saghul]

@sandr8 usually yes, unfortunately I've had some internet problems lately which prevent me from being around as much as I'd like to.

I looked a bit deeper and indeed, we should check for the family and return an error. While this is fixed (may take a while) I suggest you create 2 tcp handles, one for IPv4 and one for IPv6 addresses, and try the results of getaddrinfo in the respective handle. This would also allow you to try IPv4 and IPv6 in parallel, as happy-eyeballs suggests.

[saghul]

Closing. As discussed on IRC, we cannot re-create the socket with a different family because the socket might have been created and bound earlier y the user calling uv_tcp_bind.

This will hopefully be more clear when we modify the API to create sockets early (in init), as follows:

int uv_tcp_init(uv_loop_t* loop, uv_tcp_t* handle, int family)
...
r =  uv_tcp_init(loop, handle, AF_INET6);
....