Skip to content

Commit

Permalink
Use more suitable atomic orderings instead of SeqCst
Browse files Browse the repository at this point in the history
Single-time initialization is one of clear cases when Acquire and Release semantics are very well suited.
  • Loading branch information
AngelicosPhosphoros committed Dec 17, 2023
1 parent 6042ec8 commit a3aeef8
Showing 1 changed file with 33 additions and 17 deletions.
50 changes: 33 additions & 17 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -408,7 +408,7 @@ unsafe impl Sync for AtomicUsize {}
// the STATE static which determines whether LOGGER has been initialized yet.
static mut LOGGER: &dyn Log = &NopLogger;

static STATE: AtomicUsize = AtomicUsize::new(0);
static STATE: AtomicUsize = AtomicUsize::new(UNINITIALIZED);

// There are three different states that we care about: the logger's
// uninitialized, the logger's initializing (set_logger's been called but
Expand Down Expand Up @@ -1366,31 +1366,45 @@ fn set_logger_inner<F>(make_logger: F) -> Result<(), SetLoggerError>
where
F: FnOnce() -> &'static dyn Log,
{
let old_state = match STATE.compare_exchange(
// This method doesn't read from `LOGGER` variable at all,
// and if it changes it, it does another store to `STATE` after that.
// We don't need to synchronize CPU caches using memory ordering in this function
// even if another thread set `LOGGER` variable
// because all read accesses to `LOGGER` do that synchronization anyway.
let (Ok(old_state) | Err(old_state)) = STATE.compare_exchange(
UNINITIALIZED,
INITIALIZING,
Ordering::SeqCst,
Ordering::SeqCst,
) {
Ok(s) | Err(s) => s,
};
Ordering::Relaxed,
Ordering::Relaxed,
);
match old_state {
UNINITIALIZED => {
unsafe {
LOGGER = make_logger();
}
STATE.store(INITIALIZED, Ordering::SeqCst);
// Use release ordering here to make write to `LOGGER` static
// and write to any contents of object pointed by `LOGGER` visible
// to other threads after they load `INITIALIZED` `STATE` using Acquire.
STATE.store(INITIALIZED, Ordering::Release);
Ok(())
}
INITIALIZING => {
while STATE.load(Ordering::SeqCst) == INITIALIZING {
// TODO: replace with `hint::spin_loop` once MSRV is 1.49.0.
#[allow(deprecated)]
std::sync::atomic::spin_loop_hint();
// Wait until other thread writes a fat reference and a usize
// so wait would be very short so it is OK to use spinlock here.

// We don't need to use Acquire here because all ways to access
// `LOGGER` from public interface do `Acquire` load before that.
// We need to wait here to ensure that `logger()` function returns
// the valid logger after call.

while STATE.load(Ordering::Relaxed) == INITIALIZING {
std::hint::spin_loop();
}

Err(SetLoggerError(()))
}
_ => Err(SetLoggerError(())),
INITIALIZED => Err(SetLoggerError(())),
unexpected_state => unreachable!("log::STATE has invalid value {}", unexpected_state),
}
}

Expand All @@ -1414,17 +1428,18 @@ where
///
/// [`set_logger`]: fn.set_logger.html
pub unsafe fn set_logger_racy(logger: &'static dyn Log) -> Result<(), SetLoggerError> {
match STATE.load(Ordering::SeqCst) {
match STATE.load(Ordering::Relaxed) {
UNINITIALIZED => {
LOGGER = logger;
STATE.store(INITIALIZED, Ordering::SeqCst);
STATE.store(INITIALIZED, Ordering::Release);
Ok(())
}
INITIALIZING => {
// This is just plain UB, since we were racing another initialization function
unreachable!("set_logger_racy must not be used with other initialization functions")
}
_ => Err(SetLoggerError(())),
INITIALIZED => Err(SetLoggerError(())),
unexpected_state => unreachable!("log::STATE has invalid value {}", unexpected_state),
}
}

Expand Down Expand Up @@ -1466,7 +1481,8 @@ impl error::Error for ParseLevelError {}
///
/// If a logger has not been set, a no-op implementation is returned.
pub fn logger() -> &'static dyn Log {
if STATE.load(Ordering::SeqCst) != INITIALIZED {
// Use Acquire to pair it with Release store in `set_logger_inner`.
if STATE.load(Ordering::Acquire) != INITIALIZED {
static NOP: NopLogger = NopLogger;
&NOP
} else {
Expand Down

0 comments on commit a3aeef8

Please sign in to comment.