From: Brad Fitzpatrick <bradfitz@golang.org>
Date: Tue, 5 Feb 2013 04:26:25 +0000 (-0800)
Subject: net/http: fix Server blocking after a Handler's Write fails
X-Git-Tag: go1.1rc2~1139
X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=d1e16d06b4df98930b5c6b0775cdd414dfdebd50;p=gostls13.git

net/http: fix Server blocking after a Handler's Write fails

If a Handle's Write to a ResponseWriter fails (e.g. via a
net.Conn WriteDeadline via WriteTimeout on the Server), the
Server was blocking forever waiting for reads on that
net.Conn, even after a Write failed.

Instead, once we see a Write fail, close the connection,
since it's then dead to us anyway.

Fixes #4741

R=golang-dev, adg
CC=golang-dev
https://golang.org/cl/7301043
---

diff --git a/src/pkg/net/http/serve_test.go b/src/pkg/net/http/serve_test.go
index 6c97eaf637..dc07a8969d 100644
--- a/src/pkg/net/http/serve_test.go
+++ b/src/pkg/net/http/serve_test.go
@@ -326,6 +326,66 @@ func TestServerTimeouts(t *testing.T) {
 	}
 }
 
+// golang.org/issue/4741 -- setting only a write timeout that triggers
+// shouldn't cause a handler to block forever on reads (next HTTP
+// request) that will never happen.
+func TestOnlyWriteTimeout(t *testing.T) {
+	var conn net.Conn
+	var afterTimeoutErrc = make(chan error, 1)
+	ts := httptest.NewUnstartedServer(HandlerFunc(func(w ResponseWriter, req *Request) {
+		buf := make([]byte, 512<<10)
+		_, err := w.Write(buf)
+		if err != nil {
+			t.Errorf("handler Write error: %v", err)
+			return
+		}
+		conn.SetWriteDeadline(time.Now().Add(-30 * time.Second))
+		_, err = w.Write(buf)
+		afterTimeoutErrc <- err
+	}))
+	ts.Listener = trackLastConnListener{ts.Listener, &conn}
+	ts.Start()
+	defer ts.Close()
+
+	tr := &Transport{DisableKeepAlives: false}
+	defer tr.CloseIdleConnections()
+	c := &Client{Transport: tr}
+
+	errc := make(chan error)
+	go func() {
+		res, err := c.Get(ts.URL)
+		if err != nil {
+			errc <- err
+			return
+		}
+		_, err = io.Copy(ioutil.Discard, res.Body)
+		errc <- err
+	}()
+	select {
+	case err := <-errc:
+		if err == nil {
+			t.Errorf("expected an error from Get request")
+		}
+	case <-time.After(5 * time.Second):
+		t.Fatal("timeout waiting for Get error")
+	}
+	if err := <-afterTimeoutErrc; err == nil {
+		t.Error("expected write error after timeout")
+	}
+}
+
+// trackLastConnListener tracks the last net.Conn that was accepted.
+type trackLastConnListener struct {
+	net.Listener
+	last *net.Conn // destination
+}
+
+func (l trackLastConnListener) Accept() (c net.Conn, err error) {
+	c, err = l.Listener.Accept()
+	*l.last = c
+	return
+}
+
 // TestIdentityResponse verifies that a handler can unset
 func TestIdentityResponse(t *testing.T) {
 	handler := HandlerFunc(func(rw ResponseWriter, req *Request) {
diff --git a/src/pkg/net/http/server.go b/src/pkg/net/http/server.go
index e70d129e7e..a965a0e9f9 100644
--- a/src/pkg/net/http/server.go
+++ b/src/pkg/net/http/server.go
@@ -223,6 +223,7 @@ func (cw *chunkWriter) Write(p []byte) (n int, err error) {
 	if cw.chunking {
 		_, err = fmt.Fprintf(cw.res.conn.buf, "%x\r\n", len(p))
 		if err != nil {
+			cw.res.conn.rwc.Close()
 			return
 		}
 	}
@@ -230,6 +231,9 @@ func (cw *chunkWriter) Write(p []byte) (n int, err error) {
 	if cw.chunking && err == nil {
 		_, err = cw.res.conn.buf.Write(crlf)
 	}
+	if err != nil {
+		cw.res.conn.rwc.Close()
+	}
 	return
 }