Commit c870d56f authored by Brad Fitzpatrick's avatar Brad Fitzpatrick

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: 's avatarDmitri Shuralyov <dmitshur@golang.org>
parent 95d06ab6
...@@ -448,6 +448,9 @@ func (m *maxLatencyWriter) Write(p []byte) (n int, err error) { ...@@ -448,6 +448,9 @@ func (m *maxLatencyWriter) Write(p []byte) (n int, err error) {
func (m *maxLatencyWriter) delayedFlush() { func (m *maxLatencyWriter) delayedFlush() {
m.mu.Lock() m.mu.Lock()
defer m.mu.Unlock() defer m.mu.Unlock()
if !m.flushPending { // if stop was called but AfterFunc already started this goroutine
return
}
m.dst.Flush() m.dst.Flush()
m.flushPending = false m.flushPending = false
} }
...@@ -455,6 +458,7 @@ func (m *maxLatencyWriter) delayedFlush() { ...@@ -455,6 +458,7 @@ func (m *maxLatencyWriter) delayedFlush() {
func (m *maxLatencyWriter) stop() { func (m *maxLatencyWriter) stop() {
m.mu.Lock() m.mu.Lock()
defer m.mu.Unlock() defer m.mu.Unlock()
m.flushPending = false
if m.t != nil { if m.t != nil {
m.t.Stop() m.t.Stop()
} }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment