From 01e3b4fc6aa13e126f61782ad42aa51ba490b302 Mon Sep 17 00:00:00 2001
From: Brad Fitzpatrick <bradfitz@golang.org>
Date: Wed, 29 Jan 2014 11:23:45 +0100
Subject: [PATCH] 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       | 11 ++++++++
 src/pkg/net/http/transport_test.go | 43 ++++++++++++++++++++++++++++++
 2 files changed, 54 insertions(+)

diff --git a/src/pkg/net/http/transfer.go b/src/pkg/net/http/transfer.go
index 344b1ba242..2eec9d9abc 100644
--- a/src/pkg/net/http/transfer.go
+++ b/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
 }
 
diff --git a/src/pkg/net/http/transport_test.go b/src/pkg/net/http/transport_test.go
index cb54a7b419..21a1f114d3 100644
--- a/src/pkg/net/http/transport_test.go
+++ b/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)
-- 
2.48.1