Skip to content

Commit

Permalink
Rollup merge of #127813 - ChrisDenton:win-futex, r=joboet
Browse files Browse the repository at this point in the history
Prevent double reference in generic futex

In the Windows futex implementation we were a little lax at allowing references to references (i.e. `&&`) which can lead to deadlocks due to reading the wrong memory address. This uses a trait to tighten the constraints and ensure this doesn't happen.

r? libs
  • Loading branch information
tgross35 authored Jul 17, 2024
2 parents e1e6065 + 0585c4a commit 8352966
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 5 deletions.
11 changes: 7 additions & 4 deletions library/std/src/sys/pal/windows/futex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ pub type SmallAtomic = AtomicU8;
/// Must be the underlying type of SmallAtomic
pub type SmallPrimitive = u8;

pub unsafe trait Futex {}
pub unsafe trait Waitable {
type Atomic;
}
Expand All @@ -24,6 +25,7 @@ macro_rules! unsafe_waitable_int {
unsafe impl Waitable for $int {
type Atomic = $atomic;
}
unsafe impl Futex for $atomic {}
)*
};
}
Expand All @@ -46,6 +48,7 @@ unsafe impl<T> Waitable for *const T {
unsafe impl<T> Waitable for *mut T {
type Atomic = AtomicPtr<T>;
}
unsafe impl<T> Futex for AtomicPtr<T> {}

pub fn wait_on_address<W: Waitable>(
address: &W::Atomic,
Expand All @@ -61,14 +64,14 @@ pub fn wait_on_address<W: Waitable>(
}
}

pub fn wake_by_address_single<T>(address: &T) {
pub fn wake_by_address_single<T: Futex>(address: &T) {
unsafe {
let addr = ptr::from_ref(address).cast::<c_void>();
c::WakeByAddressSingle(addr);
}
}

pub fn wake_by_address_all<T>(address: &T) {
pub fn wake_by_address_all<T: Futex>(address: &T) {
unsafe {
let addr = ptr::from_ref(address).cast::<c_void>();
c::WakeByAddressAll(addr);
Expand All @@ -80,11 +83,11 @@ pub fn futex_wait<W: Waitable>(futex: &W::Atomic, expected: W, timeout: Option<D
wait_on_address(futex, expected, timeout) || api::get_last_error() != WinError::TIMEOUT
}

pub fn futex_wake<T>(futex: &T) -> bool {
pub fn futex_wake<T: Futex>(futex: &T) -> bool {
wake_by_address_single(futex);
false
}

pub fn futex_wake_all<T>(futex: &T) {
pub fn futex_wake_all<T: Futex>(futex: &T) {
wake_by_address_all(futex)
}
2 changes: 1 addition & 1 deletion library/std/src/sys/sync/once/futex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ impl<'a> Drop for CompletionGuard<'a> {
// up on the Once. `futex_wake_all` does its own synchronization, hence
// we do not need `AcqRel`.
if self.state.swap(self.set_state_on_drop_to, Release) == QUEUED {
futex_wake_all(&self.state);
futex_wake_all(self.state);
}
}
}
Expand Down

0 comments on commit 8352966

Please sign in to comment.