hashicorp / go-metrics

A Golang library for exporting performance and runtime metrics to external metrics systems (i.e. statsite, statsd)
MIT License
1.46k stars 177 forks source link

InmemSink: create a new RWMutex , because they are not safe to copy #124

Closed dnephin closed 3 years ago

dnephin commented 3 years ago

We noticed some Consul tests deadlock and timeout when run with the race detector (example). I tracked down the problem to this line in InmemSink.Data.

In 1d3de9f7002ebf98cf5c2a86cd03b0dbf1f8e885 there was a change made to fix some data races in this method. It looks like the RWMutex field was missed, possibly because the issue isn't a data race itself, so isn't reported by the race detector.

RWMutex are not safe for copying, so we need to create a new instance on the copy of IntervalMetrics.

Before this change I was able to pretty reliable cause the test to deadlock in under 200 runs. After this change the test no longer deadlocks after 1000+ runs. I haven't looked at the RWMutex internals, but I assume the docs mention they can not be copied for this reason.