net/http: reuse client connections earlier when Content-Length is set

Set EOF on the final Read of a body with a Content-Length, which
will cause clients to recycle their connection immediately upon
the final Read, rather than waiting for another Read or Close
(neither of which might come).  This happens often when client
code is simply something like:

  err := json.NewDecoder(resp.Body).Decode(&dest)
  ...

Then there's usually no subsequent Read. Even if the client
calls Close (which they should): in Go 1.1, the body was
slurped to EOF, but in Go 1.2, that was then treated as a
Close-before-EOF and the underlying connection was closed.
But that's assuming the user even calls Close. Many don't.
Reading to EOF also causes a connection be reused. Now the EOF
arrives earlier.

This CL only addresses the Content-Length case. A future CL
will address the chunked case.

LGTM=adg
R=adg
CC=golang-codereviews
https://golang.org/cl/49570044

src/pkg/net/http/transfer.go

@@ -559,6 +559,17 @@ func (b *body) readLocked(p []byte) (n int, err error) {
 		}
 	}
 
+	// If we can return an EOF here along with the read data, do
+	// so. This is optional per the io.Reader contract, but doing
+	// so helps the HTTP transport code recycle its connection
+	// earlier (since it will see this EOF itself), even if the
+	// client doesn't do future reads or Close.
+	if err == nil && n > 0 {
+		if lr, ok := b.src.(*io.LimitedReader); ok && lr.N == 0 {
+			err = io.EOF
+		}
+	}
+
 	return n, err
 }
 

src/pkg/net/http/transport_test.go

@@ -271,6 +271,49 @@ func TestTransportIdleCacheKeys(t *testing.T) {
 	}
 }
 
+// Tests that the HTTP transport re-uses connections when a client
+// reads to the end of a response Body without closing it.
+func TestTransportReadToEndReusesConn(t *testing.T) {
+	defer afterTest(t)
+	const msg = "foobar"
+
+	addrSeen := make(map[string]int)
+	ts := httptest.NewServer(HandlerFunc(func(w ResponseWriter, r *Request) {
+		addrSeen[r.RemoteAddr]++
+		w.Header().Set("Content-Type", strconv.Itoa(len(msg)))
+		w.WriteHeader(200)
+		w.Write([]byte(msg))
+	}))
+	defer ts.Close()
+
+	buf := make([]byte, len(msg))
+
+	for i := 0; i < 3; i++ {
+		res, err := http.Get(ts.URL)
+		if err != nil {
+			t.Errorf("Get: %v", err)
+			continue
+		}
+		// We want to close this body eventually (before the
+		// defer afterTest at top runs), but not before the
+		// len(addrSeen) check at the bottom of this test,
+		// since Closing this early in the loop would risk
+		// making connections be re-used for the wrong reason.
+		defer res.Body.Close()
+
+		if res.ContentLength != int64(len(msg)) {
+			t.Errorf("res.ContentLength = %d; want %d", res.ContentLength, len(msg))
+		}
+		n, err := res.Body.Read(buf)
+		if n != len(msg) || err != io.EOF {
+			t.Errorf("Read = %v, %v; want 6, EOF", n, err)
+		}
+	}
+	if len(addrSeen) != 1 {
+		t.Errorf("server saw %d distinct client addresses; want 1", len(addrSeen))
+	}
+}
+
 func TestTransportMaxPerHostIdleConns(t *testing.T) {
 	defer afterTest(t)
 	resch := make(chan string)