Skip to content

Commit 91c5e7c

Browse files
authored
Rollup merge of #89017 - the8472:fix-u64-time-monotonizer, r=kennytm
fix potential race in AtomicU64 time monotonizer The AtomicU64-based monotonizer introduced in #83093 is incorrect because several threads could try to update the value concurrently and a thread which doesn't have the newest value among all the updates could win. That bug probably has little real world impact since it doesn't make observed time worse than hardware clocks. The worst case would probably be a thread which has a clock that is behind by several cycles observing several inconsistent fixups, which should be similar to observing the unfiltered backslide in the first place. New benchmarks, they don't look as good as the original PR but still an improvement compared to the mutex. I don't know why the contended mutex case is faster now than in the previous benchmarks. ``` actually_monotonic() == true: test time::tests::instant_contention_01_threads ... bench: 44 ns/iter (+/- 0) test time::tests::instant_contention_02_threads ... bench: 45 ns/iter (+/- 0) test time::tests::instant_contention_04_threads ... bench: 45 ns/iter (+/- 0) test time::tests::instant_contention_08_threads ... bench: 45 ns/iter (+/- 0) test time::tests::instant_contention_16_threads ... bench: 46 ns/iter (+/- 0) atomic u64: test time::tests::instant_contention_01_threads ... bench: 66 ns/iter (+/- 0) test time::tests::instant_contention_02_threads ... bench: 287 ns/iter (+/- 14) test time::tests::instant_contention_04_threads ... bench: 296 ns/iter (+/- 43) test time::tests::instant_contention_08_threads ... bench: 604 ns/iter (+/- 163) test time::tests::instant_contention_16_threads ... bench: 1,147 ns/iter (+/- 29) mutex: test time::tests::instant_contention_01_threads ... bench: 78 ns/iter (+/- 0) test time::tests::instant_contention_02_threads ... bench: 652 ns/iter (+/- 275) test time::tests::instant_contention_04_threads ... bench: 900 ns/iter (+/- 32) test time::tests::instant_contention_08_threads ... bench: 1,927 ns/iter (+/- 62) test time::tests::instant_contention_16_threads ... bench: 3,748 ns/iter (+/- 146) ```
2 parents 1c3fce0 + 57465d9 commit 91c5e7c

File tree

2 files changed

+30
-28
lines changed

2 files changed

+30
-28
lines changed

library/std/src/lib.rs

+1
Original file line numberDiff line numberDiff line change
@@ -234,6 +234,7 @@
234234
#![feature(atomic_mut_ptr)]
235235
#![feature(auto_traits)]
236236
#![feature(bench_black_box)]
237+
#![feature(bool_to_option)]
237238
#![feature(box_syntax)]
238239
#![feature(c_unwind)]
239240
#![feature(c_variadic)]

library/std/src/time/monotonic.rs

+29-28
Original file line numberDiff line numberDiff line change
@@ -37,35 +37,36 @@ pub mod inner {
3737
// This could be a problem for programs that call instants at intervals greater
3838
// than 68 years. Interstellar probes may want to ensure that actually_monotonic() is true.
3939
let packed = (secs << 32) | nanos;
40-
let old = mono.load(Relaxed);
41-
42-
if old == UNINITIALIZED || packed.wrapping_sub(old) < u64::MAX / 2 {
43-
mono.store(packed, Relaxed);
44-
raw
45-
} else {
46-
// Backslide occurred. We reconstruct monotonized time from the upper 32 bit of the
47-
// passed in value and the 64bits loaded from the atomic
48-
let seconds_lower = old >> 32;
49-
let mut seconds_upper = secs & 0xffff_ffff_0000_0000;
50-
if secs & 0xffff_ffff > seconds_lower {
51-
// Backslide caused the lower 32bit of the seconds part to wrap.
52-
// This must be the case because the seconds part is larger even though
53-
// we are in the backslide branch, i.e. the seconds count should be smaller or equal.
54-
//
55-
// We assume that backslides are smaller than 2^32 seconds
56-
// which means we need to add 1 to the upper half to restore it.
57-
//
58-
// Example:
59-
// most recent observed time: 0xA1_0000_0000_0000_0000u128
60-
// bits stored in AtomicU64: 0x0000_0000_0000_0000u64
61-
// backslide by 1s
62-
// caller time is 0xA0_ffff_ffff_0000_0000u128
63-
// -> we can fix up the upper half time by adding 1 << 32
64-
seconds_upper = seconds_upper.wrapping_add(0x1_0000_0000);
40+
let updated = mono.fetch_update(Relaxed, Relaxed, |old| {
41+
(old == UNINITIALIZED || packed.wrapping_sub(old) < u64::MAX / 2).then_some(packed)
42+
});
43+
match updated {
44+
Ok(_) => raw,
45+
Err(newer) => {
46+
// Backslide occurred. We reconstruct monotonized time from the upper 32 bit of the
47+
// passed in value and the 64bits loaded from the atomic
48+
let seconds_lower = newer >> 32;
49+
let mut seconds_upper = secs & 0xffff_ffff_0000_0000;
50+
if secs & 0xffff_ffff > seconds_lower {
51+
// Backslide caused the lower 32bit of the seconds part to wrap.
52+
// This must be the case because the seconds part is larger even though
53+
// we are in the backslide branch, i.e. the seconds count should be smaller or equal.
54+
//
55+
// We assume that backslides are smaller than 2^32 seconds
56+
// which means we need to add 1 to the upper half to restore it.
57+
//
58+
// Example:
59+
// most recent observed time: 0xA1_0000_0000_0000_0000u128
60+
// bits stored in AtomicU64: 0x0000_0000_0000_0000u64
61+
// backslide by 1s
62+
// caller time is 0xA0_ffff_ffff_0000_0000u128
63+
// -> we can fix up the upper half time by adding 1 << 32
64+
seconds_upper = seconds_upper.wrapping_add(0x1_0000_0000);
65+
}
66+
let secs = seconds_upper | seconds_lower;
67+
let nanos = newer as u32;
68+
ZERO.checked_add_duration(&Duration::new(secs, nanos)).unwrap()
6569
}
66-
let secs = seconds_upper | seconds_lower;
67-
let nanos = old as u32;
68-
ZERO.checked_add_duration(&Duration::new(secs, nanos)).unwrap()
6970
}
7071
}
7172
}

0 commit comments

Comments
 (0)