Project

General

Profile

Bug #14349

Fix Net::HTTP documentation around connection reuse

Added by rohitpaulk (Paul Kuruvilla) 4 months ago. Updated 3 months ago.

Status:
Closed
Priority:
Normal
Assignee:
-
Target version:
-
ruby -v:
ruby 2.5.0p0 (2017-12-25 revision 61468) [x86_64-darwin17]
[ruby-core:84815]

Description

From Net::HTTP's docs:

::start immediately creates a connection to an HTTP server which is kept open for the duration of the block. The connection will remain open for multiple requests in the block if the server indicates it supports persistent connections.

If you wish to re-use a connection across multiple HTTP requests without automatically closing it you can use ::new instead of ::start. request will automatically open a connection to the server if one is not currently open. You can manually close the connection with finish.

According to the above, I'd expect the following scripts to reuse the underlying HTTP connection:

Net::HTTP.start('httpbin.org', port=443, use_ssl: true) { |http|
  10.times { http.get("/") }
}
http = Net::HTTP.new('httpbin.org', 443)
http.use_ssl = true
10.times { http.get("/") }
http.finish

The first one does indeed reuse connections, but the second doesn't. From a debug script (attached):

------------
Using #start
------------
Connection Alive? true
Connection Alive? true
Connection Alive? true
Connection Alive? true
Connection Alive? true
Connection Alive? true
Connection Alive? true
Connection Alive? true
Connection Alive? true
Connection Alive? true

Time taken: 4.11464

----------
Using #new
----------
Connection Alive? false
Connection Alive? false
Connection Alive? false
Connection Alive? false
Connection Alive? false
Connection Alive? false
Connection Alive? false
Connection Alive? false
Connection Alive? false
Connection Alive? false

Time taken: 12.337512

This happens because when start is called without a block and a connection doesn't exist, it proxies to a call with a block. And when a block is passed to start, it automatically calls finish at the end.

I've attached a patch that I think solves the issue. With the patch, sockets will still get closed after single requests through methods like Net::HTTP.get, since they use the block form for start.

Command to run the new test that was added: make test-all TESTS='net/http/test_http.rb -n test_keep_alive_using_new'

net-http-reuse-connections.patch (1.32 KB) net-http-reuse-connections.patch rohitpaulk (Paul Kuruvilla), 01/10/2018 05:04 PM
debug_script.rb (1.14 KB) debug_script.rb rohitpaulk (Paul Kuruvilla), 01/10/2018 05:32 PM
fix-net-http-documentation.patch (1.37 KB) fix-net-http-documentation.patch https://github.com/ruby/ruby/pull/1794 rohitpaulk (Paul Kuruvilla), 01/21/2018 01:21 PM

Associated revisions

Revision 3474d5c3
Added by normal 3 months ago

net/http: fix documentation for HTTP connection reuse

Thanks to Paul Kuruvilla rohitpaulk@gmail.com for the patch

  • lib/net/http.rb: fix documentation for HTTP connection reuse [Bug #14349]

From: Paul Kuruvilla rohitpaulk@gmail.com

git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@62113 b2dd03c8-39d4-4d8f-98ff-823fe69b080e

Revision 62113
Added by normalperson (Eric Wong) 3 months ago

net/http: fix documentation for HTTP connection reuse

Thanks to Paul Kuruvilla rohitpaulk@gmail.com for the patch

  • lib/net/http.rb: fix documentation for HTTP connection reuse [Bug #14349]

From: Paul Kuruvilla rohitpaulk@gmail.com

History

#1 [ruby-core:84816] Updated by rohitpaulk (Paul Kuruvilla) 4 months ago

This happens because when start is called without a block and a connection doesn't exist, it proxies to a call with a block.

Oops, this should've read: "When request is called and a connection doesn't exist, it proxies to a start call with a block."

#2 [ruby-core:84820] Updated by jeremyevans0 (Jeremy Evans) 4 months ago

I think this is a bad idea. If you do:

http = Net::HTTP.new('httpbin.org', 80)
http.get("/")

then it doesn't close the connection when making the GET request, and that's a bad thing. This breaks backwards compatibility and can result in connections being left open that were previously closed.

If you want to use persistent connections without calling start with a block, you need to call #start and #finish manually:

http = Net::HTTP.new('httpbin.org', 80)
http.start
10.times { http.get("/") }
http.finish

I do agree the documentation doesn't match the code. I think the bad section of the documentation should be removed or reworded.

#3 [ruby-core:84822] Updated by normalperson (Eric Wong) 4 months ago

rohitpaulk@gmail.com wrote:

From Net::HTTP's docs:

If you wish to re-use a connection across multiple HTTP
requests without automatically closing it you can use ::new
instead of ::start. request will automatically open a
connection to the server if one is not currently open. You
can manually close the connection with finish.

I guess this is a documentation bug and it should mention #start
along with ::new. Perhaps you can help reword this?

I've attached a patch that I think solves the issue. With the
patch, sockets will still get closed after single requests
through methods like Net::HTTP.get, since they use the block
form for start.

What Jeremy said; this may cause unintended resource exhaustion
on the client side. It may not be a big problem on the C Ruby GC;
but it may be on other VMs.

#4 [ruby-core:84956] Updated by rohitpaulk (Paul Kuruvilla) 3 months ago

Makes sense, I've attached a patch that fixes the documentation (generated from https://github.com/ruby/ruby/pull/1794).

#5 Updated by rohitpaulk (Paul Kuruvilla) 3 months ago

  • Subject changed from Net::HTTP doesn't reuse connections when used via ::new to Fix Net::HTTP documentation around connection reuse

#6 Updated by Anonymous 3 months ago

  • Status changed from Open to Closed

Applied in changeset trunk|r62113.


net/http: fix documentation for HTTP connection reuse

Thanks to Paul Kuruvilla rohitpaulk@gmail.com for the patch

  • lib/net/http.rb: fix documentation for HTTP connection reuse [Bug #14349]

From: Paul Kuruvilla rohitpaulk@gmail.com

Also available in: Atom PDF