oxy icon indicating copy to clipboard operation
oxy copied to clipboard

Data race in memmetrics/counter. Locking problem.

Open sbunce opened this issue 5 years ago • 1 comments

Showed up in our internal application. Counter is being incremented concurrently by multiple goroutines because it's not protected by a lock. I'll see if I can come up with a PR to fix this.

==================
WARNING: DATA RACE
Read at 0x00c0000c05d8 by goroutine 49:
  github.com/vulcand/oxy/memmetrics.(*RollingCounter).incBucketValue()
      /home/seth.bunce/go/src/<omit>/vendor/github.com/vulcand/oxy/memmetrics/counter.go:132 +0x1cd
  github.com/vulcand/oxy/memmetrics.(*RollingCounter).Inc()
      /home/seth.bunce/go/src/<omit>/vendor/github.com/vulcand/oxy/memmetrics/counter.go:120 +0x50
  github.com/vulcand/oxy/memmetrics.(*RTMetrics).Record()
      /home/seth.bunce/go/src/<omit>/vendor/github.com/vulcand/oxy/memmetrics/roundtrip.go:211 +0x58
  github.com/vulcand/oxy/cbreaker.(*CircuitBreaker).serve()
      /home/seth.bunce/go/src/<omit>/vendor/github.com/vulcand/oxy/cbreaker/cbreaker.go:177 +0x2cd
  github.com/vulcand/oxy/cbreaker.(*CircuitBreaker).ServeHTTP()
      /home/seth.bunce/go/src/<omit>/vendor/github.com/vulcand/oxy/cbreaker/cbreaker.go:124 +0x191
  robot.car/vault-guard/pkg/handler.(*Handler).metricMiddleWare.func1()
      /home/seth.bunce/go/src/<omit>/pkg/handler/metric.go:61 +0x158
  net/http.HandlerFunc.ServeHTTP()
      /home/seth.bunce/src/go/src/net/http/server.go:2042 +0x51
  net/http.serverHandler.ServeHTTP()
      /home/seth.bunce/src/go/src/net/http/server.go:2843 +0xca
  net/http.(*conn).serve()
      /home/seth.bunce/src/go/src/net/http/server.go:1925 +0x84c

Previous write at 0x00c0000c05d8 by goroutine 95:
  github.com/vulcand/oxy/memmetrics.(*RollingCounter).incBucketValue()
      /home/seth.bunce/go/src/<omit>/vendor/github.com/vulcand/oxy/memmetrics/counter.go:133 +0x1eb
  github.com/vulcand/oxy/memmetrics.(*RollingCounter).Inc()
      /home/seth.bunce/go/src/<omit>/vendor/github.com/vulcand/oxy/memmetrics/counter.go:120 +0x50
  github.com/vulcand/oxy/memmetrics.(*RTMetrics).Record()
      /home/seth.bunce/go/src/<omit>/vendor/github.com/vulcand/oxy/memmetrics/roundtrip.go:211 +0x58
  github.com/vulcand/oxy/cbreaker.(*CircuitBreaker).serve()
      /home/seth.bunce/go/src/<omit>/vendor/github.com/vulcand/oxy/cbreaker/cbreaker.go:177 +0x2cd
  github.com/vulcand/oxy/cbreaker.(*CircuitBreaker).ServeHTTP()
      /home/seth.bunce/go/src/<omit>/vendor/github.com/vulcand/oxy/cbreaker/cbreaker.go:124 +0x191
  robot.car/vault-guard/pkg/handler.(*Handler).metricMiddleWare.func1()
      /home/seth.bunce/go/src/<omit>/pkg/handler/metric.go:61 +0x158
  net/http.HandlerFunc.ServeHTTP()
      /home/seth.bunce/src/go/src/net/http/server.go:2042 +0x51
  net/http.serverHandler.ServeHTTP()
      /home/seth.bunce/src/go/src/net/http/server.go:2843 +0xca
  net/http.(*conn).serve()
      /home/seth.bunce/src/go/src/net/http/server.go:1925 +0x84c

Goroutine 49 (running) created at:
  net/http.(*Server).Serve()
      /home/seth.bunce/src/go/src/net/http/server.go:2969 +0x5d3
  net/http.(*Server).ListenAndServe()
      /home/seth.bunce/src/go/src/net/http/server.go:2866 +0x105

Goroutine 95 (running) created at:
  net/http.(*Server).Serve()
      /home/seth.bunce/src/go/src/net/http/server.go:2969 +0x5d3
  net/http.(*Server).ListenAndServe()
      /home/seth.bunce/src/go/src/net/http/server.go:2866 +0x105
==================

sbunce avatar Nov 05 '20 19:11 sbunce

PR to fix the data race and simplify the locking in the memmetrics package. https://github.com/vulcand/oxy/pull/209

sbunce avatar Nov 05 '20 23:11 sbunce