1berry/ruby
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Do not save ResolutionError if resolution succeeds for any address family (#12678)

Browse Source 
* Do not save ResolutionError if resolution succeeds for any address family

Socket with Happy Eyeballs Version 2 performs connection attempts and name resolution in parallel.

In the existing implementation, if a connection attempt failed for one address family while name resolution was still in progress for the other, and that name resolution later failed, the method would terminate with a name resolution error.
This behavior was intended to ensure that the final error reflected the most recent failure, potentially overriding an earlier error.

However, [Bug #21088](https://bugs.ruby-lang.org/issues/21088) made me realize that terminating with a name resolution error is unnatural when name resolution succeeded for at least one address family.

This PR modifies the behavior so that if name resolution succeeds for one address family, any name resolution error from the other is not saved.

This PR includes the following changes:

* Do not display select(2) as the system call that caused the raised error, as it is for internal processing

* Fix bug: Get errno with Socket::SO_ERROR in Windows environment with a workaround for tests not passing
This commit is contained in:
Misaki Shioi 2025-02-03 20:26:47 +09:00 committed by GitHub
parent f84d75eecc
commit 1683dadb19
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
Notes: git 2025-02-03 11:27:06 +00:00 
Merged-By: shioimm <shioi.mm@gmail.com>
4 changed files with 49 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

33
ext/socket/ipsocket.c
View File

@ -892,7 +892,6 @@ init_fast_fallback_inetsock_internal(VALUE v)
}
status = rb_thread_fd_select(nfds, &arg->readfds, &arg->writefds, NULL, delay_p);
syscall = "select(2)";
now = current_clocktime_ts();
if (is_timeout_tv(resolution_delay_expires_at, now)) {
@ -998,9 +997,11 @@ init_fast_fallback_inetsock_internal(VALUE v)
if (arg->getaddrinfo_entries[IPV6_ENTRY_POS]->err &&
arg->getaddrinfo_entries[IPV6_ENTRY_POS]->err != EAI_ADDRFAMILY) {
last_error.type = RESOLUTION_ERROR;
last_error.ecode = arg->getaddrinfo_entries[IPV6_ENTRY_POS]->err;
syscall = "getaddrinfo(3)";
if (!resolution_store.v4.finished || resolution_store.v4.has_error) {
last_error.type = RESOLUTION_ERROR;
last_error.ecode = arg->getaddrinfo_entries[IPV6_ENTRY_POS]->err;
syscall = "getaddrinfo(3)";
}
resolution_store.v6.has_error = true;
} else {
resolution_store.v6.ai = arg->getaddrinfo_entries[IPV6_ENTRY_POS]->ai;
@ -1015,9 +1016,11 @@ init_fast_fallback_inetsock_internal(VALUE v)
resolution_store.v4.finished = true;
if (arg->getaddrinfo_entries[IPV4_ENTRY_POS]->err) {
last_error.type = RESOLUTION_ERROR;
last_error.ecode = arg->getaddrinfo_entries[IPV4_ENTRY_POS]->err;
syscall = "getaddrinfo(3)";
if (!resolution_store.v6.finished || resolution_store.v6.has_error) {
last_error.type = RESOLUTION_ERROR;
last_error.ecode = arg->getaddrinfo_entries[IPV4_ENTRY_POS]->err;
syscall = "getaddrinfo(3)";
}
resolution_store.v4.has_error = true;
} else {
resolution_store.v4.ai = arg->getaddrinfo_entries[IPV4_ENTRY_POS]->ai;
@ -1057,9 +1060,11 @@ init_fast_fallback_inetsock_internal(VALUE v)
resolution_store.v6.finished = true;
if (arg->getaddrinfo_entries[IPV6_ENTRY_POS]->err) {
last_error.type = RESOLUTION_ERROR;
last_error.ecode = arg->getaddrinfo_entries[IPV6_ENTRY_POS]->err;
syscall = "getaddrinfo(3)";
if (!resolution_store.v4.finished || resolution_store.v4.has_error) {
last_error.type = RESOLUTION_ERROR;
last_error.ecode = arg->getaddrinfo_entries[IPV6_ENTRY_POS]->err;
syscall = "getaddrinfo(3)";
}
resolution_store.v6.has_error = true;
} else {
resolution_store.v6.ai = arg->getaddrinfo_entries[IPV6_ENTRY_POS]->ai;
@ -1075,9 +1080,11 @@ init_fast_fallback_inetsock_internal(VALUE v)
resolution_store.v4.finished = true;
if (arg->getaddrinfo_entries[IPV4_ENTRY_POS]->err) {
last_error.type = RESOLUTION_ERROR;
last_error.ecode = arg->getaddrinfo_entries[IPV4_ENTRY_POS]->err;
syscall = "getaddrinfo(3)";
if (!resolution_store.v6.finished || resolution_store.v6.has_error) {
last_error.type = RESOLUTION_ERROR;
last_error.ecode = arg->getaddrinfo_entries[IPV4_ENTRY_POS]->err;
syscall = "getaddrinfo(3)";
}
resolution_store.v4.has_error = true;
} else {
resolution_store.v4.ai = arg->getaddrinfo_entries[IPV4_ENTRY_POS]->ai;

9
ext/socket/lib/socket.rb
View File

@ -833,7 +833,7 @@ class Socket < BasicSocket
if except_sockets&.any?
except_sockets.each do |except_socket|
failed_ai = connecting_sockets.delete except_socket
sockopt = except_socket.getsockopt(Socket::SOL_SOCKET, Socket::SO_CONNECT_TIME)
sockopt = except_socket.getsockopt(Socket::SOL_SOCKET, Socket::SO_ERROR)
except_socket.close
ip_address = failed_ai.ipv6? ? "[#{failed_ai.ip_address}]" : failed_ai.ip_address
last_error = SystemCallError.new("connect(2) for #{ip_address}:#{failed_ai.ip_port}", sockopt.int)
@ -862,7 +862,10 @@ class Socket < BasicSocket
unless (Socket.const_defined?(:EAI_ADDRFAMILY)) &&
(result.is_a?(Socket::ResolutionError)) &&
(result.error_code == Socket::EAI_ADDRFAMILY)
last_error = result
other = family_name == :ipv6 ? :ipv4 : :ipv6
if !resolution_store.resolved?(other) || !resolution_store.resolved_successfully?(other)
last_error = result
end
end
else
resolution_store.add_resolved(family_name, result)
@ -1068,7 +1071,7 @@ class Socket < BasicSocket
end
def resolved_successfully?(family)
resolved?(family) && !!@error_dict[family]
resolved?(family) && !@error_dict[family]
end
def resolved_all_families?

22
test/socket/test_socket.rb
View File

@ -995,6 +995,28 @@ class TestSocket < Test::Unit::TestCase
RUBY
end
def test_tcp_socket_hostname_resolution_failed_after_connection_failure
opts = %w[-rsocket -W1]
assert_separately opts, <<~RUBY
server = TCPServer.new("127.0.0.1", 0)
port = server.connect_address.ip_port
Addrinfo.define_singleton_method(:getaddrinfo) do |_, _, family, *_|
case family
when Socket::AF_INET6 then sleep(0.1); raise Socket::ResolutionError
when Socket::AF_INET then [Addrinfo.tcp("127.0.0.1", port)]
end
end
server.close
# SystemCallError is a workaround for Windows environment
assert_raise(Errno::ECONNREFUSED, SystemCallError) do
Socket.tcp("localhost", port)
end
RUBY
end
def test_tcp_socket_v6_address_passed
opts = %w[-rsocket -W1]
assert_separately opts, <<~RUBY

2
test/socket/test_tcp.rb
View File

@ -316,7 +316,7 @@ class TestSocket_TCPSocket < Test::Unit::TestCase
port = server.connect_address.ip_port
server.close
assert_raise(Socket::ResolutionError) do
assert_raise(Errno::ECONNREFUSED) do
TCPSocket.new(
"localhost",
port,