From c870d56f98eab5370726afd223fe0ab14d9e88ab Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Tue, 9 Oct 2018 16:01:20 -0700 Subject: [PATCH] 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 TryBot-Result: Gobot Gobot Reviewed-by: Dmitri Shuralyov --- src/net/http/httputil/reverseproxy.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/net/http/httputil/reverseproxy.go b/src/net/http/httputil/reverseproxy.go index 1efcbd3bbc..f82d820a43 100644 --- 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() } -- 2.50.0