Skip to content

Commit c45c600

Browse files
authored
Add open_timeout as an overall timeout option for Socket.tcp (ruby#13368)
* Add `open_timeout` as an overall timeout option for `Socket.tcp` [Background] Currently, `TCPSocket.new` and `Socket.tcp` accept two kind of timeout options: - `resolv_timeout`, which controls the timeout for DNS resolution - `connect_timeout`, which controls the timeout for the connection attempt With the introduction of Happy Eyeballs Version 2 (as per [RFC 8305](https://datatracker.ietf.org/doc/html/rfc8305)) in[ Feature #20108](https://bugs.ruby-lang.org/issues/20108) and [Feature #20782](https://bugs.ruby-lang.org/issues/20782), both address resolution and connection attempts are now parallelized. As a result, the sum of `resolv_timeout` and `connect_timeout` no longer represents the total timeout duration. This is because, in HEv2, name resolution and connection attempts are performed concurrently, causing the two timeouts to overlap. Example: When `resolv_timeout: 200ms` and `connect_timeout: 100ms` are set: 1. An IPv6 address is resolved after the method starts immediately (IPv4 is still being resolved). 2. A connection attempt is initiated to the IPv6 address 3. After 100ms, `connect_timeout` is exceeded. However, since `resolv_timeout` still has 100ms left, the IPv4 resolution continues. 4. After 200ms from the start, the method raises a `resolv_timeout` error. In this case, the total elapsed time before a timeout is 200ms, not the expected 300ms (100ms + 200ms). Furthermore, in HEv2, connection attempts are also parallelized. It starts a new connection attempts every 250ms for resolved addresses. This makes the definition of `connect_timeout` even more ambiguous—specifically, it becomes unclear from which point the timeout is counted. Additionally, these methods initiate new connection attempts every 250ms (Connection Attempt Delay) for each candidate address, thereby parallelizing connection attempts. However, this behavior makes it unclear from which point in time the connect_timeout is actually measured. Currently, a `connect_timeout` is raised only after the last connection attempt exceeds the timeout. Example: When `connect_timeout: 100ms` is set and 3 address candidates: 1. Start a connection attempt to the address `a` 2. 250ms after step 1, start a new connection attempt to the address `b` 3. 500ms after step 1, start a new connection attempt to the address `c` 4. 1000ms after step 3 (1000ms after starting the connection to `c`, 1250ms after starting the connection to `b,` and 1500ms after starting the connection to `a`) `connect_timeout` is raised This behavior aims to favor successful connections by allowing more time for each attempt, but it results in a timeout model that is difficult to reason about. These methods have supported `resolv_timeout` and `connect_timeout` options even before the introduction of HEv2. However, in many use cases, it would be more convenient if a timeout occurred after a specified duration from the start of the method. Similar functions in other languages (such as PHP, Python, and Go) typically allow specifying only an overall timeout. [Proposal] I propose adding an `open_timeout` option to `Socket.tcp` in this PR, which triggers a timeout after a specified duration has elapsed from the start of the method. The name `open_timeout` aligns with the existing accessor used in `Net::HTTP`. If `open_timeout` is specified together with `resolv_timeout` and `connect_timeout`, I propose that only `open_timeout` be used and the others be ignored. While it is possible to support combinations of `open_timeout`, `resolv_timeout`, and `connect_timeout`, doing so would require defining which timeout takes precedence in which situations. In this case, I believe it is more valuable to keep the behavior simple and easy to understand, rather than supporting more complex use cases. If this proposal is accepted, I also plan to extend `open_timeout` support to `TCPSocket.new`. While the long-term future of `resolv_timeout` and `connect_timeout` may warrant further discussion, I believe the immediate priority is to offer a straightforward way to specify an overall timeout. [Outcome] If `open_timeout` is also supported by `TCPSocket.new`, users would be able to manage total connection timeouts directly in `Net::HTTP#connect` without relying on `Timeout.timeout`. https://github.com/ruby/ruby/blob/aa0f689bf45352c4a592e7f1a044912c40435266/lib/net/http.rb#L1657 --- * Raise an exception if it is specified together with other timeout options > If open_timeout is specified together with resolv_timeout and connect_timeout, I propose that only open_timeout be used and the others be ignored. Since this approach may be unclear to users, I’ve decided to explicitly raise an `ArgumentError` if these options are specified together. * Add doc * Fix: open_timeout error should be raised even if there are still addresses that have not been tried
1 parent 15084fb commit c45c600

File tree

2 files changed

+51
-9
lines changed

2 files changed

+51
-9
lines changed

ext/socket/lib/socket.rb

Lines changed: 25 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -643,6 +643,7 @@ def accept_nonblock(exception: true)
643643
#
644644
# [:resolv_timeout] Specifies the timeout in seconds from when the hostname resolution starts.
645645
# [:connect_timeout] This method sequentially attempts connecting to all candidate destination addresses.<br>The +connect_timeout+ specifies the timeout in seconds from the start of the connection attempt to the last candidate.<br>By default, all connection attempts continue until the timeout occurs.<br>When +fast_fallback:false+ is explicitly specified,<br>a timeout is set for each connection attempt and any connection attempt that exceeds its timeout will be canceled.
646+
# [:open_timeout] Specifies the timeout in seconds from the start of the method execution.<br>If this timeout is reached while there are still addresses that have not yet been attempted for connection, no further attempts will be made.
646647
# [:fast_fallback] Enables the Happy Eyeballs Version 2 algorithm (enabled by default).
647648
#
648649
# If a block is given, the block is called with the socket.
@@ -656,11 +657,16 @@ def accept_nonblock(exception: true)
656657
# sock.close_write
657658
# puts sock.read
658659
# }
659-
def self.tcp(host, port, local_host = nil, local_port = nil, connect_timeout: nil, resolv_timeout: nil, fast_fallback: tcp_fast_fallback, &) # :yield: socket
660+
def self.tcp(host, port, local_host = nil, local_port = nil, connect_timeout: nil, resolv_timeout: nil, open_timeout: nil, fast_fallback: tcp_fast_fallback, &) # :yield: socket
661+
662+
if open_timeout && (connect_timeout || resolv_timeout)
663+
raise ArgumentError, "Cannot specify open_timeout along with connect_timeout or resolv_timeout"
664+
end
665+
660666
sock = if fast_fallback && !(host && ip_address?(host))
661-
tcp_with_fast_fallback(host, port, local_host, local_port, connect_timeout:, resolv_timeout:)
667+
tcp_with_fast_fallback(host, port, local_host, local_port, connect_timeout:, resolv_timeout:, open_timeout:)
662668
else
663-
tcp_without_fast_fallback(host, port, local_host, local_port, connect_timeout:, resolv_timeout:)
669+
tcp_without_fast_fallback(host, port, local_host, local_port, connect_timeout:, resolv_timeout:, open_timeout:)
664670
end
665671

666672
if block_given?
@@ -674,7 +680,7 @@ def self.tcp(host, port, local_host = nil, local_port = nil, connect_timeout: ni
674680
end
675681
end
676682

677-
def self.tcp_with_fast_fallback(host, port, local_host = nil, local_port = nil, connect_timeout: nil, resolv_timeout: nil)
683+
def self.tcp_with_fast_fallback(host, port, local_host = nil, local_port = nil, connect_timeout: nil, resolv_timeout: nil, open_timeout: nil)
678684
if local_host || local_port
679685
local_addrinfos = Addrinfo.getaddrinfo(local_host, local_port, nil, :STREAM, timeout: resolv_timeout)
680686
resolving_family_names = local_addrinfos.map { |lai| ADDRESS_FAMILIES.key(lai.afamily) }.uniq
@@ -692,6 +698,7 @@ def self.tcp_with_fast_fallback(host, port, local_host = nil, local_port = nil,
692698
resolution_delay_expires_at = nil
693699
connection_attempt_delay_expires_at = nil
694700
user_specified_connect_timeout_at = nil
701+
user_specified_open_timeout_at = open_timeout ? now + open_timeout : nil
695702
last_error = nil
696703
last_error_from_thread = false
697704

@@ -784,7 +791,10 @@ def self.tcp_with_fast_fallback(host, port, local_host = nil, local_port = nil,
784791

785792
ends_at =
786793
if resolution_store.any_addrinfos?
787-
resolution_delay_expires_at || connection_attempt_delay_expires_at
794+
[(resolution_delay_expires_at || connection_attempt_delay_expires_at),
795+
user_specified_open_timeout_at].compact.min
796+
elsif user_specified_open_timeout_at
797+
user_specified_open_timeout_at
788798
else
789799
[user_specified_resolv_timeout_at, user_specified_connect_timeout_at].compact.max
790800
end
@@ -885,6 +895,8 @@ def self.tcp_with_fast_fallback(host, port, local_host = nil, local_port = nil,
885895
end
886896
end
887897

898+
raise(Errno::ETIMEDOUT, 'user specified timeout') if expired?(now, user_specified_open_timeout_at)
899+
888900
if resolution_store.empty_addrinfos?
889901
if connecting_sockets.empty? && resolution_store.resolved_all_families?
890902
if last_error_from_thread
@@ -912,7 +924,7 @@ def self.tcp_with_fast_fallback(host, port, local_host = nil, local_port = nil,
912924
end
913925
end
914926

915-
def self.tcp_without_fast_fallback(host, port, local_host, local_port, connect_timeout:, resolv_timeout:)
927+
def self.tcp_without_fast_fallback(host, port, local_host, local_port, connect_timeout:, resolv_timeout:, open_timeout:)
916928
last_error = nil
917929
ret = nil
918930

@@ -921,17 +933,21 @@ def self.tcp_without_fast_fallback(host, port, local_host, local_port, connect_t
921933
local_addr_list = Addrinfo.getaddrinfo(local_host, local_port, nil, :STREAM, nil)
922934
end
923935

924-
Addrinfo.foreach(host, port, nil, :STREAM, timeout: resolv_timeout) {|ai|
936+
timeout = open_timeout ? open_timeout : resolv_timeout
937+
starts_at = current_clock_time
938+
939+
Addrinfo.foreach(host, port, nil, :STREAM, timeout:) {|ai|
925940
if local_addr_list
926941
local_addr = local_addr_list.find {|local_ai| local_ai.afamily == ai.afamily }
927942
next unless local_addr
928943
else
929944
local_addr = nil
930945
end
931946
begin
947+
timeout = open_timeout ? open_timeout - (current_clock_time - starts_at) : connect_timeout
932948
sock = local_addr ?
933-
ai.connect_from(local_addr, timeout: connect_timeout) :
934-
ai.connect(timeout: connect_timeout)
949+
ai.connect_from(local_addr, timeout:) :
950+
ai.connect(timeout:)
935951
rescue SystemCallError
936952
last_error = $!
937953
next

test/socket/test_socket.rb

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -937,6 +937,32 @@ def test_tcp_socket_resolv_timeout_with_connection_failure
937937
RUBY
938938
end
939939

940+
def test_tcp_socket_open_timeout
941+
opts = %w[-rsocket -W1]
942+
assert_separately opts, <<~RUBY
943+
Addrinfo.define_singleton_method(:getaddrinfo) do |_, _, family, *_|
944+
if family == Socket::AF_INET6
945+
sleep
946+
else
947+
[Addrinfo.tcp("127.0.0.1", 12345)]
948+
end
949+
end
950+
951+
assert_raise(Errno::ETIMEDOUT) do
952+
Socket.tcp("localhost", 12345, open_timeout: 0.01)
953+
end
954+
RUBY
955+
end
956+
957+
def test_tcp_socket_open_timeout_with_other_timeouts
958+
opts = %w[-rsocket -W1]
959+
assert_separately opts, <<~RUBY
960+
assert_raise(ArgumentError) do
961+
Socket.tcp("localhost", 12345, open_timeout: 0.01, resolv_timout: 0.01)
962+
end
963+
RUBY
964+
end
965+
940966
def test_tcp_socket_one_hostname_resolution_succeeded_at_least
941967
opts = %w[-rsocket -W1]
942968
assert_separately opts, <<~RUBY

0 commit comments

Comments
 (0)