diff --git a/lib/httpx/connection.rb b/lib/httpx/connection.rb index 06a44187..6f905d03 100644 --- a/lib/httpx/connection.rb +++ b/lib/httpx/connection.rb @@ -62,6 +62,7 @@ def initialize(uri, options) @pending = [] @inflight = 0 @keep_alive_timeout = @options.timeout[:keep_alive_timeout] + @idle_timeout = @options.timeout[:idle_timeout] @no_more_requests_counter = 0 if @options.io @@ -98,6 +99,16 @@ def addresses? @io && @io.addresses? end + def idle_timeout_expired? + @response_received_at && @idle_timeout && + Utils.elapsed_time(@response_received_at) > @idle_timeout + end + + def keep_alive_timeout_expired? + @response_received_at && @keep_alive_timeout && + Utils.elapsed_time(@response_received_at) > @keep_alive_timeout + end + def match?(uri, options) return false if !used? && (@state == :closing || @state == :closed) @@ -310,8 +321,7 @@ def send(request) return @coalesced_connection.send(request) if @coalesced_connection if @parser && !@write_buffer.full? - if @response_received_at && @keep_alive_timeout && - Utils.elapsed_time(@response_received_at) > @keep_alive_timeout + if keep_alive_timeout_expired? # when pushing a request into an existing connection, we have to check whether there # is the possibility that the connection might have extended the keep alive timeout. # for such cases, we want to ping for availability before deciding to shovel requests. diff --git a/lib/httpx/options.rb b/lib/httpx/options.rb index f73474de..424b858c 100644 --- a/lib/httpx/options.rb +++ b/lib/httpx/options.rb @@ -11,7 +11,7 @@ class Options SETTINGS_TIMEOUT = 10 CLOSE_HANDSHAKE_TIMEOUT = 10 CONNECT_TIMEOUT = READ_TIMEOUT = WRITE_TIMEOUT = 60 - REQUEST_TIMEOUT = OPERATION_TIMEOUT = nil + REQUEST_TIMEOUT = OPERATION_TIMEOUT = IDLE_TIMEOUT = nil RESOLVER_TYPES = %i[memory file].freeze # default value used for "user-agent" header, when not overridden. @@ -79,8 +79,8 @@ def method_added(meth) # :decompress_response_body :: whether to auto-decompress response body (defaults to true). # :compress_request_body :: whether to auto-decompress response body (defaults to true) # :timeout :: hash of timeout configurations (supports :connect_timeout, :settings_timeout, - # :operation_timeout, :keep_alive_timeout, :read_timeout, :write_timeout - # and :request_timeout + # :operation_timeout, :keep_alive_timeout, :idle_timeout, + # :read_timeout, :write_timeout and :request_timeout # :headers :: hash of HTTP headers (ex: { "x-custom-foo" => "bar" }) # :window_size :: number of bytes to read from a socket # :buffer_size :: internal read and write buffer size in bytes @@ -450,6 +450,8 @@ def option_timeout(value) next if val.nil? raise TypeError, ":#{key} must be numeric" unless val.is_a?(Numeric) + + raise TypeError, ":#{key} must be positive" unless val.positive? end timeout_hash @@ -559,6 +561,7 @@ def access_option(obj, k, ivar_map) close_handshake_timeout: CLOSE_HANDSHAKE_TIMEOUT, operation_timeout: OPERATION_TIMEOUT, keep_alive_timeout: KEEP_ALIVE_TIMEOUT, + idle_timeout: IDLE_TIMEOUT, read_timeout: READ_TIMEOUT, write_timeout: WRITE_TIMEOUT, request_timeout: REQUEST_TIMEOUT, diff --git a/lib/httpx/pool.rb b/lib/httpx/pool.rb index dd5385d7..3be49e67 100644 --- a/lib/httpx/pool.rb +++ b/lib/httpx/pool.rb @@ -186,13 +186,24 @@ def inspect private def acquire_connection(uri, options) - idx = @connections.find_index do |connection| - connection.match?(uri, options) - end + conn = nil + + @connections.delete_if do |connection| + if connection.state == :inactive && connection.idle_timeout_expired? + # discard connections which have been open and idle for too long + connection.force_close + next(true) + end - return unless idx + if connection.match?(uri, options) + conn = connection + break + end + + false + end - @connections.delete_at(idx) + conn end def checkout_new_connection(uri, options) diff --git a/lib/httpx/selector.rb b/lib/httpx/selector.rb index 3c52473d..666d9030 100644 --- a/lib/httpx/selector.rb +++ b/lib/httpx/selector.rb @@ -102,8 +102,20 @@ def each_connection(&block) end def find_connection(request_uri, options) - each_connection.find do |connection| - connection.match?(request_uri, options) + loop do + conn = each_connection.find do |connection| + connection.match?(request_uri, options) + end + + return unless conn + + if conn.idle_timeout_expired? + # discard connections which have been idle for too long + conn.force_close + next + end + + return conn end end diff --git a/sig/connection.rbs b/sig/connection.rbs index 7af5409f..8eb834e3 100644 --- a/sig/connection.rbs +++ b/sig/connection.rbs @@ -39,6 +39,7 @@ module HTTPX @inflight: Integer @max_concurrent_requests: Integer? @keep_alive_timeout: Numeric? + @idle_timeout: Numeric? @timeout: Numeric? @current_timeout: Numeric? @parser: Object & _Parser @@ -63,6 +64,10 @@ module HTTPX def addresses?: () -> boolish + def idle_timeout_expired?: () -> boolish + + def keep_alive_timeout_expired?: () -> boolish + def match?: (http_uri uri, Options options) -> bool def mergeable?: (Connection connection) -> bool diff --git a/sig/options.rbs b/sig/options.rbs index 1f783281..68bb1e39 100644 --- a/sig/options.rbs +++ b/sig/options.rbs @@ -8,11 +8,12 @@ module HTTPX CONNECT_TIMEOUT: Integer READ_TIMEOUT: Integer WRITE_TIMEOUT: Integer - REQUEST_TIMEOUT: Integer - OPERATION_TIMEOUT: Integer + REQUEST_TIMEOUT: nil + OPERATION_TIMEOUT: nil KEEP_ALIVE_TIMEOUT: Integer SETTINGS_TIMEOUT: Integer CLOSE_HANDSHAKE_TIMEOUT: Integer + IDLE_TIMEOUT: nil SET_TEMPORARY_NAME: ^(Class klass, ?Symbol pl) -> void DEFAULT_OPTIONS: Hash[Symbol, untyped] @@ -21,7 +22,7 @@ module HTTPX RESOLVER_TYPES: Array[Symbol] USER_AGENT: String - type timeout_type = :connect_timeout | :settings_timeout | :close_handshake_timeout | :operation_timeout | :keep_alive_timeout | :read_timeout | :write_timeout | :request_timeout + type timeout_type = :connect_timeout | :settings_timeout | :close_handshake_timeout | :operation_timeout | :keep_alive_timeout | :idle_timeout | :read_timeout | :write_timeout | :request_timeout type timeout = Hash[timeout_type, Numeric?] type redact_value = :headers | :body | bool type resolver_cache_option = :memory | :file | (Object & Resolver::_Cache) diff --git a/test/options_test.rb b/test/options_test.rb index 93de38b2..976f4889 100644 --- a/test/options_test.rb +++ b/test/options_test.rb @@ -137,6 +137,7 @@ def test_options_merge_attributes_match settings_timeout: 10, close_handshake_timeout: 10, operation_timeout: nil, + idle_timeout: nil, keep_alive_timeout: 20, read_timeout: 60, write_timeout: 60, diff --git a/test/session_test.rb b/test/session_test.rb index d9173313..8204581c 100644 --- a/test/session_test.rb +++ b/test/session_test.rb @@ -234,6 +234,20 @@ def test_session_timeout_keep_alive_timeout_multi_request_pings_once end end + def test_session_timeout_idle_timeout_no_ping + uri = build_uri("/get") + + HTTPX.plugin(SessionWithPool).with(timeout: { keep_alive_timeout: 2, idle_timeout: 2 }).wrap do |http| + response1 = http.get(uri) + sleep(3) + response2 = http.get(uri) + + verify_status(response1, 200) + verify_status(response2, 200) + assert http.ping_count.zero?, "session should have pinged after timeout (#{http.ping_count})" + end + end + def test_session_response_peer_address uri = URI(build_uri("/get")) response = HTTPX.get(uri) diff --git a/test/support/requests/plugins/persistent.rb b/test/support/requests/plugins/persistent.rb index 525fdec0..3665cef8 100644 --- a/test/support/requests/plugins/persistent.rb +++ b/test/support/requests/plugins/persistent.rb @@ -133,6 +133,24 @@ def test_persistent_retry_http2_goaway end end unless RUBY_ENGINE == "jruby" + def test_persistent_idle_timeout_no_ping + return unless origin.start_with?("https") + + uri = build_uri("/get") + + http = HTTPX.plugin(SessionWithPool) + .plugin(RequestInspector) + .plugin(:persistent) + .with(timeout: { keep_alive_timeout: 2, idle_timeout: 2 }) + response1 = http.get(uri) + sleep(3) + response2 = http.get(uri) + + verify_status(response1, 200) + verify_status(response2, 200) + assert http.ping_count.zero?, "session should have pinged after timeout (#{http.ping_count})" + end + def test_persistent_proxy_retry_http2_goaway return unless origin.start_with?("https")