diff options
| author | Alex Crichton <alex@alexcrichton.com> | 2019-04-03 12:18:38 -0700 |
|---|---|---|
| committer | Alex Crichton <alex@alexcrichton.com> | 2019-04-04 07:19:14 -0700 |
| commit | cb57484dcaeb4881c73f8a1174d4d5661ca2bbe2 (patch) | |
| tree | e4544a0b7059c5bfd2ec95a2bd8a51b917c76e1e /src/libstd/sys | |
| parent | f8673e0ad85e98997faa76fa7edc99c5825f46ee (diff) | |
| download | rust-cb57484dcaeb4881c73f8a1174d4d5661ca2bbe2.tar.gz rust-cb57484dcaeb4881c73f8a1174d4d5661ca2bbe2.zip | |
std: Avoid usage of `Once` in `Instant`
This commit removes usage of `Once` from the internal implementation of time utilities on OSX and Windows. It turns out that we accidentally hit a deadlock today (#59020) via events that look like: * A thread invokes `park_timeout` * Internally, only on OSX, `park_timeout` calls `Instant::elapsed` * Inside of `Instant::elapsed` on OSX we enter a `Once` to initialize global timer data * Inside of `Once`, it attempts to `park` This means on the same stack frame, when there's contention, we're calling `park` from inside `park_timeout`, causing a deadlock! The solution implemented in this commit was to remove usage of `Once` and instead just do a small dance with atomics. There's no real need we need to guarantee that the global information is only learned once, only that it's only *stored* once. This implementation may have multiple threads invoke `mach_timebase_info`, but only one will store the global information which will amortize the cost for all other threads. A similar fix has been applied to windows to be uniform across our implementations, but looking at the code on Windows no deadlock was possible. This is purely just a consistency update for Windows and in theory a slightly leaner implementation. Closes #59020
Diffstat (limited to 'src/libstd/sys')
| -rw-r--r-- | src/libstd/sys/unix/time.rs | 27 | ||||
| -rw-r--r-- | src/libstd/sys/windows/time.rs | 24 |
2 files changed, 38 insertions, 13 deletions
diff --git a/src/libstd/sys/unix/time.rs b/src/libstd/sys/unix/time.rs index 127ae6aa104..e21c32cd91b 100644 --- a/src/libstd/sys/unix/time.rs +++ b/src/libstd/sys/unix/time.rs @@ -114,7 +114,8 @@ impl Hash for Timespec { #[cfg(any(target_os = "macos", target_os = "ios"))] mod inner { use crate::fmt; - use crate::sync::Once; + use crate::mem; + use crate::sync::atomic::{AtomicUsize, Ordering::SeqCst}; use crate::sys::cvt; use crate::sys_common::mul_div_u64; use crate::time::Duration; @@ -229,18 +230,30 @@ mod inner { Some(mul_div_u64(nanos, info.denom as u64, info.numer as u64)) } - fn info() -> &'static libc::mach_timebase_info { + fn info() -> libc::mach_timebase_info { static mut INFO: libc::mach_timebase_info = libc::mach_timebase_info { numer: 0, denom: 0, }; - static ONCE: Once = Once::new(); + static STATE: AtomicUsize = AtomicUsize::new(0); unsafe { - ONCE.call_once(|| { - libc::mach_timebase_info(&mut INFO); - }); - &INFO + // If a previous thread has filled in this global state, use that. + if STATE.load(SeqCst) == 2 { + return INFO; + } + + // ... otherwise learn for ourselves ... + let mut info = mem::zeroed(); + libc::mach_timebase_info(&mut info); + + // ... and attempt to be the one thread that stores it globally for + // all other threads + if STATE.compare_exchange(0, 1, SeqCst, SeqCst).is_ok() { + INFO = info; + STATE.store(2, SeqCst); + } + return info; } } } diff --git a/src/libstd/sys/windows/time.rs b/src/libstd/sys/windows/time.rs index 4c9d2aee157..e0f0e3a1a45 100644 --- a/src/libstd/sys/windows/time.rs +++ b/src/libstd/sys/windows/time.rs @@ -173,7 +173,7 @@ fn intervals2dur(intervals: u64) -> Duration { mod perf_counter { use super::{NANOS_PER_SEC}; - use crate::sync::Once; + use crate::sync::atomic::{AtomicUsize, Ordering::SeqCst}; use crate::sys_common::mul_div_u64; use crate::sys::c; use crate::sys::cvt; @@ -210,13 +210,25 @@ mod perf_counter { fn frequency() -> c::LARGE_INTEGER { static mut FREQUENCY: c::LARGE_INTEGER = 0; - static ONCE: Once = Once::new(); + static STATE: AtomicUsize = AtomicUsize::new(0); unsafe { - ONCE.call_once(|| { - cvt(c::QueryPerformanceFrequency(&mut FREQUENCY)).unwrap(); - }); - FREQUENCY + // If a previous thread has filled in this global state, use that. + if STATE.load(SeqCst) == 2 { + return FREQUENCY; + } + + // ... otherwise learn for ourselves ... + let mut frequency = 0; + cvt(c::QueryPerformanceFrequency(&mut frequency)).unwrap(); + + // ... and attempt to be the one thread that stores it globally for + // all other threads + if STATE.compare_exchange(0, 1, SeqCst, SeqCst).is_ok() { + FREQUENCY = frequency; + STATE.store(2, SeqCst); + } + return frequency; } } |
