Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Invalidate pooled sessions for the same destination when we got a Kee…

…pAliveDisconnected exception. closes #30.

I know the original intent of #30 is 'create a new (non cached) session
when we got a KeepAliveDisconnected' but the suggested fix could create
too much session to the same site I guess... So at this time, I decided
to invalidate all sessions to the same site. It must include live
sessions (not expired) but it should not cause performance degrade in
the situation such as we got KeepAliveDisconnected.
  • Loading branch information...
commit a620b5f5aa60756c9df405d087f434cb22aeb925 1 parent 6ffacfa
@nahi authored
View
9 lib/httpclient.rb
@@ -781,6 +781,10 @@ class RetryableResponse < StandardError # :nodoc:
end
class KeepAliveDisconnected < StandardError # :nodoc:
+ attr_reader :sess
+ def initialize(sess = nil)
+ @sess = sess
+ end
end
def do_request(method, uri, query, body, extheader, &block)
@@ -884,7 +888,10 @@ def follow_redirect(method, uri, query, body, extheader, &block)
def protect_keep_alive_disconnected
begin
yield
- rescue KeepAliveDisconnected
+ rescue KeepAliveDisconnected => e
+ if e.sess
+ @session_manager.invalidate(e.sess.dest)
+ end
yield
end
end
View
32 lib/httpclient/session.rb
@@ -177,6 +177,16 @@ def keep(sess)
add_cached_session(sess)
end
+ def invalidate(site)
+ @sess_pool_mutex.synchronize do
+ @sess_pool.each do |sess|
+ if sess.dest == site
+ sess.invalidate
+ end
+ end
+ end
+ end
+
private
def open(uri, via_proxy = false)
@@ -227,7 +237,9 @@ def get_cached_session(uri)
@sess_pool_mutex.synchronize do
new_pool = []
@sess_pool.each do |s|
- if !cached && s.dest.match(uri)
+ if s.invalidated?
+ s.close # close & remove from the pool
+ elsif !cached && s.dest.match(uri)
cached = s
else
new_pool << s
@@ -515,6 +527,7 @@ class Session
def initialize(client, dest, agent_name, from)
@client = client
@dest = dest
+ @invalidated = false
@proxy = nil
@socket_sync = true
@requested_version = nil
@@ -567,13 +580,14 @@ def query(req)
rescue Errno::ECONNABORTED, Errno::ECONNRESET, Errno::EPIPE
# JRuby can raise IOError instead of ECONNRESET for now
close
- raise KeepAliveDisconnected.new
+ raise KeepAliveDisconnected.new(self)
rescue HTTPClient::TimeoutError
close
raise
rescue
+ close
if SSLEnabled and $!.is_a?(OpenSSL::SSL::SSLError)
- raise KeepAliveDisconnected.new
+ raise KeepAliveDisconnected.new(self)
else
raise
end
@@ -597,6 +611,14 @@ def closed?
@state == :INIT
end
+ def invalidate
+ @invalidated = true
+ end
+
+ def invalidated?
+ @invalidated
+ end
+
def get_header
begin
if @state != :META
@@ -801,12 +823,12 @@ def parse_header
initial_line = @socket.gets("\n")
if initial_line.nil?
close
- raise KeepAliveDisconnected.new
+ raise KeepAliveDisconnected.new(self)
end
rescue Errno::ECONNABORTED, Errno::ECONNRESET, Errno::EPIPE, IOError
# JRuby can raise IOError instead of ECONNRESET for now
close
- raise KeepAliveDisconnected.new
+ raise KeepAliveDisconnected.new(self)
end
begin
if StatusParseRegexp !~ initial_line
View
39 test/test_httpclient.rb
@@ -415,6 +415,13 @@ def test_redirect_non_https
# trying to normal endpoint with SSL -> SSL negotiation failure
@client.get_content(https_url)
end
+ #
+ # https -> http with strict_redirect_uri_callback
+ @client.redirect_uri_callback = @client.method(:strict_redirect_uri_callback)
+ @client.test_loopback_http_response << redirect_to_http
+ assert_raises(HTTPClient::BadResponseError) do
+ @client.get_content(https_url)
+ end
end
def test_redirect_relative
@@ -1160,6 +1167,38 @@ def test_session_manager
assert_equal(@client.debug_dev, mgr.debug_dev)
end
+ def test_keepalive_disconnected
+ client = HTTPClient.new
+ server = TCPServer.open('127.0.0.1', 0)
+ server.listen(30) # set enough backlogs
+ addr = server.addr
+ server_thread = Thread.new {
+ 5.times do
+ # emulate 5 keep-alive connections
+ sock = server.accept
+ sock.gets
+ sock.gets
+ sock.write("HTTP/1.1 200 OK\r\n")
+ sock.write("Content-Length: 5\r\n")
+ sock.write("\r\n")
+ sock.write("12345")
+ end
+ while true
+ # then emulate KeepAliveDisconnected
+ sock = server.accept
+ sock.close
+ end
+ }
+ 10.times.to_enum.map {
+ Thread.new {
+ begin
+ client.get("http://127.0.0.1:#{addr[1]}/")
+ rescue HTTPClient::KeepAliveDisconnected
+ end
+ }
+ }.map { |t| t.join }
+ end
+
private
def check_query_get(query)
Please sign in to comment.
Something went wrong with that request. Please try again.