From: John Newlin <jnewlin@google.com>
Date: Thu, 26 Dec 2013 19:52:14 +0000 (-0800)
Subject: net/http: Release reference to chunkWriter's bufio.Writer on hijack
X-Git-Tag: go1.3beta1~1109
X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=92b5e16147b26dcea216b48b380566b900b4916b;p=gostls13.git

net/http: Release reference to chunkWriter's bufio.Writer on hijack

When a connection is hijacked, release the reference to the bufio.Writer
that is used with the chunkWriter.  The chunkWriter is not used after
the connection is hijacked.

Also add a test to check that double Hijack calls do something sensible.

benchmark                old ns/op    new ns/op    delta
BenchmarkServerHijack        24137        20629  -14.53%

benchmark               old allocs   new allocs    delta
BenchmarkServerHijack           21           19   -9.52%

benchmark                old bytes    new bytes    delta
BenchmarkServerHijack        11774         9667  -17.90%

R=bradfitz, dave, chris.cahoon
CC=golang-codereviews
https://golang.org/cl/39440044
---

diff --git a/src/pkg/net/http/serve_test.go b/src/pkg/net/http/serve_test.go
index af33b5e10b..90e878e2b6 100644
--- a/src/pkg/net/http/serve_test.go
+++ b/src/pkg/net/http/serve_test.go
@@ -1934,6 +1934,31 @@ func TestWriteAfterHijack(t *testing.T) {
 	}
 }
 
+func TestDoubleHijack(t *testing.T) {
+	req := reqBytes("GET / HTTP/1.1\nHost: golang.org")
+	var buf bytes.Buffer
+	conn := &rwTestConn{
+		Reader: bytes.NewReader(req),
+		Writer: &buf,
+		closec: make(chan bool, 1),
+	}
+	handler := HandlerFunc(func(rw ResponseWriter, r *Request) {
+		conn, _, err := rw.(Hijacker).Hijack()
+		if err != nil {
+			t.Error(err)
+			return
+		}
+		_, _, err = rw.(Hijacker).Hijack()
+		if err == nil {
+			t.Errorf("got err = nil;  want err != nil")
+		}
+		conn.Close()
+	})
+	ln := &oneConnListener{conn: conn}
+	go Serve(ln, handler)
+	<-conn.closec
+}
+
 // http://code.google.com/p/go/issues/detail?id=5955
 // Note that this does not test the "request too large"
 // exit path from the http server. This is intentional;
diff --git a/src/pkg/net/http/server.go b/src/pkg/net/http/server.go
index 3b80d45fdd..7ebd8575f3 100644
--- a/src/pkg/net/http/server.go
+++ b/src/pkg/net/http/server.go
@@ -1198,7 +1198,14 @@ func (w *response) Hijack() (rwc net.Conn, buf *bufio.ReadWriter, err error) {
 	if w.wroteHeader {
 		w.cw.flush()
 	}
-	return w.conn.hijack()
+	// Release the bufioWriter that writes to the chunk writer, it is not
+	// used after a connection has been hijacked.
+	rwc, buf, err = w.conn.hijack()
+	if err == nil {
+		putBufioWriter(w.w)
+		w.w = nil
+	}
+	return rwc, buf, err
 }
 
 func (w *response) CloseNotify() <-chan bool {