Skip to content
Permalink
Browse files

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
  • Loading branch information...
alexcrichton committed Apr 3, 2019
1 parent f8673e0 commit cb57484dcaeb4881c73f8a1174d4d5661ca2bbe2
Showing with 65 additions and 13 deletions.
  1. +20 −7 src/libstd/sys/unix/time.rs
  2. +18 −6 src/libstd/sys/windows/time.rs
  3. +27 −0 src/test/run-pass/issue-59020.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;
}
}
}
@@ -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;
}
}

@@ -0,0 +1,27 @@
// edition:2018
// run-pass
// ignore-emscripten no threads support

use std::thread;
use std::time::Duration;

fn main() {
let t1 = thread::spawn(|| {
let sleep = Duration::new(0,100_000);
for _ in 0..100 {
println!("Parking1");
thread::park_timeout(sleep);
}
});

let t2 = thread::spawn(|| {
let sleep = Duration::new(0,100_000);
for _ in 0..100 {
println!("Parking2");
thread::park_timeout(sleep);
}
});

t1.join().expect("Couldn't join thread 1");
t2.join().expect("Couldn't join thread 2");
}

0 comments on commit cb57484

Please sign in to comment.
You can’t perform that action at this time.