From 2f87b24d4787f79f4d5a426abdfc66b44fcee8d8 Mon Sep 17 00:00:00 2001 From: Oliver Gould Date: Mon, 31 Jan 2022 23:34:03 +0000 Subject: [PATCH] Avoid time operations that can panic We have reports of runtime panics (linkerd/linkerd2#7748) that sound a lot like rust-lang/rust#86470. We don't have any evidence that these panics originate in hyper, but #2385 reports a similar issue. Even though this is almost definitely a bug in Rust, it seems most prudent to actively avoid the uses of `Instant` that are prone to this bug. This change replaces uses of `Instant::elapsed` and `Instant::sub` with calls to `Instant::saturating_duration_since` to prevent this class of panic. These fixes should ultimately be made in the standard library, but this change lets us avoid this problem while we wait for those fixes. See also hyperium/hyper#2746 --- tower/src/hedge/latency.rs | 2 +- tower/src/hedge/rotating_histogram.rs | 2 +- tower/src/limit/rate/service.rs | 2 +- tower/src/load/peak_ewma.rs | 5 +++-- tower/src/retry/budget.rs | 2 +- 5 files changed, 7 insertions(+), 6 deletions(-) diff --git a/tower/src/hedge/latency.rs b/tower/src/hedge/latency.rs index 5f99642ba..b1a8b2459 100644 --- a/tower/src/hedge/latency.rs +++ b/tower/src/hedge/latency.rs @@ -82,7 +82,7 @@ where let this = self.project(); let rsp = ready!(this.inner.poll(cx)).map_err(Into::into)?; - let duration = Instant::now() - *this.start; + let duration = Instant::now().saturating_duration_since(*this.start); this.rec.record(duration); Poll::Ready(Ok(rsp)) } diff --git a/tower/src/hedge/rotating_histogram.rs b/tower/src/hedge/rotating_histogram.rs index d6190ddb1..4b07a1593 100644 --- a/tower/src/hedge/rotating_histogram.rs +++ b/tower/src/hedge/rotating_histogram.rs @@ -39,7 +39,7 @@ impl RotatingHistogram { } fn maybe_rotate(&mut self) { - let delta = Instant::now() - self.last_rotation; + let delta = Instant::now().saturating_duration_since(self.last_rotation); // TODO: replace with delta.duration_div when it becomes stable. let rotations = (nanos(delta) / nanos(self.period)) as u32; if rotations >= 2 { diff --git a/tower/src/limit/rate/service.rs b/tower/src/limit/rate/service.rs index 550c92d8a..b7604603b 100644 --- a/tower/src/limit/rate/service.rs +++ b/tower/src/limit/rate/service.rs @@ -73,7 +73,7 @@ where match self.state { State::Ready { .. } => return Poll::Ready(ready!(self.inner.poll_ready(cx))), State::Limited => { - if let Poll::Pending = Pin::new(&mut self.sleep).poll(cx) { + if Pin::new(&mut self.sleep).poll(cx).is_pending() { tracing::trace!("rate limit exceeded; sleeping."); return Poll::Pending; } diff --git a/tower/src/load/peak_ewma.rs b/tower/src/load/peak_ewma.rs index e48c55a20..61ac2011f 100644 --- a/tower/src/load/peak_ewma.rs +++ b/tower/src/load/peak_ewma.rs @@ -91,6 +91,7 @@ const NANOS_PER_MILLI: f64 = 1_000_000.0; impl PeakEwma { /// Wraps an `S`-typed service so that its load is tracked by the EWMA of its peak latency. pub fn new(service: S, default_rtt: Duration, decay_ns: f64, completion: C) -> Self { + debug_assert!(decay_ns > 0.0, "decay_ns must be positive"); Self { service, decay_ns, @@ -241,7 +242,7 @@ impl RttEstimate { recv_at, sent_at ); - let rtt = nanos(recv_at - sent_at); + let rtt = nanos(recv_at.saturating_duration_since(sent_at)); let now = Instant::now(); debug_assert!( @@ -264,7 +265,7 @@ impl RttEstimate { // prior estimate according to how much time has elapsed since the last // update. The inverse of the decay is used to scale the estimate towards the // observed RTT value. - let elapsed = nanos(now - self.update_at); + let elapsed = nanos(now.saturating_duration_since(self.update_at)); let decay = (-elapsed / decay_ns).exp(); let recency = 1.0 - decay; let next_estimate = (self.rtt_ns * decay) + (rtt * recency); diff --git a/tower/src/retry/budget.rs b/tower/src/retry/budget.rs index ccecce01a..3705f8275 100644 --- a/tower/src/retry/budget.rs +++ b/tower/src/retry/budget.rs @@ -175,7 +175,7 @@ impl Bucket { let mut gen = self.generation.lock().expect("generation lock"); let now = Instant::now(); - let diff = now - gen.time; + let diff = now.saturating_duration_since(gen.time); if diff < self.window { // not expired yet return;