about summary refs log tree commit diff
path: root/src/libstd/sys
diff options
context:
space:
mode:
authorAlex Crichton <alex@alexcrichton.com>2019-04-03 12:18:38 -0700
committerAlex Crichton <alex@alexcrichton.com>2019-04-04 07:19:14 -0700
commitcb57484dcaeb4881c73f8a1174d4d5661ca2bbe2 (patch)
treee4544a0b7059c5bfd2ec95a2bd8a51b917c76e1e /src/libstd/sys
parentf8673e0ad85e98997faa76fa7edc99c5825f46ee (diff)
downloadrust-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.rs27
-rw-r--r--src/libstd/sys/windows/time.rs24
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;
         }
     }