From a0e4c1695854db9bf2f4aae3229026608675307d Mon Sep 17 00:00:00 2001 From: Chris Denton Date: Thu, 4 Aug 2022 12:26:40 +0100 Subject: [PATCH] Update after code review --- library/std/src/sys/windows/c.rs | 6 ++++++ library/std/src/sys/windows/compat.rs | 12 ++++++------ 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/library/std/src/sys/windows/c.rs b/library/std/src/sys/windows/c.rs index c340baf2a4a2a..c5a30f8bac86d 100644 --- a/library/std/src/sys/windows/c.rs +++ b/library/std/src/sys/windows/c.rs @@ -1254,10 +1254,16 @@ compat_fn_with_fallback! { pub static SYNCH_API: &CStr = ansi_str!("api-ms-win-core-synch-l1-2-0"); #[allow(unused)] fn WakeByAddressSingle(Address: LPVOID) -> () { + // This fallback is currently tightly coupled to its use in Parker::unpark. + // + // FIXME: If `WakeByAddressSingle` needs to be used anywhere other than + // Parker::unpark then this fallback will be wrong and will need to be decoupled. crate::sys::windows::thread_parker::unpark_keyed_event(Address) } } pub use crate::sys::compat::WaitOnAddress; +// Change exported name of `WakeByAddressSingle` to make the strange fallback +// behaviour clear. pub use WakeByAddressSingle::call as wake_by_address_single_or_unpark_keyed_event; compat_fn_with_fallback! { diff --git a/library/std/src/sys/windows/compat.rs b/library/std/src/sys/windows/compat.rs index ca5b223513498..473544c4d4f7b 100644 --- a/library/std/src/sys/windows/compat.rs +++ b/library/std/src/sys/windows/compat.rs @@ -7,14 +7,14 @@ //! `GetModuleHandle` and `GetProcAddress` to look up DLL entry points at //! runtime. //! -//! This is implemented simply by storing a function pointer in an atomic -//! and using relaxed ordering to load it. This means that calling it will be no -//! more expensive then calling any other dynamically imported function. +//! This is implemented simply by storing a function pointer in an atomic. +//! Loading and calling this function will have little or no overhead +//! compared with calling any other dynamically imported function. //! //! The stored function pointer starts out as an importer function which will //! swap itself with the real function when it's called for the first time. If //! the real function can't be imported then a fallback function is used in its -//! place. While this is zero cost for the happy path (where the function is +//! place. While this is low cost for the happy path (where the function is //! already loaded) it does mean there's some overhead the first time the //! function is called. In the worst case, multiple threads may all end up //! importing the same function unnecessarily. @@ -175,7 +175,7 @@ pub mod WaitOnAddress { #[inline(always)] pub fn option() -> Option { - let f = WAIT_ON_ADDRESS.load(Ordering::Relaxed); + let f = WAIT_ON_ADDRESS.load(Ordering::Acquire); if !f.is_null() { Some(unsafe { mem::transmute(f) }) } else { try_load() } } @@ -185,7 +185,7 @@ pub mod WaitOnAddress { // load the module let mut wait_on_address = None; if let Some(func) = try_load_inner() { - WAIT_ON_ADDRESS.store(func.as_ptr(), Ordering::Relaxed); + WAIT_ON_ADDRESS.store(func.as_ptr(), Ordering::Release); wait_on_address = Some(unsafe { mem::transmute(func) }); } // Don't try to load the module again even if loading failed.