From 255a3f3e183150e30d411628d5996bd3a183bd6f Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Wed, 19 Dec 2018 10:29:23 -0800 Subject: [PATCH] std: Force `Instant::now()` to be monotonic This commit is an attempt to force `Instant::now` to be monotonic through any means possible. We tried relying on OS/hardware/clock implementations, but those seem buggy enough that we can't rely on them in practice. This commit implements the same hammer Firefox recently implemented (noted in #56612) which is to just keep whatever the lastest `Instant::now()` return value was in memory, returning that instead of the OS looks like it's moving backwards. Closes #48514 Closes #49281 cc #51648 cc #56560 Closes #56612 Closes #56940 --- src/librustc/util/profiling.rs | 17 ++----------- src/libstd/sys/cloudabi/time.rs | 8 +++++++ src/libstd/sys/redox/time.rs | 8 +++++++ src/libstd/sys/unix/time.rs | 40 +++++++++++++++++++++---------- src/libstd/sys/wasm/time.rs | 8 +++++++ src/libstd/sys/windows/time.rs | 8 +++++++ src/libstd/time.rs | 42 ++++++++++++++++++++++++++++++++- 7 files changed, 103 insertions(+), 28 deletions(-) diff --git a/src/librustc/util/profiling.rs b/src/librustc/util/profiling.rs index d598709ae3a..a73a2d79d30 100644 --- a/src/librustc/util/profiling.rs +++ b/src/librustc/util/profiling.rs @@ -2,7 +2,7 @@ use session::config::Options; use std::fs; use std::io::{self, StderrLock, Write}; -use std::time::{Duration, Instant}; +use std::time::Instant; macro_rules! define_categories { ($($name:ident,)*) => { @@ -203,20 +203,7 @@ impl SelfProfiler { } fn stop_timer(&mut self) -> u64 { - let elapsed = if cfg!(windows) { - // On Windows, timers don't always appear to be monotonic (see #51648) - // which can lead to panics when calculating elapsed time. - // Work around this by testing to see if the current time is less than - // our recorded time, and if it is, just returning 0. - let now = Instant::now(); - if self.current_timer >= now { - Duration::new(0, 0) - } else { - self.current_timer.elapsed() - } - } else { - self.current_timer.elapsed() - }; + let elapsed = self.current_timer.elapsed(); self.current_timer = Instant::now(); diff --git a/src/libstd/sys/cloudabi/time.rs b/src/libstd/sys/cloudabi/time.rs index 906beba4ae1..545e3c0ce84 100644 --- a/src/libstd/sys/cloudabi/time.rs +++ b/src/libstd/sys/cloudabi/time.rs @@ -25,6 +25,14 @@ impl Instant { } } + pub fn actually_monotonic() -> bool { + true + } + + pub const fn zero() -> Instant { + Instant { t: 0 } + } + pub fn sub_instant(&self, other: &Instant) -> Duration { let diff = self.t .checked_sub(other.t) diff --git a/src/libstd/sys/redox/time.rs b/src/libstd/sys/redox/time.rs index b3272350657..401b7012aa7 100644 --- a/src/libstd/sys/redox/time.rs +++ b/src/libstd/sys/redox/time.rs @@ -128,6 +128,14 @@ impl Instant { Instant { t: now(syscall::CLOCK_MONOTONIC) } } + pub const fn zero() -> Instant { + Instant { t: Timespec { t: syscall::TimeSpec { tv_sec: 0, tv_nsec: 0 } } } + } + + pub fn actually_monotonic() -> bool { + false + } + pub fn sub_instant(&self, other: &Instant) -> Duration { self.t.sub_timespec(&other.t).unwrap_or_else(|_| { panic!("specified instant was later than self") diff --git a/src/libstd/sys/unix/time.rs b/src/libstd/sys/unix/time.rs index 073c0d0686d..4a655714f99 100644 --- a/src/libstd/sys/unix/time.rs +++ b/src/libstd/sys/unix/time.rs @@ -14,6 +14,12 @@ struct Timespec { } impl Timespec { + const fn zero() -> Timespec { + Timespec { + t: libc::timespec { tv_sec: 0, tv_nsec: 0 }, + } + } + fn sub_timespec(&self, other: &Timespec) -> Result { if self >= other { Ok(if self.t.tv_nsec >= other.t.tv_nsec { @@ -128,12 +134,7 @@ mod inner { } pub const UNIX_EPOCH: SystemTime = SystemTime { - t: Timespec { - t: libc::timespec { - tv_sec: 0, - tv_nsec: 0, - }, - }, + t: Timespec::zero(), }; impl Instant { @@ -141,6 +142,14 @@ mod inner { Instant { t: unsafe { libc::mach_absolute_time() } } } + pub const fn zero() -> Instant { + Instant { t: 0 } + } + + pub fn actually_monotonic() -> bool { + true + } + pub fn sub_instant(&self, other: &Instant) -> Duration { let info = info(); let diff = self.t.checked_sub(other.t) @@ -258,12 +267,7 @@ mod inner { } pub const UNIX_EPOCH: SystemTime = SystemTime { - t: Timespec { - t: libc::timespec { - tv_sec: 0, - tv_nsec: 0, - }, - }, + t: Timespec::zero(), }; impl Instant { @@ -271,6 +275,18 @@ mod inner { Instant { t: now(libc::CLOCK_MONOTONIC) } } + pub const fn zero() -> Instant { + Instant { + t: Timespec::zero(), + } + } + + pub fn actually_monotonic() -> bool { + (cfg!(target_os = "linux") && cfg!(target_arch = "x86_64")) || + (cfg!(target_os = "linux") && cfg!(target_arch = "x86")) || + false // last clause, used so `||` is always trailing above + } + pub fn sub_instant(&self, other: &Instant) -> Duration { self.t.sub_timespec(&other.t).unwrap_or_else(|_| { panic!("specified instant was later than self") diff --git a/src/libstd/sys/wasm/time.rs b/src/libstd/sys/wasm/time.rs index a5c41d4a96e..31798466fed 100644 --- a/src/libstd/sys/wasm/time.rs +++ b/src/libstd/sys/wasm/time.rs @@ -14,6 +14,14 @@ impl Instant { Instant(TimeSysCall::perform(TimeClock::Monotonic)) } + pub const fn zero() -> Instant { + Instant(Duration::from_secs(0)) + } + + pub fn actually_monotonic() -> bool { + false + } + pub fn sub_instant(&self, other: &Instant) -> Duration { self.0 - other.0 } diff --git a/src/libstd/sys/windows/time.rs b/src/libstd/sys/windows/time.rs index 60c96959186..8e8e9195cf4 100644 --- a/src/libstd/sys/windows/time.rs +++ b/src/libstd/sys/windows/time.rs @@ -40,6 +40,14 @@ impl Instant { t } + pub fn actually_monotonic() -> bool { + false + } + + pub const fn zero() -> Instant { + Instant { t: 0 } + } + pub fn sub_instant(&self, other: &Instant) -> Duration { // Values which are +- 1 need to be considered as basically the same // units in time due to various measurement oddities, according to diff --git a/src/libstd/time.rs b/src/libstd/time.rs index eb67b718dbd..72a5a070233 100644 --- a/src/libstd/time.rs +++ b/src/libstd/time.rs @@ -12,11 +12,13 @@ #![stable(feature = "time", since = "1.3.0")] +use cmp; use error::Error; use fmt; use ops::{Add, Sub, AddAssign, SubAssign}; use sys::time; use sys_common::FromInner; +use sys_common::mutex::Mutex; #[stable(feature = "time", since = "1.3.0")] pub use core::time::Duration; @@ -150,7 +152,45 @@ impl Instant { /// ``` #[stable(feature = "time2", since = "1.8.0")] pub fn now() -> Instant { - Instant(time::Instant::now()) + let os_now = time::Instant::now(); + + // And here we come upon a sad state of affairs. The whole point of + // `Instant` is that it's monotonically increasing. We've found in the + // wild, however, that it's not actually monotonically increasing for + // one reason or another. These appear to be OS and hardware level bugs, + // and there's not really a whole lot we can do about them. Here's a + // taste of what we've found: + // + // * #48514 - OpenBSD, x86_64 + // * #49281 - linux arm64 and s390x + // * #51648 - windows, x86 + // * #56560 - windows, x86_64, AWS + // * #56612 - windows, x86, vm (?) + // * #56940 - linux, arm64 + // * https://bugzilla.mozilla.org/show_bug.cgi?id=1487778 - a similar + // Firefox bug + // + // It simply seems that this it just happens so that a lot in the wild + // we're seeing panics across various platforms where consecutive calls + // to `Instant::now`, such as via the `elapsed` function, are panicking + // as they're going backwards. Placed here is a last-ditch effort to try + // to fix things up. We keep a global "latest now" instance which is + // returned instead of what the OS says if the OS goes backwards. + // + // To hopefully mitigate the impact of this though a few platforms are + // whitelisted as "these at least haven't gone backwards yet". + if time::Instant::actually_monotonic() { + return Instant(os_now) + } + + static LOCK: Mutex = Mutex::new(); + static mut LAST_NOW: time::Instant = time::Instant::zero(); + unsafe { + let _lock = LOCK.lock(); + let now = cmp::max(LAST_NOW, os_now); + LAST_NOW = now; + Instant(now) + } } /// Returns the amount of time elapsed from another instant to this one.