about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2024-03-06 12:19:40 +0000
committerbors <bors@rust-lang.org>2024-03-06 12:19:40 +0000
commit3314d5ce4c209e840c2e4b2c4442f6e031ae0989 (patch)
tree76e2b5df951aadb1db86b55c6efcb1f63906320e
parent09bc67b9158392361780e779d32997f14cc75c39 (diff)
parentcf83d83c77eb9c5a08debc818156815ae5bf759f (diff)
downloadrust-3314d5ce4c209e840c2e4b2c4442f6e031ae0989.tar.gz
rust-3314d5ce4c209e840c2e4b2c4442f6e031ae0989.zip
Auto merge of #121956 - ChrisDenton:srwlock, r=joboet
Windows: Implement condvar, mutex and rwlock using futex

Well, the Windows equivalent: [`WaitOnAddress`,](https://learn.microsoft.com/en-us/windows/win32/api/synchapi/nf-synchapi-waitonaddress) [`WakeByAddressSingle`](https://learn.microsoft.com/en-us/windows/win32/api/synchapi/nf-synchapi-wakebyaddresssingle) and [`WakeByAddressAll`](https://learn.microsoft.com/en-us/windows/win32/api/synchapi/nf-synchapi-wakebyaddressall).

Note that Windows flavoured futexes can be different sizes (1, 2, 4 or 8 bytes). I took advantage of that in the `Mutex` implementation.

I also edited the Mutex implementation a bit more than necessary. I was having trouble keeping in my head what 0, 1 and 2 meant so I replaced them with consts.

I *think* we're maybe spinning a bit much. `WaitOnAddress` seems to be looping quite a bit too. But for now I've keep the implementations the same. I do wonder if it'd be worth reducing or removing our spinning on Windows.

This also adds a new shim to miri, because of course it does.

Fixes #121949
-rw-r--r--library/std/src/sys/locks/condvar/mod.rs7
-rw-r--r--library/std/src/sys/locks/condvar/windows7.rs (renamed from library/std/src/sys/locks/condvar/windows.rs)0
-rw-r--r--library/std/src/sys/locks/mutex/futex.rs54
-rw-r--r--library/std/src/sys/locks/mutex/mod.rs7
-rw-r--r--library/std/src/sys/locks/mutex/windows7.rs (renamed from library/std/src/sys/locks/mutex/windows.rs)0
-rw-r--r--library/std/src/sys/locks/rwlock/mod.rs7
-rw-r--r--library/std/src/sys/locks/rwlock/windows7.rs (renamed from library/std/src/sys/locks/rwlock/windows.rs)0
-rw-r--r--library/std/src/sys/pal/windows/c.rs4
-rw-r--r--library/std/src/sys/pal/windows/futex.rs85
-rw-r--r--library/std/src/sys/pal/windows/mod.rs2
-rw-r--r--src/tools/miri/src/shims/windows/foreign_items.rs6
-rw-r--r--src/tools/miri/src/shims/windows/sync.rs15
-rw-r--r--tests/debuginfo/mutex.rs2
-rw-r--r--tests/debuginfo/rwlock-read.rs2
14 files changed, 159 insertions, 32 deletions
diff --git a/library/std/src/sys/locks/condvar/mod.rs b/library/std/src/sys/locks/condvar/mod.rs
index 126a42a2a4c..6849cacf88e 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
index 28a288335d2..28a288335d2 100644
--- a/library/std/src/sys/locks/condvar/windows.rs
+++ b/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 c01229586c3..7427cae94d6 100644
--- a/library/std/src/sys/locks/mutex/futex.rs
+++ b/library/std/src/sys/locks/mutex/futex.rs
@@ -1,30 +1,42 @@
 use crate::sync::atomic::{
-    AtomicU32,
+    self,
     Ordering::{Acquire, Relaxed, Release},
 };
 use crate::sys::futex::{futex_wait, futex_wake};
 
+cfg_if::cfg_if! {
+if #[cfg(windows)] {
+    // On Windows we can have a smol futex
+    type Atomic = atomic::AtomicU8;
+    type State = u8;
+} else {
+    type Atomic = atomic::AtomicU32;
+    type State = u32;
+}
+}
+
 pub struct Mutex {
-    /// 0: unlocked
-    /// 1: locked, no other threads waiting
-    /// 2: locked, and other threads waiting (contended)
-    futex: AtomicU32,
+    futex: Atomic,
 }
 
