From f31a00de016a534cdb5e48c6767f82a7a27e1b07 Mon Sep 17 00:00:00 2001 From: Klaus Post Date: Fri, 13 Aug 2021 16:30:03 +0200 Subject: [PATCH] fix: http stats race in traffic metering (#12956) Traffic metering was not protected against concurrent updates. ``` WARNING: DATA RACE Read at 0x00c02b0dace8 by goroutine 235: github.com/minio/minio/cmd.setHTTPStatsHandler.func1() d:/minio/minio/cmd/generic-handlers.go:360 +0x27d net/http.HandlerFunc.ServeHTTP() ... Previous write at 0x00c02b0dace8 by goroutine 994: github.com/minio/minio/internal/http/stats.(*IncomingTrafficMeter).Read() d:/minio/minio/internal/http/stats/http-traffic-recorder.go:34 +0xd2 ``` --- cmd/http-stats.go | 8 ++++---- internal/http/stats/http-traffic-recorder.go | 18 ++++++++++-------- 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/cmd/http-stats.go b/cmd/http-stats.go index ca306908a..fe7d7998a 100644 --- a/cmd/http-stats.go +++ b/cmd/http-stats.go @@ -38,12 +38,12 @@ type ConnStats struct { } // Increase total input bytes -func (s *ConnStats) incInputBytes(n int) { +func (s *ConnStats) incInputBytes(n int64) { atomic.AddUint64(&s.totalInputBytes, uint64(n)) } // Increase total output bytes -func (s *ConnStats) incOutputBytes(n int) { +func (s *ConnStats) incOutputBytes(n int64) { atomic.AddUint64(&s.totalOutputBytes, uint64(n)) } @@ -58,12 +58,12 @@ func (s *ConnStats) getTotalOutputBytes() uint64 { } // Increase outbound input bytes -func (s *ConnStats) incS3InputBytes(n int) { +func (s *ConnStats) incS3InputBytes(n int64) { atomic.AddUint64(&s.s3InputBytes, uint64(n)) } // Increase outbound output bytes -func (s *ConnStats) incS3OutputBytes(n int) { +func (s *ConnStats) incS3OutputBytes(n int64) { atomic.AddUint64(&s.s3OutputBytes, uint64(n)) } diff --git a/internal/http/stats/http-traffic-recorder.go b/internal/http/stats/http-traffic-recorder.go index dbee1a3e7..e5cfe3cfb 100644 --- a/internal/http/stats/http-traffic-recorder.go +++ b/internal/http/stats/http-traffic-recorder.go @@ -20,37 +20,39 @@ package stats import ( "io" "net/http" + "sync/atomic" ) // IncomingTrafficMeter counts the incoming bytes from the underlying request.Body. type IncomingTrafficMeter struct { + countBytes int64 io.ReadCloser - countBytes int } // Read calls the underlying Read and counts the transferred bytes. func (r *IncomingTrafficMeter) Read(p []byte) (n int, err error) { n, err = r.ReadCloser.Read(p) - r.countBytes += n + atomic.AddInt64(&r.countBytes, int64(n)) + return n, err } // BytesCount returns the number of transferred bytes -func (r IncomingTrafficMeter) BytesCount() int { - return r.countBytes +func (r IncomingTrafficMeter) BytesCount() int64 { + return atomic.LoadInt64(&r.countBytes) } // OutgoingTrafficMeter counts the outgoing bytes through the responseWriter. type OutgoingTrafficMeter struct { + countBytes int64 // wrapper for underlying http.ResponseWriter. http.ResponseWriter - countBytes int } // Write calls the underlying write and counts the output bytes func (w *OutgoingTrafficMeter) Write(p []byte) (n int, err error) { n, err = w.ResponseWriter.Write(p) - w.countBytes += n + atomic.AddInt64(&w.countBytes, int64(n)) return n, err } @@ -60,6 +62,6 @@ func (w *OutgoingTrafficMeter) Flush() { } // BytesCount returns the number of transferred bytes -func (w OutgoingTrafficMeter) BytesCount() int { - return w.countBytes +func (w OutgoingTrafficMeter) BytesCount() int64 { + return atomic.LoadInt64(&w.countBytes) }