*: Move to parking_lot and thus make owning_ref obsolete (prometh…

…eus#78)

Before `proemtheus-client` would use the `owning_ref` crate to map the target
of a `std::sync::RwLockReadGuard`. `owning_ref` has multiple unsoundness
issues, see https://rustsec.org/advisories/RUSTSEC-2022-0040.html. Instead of
replacing `owning_ref` with a similar crate, we switch to locking via
`parking_lot` which supports the above mapping natively.

Signed-off-by: Max Inden <[email protected]>

CHANGELOG.md

@@ -4,6 +4,18 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [0.18.0] - unreleased
+
+### Changed
+
+- Use `parking_lot` instead of `std::sync::*`.
+
+  Before `proemtheus-client` would use the `owning_ref` crate to map the target
+  of a `std::sync::RwLockReadGuard`. `owning_ref` has multiple unsoundness
+  issues, see https://rustsec.org/advisories/RUSTSEC-2022-0040.html. Instead of
+  replacing `owning_ref` with a similar crate, we switch to locking via
+  `parking_lot` which supports the above mapping natively.
+
 ## [0.17.0]
 
 ### Changed

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "prometheus-client"
-version = "0.17.0"
+version = "0.18.0"
 authors = ["Max Inden <[email protected]>"]
 edition = "2021"
 description = "Open Metrics client library allowing users to natively instrument applications."
@@ -19,7 +19,7 @@ members = ["derive-text-encode", "derive-proto-encode"]
 [dependencies]
 dtoa = "1.0"
 itoa = "1.0"
-owning_ref = "0.4"
+parking_lot = "0.12"
 prometheus-client-derive-proto-encode = { version = "0.1.0", path = "derive-proto-encode", optional = true }
 prometheus-client-derive-text-encode = { version = "0.3.0", path = "derive-text-encode" }
 prost = { version = "0.9.0", optional = true }

src/encoding/text.rs

@@ -436,7 +436,7 @@ where
 {
     fn encode(&self, encoder: Encoder) -> Result<(), std::io::Error> {
         let (value, exemplar) = self.get();
-        encode_counter_with_maybe_exemplar(value, exemplar.as_ref().as_ref(), encoder)
+        encode_counter_with_maybe_exemplar(value, exemplar.as_ref(), encoder)
     }
 
     fn metric_type(&self) -> MetricType {

src/metrics/exemplar.rs

@@ -4,13 +4,13 @@
 
 use super::counter::{self, Counter};
 use super::histogram::Histogram;
-use owning_ref::OwningRef;
+use parking_lot::{MappedRwLockReadGuard, RwLock, RwLockReadGuard};
 use std::collections::HashMap;
 #[cfg(any(target_arch = "mips", target_arch = "powerpc"))]
 use std::sync::atomic::AtomicU32;
 #[cfg(not(any(target_arch = "mips", target_arch = "powerpc")))]
 use std::sync::atomic::AtomicU64;
-use std::sync::{Arc, RwLock, RwLockReadGuard};
+use std::sync::Arc;
 
 #[derive(Debug)]
 pub struct Exemplar<S, V> {
@@ -74,7 +74,7 @@ impl<S, N: Clone, A: counter::Atomic<N>> CounterWithExemplar<S, N, A> {
     /// Increase the [`CounterWithExemplar`] by `v`, updating the [`Exemplar`]
     /// if a label set is provided, returning the previous value.
     pub fn inc_by(&self, v: N, label_set: Option<S>) -> N {
-        let mut inner = self.inner.write().expect("Lock not to be poisoned.");
+        let mut inner = self.inner.write();
 
         inner.exemplar = label_set.map(|label_set| Exemplar {
             label_set,
@@ -86,10 +86,10 @@ impl<S, N: Clone, A: counter::Atomic<N>> CounterWithExemplar<S, N, A> {
 
     /// Get the current value of the [`CounterWithExemplar`] as well as its
     /// [`Exemplar`] if any.
-    pub fn get(&self) -> (N, RwLockGuardedCounterWithExemplar<S, N, A>) {
-        let inner = self.inner.read().expect("Lock not to be poisoned.");
+    pub fn get(&self) -> (N, MappedRwLockReadGuard<Option<Exemplar<S, N>>>) {
+        let inner = self.inner.read();
         let value = inner.counter.get();
-        let exemplar = OwningRef::new(inner).map(|inner| &inner.exemplar);
+        let exemplar = RwLockReadGuard::map(inner, |inner| &inner.exemplar);
         (value, exemplar)
     }
 
@@ -101,15 +101,11 @@ impl<S, N: Clone, A: counter::Atomic<N>> CounterWithExemplar<S, N, A> {
     /// The caller of this function has to uphold the property of an Open
     /// Metrics counter namely that the value is monotonically increasing, i.e.
     /// either stays the same or increases.
-    pub fn inner(&self) -> OwningRef<RwLockReadGuard<CounterWithExemplarInner<S, N, A>>, A> {
-        OwningRef::new(self.inner.read().expect("Lock not to be poisoned."))
-            .map(|inner| inner.counter.inner())
+    pub fn inner(&self) -> MappedRwLockReadGuard<A> {
+        RwLockReadGuard::map(self.inner.read(), |inner| inner.counter.inner())
     }
 }
 
-type RwLockGuardedCounterWithExemplar<'a, S, N, A> =
-    OwningRef<RwLockReadGuard<'a, CounterWithExemplarInner<S, N, A>>, Option<Exemplar<S, N>>>;
-
 /////////////////////////////////////////////////////////////////////////////////
 // Histogram
 
@@ -153,7 +149,7 @@ impl<S> HistogramWithExemplars<S> {
     }
 
     pub fn observe(&self, v: f64, label_set: Option<S>) {
-        let mut inner = self.inner.write().expect("Lock not to be poisoned.");
+        let mut inner = self.inner.write();
         let bucket = inner.histogram.observe_and_bucket(v);
         if let (Some(bucket), Some(label_set)) = (bucket, label_set) {
             inner.exemplars.insert(
@@ -167,6 +163,6 @@ impl<S> HistogramWithExemplars<S> {
     }
 
     pub(crate) fn inner(&self) -> RwLockReadGuard<HistogramWithExemplarsInner<S>> {
-        self.inner.read().expect("Lock not to be poisoned.")
+        self.inner.read()
     }
 }

src/metrics/family.rs

@@ -3,9 +3,9 @@
 //! See [`Family`] for details.
 
 use super::{MetricType, TypedMetric};
-use owning_ref::OwningRef;
+use parking_lot::{MappedRwLockReadGuard, RwLock, RwLockReadGuard};
 use std::collections::HashMap;
-use std::sync::{Arc, RwLock, RwLockReadGuard};
+use std::sync::Arc;
 
 /// Representation of the OpenMetrics *MetricFamily* data type.
 ///
@@ -214,29 +214,27 @@ impl<S: Clone + std::hash::Hash + Eq, M, C: MetricConstructor<M>> Family<S, M, C
     /// // calls.
     /// family.get_or_create(&vec![("method".to_owned(), "GET".to_owned())]).inc();
     /// ```
-    pub fn get_or_create(&self, label_set: &S) -> OwningRef<RwLockReadGuard<HashMap<S, M>>, M> {
-        let read_guard = self.metrics.read().expect("Lock not to be poisoned.");
+    pub fn get_or_create(&self, label_set: &S) -> MappedRwLockReadGuard<M> {
         if let Ok(metric) =
-            OwningRef::new(read_guard).try_map(|metrics| metrics.get(label_set).ok_or(()))
+            RwLockReadGuard::try_map(self.metrics.read(), |metrics| metrics.get(label_set))
         {
             return metric;
         }
 
-        let mut write_guard = self.metrics.write().unwrap();
+        let mut write_guard = self.metrics.write();
         write_guard.insert(label_set.clone(), self.constructor.new_metric());
 
         drop(write_guard);
 
-        let read_guard = self.metrics.read().unwrap();
-        OwningRef::new(read_guard).map(|metrics| {
+        RwLockReadGuard::map(self.metrics.read(), |metrics| {
             metrics
                 .get(label_set)
                 .expect("Metric to exist after creating it.")
         })
     }
 
     pub(crate) fn read(&self) -> RwLockReadGuard<HashMap<S, M>> {
-        self.metrics.read().unwrap()
+        self.metrics.read()
     }
 }
 

src/metrics/histogram.rs

@@ -3,9 +3,9 @@
 //! See [`Histogram`] for details.
 
 use super::{MetricType, TypedMetric};
-use owning_ref::OwningRef;
+use parking_lot::{MappedRwLockReadGuard, RwLock, RwLockReadGuard};
 use std::iter::{self, once};
-use std::sync::{Arc, Mutex, MutexGuard};
+use std::sync::Arc;
 
 /// Open Metrics [`Histogram`] to measure distributions of discrete events.
 ///
@@ -33,7 +33,7 @@ use std::sync::{Arc, Mutex, MutexGuard};
 // https://github.com/tikv/rust-prometheus/pull/314.
 #[derive(Debug)]
 pub struct Histogram {
-    inner: Arc<Mutex<Inner>>,
+    inner: Arc<RwLock<Inner>>,
 }
 
 impl Clone for Histogram {
@@ -56,7 +56,7 @@ pub(crate) struct Inner {
 impl Histogram {
     pub fn new(buckets: impl Iterator<Item = f64>) -> Self {
         Self {
-            inner: Arc::new(Mutex::new(Inner {
+            inner: Arc::new(RwLock::new(Inner {
                 sum: Default::default(),
                 count: Default::default(),
                 buckets: buckets
@@ -78,7 +78,7 @@ impl Histogram {
     /// Needed in
     /// [`HistogramWithExemplars`](crate::metrics::exemplar::HistogramWithExemplars).
     pub(crate) fn observe_and_bucket(&self, v: f64) -> Option<usize> {
-        let mut inner = self.inner.lock().unwrap();
+        let mut inner = self.inner.write();
         inner.sum += v;
         inner.count += 1;
 
@@ -97,17 +97,15 @@ impl Histogram {
         }
     }
 
-    pub(crate) fn get(&self) -> (f64, u64, MutexGuardedBuckets) {
-        let inner = self.inner.lock().unwrap();
+    pub(crate) fn get(&self) -> (f64, u64, MappedRwLockReadGuard<Vec<(f64, u64)>>) {
+        let inner = self.inner.read();
         let sum = inner.sum;
         let count = inner.count;
-        let buckets = OwningRef::new(inner).map(|inner| &inner.buckets);
+        let buckets = RwLockReadGuard::map(inner, |inner| &inner.buckets);
         (sum, count, buckets)
     }
 }
 
-pub(crate) type MutexGuardedBuckets<'a> = OwningRef<MutexGuard<'a, Inner>, Vec<(f64, u64)>>;
-
 impl TypedMetric for Histogram {
     const TYPE: MetricType = MetricType::Histogram;
 }