+const UNLOCKED: State = 0;
+const LOCKED: State = 1; // locked, no other threads waiting
+const CONTENDED: State = 2; // locked, and other threads waiting (contended)
+
 impl Mutex {
     #[inline]
     pub const fn new() -> Self {
-        Self { futex: AtomicU32::new(0) }
+        Self { futex: Atomic::new(UNLOCKED) }
     }
 
     #[inline]
     pub fn try_lock(&self) -> bool {
-        self.futex.compare_exchange(0, 1, Acquire, Relaxed).is_ok()
+        self.futex.compare_exchange(UNLOCKED, LOCKED, Acquire, Relaxed).is_ok()
     }
 
     #[inline]
     pub fn lock(&self) {
-        if self.futex.compare_exchange(0, 1, Acquire, Relaxed).is_err() {
+        if self.futex.compare_exchange(UNLOCKED, LOCKED, Acquire, Relaxed).is_err() {
             self.lock_contended();
         }
     }
@@ -36,8 +48,8 @@ impl Mutex {
 
         // 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) {
+        if state == UNLOCKED {
+            match self.futex.compare_exchange(UNLOCKED, LOCKED, Acquire, Relaxed) {
                 Ok(_) => return, // Locked!
                 Err(s) => state = s,
             }
@@ -45,31 +57,31 @@ impl Mutex {
 
         loop {
             // Put the lock in contended state.
-            // We avoid an unnecessary write if it as already set to 2,
+            // We avoid an unnecessary write if it as already set to CONTENDED,
             // 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.
+            if state != CONTENDED && self.futex.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 2.
-            futex_wait(&self.futex, 2, None);
+            // Wait for the futex to change state, assuming it is still CONTENDED.
+            futex_wait(&self.futex, CONTENDED, None);
 
             // Spin again after waking up.
             state = self.spin();
         }
     }
 
-    fn spin(&self) -> u32 {
+    fn spin(&self) -> State {
         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 {
+            // We stop spinning when the mutex is UNLOCKED,
+            // but also when it's CONTENDED.
+            if state != LOCKED || spin == 0 {
                 return state;
             }
 
@@ -80,9 +92,9 @@ impl Mutex {
 
     #[inline]
     pub unsafe fn unlock(&self) {
-        if self.futex.swap(0, Release) == 2 {
+        if self.futex.swap(UNLOCKED, Release) == CONTENDED {
             // We only wake up one thread. When that thread locks the mutex, it
-            // will mark the mutex as contended (2) (see lock_contended above),
+            // 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();
diff --git a/library/std/src/sys/locks/mutex/mod.rs b/library/std/src/sys/locks/mutex/mod.rs
index 710cb91fb14..73d9bd273de 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
index ef2f84082cd..ef2f84082cd 100644
--- a/library/std/src/sys/locks/mutex/windows.rs
+++ b/library/std/src/sys/locks/mutex/windows7.rs
diff --git a/library/std/src/sys/locks/rwlock/mod.rs b/library/std/src/sys/locks/rwlock/mod.rs
index 0564f1fe6fa..675931c64bd 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
index e69415baac4..e69415baac4 100644
--- a/library/std/src/sys/locks/rwlock/windows.rs
+++ b/library/std/src/sys/locks/rwlock/windows7.rs
diff --git a/library/std/src/sys/pal/windows/c.rs b/library/std/src/sys/pal/windows/c.rs
index afa92409404..584e17cd196 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/futex.rs b/library/std/src/sys/pal/windows/futex.rs
new file mode 100644
index 00000000000..bc19c402d9c
--- /dev/null
+++ b/library/std/src/sys/pal/windows/futex.rs
@@ -0,0 +1,85 @@
+use super::api;
+use crate::sys::c;
+use crate::sys::dur2timeout;
+use core::ffi::c_void;
+use core::mem;
+use core::ptr;
+use core::sync::atomic::{
+    AtomicBool, AtomicI16, AtomicI32, AtomicI64, AtomicI8, AtomicIsize, AtomicPtr, AtomicU16,
+    AtomicU32, AtomicU64, AtomicU8, AtomicUsize,
+};
+use core::time::Duration;
+
+pub unsafe trait Waitable {
+    type Atomic;
+}
+macro_rules! unsafe_waitable_int {
+    ($(($int:ty, $atomic:ty)),*$(,)?) => {
+        $(
+            unsafe impl Waitable for $int {
+                type Atomic = $atomic;
+            }
+        )*
+    };
+}
+unsafe_waitable_int! {
+    (bool, AtomicBool),
+    (i8, AtomicI8),
+    (i16, AtomicI16),
+    (i32, AtomicI32),
+    (i64, AtomicI64),
+    (isize, AtomicIsize),
+    (u8, AtomicU8),
+    (u16, AtomicU16),
+    (u32, AtomicU32),
+    (u64, AtomicU64),
+    (usize, AtomicUsize),
+}
+unsafe impl<T> Waitable for *const T {
+    type Atomic = AtomicPtr<T>;
+}
+unsafe impl<T> Waitable for *mut T {
+    type Atomic = AtomicPtr<T>;
+}
+
+pub fn wait_on_address<W: Waitable>(
+    address: &W::Atomic,
+    compare: W,
+    timeout: Option<Duration>,
+) -> bool {
+    unsafe {
+        let addr = ptr::from_ref(address).cast::<c_void>();
+        let size = mem::size_of::<W>();
+        let compare_addr = ptr::addr_of!(compare).cast::<c_void>();
+        let timeout = timeout.map(dur2timeout).unwrap_or(c::INFINITE);
+        c::WaitOnAddress(addr, compare_addr, size, timeout) == c::TRUE
+    }
+}
+
+pub fn wake_by_address_single<T>(address: &T) {
+    unsafe {
+        let addr = ptr::from_ref(address).cast::<c_void>();
+        c::WakeByAddressSingle(addr);
+    }
+}
+
+pub fn wake_by_address_all<T>(address: &T) {
+    unsafe {
+        let addr = ptr::from_ref(address).cast::<c_void>();
+        c::WakeByAddressAll(addr);
+    }
+}
+
+pub fn futex_wait<W: Waitable>(futex: &W::Atomic, expected: W, timeout: Option<Duration>) -> bool {
+    // return false only on timeout
+    wait_on_address(futex, expected, timeout) || api::get_last_error().code != c::ERROR_TIMEOUT
+}
+
+pub fn futex_wake<T>(futex: &T) -> bool {
+    wake_by_address_single(futex);
+    false
+}
+
+pub fn futex_wake_all<T>(futex: &T) {
+    wake_by_address_all(futex)
+}
diff --git a/library/std/src/sys/pal/windows/mod.rs b/library/std/src/sys/pal/windows/mod.rs
index a53c4034d06..6a561518fad 100644
--- a/library/std/src/sys/pal/windows/mod.rs
+++ b/library/std/src/sys/pal/windows/mod.rs
@@ -17,6 +17,8 @@ pub mod args;
 pub mod c;
 pub mod env;
 pub mod fs;
+#[cfg(not(target_vendor = "win7"))]
+pub mod futex;
 pub mod handle;
 pub mod io;
 pub mod net;
diff --git a/src/tools/miri/src/shims/windows/foreign_items.rs b/src/tools/miri/src/shims/windows/foreign_items.rs
index 734737a86dd..f56ea06dbe3 100644
--- a/src/tools/miri/src/shims/windows/foreign_items.rs
+++ b/src/tools/miri/src/shims/windows/foreign_items.rs
@@ -366,6 +366,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 2b9801fea68..1ce385aaaba 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,
diff --git a/tests/debuginfo/mutex.rs b/tests/debuginfo/mutex.rs
index 1201da0dd7b..affc1558ffa 100644
--- a/tests/debuginfo/mutex.rs
+++ b/tests/debuginfo/mutex.rs
@@ -10,7 +10,7 @@
 //
 // cdb-command:dx m,d
 // cdb-check:m,d              [Type: std::sync::mutex::Mutex<i32>]
-// cdb-check:    [...] inner            [Type: std::sys::locks::mutex::windows::Mutex]
+// cdb-check:    [...] inner            [Type: std::sys::locks::mutex::futex::Mutex]
 // cdb-check:    [...] poison           [Type: std::sync::poison::Flag]
 // cdb-check:    [...] data             : 0 [Type: core::cell::UnsafeCell<i32>]
 
diff --git a/tests/debuginfo/rwlock-read.rs b/tests/debuginfo/rwlock-read.rs
index 7abbfd70ffb..76dbc73a1e9 100644
--- a/tests/debuginfo/rwlock-read.rs
+++ b/tests/debuginfo/rwlock-read.rs
@@ -16,7 +16,7 @@
 // cdb-command:dx r
 // cdb-check:r                [Type: std::sync::rwlock::RwLockReadGuard<i32>]
 // cdb-check:    [...] data             : NonNull([...]: 0) [Type: core::ptr::non_null::NonNull<i32>]
-// cdb-check:    [...] inner_lock       : [...] [Type: std::sys::locks::rwlock::windows::RwLock *]
+// cdb-check:    [...] inner_lock       : [...] [Type: std::sys::locks::rwlock::futex::RwLock *]
 
 #[allow(unused_variables)]