net/http: eliminate a goroutine leak in (*persistConn.addTLS)

In case of a handshake timeout, the goroutine running addTLS
closes the underlying connection, which should unblock the call
to tlsConn.HandshakeContext. However, it didn't then wait for
HandshakeContext to actually return.

I thought this might have something to do with #57602, but as
far as I can tell it does not. Still, it seems best to avoid the leak:
if tracing is enabled we emit a TLSHandshakeDone event, and it seems
misleading to produce that event when the handshake is still in
progress.

For #57602.

Change-Id: Ibfc0cf4ef8df2ccf11d8897f23d7d79ee482d5fb
Reviewed-on: https://go-review.googlesource.com/c/go/+/529755
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Damien Neil <dneil@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
Commit-Queue: Bryan Mills <bcmills@google.com>

diff --git a/src/net/http/transport.go b/src/net/http/transport.go
index ac7477ea1d8d939ad021f5fb0c48831d94e503fa..1cf41a54740de4641a74c672fa487f914156afef 100644 (file)
--- a/src/net/http/transport.go
+++ b/src/net/http/transport.go
@@ -1577,6 +1577,11 @@ func (pconn *persistConn) addTLS(ctx context.Context, name string, trace *httptr
        }()
        if err := <-errc; err != nil {
                plainConn.Close()
+               if err == (tlsHandshakeTimeoutError{}) {
+                       // Now that we have closed the connection,
+                       // wait for the call to HandshakeContext to return.
+                       <-errc
+               }
                if trace != nil && trace.TLSHandshakeDone != nil {
                        trace.TLSHandshakeDone(tls.ConnectionState{}, err)
                }