From 508a095af63af8e6239718ffee8919d04c142e01 Mon Sep 17 00:00:00 2001 From: Chris Denton Date: Sun, 3 Mar 2024 22:06:00 +0000 Subject: [PATCH] Windows: Implement mutex using futex Well, the Windows equivalent: `WaitOnAddress`, `WakeByAddressSingle` and `WakeByAddressAll`. --- library/std/src/sys/locks/condvar/futex.rs | 6 ++ library/std/src/sys/locks/condvar/mod.rs | 7 +- .../locks/condvar/{windows.rs => windows7.rs} | 0 library/std/src/sys/locks/mutex/futex.rs | 101 ++---------------- library/std/src/sys/locks/mutex/futex/unix.rs | 1 + .../std/src/sys/locks/mutex/futex/windows.rs | 65 +++++++++++ library/std/src/sys/locks/mutex/mod.rs | 7 +- .../locks/mutex/{windows.rs => windows7.rs} | 0 library/std/src/sys/locks/rwlock/futex.rs | 15 +++ library/std/src/sys/locks/rwlock/mod.rs | 7 +- .../locks/rwlock/{windows.rs => windows7.rs} | 0 library/std/src/sys/pal/windows/api.rs | 39 +++++++ library/std/src/sys/pal/windows/c.rs | 4 + library/std/src/sys/pal/windows/mod.rs | 2 +- .../miri/src/shims/windows/foreign_items.rs | 6 ++ src/tools/miri/src/shims/windows/sync.rs | 15 +++ 16 files changed, 171 insertions(+), 104 deletions(-) rename library/std/src/sys/locks/condvar/{windows.rs => windows7.rs} (100%) create mode 100644 library/std/src/sys/locks/mutex/futex/unix.rs create mode 100644 library/std/src/sys/locks/mutex/futex/windows.rs rename library/std/src/sys/locks/mutex/{windows.rs => windows7.rs} (100%) rename library/std/src/sys/locks/rwlock/{windows.rs => windows7.rs} (100%) diff --git a/library/std/src/sys/locks/condvar/futex.rs b/library/std/src/sys/locks/condvar/futex.rs index 3ad93ce07f753..c9f7962ea9190 100644 --- a/library/std/src/sys/locks/condvar/futex.rs +++ b/library/std/src/sys/locks/condvar/futex.rs @@ -1,4 +1,10 @@ use crate::sync::atomic::{AtomicU32, Ordering::Relaxed}; +#[cfg(windows)] +use crate::sys::api::{ + wait_on_address as futex_wait, wake_by_address_all as futex_wake_all, + wake_by_address_single as futex_wake, +}; +#[cfg(not(windows))] use crate::sys::futex::{futex_wait, futex_wake, futex_wake_all}; use crate::sys::locks::Mutex; use crate::time::Duration; diff --git a/library/std/src/sys/locks/condvar/mod.rs b/library/std/src/sys/locks/condvar/mod.rs index 126a42a2a4c00..6849cacf88e76 100644 --- a/library/std/src/sys/locks/condvar/mod.rs +++ b/library/std/src/sys/locks/condvar/mod.rs @@ -1,5 +1,6 @@ cfg_if::cfg_if! { if #[cfg(any( + all(target_os = "windows", not(target_vendor="win7")), target_os = "linux", target_os = "android", target_os = "freebsd", @@ -14,9 +15,9 @@ cfg_if::cfg_if! { } else if #[cfg(target_family = "unix")] { mod pthread; pub use pthread::Condvar; - } else if #[cfg(target_os = "windows")] { - mod windows; - pub use windows::Condvar; + } else if #[cfg(all(target_os = "windows", target_vendor = "win7"))] { + mod windows7; + pub use windows7::Condvar; } else if #[cfg(all(target_vendor = "fortanix", target_env = "sgx"))] { mod sgx; pub use sgx::Condvar; diff --git a/library/std/src/sys/locks/condvar/windows.rs b/library/std/src/sys/locks/condvar/windows7.rs similarity index 100% rename from library/std/src/sys/locks/condvar/windows.rs rename to library/std/src/sys/locks/condvar/windows7.rs diff --git a/library/std/src/sys/locks/mutex/futex.rs b/library/std/src/sys/locks/mutex/futex.rs index c01229586c302..27f25b9bb4106 100644 --- a/library/std/src/sys/locks/mutex/futex.rs +++ b/library/std/src/sys/locks/mutex/futex.rs @@ -1,96 +1,9 @@ -use crate::sync::atomic::{ - AtomicU32, - Ordering::{Acquire, Relaxed, Release}, -}; -use crate::sys::futex::{futex_wait, futex_wake}; - -pub struct Mutex { - /// 0: unlocked - /// 1: locked, no other threads waiting - /// 2: locked, and other threads waiting (contended) - futex: AtomicU32, -} - -impl Mutex { - #[inline] - pub const fn new() -> Self { - Self { futex: AtomicU32::new(0) } - } - - #[inline] - pub fn try_lock(&self) -> bool { - self.futex.compare_exchange(0, 1, Acquire, Relaxed).is_ok() - } - - #[inline] - pub fn lock(&self) { - if self.futex.compare_exchange(0, 1, Acquire, Relaxed).is_err() { - self.lock_contended(); - } - } - - #[cold] - fn lock_contended(&self) { - // Spin first to speed things up if the lock is released quickly. - let mut state = self.spin(); - - // If it's unlocked now, attempt to take the lock - // without marking it as contended. - if state == 0 { - match self.futex.compare_exchange(0, 1, Acquire, Relaxed) { - Ok(_) => return, // Locked! - Err(s) => state = s, - } - } - - loop { - // Put the lock in contended state. - // We avoid an unnecessary write if it as already set to 2, - // to be friendlier for the caches. - if state != 2 && self.futex.swap(2, Acquire) == 0 { - // We changed it from 0 to 2, so we just successfully locked it. - return; - } - - // Wait for the futex to change state, assuming it is still 2. - futex_wait(&self.futex, 2, None); - - // Spin again after waking up. - state = self.spin(); - } - } - - fn spin(&self) -> u32 { - let mut spin = 100; - loop { - // We only use `load` (and not `swap` or `compare_exchange`) - // while spinning, to be easier on the caches. - let state = self.futex.load(Relaxed); - - // We stop spinning when the mutex is unlocked (0), - // but also when it's contended (2). - if state != 1 || spin == 0 { - return state; - } - - crate::hint::spin_loop(); - spin -= 1; - } - } - - #[inline] - pub unsafe fn unlock(&self) { - if self.futex.swap(0, Release) == 2 { - // We only wake up one thread. When that thread locks the mutex, it - // will mark the mutex as contended (2) (see lock_contended above), - // which makes sure that any other waiting threads will also be - // woken up eventually. - self.wake(); - } - } - - #[cold] - fn wake(&self) { - futex_wake(&self.futex); +cfg_if::cfg_if! { + if #[cfg(windows)] { + mod windows; + pub use windows::*; + } else { + mod unix; + pub use unix::*; } } diff --git a/library/std/src/sys/locks/mutex/futex/unix.rs b/library/std/src/sys/locks/mutex/futex/unix.rs new file mode 100644 index 0000000000000..8b137891791fe --- /dev/null +++ b/library/std/src/sys/locks/mutex/futex/unix.rs @@ -0,0 +1 @@ + diff --git a/library/std/src/sys/locks/mutex/futex/windows.rs b/library/std/src/sys/locks/mutex/futex/windows.rs new file mode 100644 index 0000000000000..5f55d1f8f03d5 --- /dev/null +++ b/library/std/src/sys/locks/mutex/futex/windows.rs @@ -0,0 +1,65 @@ +use crate::sync::atomic::{ + AtomicI8, + Ordering::{Acquire, Relaxed, Release}, +}; +use crate::sys::api; + +pub struct Mutex { + state: AtomicI8, +} + +const UNLOCKED: i8 = 0; +const LOCKED: i8 = 1; +const CONTENDED: i8 = 2; + +impl Mutex { + #[inline] + pub const fn new() -> Mutex { + Mutex { state: AtomicI8::new(UNLOCKED) } + } + + #[inline] + pub fn lock(&self) { + if let Err(state) = self.state.compare_exchange(UNLOCKED, LOCKED, Acquire, Relaxed) { + self.lock_contended(state) + } + } + + #[cold] + fn lock_contended(&self, mut state: i8) { + // Note: WaitOnAddress is already quite spin-happy so we don't do any further spinning on top. + loop { + // Put the lock in contended state. + // We avoid an unnecessary write if it as already set to CONTENDED, + // to be friendlier for the caches. + if state != CONTENDED && self.state.swap(CONTENDED, Acquire) == UNLOCKED { + // We changed it from UNLOCKED to CONTENDED, so we just successfully locked it. + return; + } + // Wait for the futex to change state, assuming it is still CONTENDED. + api::wait_on_address(&self.state, CONTENDED, None); + state = self.state.load(Relaxed); + } + } + + #[inline] + pub fn try_lock(&self) -> bool { + self.state.compare_exchange(UNLOCKED, LOCKED, Acquire, Relaxed).is_ok() + } + + #[inline] + pub unsafe fn unlock(&self) { + if self.state.swap(UNLOCKED, Release) == CONTENDED { + // We only wake up one thread. When that thread locks the mutex, it + // will mark the mutex as CONTENDED (see lock_contended above), + // which makes sure that any other waiting threads will also be + // woken up eventually. + self.wake(); + } + } + + #[cold] + fn wake(&self) { + api::wake_by_address_single(&self.state); + } +} diff --git a/library/std/src/sys/locks/mutex/mod.rs b/library/std/src/sys/locks/mutex/mod.rs index 710cb91fb1473..73d9bd273de17 100644 --- a/library/std/src/sys/locks/mutex/mod.rs +++ b/library/std/src/sys/locks/mutex/mod.rs @@ -1,5 +1,6 @@ cfg_if::cfg_if! { if #[cfg(any( + all(target_os = "windows", not(target_vendor = "win7")), target_os = "linux", target_os = "android", target_os = "freebsd", @@ -19,9 +20,9 @@ cfg_if::cfg_if! { ))] { mod pthread; pub use pthread::{Mutex, raw}; - } else if #[cfg(target_os = "windows")] { - mod windows; - pub use windows::{Mutex, raw}; + } else if #[cfg(all(target_os = "windows", target_vendor = "win7"))] { + mod windows7; + pub use windows7::{Mutex, raw}; } else if #[cfg(all(target_vendor = "fortanix", target_env = "sgx"))] { mod sgx; pub use sgx::Mutex; diff --git a/library/std/src/sys/locks/mutex/windows.rs b/library/std/src/sys/locks/mutex/windows7.rs similarity index 100% rename from library/std/src/sys/locks/mutex/windows.rs rename to library/std/src/sys/locks/mutex/windows7.rs diff --git a/library/std/src/sys/locks/rwlock/futex.rs b/library/std/src/sys/locks/rwlock/futex.rs index aa0de900238f5..79f147d210386 100644 --- a/library/std/src/sys/locks/rwlock/futex.rs +++ b/library/std/src/sys/locks/rwlock/futex.rs @@ -2,6 +2,12 @@ use crate::sync::atomic::{ AtomicU32, Ordering::{Acquire, Relaxed, Release}, }; +#[cfg(windows)] +use crate::sys::api::{ + wait_on_address as futex_wait, wake_by_address_all as futex_wake_all, + wake_by_address_single as futex_wake, +}; +#[cfg(not(windows))] use crate::sys::futex::{futex_wait, futex_wake, futex_wake_all}; pub struct RwLock { @@ -291,7 +297,10 @@ impl RwLock { } /// Spin for a while, but stop directly at the given condition. + /// + /// We avoid spinning on Windows because the futex implementation spins enough. #[inline] + #[cfg(not(windows))] fn spin_until(&self, f: impl Fn(u32) -> bool) -> u32 { let mut spin = 100; // Chosen by fair dice roll. loop { @@ -304,6 +313,12 @@ impl RwLock { } } + #[inline] + #[cfg(windows)] + fn spin_until(&self, _f: impl Fn(u32) -> bool) -> u32 { + self.state.load(Relaxed) + } + #[inline] fn spin_write(&self) -> u32 { // Stop spinning when it's unlocked or when there's waiting writers, to keep things somewhat fair. diff --git a/library/std/src/sys/locks/rwlock/mod.rs b/library/std/src/sys/locks/rwlock/mod.rs index 0564f1fe6fab9..675931c64bdd3 100644 --- a/library/std/src/sys/locks/rwlock/mod.rs +++ b/library/std/src/sys/locks/rwlock/mod.rs @@ -1,5 +1,6 @@ cfg_if::cfg_if! { if #[cfg(any( + all(target_os = "windows", not(target_vendor = "win7")), target_os = "linux", target_os = "android", target_os = "freebsd", @@ -14,9 +15,9 @@ cfg_if::cfg_if! { } else if #[cfg(target_family = "unix")] { mod queue; pub use queue::RwLock; - } else if #[cfg(target_os = "windows")] { - mod windows; - pub use windows::RwLock; + } else if #[cfg(all(target_os = "windows", target_vendor = "win7"))] { + mod windows7; + pub use windows7::RwLock; } else if #[cfg(all(target_vendor = "fortanix", target_env = "sgx"))] { mod sgx; pub use sgx::RwLock; diff --git a/library/std/src/sys/locks/rwlock/windows.rs b/library/std/src/sys/locks/rwlock/windows7.rs similarity index 100% rename from library/std/src/sys/locks/rwlock/windows.rs rename to library/std/src/sys/locks/rwlock/windows7.rs diff --git a/library/std/src/sys/pal/windows/api.rs b/library/std/src/sys/pal/windows/api.rs index 90e1bff52a365..0496bc062c33f 100644 --- a/library/std/src/sys/pal/windows/api.rs +++ b/library/std/src/sys/pal/windows/api.rs @@ -155,3 +155,42 @@ pub fn get_last_error() -> WinError { pub struct WinError { pub code: u32, } + +#[cfg(not(target_vendor = "win7"))] +mod futex { + use super::*; + use crate::sys::dur2timeout; + use core::mem; + use core::time::Duration; + + #[inline(always)] + pub fn wait_on_address(address: &T, compare: U, timeout: Option) -> bool { + assert_eq!(mem::size_of::(), mem::size_of::()); + unsafe { + let addr = addr_of!(*address).cast::(); + let size = mem::size_of::(); + let compare_addr = addr_of!(compare).cast::(); + let timeout = timeout.map(dur2timeout).unwrap_or(c::INFINITE); + c::WaitOnAddress(addr, compare_addr, size, timeout) == c::TRUE + } + } + + #[inline(always)] + pub fn wake_by_address_single(address: &T) -> bool { + unsafe { + let addr = addr_of!(*address).cast::(); + c::WakeByAddressSingle(addr); + false + } + } + + #[inline(always)] + pub fn wake_by_address_all(address: &T) { + unsafe { + let addr = addr_of!(*address).cast::(); + c::WakeByAddressAll(addr); + } + } +} +#[cfg(not(target_vendor = "win7"))] +pub use futex::*; diff --git a/library/std/src/sys/pal/windows/c.rs b/library/std/src/sys/pal/windows/c.rs index afa9240940494..584e17cd19677 100644 --- a/library/std/src/sys/pal/windows/c.rs +++ b/library/std/src/sys/pal/windows/c.rs @@ -36,6 +36,7 @@ pub type LPVOID = *mut c_void; pub type LPWCH = *mut WCHAR; pub type LPWSTR = *mut WCHAR; +#[cfg(target_vendor = "win7")] pub type PSRWLOCK = *mut SRWLOCK; pub type socklen_t = c_int; @@ -50,7 +51,9 @@ pub const INVALID_HANDLE_VALUE: HANDLE = ::core::ptr::without_provenance_mut(-1i pub const EXIT_SUCCESS: u32 = 0; pub const EXIT_FAILURE: u32 = 1; +#[cfg(target_vendor = "win7")] pub const CONDITION_VARIABLE_INIT: CONDITION_VARIABLE = CONDITION_VARIABLE { Ptr: ptr::null_mut() }; +#[cfg(target_vendor = "win7")] pub const SRWLOCK_INIT: SRWLOCK = SRWLOCK { Ptr: ptr::null_mut() }; pub const INIT_ONCE_STATIC_INIT: INIT_ONCE = INIT_ONCE { Ptr: ptr::null_mut() }; @@ -373,6 +376,7 @@ extern "system" { dwmilliseconds: u32, ) -> BOOL; pub fn WakeByAddressSingle(address: *const c_void); + pub fn WakeByAddressAll(address: *const c_void); } #[cfg(target_vendor = "win7")] diff --git a/library/std/src/sys/pal/windows/mod.rs b/library/std/src/sys/pal/windows/mod.rs index a53c4034d0685..6b0bc105fc98d 100644 --- a/library/std/src/sys/pal/windows/mod.rs +++ b/library/std/src/sys/pal/windows/mod.rs @@ -39,7 +39,7 @@ cfg_if::cfg_if! { } } -mod api; +pub(in crate::sys) mod api; /// Map a Result to io::Result. trait IoResult { diff --git a/src/tools/miri/src/shims/windows/foreign_items.rs b/src/tools/miri/src/shims/windows/foreign_items.rs index fdd7fc5fad4e8..a0d3521152fae 100644 --- a/src/tools/miri/src/shims/windows/foreign_items.rs +++ b/src/tools/miri/src/shims/windows/foreign_items.rs @@ -355,6 +355,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { this.WakeByAddressSingle(ptr_op)?; } + "WakeByAddressAll" => { + let [ptr_op] = + this.check_shim(abi, Abi::System { unwind: false }, link_name, args)?; + + this.WakeByAddressAll(ptr_op)?; + } // Dynamic symbol loading "GetProcAddress" => { diff --git a/src/tools/miri/src/shims/windows/sync.rs b/src/tools/miri/src/shims/windows/sync.rs index 2b9801fea68e6..1ce385aaabad4 100644 --- a/src/tools/miri/src/shims/windows/sync.rs +++ b/src/tools/miri/src/shims/windows/sync.rs @@ -384,6 +384,21 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { Ok(()) } + fn WakeByAddressAll(&mut self, ptr_op: &OpTy<'tcx, Provenance>) -> InterpResult<'tcx> { + let this = self.eval_context_mut(); + + let ptr = this.read_pointer(ptr_op)?; + + // See the Linux futex implementation for why this fence exists. + this.atomic_fence(AtomicFenceOrd::SeqCst)?; + + while let Some(thread) = this.futex_wake(ptr.addr().bytes(), u32::MAX) { + this.unblock_thread(thread); + this.unregister_timeout_callback_if_exists(thread); + } + + Ok(()) + } fn SleepConditionVariableSRW( &mut self,