net/http/httputil: fix race/crash in previous ReverseProxy change

The previous ReverseProxy change, CL 137335, introduced a bug which could cause
a race and/or a crash.

This reliably crashed before:

$ go test -short -race -v -run=TestReverseProxyFlushInterval -count=20 net/http/httputil

The problem was a goroutine was running http.ResponseWriter.Flush
after the http.Handler's ServeHTTP completed. There was code to
prevent that (a deferred stop call) but the stop call didn't consider
the case where time.AfterFunc had already fired off a new goroutine
but that goroutine hadn't yet scheduled.

Change-Id: I06357908465a3b953efc33e63c70dec19a501adf
Reviewed-on: https://go-review.googlesource.com/c/140977
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>

diff --git a/src/net/http/httputil/reverseproxy.go b/src/net/http/httputil/reverseproxy.go
index 1efcbd3bbce643384b38a21aa0096c38daa5ccf4..f82d820a43007d614feec03398f06d25c49b9f9a 100644 (file)
--- a/src/net/http/httputil/reverseproxy.go
+++ b/src/net/http/httputil/reverseproxy.go
@@ -448,6 +448,9 @@ func (m *maxLatencyWriter) Write(p []byte) (n int, err error) {
 func (m *maxLatencyWriter) delayedFlush() {
        m.mu.Lock()
        defer m.mu.Unlock()
+       if !m.flushPending { // if stop was called but AfterFunc already started this goroutine
+               return
+       }
        m.dst.Flush()
        m.flushPending = false
 }
@@ -455,6 +458,7 @@ func (m *maxLatencyWriter) delayedFlush() {
 func (m *maxLatencyWriter) stop() {
        m.mu.Lock()
        defer m.mu.Unlock()
+       m.flushPending = false
        if m.t != nil {
                m.t.Stop()
        }