[ruby-core:98561] [Ruby master Feature#16559] Net::HTTP#request injects "Connection: close" header if #started? is false, wasting HTTP server resources

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

Backport deleted (2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN)
ruby -v deleted (2.8.0-dev, 2.7.0, 2.6.5)
Assignee set to naruse (Yui NARUSE)
Tracker changed from Bug to Feature

I don't think this is a bug fix, this just makes a different tradeoff.  This can help cases where the computer you are connecting to is overloaded by TIME_WAIT sockets.  However, by the same token, it causes TIME_WAIT sockets to accumulate on the computer you are connecting from.

Consider a situation where you have a program that needs to get the root page for hundreds of thousands of websites.  It connects to a hundred web servers concurrently:

```ruby
100.times.map do
  Thread.new do
    loop do
      break unless domain_name = input_queue.pop
      net = Net::HTTP.new(domain_name)
      output_queue.push net.get('/')
    end
  end
end.each(&:join)
```

This approach works fine currently (assuming you add the necessary error handling), because `connection: close` will be set and the server will close the connection, so the client socket will not end up in TIME_WAIT.  With your change, `connection: close` will not be set, the client will close the connection, and all client sockets will end up in TIME_WAIT until 2MSL expires.

I'm not against the patch, as it would make things more consistent, and the backwards compatibility issues are small.  You can always set the `connection` header manually if you want specific behavior.  The net/http maintainer will have to decide if the change is worth it.



----------------------------------------
Feature #16559: Net::HTTP#request injects "Connection: close" header if #started? is false, wasting HTTP server resources
https://bugs.ruby-lang.org/issues/16559#change-85858

* Author: f3ndot (Justin Bull)
* Status: Open
* Priority: Normal
* Assignee: naruse (Yui NARUSE)
----------------------------------------
Hello,

There appears to be a bug in Net::HTTP#request (and thus #get, #post, etc.) on an instance that isn't explicitly started by the programmer (by invoking #start first, or by executing #request inside a block passed to #start).

Inspecting the source code, it reveals #request will recursively call itself inside a #start block if #started? is false. This is great and as I'd expect.

However in production and in a test setup I'm observing TCP socket connections on the server-side in the "TIME_WAIT" state, indicating the socket was never properly closed. Conversely, explicitly running #request inside a #start block yields no such behaviour.

Consider the following setup, assuming you have docker:

```
docker run --rm -it -p 8080:80/tcp --user root ubuntu
apt-get update && apt-get install net-tools watch nginx
service nginx start
watch 'netstat -tunapl'
```

Running this on your host machine:

``` ruby
net = Net::HTTP.new('localhost', 8080)
50.times { net.get('/') } # is bad
```

Will spawn 50 TCP connections on the server, and will all have on TIME_WAIT for 60 seconds (different *nix OSes have different times):

```
Every 2.0s: netstat -tunapl

Active Internet connections (servers and established)
Proto Recv-Q Send-Q Local Address           Foreign Address         State       PID/Program name
tcp        0      0 0.0.0.0:80              0.0.0.0:*               LISTEN      791/nginx: master p
tcp        0      0 172.17.0.2:80           172.17.0.1:60772        TIME_WAIT   -
tcp        0      0 172.17.0.2:80           172.17.0.1:60732        TIME_WAIT   -
tcp        0      0 172.17.0.2:80           172.17.0.1:60812        TIME_WAIT   -
tcp        0      0 172.17.0.2:80           172.17.0.1:60778        TIME_WAIT   -
...
```

However running any of these incantations have no such result:

``` ruby
50.times { Net::HTTP.get(URI('http://localhost:8080/')) } # is OK
```

``` ruby
net = Net::HTTP.new('localhost', 8080)
net.start
50.times { net.get('/') } # is OK
net.finish
```

``` ruby
net = Net::HTTP.new('localhost', 8080)
50.times { net.start { net.get('/') } } # is OK
```

These TIME_WAIT connections matter because a server receiving many HTTP requests from clients using Net::HTTP in this fashion (as Faraday does[1]) the server will begin to oversaturate and timeout past a particular scale.

I've tested and reproduced this in 2.7 and 2.6.

[1]: https://github.com/lostisland/faraday/pull/1117

---Files--------------------------------
dont-default-connection-close.patch (3.77 KB)


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

Unsubscribe: <mailto:[e-mail]?subject=unsubscribe>
<http://lists.ruby-lang.org/cgi-bin/mailman/options/ruby-core>