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 af33b5e10b7e07001686e7e169ae7574ee538ce9..90e878e2b641b8bf078e7bdfe0147938f64eb0c8 100644 (file)
--- 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 3b80d45fdd3246bcd013272c5fcdd88b850849ab..7ebd8575f3beb7d1640b14451e8c0cfb12fe2aa1 100644 (file)
--- 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 {