Fix Family::get_or_create

The method should not overwrite an existing key after it obtains the write lock.

Consider the following scenario with `Family<LabelSet, Gauge>` used by threads A and B:

1. A and B both call `family.get_or_insert(&label_set)`
2. A and B both acquire read lock and finds no key
3. A gets write lock and inserts a new gauge with value 0
4. A increments returned gauge to 1
5. B gets write lock and inserts a new gauge with value 0
6. B increments returned gauge to 1
7. A decrements gauge back to 0
8. B decrements gauge which now overflows to `u64::MAX`

Signed-off-by: Anthony Ramine <[email protected]>

Author: nox

CHANGELOG.md

@@ -18,9 +18,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 - Move`Encode` trait from `prometheus_client::encoding::text` to `prometheus_client::encoding`. See [PR 83].
 
+### Fixed
+
+- Fix race condition in `Family::get_or_create`. See [PR 102].
+
 [PR 83]: https://github.com/prometheus/client_rust/pull/83
 [PR 85]: https://github.com/prometheus/client_rust/pull/85
 [PR 96]: https://github.com/prometheus/client_rust/pull/96
+[PR 102]: https://github.com/prometheus/client_rust/pull/102
 
 ## [0.18.0]
 

src/metrics/family.rs

@@ -216,22 +216,18 @@ impl<S: Clone + std::hash::Hash + Eq, M, C: MetricConstructor<M>> Family<S, M, C
     /// family.get_or_create(&vec![("method".to_owned(), "GET".to_owned())]).inc();
     /// ```
     pub fn get_or_create(&self, label_set: &S) -> MappedRwLockReadGuard<M> {
-        if let Ok(metric) =
-            RwLockReadGuard::try_map(self.metrics.read(), |metrics| metrics.get(label_set))
-        {
-            return metric;
-        }
-
-        let mut write_guard = self.metrics.write();
-        write_guard.insert(label_set.clone(), self.constructor.new_metric());
-
-        drop(write_guard);
+        loop {
+            if let Ok(metric) =
+                RwLockReadGuard::try_map(self.metrics.read(), |metrics| metrics.get(label_set))
+            {
+                return metric;
+            }
 
-        RwLockReadGuard::map(self.metrics.read(), |metrics| {
-            metrics
-                .get(label_set)
-                .expect("Metric to exist after creating it.")
-        })
+            self.metrics
+                .write()
+                .entry(label_set.clone())
+                .or_insert_with(|| self.constructor.new_metric());
+        }
     }
 
     /// Remove a label set from the metric family.