Bug #14349: Fix Net::HTTP documentation around connection reuse

• Status: Closed
• Assignee: -
• Target version: -
• ruby -v: ruby 2.5.0p0 (2017-12-25 revision 61468) [x86_64-darwin17]
• Backport: 2.3: UNKNOWN, 2.4: UNKNOWN, 2.5: UNKNOWN

[rohitpaulk (Paul Kuruvilla)]

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'

[rohitpaulk (Paul Kuruvilla)]

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."

[jeremyevans0 (Jeremy Evans)]

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.

[normalperson (Eric Wong)]

[e-mail] 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.

[rohitpaulk (Paul Kuruvilla)]

• File fix-net-http-documentation.patch fix-net-http-documentation.patch added

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

[rohitpaulk (Paul Kuruvilla)]

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

• Status changed from Open to Closed

Applied in changeset trunk|r62113.

---

net/http: fix documentation for HTTP connection reuse

Thanks to Paul Kuruvilla [e-mail] for the patch

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

From: Paul Kuruvilla [e-mail]