Skip to content

Avoid SeqCst or static mut in mach_timebase_info and QueryPerformanceFrequency caches #77727

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Oct 11, 2020
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Switch to using a single atomic and treating 0 as 'uninitialized'
  • Loading branch information
thomcc committed Oct 9, 2020
commit 59c06e9e40c39aeec955fb2359f810285c262c34
28 changes: 12 additions & 16 deletions library/std/src/sys/unix/time.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ impl Hash for Timespec {
#[cfg(any(target_os = "macos", target_os = "ios"))]
mod inner {
use crate::fmt;
use crate::sync::atomic::{AtomicBool, AtomicU64, Ordering};
use crate::sync::atomic::{AtomicU64, Ordering};
use crate::sys::cvt;
use crate::sys_common::mul_div_u64;
use crate::time::Duration;
Expand Down Expand Up @@ -232,15 +232,19 @@ mod inner {
}

fn info() -> mach_timebase_info {
static INITIALIZED: AtomicBool = AtomicBool::new(false);
// INFO_BITS conceptually is an `Option<mach_timebase_info>`. We can do
// this in 64 bits because we know 0 is never a valid value for the
// `denom` field.
//
// Encoding this as a single `AtomicU64` allows us to use `Relaxed`
// operations, as we are only interested in in the effects on a single
// memory location.
static INFO_BITS: AtomicU64 = AtomicU64::new(0);

// If a previous thread has initialized `INFO_BITS`, use that.
if INITIALIZED.load(Ordering::Acquire) {
// Note: `Relaxed` is correct here and below -- the `Acquire` /
// `Release` pair used for `INITIALIZED` ensures this load can see
// the corresponding store below.
return info_from_bits(INFO_BITS.load(Ordering::Relaxed));
// If a previous thread has initialized `INFO_BITS`, use it.
let info_bits = INFO_BITS.load(Ordering::Relaxed);
if info_bits != 0 {
return info_from_bits(info_bits);
}

// ... otherwise learn for ourselves ...
Expand All @@ -252,15 +256,7 @@ mod inner {
unsafe {
mach_timebase_info(&mut info);
}

// This is racy, but the race should be against other threads trying to
// write the same value.
INFO_BITS.store(info_to_bits(info), Ordering::Relaxed);

// The `Release` here "publishes" the store of `INFO_BITS` to other
// threads (which do a `INITIALIZED.load(Acquire)`) despite it being
// read/written w/ `Relaxed`.
INITIALIZED.store(true, Ordering::Release);
info
}

Expand Down