[ruby-core:84815] [Ruby trunk Bug#14349] Net::HTTP doesn't reuse connections when used via ::new

[ruby-core:84815] [Ruby trunk Bug#14349] Net::HTTP doesn't reuse connections when used via ::new

[rohitpaulk]

Issue #14349 has been reported by rohitpaulk (Paul Kuruvilla).

----------------------------------------
Bug #14349: Net::HTTP doesn't reuse connections when used via ::new
https://bugs.ruby-lang.org/issues/14349

* Author: rohitpaulk (Paul Kuruvilla)
* Status: Open
* Priority: Normal
* 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
----------------------------------------
From [Net::HTTP's docs](http://ruby-doc.org/stdlib-2.5.0/libdoc/net/http/rdoc/Net/HTTP.html#class-Net::HTTP-label-How+to+use+Net-3A-3AHTTP): 

> ::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'`


---Files--------------------------------
net-http-reuse-connections.patch (1.32 KB)
debug_script.rb (1.14 KB)


-- 
https://bugs.ruby-lang.org/

[ruby-core:84816] [Ruby trunk Bug#14349] Net::HTTP doesn't reuse connections when used via ::new

[rohitpaulk]

Issue #14349 has been updated by 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."

----------------------------------------
Bug #14349: Net::HTTP doesn't reuse connections when used via ::new
https://bugs.ruby-lang.org/issues/14349#change-69529

* Author: rohitpaulk (Paul Kuruvilla)
* Status: Open
* Priority: Normal
* 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
----------------------------------------
From [Net::HTTP's docs](http://ruby-doc.org/stdlib-2.5.0/libdoc/net/http/rdoc/Net/HTTP.html#class-Net::HTTP-label-How+to+use+Net-3A-3AHTTP): 

> ::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'`


---Files--------------------------------
net-http-reuse-connections.patch (1.32 KB)
debug_script.rb (1.14 KB)


-- 
https://bugs.ruby-lang.org/

[ruby-core:84820] [Ruby trunk Bug#14349] Net::HTTP doesn't reuse connections when used via ::new

[merch-redmine]

Issue #14349 has been updated by jeremyevans0 (Jeremy Evans).


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

~~~ ruby
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:


~~~ ruby
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.

----------------------------------------
Bug #14349: Net::HTTP doesn't reuse connections when used via ::new
https://bugs.ruby-lang.org/issues/14349#change-69533

* Author: rohitpaulk (Paul Kuruvilla)
* Status: Open
* Priority: Normal
* 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
----------------------------------------
From [Net::HTTP's docs](http://ruby-doc.org/stdlib-2.5.0/libdoc/net/http/rdoc/Net/HTTP.html#class-Net::HTTP-label-How+to+use+Net-3A-3AHTTP): 

> ::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'`


---Files--------------------------------
net-http-reuse-connections.patch (1.32 KB)
debug_script.rb (1.14 KB)


-- 
https://bugs.ruby-lang.org/

[ruby-core:84822] Re: [Ruby trunk Bug#14349] Net::HTTP doesn't reuse connections when used via ::new

[Eric Wong]

rohitpaulk@gmail.com wrote:
> From [Net::HTTP's docs](http://ruby-doc.org/stdlib-2.5.0/libdoc/net/http/rdoc/Net/HTTP.html#class-Net::HTTP-label-How+to+use+Net-3A-3AHTTP): 

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

[ruby-core:84956] [Ruby trunk Bug#14349] Net::HTTP doesn't reuse connections when used via ::new

[rohitpaulk]

Issue #14349 has been updated by rohitpaulk (Paul Kuruvilla).

File 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). 

----------------------------------------
Bug #14349: Net::HTTP doesn't reuse connections when used via ::new
https://bugs.ruby-lang.org/issues/14349#change-69665

* Author: rohitpaulk (Paul Kuruvilla)
* Status: Open
* Priority: Normal
* 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
----------------------------------------
From [Net::HTTP's docs](http://ruby-doc.org/stdlib-2.5.0/libdoc/net/http/rdoc/Net/HTTP.html#class-Net::HTTP-label-How+to+use+Net-3A-3AHTTP): 

> ::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'`


---Files--------------------------------
net-http-reuse-connections.patch (1.32 KB)
debug_script.rb (1.14 KB)
fix-net-http-documentation.patch (1.37 KB)


-- 
https://bugs.ruby-lang.org/