1
0
mirror of https://github.com/golang/go synced 2024-11-23 16:00:06 -07:00

net/http: don't re-use Transport connections if we've seen an EOF

This the second part of making persistent HTTPS connections to
certain servers (notably Amazon) robust.

See the story in part 1: https://golang.org/cl/76400046/

This is the http Transport change that notes whether our
net.Conn.Read has ever seen an EOF. If it has, then we use
that as an additional signal to not re-use that connection (in
addition to the HTTP response headers)

Fixes #3514

LGTM=rsc
R=agl, rsc
CC=golang-codereviews
https://golang.org/cl/79240044
This commit is contained in:
Brad Fitzpatrick 2014-03-25 10:59:09 -07:00
parent f61f18d694
commit cc2c5fc3d2
2 changed files with 84 additions and 1 deletions

View File

@ -588,7 +588,7 @@ func (t *Transport) dialConn(cm connectMethod) (*persistConn, error) {
pconn.conn = tlsConn
}
pconn.br = bufio.NewReader(pconn.conn)
pconn.br = bufio.NewReader(noteEOFReader{pconn.conn, &pconn.sawEOF})
pconn.bw = bufio.NewWriter(pconn.conn)
go pconn.readLoop()
go pconn.writeLoop()
@ -723,6 +723,7 @@ type persistConn struct {
tlsState *tls.ConnectionState
closed bool // whether conn has been closed
br *bufio.Reader // from conn
sawEOF bool // whether we've seen EOF from conn; owned by readLoop
bw *bufio.Writer // to conn
reqch chan requestAndChan // written by roundTrip; read by readLoop
writech chan writeRequest // written by roundTrip; read by writeLoop
@ -841,6 +842,9 @@ func (pc *persistConn) readLoop() {
if err != nil {
alive1 = false
}
if alive1 && pc.sawEOF {
alive1 = false
}
if alive1 && !pc.t.putIdleConn(pc) {
alive1 = false
}
@ -1134,3 +1138,16 @@ type tlsHandshakeTimeoutError struct{}
func (tlsHandshakeTimeoutError) Timeout() bool { return true }
func (tlsHandshakeTimeoutError) Temporary() bool { return true }
func (tlsHandshakeTimeoutError) Error() string { return "net/http: TLS handshake timeout" }
type noteEOFReader struct {
r io.Reader
sawEOF *bool
}
func (nr noteEOFReader) Read(p []byte) (n int, err error) {
n, err = nr.r.Read(p)
if err == io.EOF {
*nr.sawEOF = true
}
return
}

View File

@ -11,6 +11,7 @@ import (
"bytes"
"compress/gzip"
"crypto/rand"
"crypto/tls"
"errors"
"fmt"
"io"
@ -1836,6 +1837,71 @@ func TestTransportTLSHandshakeTimeout(t *testing.T) {
}
}
// Trying to repro golang.org/issue/3514
func TestTLSServerClosesConnection(t *testing.T) {
defer afterTest(t)
closedc := make(chan bool, 1)
ts := httptest.NewTLSServer(HandlerFunc(func(w ResponseWriter, r *Request) {
if strings.Contains(r.URL.Path, "/keep-alive-then-die") {
conn, _, _ := w.(Hijacker).Hijack()
conn.Write([]byte("HTTP/1.1 200 OK\r\nContent-Length: 3\r\n\r\nfoo"))
conn.Close()
closedc <- true
return
}
fmt.Fprintf(w, "hello")
}))
defer ts.Close()
tr := &Transport{
TLSClientConfig: &tls.Config{
InsecureSkipVerify: true,
},
}
defer tr.CloseIdleConnections()
client := &Client{Transport: tr}
var nSuccess = 0
var errs []error
const trials = 20
for i := 0; i < trials; i++ {
tr.CloseIdleConnections()
res, err := client.Get(ts.URL + "/keep-alive-then-die")
if err != nil {
t.Fatal(err)
}
<-closedc
slurp, err := ioutil.ReadAll(res.Body)
if err != nil {
t.Fatal(err)
}
if string(slurp) != "foo" {
t.Errorf("Got %q, want foo", slurp)
}
// Now try again and see if we successfully
// pick a new connection.
res, err = client.Get(ts.URL + "/")
if err != nil {
errs = append(errs, err)
continue
}
slurp, err = ioutil.ReadAll(res.Body)
if err != nil {
errs = append(errs, err)
continue
}
nSuccess++
}
if nSuccess > 0 {
t.Logf("successes = %d of %d", nSuccess, trials)
} else {
t.Errorf("All runs failed:")
}
for _, err := range errs {
t.Logf(" err: %v", err)
}
}
func newLocalListener(t *testing.T) net.Listener {
ln, err := net.Listen("tcp", "127.0.0.1:0")
if err != nil {