search

swift-server / swift-prometheus

Prometheus client library for Swift
https://swiftpackageindex.com/swift-server/swift-prometheus
Apache License 2.0
145 stars 30 forks source link

Fix Histogram Concurrency Issue #78

Closed Sam-Amin closed 2 years ago

Sam-Amin commented 2 years ago

Issue: In Histogram, some values were being dropped when the histogram is accessed concurrently by multiple threads.

Fix: In getOrCreateHistogram, the code was copying the dictionary into a local variable, and checking it for the label without a lock. If not there, it would take the lock again to add it. But in line 224, after taking the lock again, it was rechecking the local variable, which of course would not have changed in the meantime. But the original dictionary could have changed, and that's the one we should check the second time.

Improved the concurrency test, as it was missing the bug by only checking the total count and sum without labels.

ktoso commented 2 years ago

Nice catch, thank you @Sam-Amin !

Sorry you've hit this but very thankful for the PR :)

ktoso commented 2 years ago

I'll cut a 1.0.1 with this