From 038fc2a6544df8ba4bff04ff1e221c109f235bc0 Mon Sep 17 00:00:00 2001 From: Max Inden Date: Tue, 16 Aug 2022 05:18:52 +0200 Subject: [PATCH] *: Move to `parking_lot` and thus make `owning_ref` obsolete (#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 --- CHANGELOG.md | 12 ++++++++++++ Cargo.toml | 4 ++-- src/encoding/text.rs | 2 +- src/metrics/exemplar.rs | 24 ++++++++++-------------- src/metrics/family.rs | 16 +++++++--------- src/metrics/histogram.rs | 18 ++++++++---------- 6 files changed, 40 insertions(+), 36 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d881544c..1fb69ffc 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/Cargo.toml b/Cargo.toml index 7cc00f35..8875fd4d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "prometheus-client" -version = "0.17.0" +version = "0.18.0" authors = ["Max Inden "] 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 } diff --git a/src/encoding/text.rs b/src/encoding/text.rs index edb03c1c..c3e774a2 100644 --- a/src/encoding/text.rs +++ b/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 { diff --git a/src/metrics/exemplar.rs b/src/metrics/exemplar.rs index 485712f6..b08fdc39 100644 --- a/src/metrics/exemplar.rs +++ b/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 { @@ -74,7 +74,7 @@ impl> CounterWithExemplar { /// 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) -> 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> CounterWithExemplar { /// Get the current value of the [`CounterWithExemplar`] as well as its /// [`Exemplar`] if any. - pub fn get(&self) -> (N, RwLockGuardedCounterWithExemplar) { - let inner = self.inner.read().expect("Lock not to be poisoned."); + pub fn get(&self) -> (N, MappedRwLockReadGuard>>) { + 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> CounterWithExemplar { /// 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>, A> { - OwningRef::new(self.inner.read().expect("Lock not to be poisoned.")) - .map(|inner| inner.counter.inner()) + pub fn inner(&self) -> MappedRwLockReadGuard { + RwLockReadGuard::map(self.inner.read(), |inner| inner.counter.inner()) } } -type RwLockGuardedCounterWithExemplar<'a, S, N, A> = - OwningRef>, Option>>; - ///////////////////////////////////////////////////////////////////////////////// // Histogram @@ -153,7 +149,7 @@ impl HistogramWithExemplars { } pub fn observe(&self, v: f64, label_set: Option) { - 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 HistogramWithExemplars { } pub(crate) fn inner(&self) -> RwLockReadGuard> { - self.inner.read().expect("Lock not to be poisoned.") + self.inner.read() } } diff --git a/src/metrics/family.rs b/src/metrics/family.rs index be945075..fb5dbeb6 100644 --- a/src/metrics/family.rs +++ b/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,21 +214,19 @@ impl> Family OwningRef>, M> { - let read_guard = self.metrics.read().expect("Lock not to be poisoned."); + pub fn get_or_create(&self, label_set: &S) -> MappedRwLockReadGuard { 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.") @@ -236,7 +234,7 @@ impl> Family RwLockReadGuard> { - self.metrics.read().unwrap() + self.metrics.read() } } diff --git a/src/metrics/histogram.rs b/src/metrics/histogram.rs index c025c4e8..e641d768 100644 --- a/src/metrics/histogram.rs +++ b/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>, + inner: Arc>, } impl Clone for Histogram { @@ -56,7 +56,7 @@ pub(crate) struct Inner { impl Histogram { pub fn new(buckets: impl Iterator) -> 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 { - 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>) { + 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, Vec<(f64, u64)>>; - impl TypedMetric for Histogram { const TYPE: MetricType = MetricType::Histogram; }