Skip to content

Commit e4068fe

Browse files
fix(s2n-quic-dc-metrics): Avoid panicking in Counter::increment with >u32::MAX count (#2995)
1 parent 5de081c commit e4068fe

2 files changed

Lines changed: 32 additions & 3 deletions

File tree

dc/s2n-quic-dc-metrics/src/counter.rs

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,9 +52,19 @@ impl Counter {
5252
}
5353
}
5454

55-
#[track_caller]
5655
pub fn increment(&self, count: u64) {
57-
assert!(count <= u32::MAX as u64);
56+
// If we get a particularly large count, directly serialize it into the underlying
57+
// aggregate. We expect this to be rare, so acquiring the lock across all counters should
58+
// be relatively cheap.
59+
//
60+
// It's possible to shift the exact threshold if we wanted to (e.g., by reducing the
61+
// counter index bits, or having a more complicated serialization scheme), but we don't
62+
// expect this to matter much in practice. If the event recorded is this large there's
63+
// probably a good deal of compute needed to produce it in the first place.
64+
if count > (u32::MAX as u64) {
65+
self.channels.lock_aggregate()[self.counter as usize].value += count;
66+
return;
67+
}
5868
self.channels
5969
.send_event(((self.counter as u64) << 32) | count);
6070
}
@@ -76,3 +86,18 @@ fn basic() {
7686

7787
assert_eq!(registry.take_current_metrics_line(), "a=5,b=9");
7888
}
89+
90+
#[test]
91+
fn check_u64_max() {
92+
let registry = crate::Registry::new();
93+
let a = registry.register_counter(String::from("a"), None);
94+
let b = registry.register_counter(String::from("b"), None);
95+
96+
a.increment(u64::MAX);
97+
b.increment(u32::MAX as u64);
98+
99+
assert_eq!(
100+
registry.take_current_metrics_line(),
101+
format!("a={},b={}", u64::MAX, u32::MAX)
102+
);
103+
}

dc/s2n-quic-dc-metrics/src/rseq.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ use std::{
99
ptr::NonNull,
1010
sync::{
1111
atomic::{AtomicBool, AtomicPtr, AtomicU64, Ordering},
12-
Mutex,
12+
Mutex, MutexGuard,
1313
},
1414
};
1515

@@ -249,6 +249,10 @@ impl<T: Absorb> Channels<T> {
249249
self.empty_pages.push(page);
250250
}
251251

252+
pub(crate) fn lock_aggregate(&self) -> MutexGuard<'_, Vec<T>> {
253+
self.aggregate.lock().expect("propagate panic")
254+
}
255+
252256
#[cfg(not(target_os = "linux"))]
253257
pub(crate) fn send_event(&self, event: u64) {
254258
self.fallback_push(event)

0 commit comments

Comments
 (0)