Skip to content

Commit

Permalink
Avoid time operations that can panic
Browse files Browse the repository at this point in the history
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
  • Loading branch information
olix0r committed Jan 31, 2022
1 parent 20d7b55 commit 2f87b24
Show file tree
Hide file tree
Showing 5 changed files with 7 additions and 6 deletions.
2 changes: 1 addition & 1 deletion tower/src/hedge/latency.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
Expand Down
2 changes: 1 addition & 1 deletion tower/src/hedge/rotating_histogram.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion tower/src/limit/rate/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
5 changes: 3 additions & 2 deletions tower/src/load/peak_ewma.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ const NANOS_PER_MILLI: f64 = 1_000_000.0;
impl<S, C> PeakEwma<S, C> {
/// 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,
Expand Down Expand Up @@ -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!(
Expand All @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion tower/src/retry/budget.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

0 comments on commit 2f87b24

Please sign in to comment.