Skip to content

Commit acad59a

Browse files
committed
Auto merge of #56988 - alexcrichton:monotonic-instant, r=sfackler
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
2 parents 8e2063d + 255a3f3 commit acad59a

File tree

7 files changed

+103
-28
lines changed

7 files changed

+103
-28
lines changed

src/librustc/util/profiling.rs

+2-15
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ use session::config::Options;
22

33
use std::fs;
44
use std::io::{self, StderrLock, Write};
5-
use std::time::{Duration, Instant};
5+
use std::time::Instant;
66

77
macro_rules! define_categories {
88
($($name:ident,)*) => {
@@ -203,20 +203,7 @@ impl SelfProfiler {
203203
}
204204

205205
fn stop_timer(&mut self) -> u64 {
206-
let elapsed = if cfg!(windows) {
207-
// On Windows, timers don't always appear to be monotonic (see #51648)
208-
// which can lead to panics when calculating elapsed time.
209-
// Work around this by testing to see if the current time is less than
210-
// our recorded time, and if it is, just returning 0.
211-
let now = Instant::now();
212-
if self.current_timer >= now {
213-
Duration::new(0, 0)
214-
} else {
215-
self.current_timer.elapsed()
216-
}
217-
} else {
218-
self.current_timer.elapsed()
219-
};
206+
let elapsed = self.current_timer.elapsed();
220207

221208
self.current_timer = Instant::now();
222209

src/libstd/sys/cloudabi/time.rs

+8
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,14 @@ impl Instant {
2525
}
2626
}
2727

28+
pub fn actually_monotonic() -> bool {
29+
true
30+
}
31+
32+
pub const fn zero() -> Instant {
33+
Instant { t: 0 }
34+
}
35+
2836
pub fn sub_instant(&self, other: &Instant) -> Duration {
2937
let diff = self.t
3038
.checked_sub(other.t)

src/libstd/sys/redox/time.rs

+8
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,14 @@ impl Instant {
128128
Instant { t: now(syscall::CLOCK_MONOTONIC) }
129129
}
130130

131+
pub const fn zero() -> Instant {
132+
Instant { t: Timespec { t: syscall::TimeSpec { tv_sec: 0, tv_nsec: 0 } } }
133+
}
134+
135+
pub fn actually_monotonic() -> bool {
136+
false
137+
}
138+
131139
pub fn sub_instant(&self, other: &Instant) -> Duration {
132140
self.t.sub_timespec(&other.t).unwrap_or_else(|_| {
133141
panic!("specified instant was later than self")

src/libstd/sys/unix/time.rs

+28-12
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,12 @@ struct Timespec {
1414
}
1515

1616
impl Timespec {
17+
const fn zero() -> Timespec {
18+
Timespec {
19+
t: libc::timespec { tv_sec: 0, tv_nsec: 0 },
20+
}
21+
}
22+
1723
fn sub_timespec(&self, other: &Timespec) -> Result<Duration, Duration> {
1824
if self >= other {
1925
Ok(if self.t.tv_nsec >= other.t.tv_nsec {
@@ -128,19 +134,22 @@ mod inner {
128134
}
129135

130136
pub const UNIX_EPOCH: SystemTime = SystemTime {
131-
t: Timespec {
132-
t: libc::timespec {
133-
tv_sec: 0,
134-
tv_nsec: 0,
135-
},
136-
},
137+
t: Timespec::zero(),
137138
};
138139

139140
impl Instant {
140141
pub fn now() -> Instant {
141142
Instant { t: unsafe { libc::mach_absolute_time() } }
142143
}
143144

145+
pub const fn zero() -> Instant {
146+
Instant { t: 0 }
147+
}
148+
149+
pub fn actually_monotonic() -> bool {
150+
true
151+
}
152+
144153
pub fn sub_instant(&self, other: &Instant) -> Duration {
145154
let info = info();
146155
let diff = self.t.checked_sub(other.t)
@@ -258,19 +267,26 @@ mod inner {
258267
}
259268

260269
pub const UNIX_EPOCH: SystemTime = SystemTime {
261-
t: Timespec {
262-
t: libc::timespec {
263-
tv_sec: 0,
264-
tv_nsec: 0,
265-
},
266-
},
270+
t: Timespec::zero(),
267271
};
268272

269273
impl Instant {
270274
pub fn now() -> Instant {
271275
Instant { t: now(libc::CLOCK_MONOTONIC) }
272276
}
273277

278+
pub const fn zero() -> Instant {
279+
Instant {
280+
t: Timespec::zero(),
281+
}
282+
}
283+
284+
pub fn actually_monotonic() -> bool {
285+
(cfg!(target_os = "linux") && cfg!(target_arch = "x86_64")) ||
286+
(cfg!(target_os = "linux") && cfg!(target_arch = "x86")) ||
287+
false // last clause, used so `||` is always trailing above
288+
}
289+
274290
pub fn sub_instant(&self, other: &Instant) -> Duration {
275291
self.t.sub_timespec(&other.t).unwrap_or_else(|_| {
276292
panic!("specified instant was later than self")

src/libstd/sys/wasm/time.rs

+8
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,14 @@ impl Instant {
1414
Instant(TimeSysCall::perform(TimeClock::Monotonic))
1515
}
1616

17+
pub const fn zero() -> Instant {
18+
Instant(Duration::from_secs(0))
19+
}
20+
21+
pub fn actually_monotonic() -> bool {
22+
false
23+
}
24+
1725
pub fn sub_instant(&self, other: &Instant) -> Duration {
1826
self.0 - other.0
1927
}

src/libstd/sys/windows/time.rs

+8
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,14 @@ impl Instant {
4040
t
4141
}
4242

43+
pub fn actually_monotonic() -> bool {
44+
false
45+
}
46+
47+
pub const fn zero() -> Instant {
48+
Instant { t: 0 }
49+
}
50+
4351
pub fn sub_instant(&self, other: &Instant) -> Duration {
4452
// Values which are +- 1 need to be considered as basically the same
4553
// units in time due to various measurement oddities, according to

src/libstd/time.rs

+41-1
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,13 @@
1212
1313
#![stable(feature = "time", since = "1.3.0")]
1414

15+
use cmp;
1516
use error::Error;
1617
use fmt;
1718
use ops::{Add, Sub, AddAssign, SubAssign};
1819
use sys::time;
1920
use sys_common::FromInner;
21+
use sys_common::mutex::Mutex;
2022

2123
#[stable(feature = "time", since = "1.3.0")]
2224
pub use core::time::Duration;
@@ -153,7 +155,45 @@ impl Instant {
153155
/// ```
154156
#[stable(feature = "time2", since = "1.8.0")]
155157
pub fn now() -> Instant {
156-
Instant(time::Instant::now())
158+
let os_now = time::Instant::now();
159+
160+
// And here we come upon a sad state of affairs. The whole point of
161+
// `Instant` is that it's monotonically increasing. We've found in the
162+
// wild, however, that it's not actually monotonically increasing for
163+
// one reason or another. These appear to be OS and hardware level bugs,
164+
// and there's not really a whole lot we can do about them. Here's a
165+
// taste of what we've found:
166+
//
167+
// * #48514 - OpenBSD, x86_64
168+
// * #49281 - linux arm64 and s390x
169+
// * #51648 - windows, x86
170+
// * #56560 - windows, x86_64, AWS
171+
// * #56612 - windows, x86, vm (?)
172+
// * #56940 - linux, arm64
173+
// * https://bugzilla.mozilla.org/show_bug.cgi?id=1487778 - a similar
174+
// Firefox bug
175+
//
176+
// It simply seems that this it just happens so that a lot in the wild
177+
// we're seeing panics across various platforms where consecutive calls
178+
// to `Instant::now`, such as via the `elapsed` function, are panicking
179+
// as they're going backwards. Placed here is a last-ditch effort to try
180+
// to fix things up. We keep a global "latest now" instance which is
181+
// returned instead of what the OS says if the OS goes backwards.
182+
//
183+
// To hopefully mitigate the impact of this though a few platforms are
184+
// whitelisted as "these at least haven't gone backwards yet".
185+
if time::Instant::actually_monotonic() {
186+
return Instant(os_now)
187+
}
188+
189+
static LOCK: Mutex = Mutex::new();
190+
static mut LAST_NOW: time::Instant = time::Instant::zero();
191+
unsafe {
192+
let _lock = LOCK.lock();
193+
let now = cmp::max(LAST_NOW, os_now);
194+
LAST_NOW = now;
195+
Instant(now)
196+
}
157197
}
158198

159199
/// Returns the amount of time elapsed from another instant to this one.

0 commit comments

Comments
 (0)