Bug: Underlying TCP connection leak when using HTTP protocol

[kavirajk]

I heard from internal team that when closing HTTP connection, the underlying TCP connection may not get closed consistently.

Did some investigation and creating this issue to track it.

This behavior is even when client.Close() is explicitly called and noticed underlying TCP connections are not getting closed.

Here is my theory

Problem 1 - Race condition

When client.Close() is explicitly invoked, according to this code, we do two things

     ch.closeOnce.Do(func() {
		err = ch.idle.Close()
		ch.closed.Store(true)
	})

(a). drain the existing idle connections from pool.
(b) mark the connection pool is closed. Note these two are not atomic operation. Some other actors may try to put the connection back in the pool meanwhile (say "after" draining the idle pool and "before" connection pool is marked as closed).

You can confirm this behavior by looking at connection pool's release code

if ch.closed.Load() {
		conn.close()
		return
	}

	ch.idle.Put(conn)

So there a race condition. The connection that was still in-flight and tried to put it back to pool may not get drained (so not closed correctly)

Problem 2 - Connection pool's release doesn't close the connection in some case.

Even without the race condition, The idle.Put() api just return without closing the connection when connectionPool.closed() is set

	if i.closed() {
		return
	}

We should close that connection at any cost to avoid such connection leak.

Proposals

1. Fix the order in Close(). First mark the idle pool as closed then drain it. So that no more connection cannot be added while draining.
2. Put() api should close the connection even when pool is closed.