From afe1a256ce6612c8c98b5f8a15f797dced1cf696 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Thu, 28 Apr 2022 10:32:11 +0200 Subject: [PATCH 01/11] Use futex-based locks and thread parker on OpenBSD. --- library/std/src/sys/unix/futex.rs | 52 ++++++++++++++++++- library/std/src/sys/unix/locks/mod.rs | 1 + .../std/src/sys_common/thread_parker/mod.rs | 1 + 3 files changed, 53 insertions(+), 1 deletion(-) diff --git a/library/std/src/sys/unix/futex.rs b/library/std/src/sys/unix/futex.rs index c12ee169e797a..f86dd560d6339 100644 --- a/library/std/src/sys/unix/futex.rs +++ b/library/std/src/sys/unix/futex.rs @@ -1,7 +1,8 @@ #![cfg(any( target_os = "linux", target_os = "android", - all(target_os = "emscripten", target_feature = "atomics") + all(target_os = "emscripten", target_feature = "atomics"), + target_os = "openbsd", ))] use crate::sync::atomic::AtomicU32; @@ -81,6 +82,55 @@ pub fn futex_wake_all(futex: &AtomicU32) { } } +#[cfg(target_os = "openbsd")] +pub fn futex_wait(futex: &AtomicU32, expected: u32, timeout: Option) -> bool { + use crate::convert::TryInto; + use crate::ptr::{null, null_mut}; + let timespec = timeout.and_then(|d| { + Some(libc::timespec { + // Sleep forever if the timeout is longer than fits in a timespec. + tv_sec: d.as_secs().try_into().ok()?, + // This conversion never truncates, as subsec_nanos is always <1e9. + tv_nsec: d.subsec_nanos() as _, + }) + }); + + let r = unsafe { + libc::futex( + futex as *const AtomicU32 as *mut u32, + libc::FUTEX_WAIT, + expected as i32, + timespec.as_ref().map_or(null(), |t| t as *const libc::timespec), + null_mut(), + ) + }; + + r == 0 || super::os::errno() != libc::ETIMEDOUT +} + +#[cfg(target_os = "openbsd")] +pub fn futex_wake(futex: &AtomicU32) -> bool { + use crate::ptr::{null, null_mut}; + unsafe { + libc::futex(futex as *const AtomicU32 as *mut u32, libc::FUTEX_WAKE, 1, null(), null_mut()) + > 0 + } +} + +#[cfg(target_os = "openbsd")] +pub fn futex_wake_all(futex: &AtomicU32) { + use crate::ptr::{null, null_mut}; + unsafe { + libc::futex( + futex as *const AtomicU32 as *mut u32, + libc::FUTEX_WAKE, + i32::MAX, + null(), + null_mut(), + ); + } +} + #[cfg(target_os = "emscripten")] extern "C" { fn emscripten_futex_wake(addr: *const AtomicU32, count: libc::c_int) -> libc::c_int; diff --git a/library/std/src/sys/unix/locks/mod.rs b/library/std/src/sys/unix/locks/mod.rs index 3e39c8b9b23e7..f7ec4763a0463 100644 --- a/library/std/src/sys/unix/locks/mod.rs +++ b/library/std/src/sys/unix/locks/mod.rs @@ -3,6 +3,7 @@ cfg_if::cfg_if! { target_os = "linux", target_os = "android", all(target_os = "emscripten", target_feature = "atomics"), + target_os = "openbsd", ))] { mod futex; mod futex_rwlock; diff --git a/library/std/src/sys_common/thread_parker/mod.rs b/library/std/src/sys_common/thread_parker/mod.rs index ea0204cd357ba..bb6a40f837984 100644 --- a/library/std/src/sys_common/thread_parker/mod.rs +++ b/library/std/src/sys_common/thread_parker/mod.rs @@ -3,6 +3,7 @@ cfg_if::cfg_if! { target_os = "linux", target_os = "android", all(target_arch = "wasm32", target_feature = "atomics"), + target_os = "openbsd", ))] { mod futex; pub use futex::Parker; From 2dfad1e3f81d649124bba15f96fd8e96431cd4fc Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Thu, 28 Apr 2022 11:03:30 +0200 Subject: [PATCH 02/11] Use futex-based locks and thread parker on NetBSD. --- library/std/src/sys/unix/futex.rs | 75 +++++++++++++------ library/std/src/sys/unix/locks/mod.rs | 1 + .../std/src/sys_common/thread_parker/mod.rs | 1 + 3 files changed, 53 insertions(+), 24 deletions(-) diff --git a/library/std/src/sys/unix/futex.rs b/library/std/src/sys/unix/futex.rs index f86dd560d6339..64ed146b0367a 100644 --- a/library/std/src/sys/unix/futex.rs +++ b/library/std/src/sys/unix/futex.rs @@ -3,17 +3,21 @@ target_os = "android", all(target_os = "emscripten", target_feature = "atomics"), target_os = "openbsd", + target_os = "netbsd", ))] use crate::sync::atomic::AtomicU32; use crate::time::Duration; +#[cfg(target_os = "netbsd")] +pub const SYS___futex: i32 = 166; + /// Wait for a futex_wake operation to wake us. /// /// Returns directly if the futex doesn't hold the expected value. /// /// Returns false on timeout, and true in all other cases. -#[cfg(any(target_os = "linux", target_os = "android"))] +#[cfg(any(target_os = "linux", target_os = "android", target_os = "netbsd"))] pub fn futex_wait(futex: &AtomicU32, expected: u32, timeout: Option) -> bool { use super::time::Timespec; use crate::ptr::null; @@ -34,15 +38,32 @@ pub fn futex_wait(futex: &AtomicU32, expected: u32, timeout: Option) - // Use FUTEX_WAIT_BITSET rather than FUTEX_WAIT to be able to give an // absolute time rather than a relative time. let r = unsafe { - libc::syscall( - libc::SYS_futex, - futex as *const AtomicU32, - libc::FUTEX_WAIT_BITSET | libc::FUTEX_PRIVATE_FLAG, - expected, - timespec.as_ref().map_or(null(), |t| &t.t as *const libc::timespec), - null::(), // This argument is unused for FUTEX_WAIT_BITSET. - !0u32, // A full bitmask, to make it behave like a regular FUTEX_WAIT. - ) + cfg_if::cfg_if! { + if #[cfg(target_os = "netbsd")] { + // Netbsd's futex syscall takes addr2 and val2 as separate arguments. + // (Both are unused for FUTEX_WAIT[_BITSET].) + libc::syscall( + SYS___futex, + futex as *const AtomicU32, + libc::FUTEX_WAIT_BITSET | libc::FUTEX_PRIVATE_FLAG, + expected, + timespec.as_ref().map_or(null(), |t| &t.t as *const libc::timespec), + null::(), // addr2: This argument is unused for FUTEX_WAIT_BITSET. + 0, // val2: This argument is unused for FUTEX_WAIT_BITSET. + !0u32, // val3 / bitmask: A full bitmask, to make it behave like a regular FUTEX_WAIT. + ) + } else { + libc::syscall( + libc::SYS_futex, + futex as *const AtomicU32, + libc::FUTEX_WAIT_BITSET | libc::FUTEX_PRIVATE_FLAG, + expected, + timespec.as_ref().map_or(null(), |t| &t.t as *const libc::timespec), + null::(), // This argument is unused for FUTEX_WAIT_BITSET. + !0u32, // A full bitmask, to make it behave like a regular FUTEX_WAIT. + ) + } + } }; match (r < 0).then(super::os::errno) { @@ -57,28 +78,34 @@ pub fn futex_wait(futex: &AtomicU32, expected: u32, timeout: Option) - /// /// Returns true if this actually woke up such a thread, /// or false if no thread was waiting on this futex. -#[cfg(any(target_os = "linux", target_os = "android"))] +#[cfg(any(target_os = "linux", target_os = "android", target_os = "netbsd"))] pub fn futex_wake(futex: &AtomicU32) -> bool { + let ptr = futex as *const AtomicU32; + let op = libc::FUTEX_WAKE | libc::FUTEX_PRIVATE_FLAG; unsafe { - libc::syscall( - libc::SYS_futex, - futex as *const AtomicU32, - libc::FUTEX_WAKE | libc::FUTEX_PRIVATE_FLAG, - 1, - ) > 0 + cfg_if::cfg_if! { + if #[cfg(target_os = "netbsd")] { + libc::syscall(SYS___futex, ptr, op, 1) > 0 + } else { + libc::syscall(libc::SYS_futex, ptr, op, 1) > 0 + } + } } } /// Wake up all threads that are waiting on futex_wait on this futex. -#[cfg(any(target_os = "linux", target_os = "android"))] +#[cfg(any(target_os = "linux", target_os = "android", target_os = "netbsd"))] pub fn futex_wake_all(futex: &AtomicU32) { + let ptr = futex as *const AtomicU32; + let op = libc::FUTEX_WAKE | libc::FUTEX_PRIVATE_FLAG; unsafe { - libc::syscall( - libc::SYS_futex, - futex as *const AtomicU32, - libc::FUTEX_WAKE | libc::FUTEX_PRIVATE_FLAG, - i32::MAX, - ); + cfg_if::cfg_if! { + if #[cfg(target_os = "netbsd")] { + libc::syscall(SYS___futex, ptr, op, i32::MAX); + } else { + libc::syscall(libc::SYS_futex, ptr, op, i32::MAX); + } + } } } diff --git a/library/std/src/sys/unix/locks/mod.rs b/library/std/src/sys/unix/locks/mod.rs index f7ec4763a0463..61fc0d5841dc9 100644 --- a/library/std/src/sys/unix/locks/mod.rs +++ b/library/std/src/sys/unix/locks/mod.rs @@ -4,6 +4,7 @@ cfg_if::cfg_if! { target_os = "android", all(target_os = "emscripten", target_feature = "atomics"), target_os = "openbsd", + target_os = "netbsd", ))] { mod futex; mod futex_rwlock; diff --git a/library/std/src/sys_common/thread_parker/mod.rs b/library/std/src/sys_common/thread_parker/mod.rs index bb6a40f837984..4ff76aea23dc4 100644 --- a/library/std/src/sys_common/thread_parker/mod.rs +++ b/library/std/src/sys_common/thread_parker/mod.rs @@ -4,6 +4,7 @@ cfg_if::cfg_if! { target_os = "android", all(target_arch = "wasm32", target_feature = "atomics"), target_os = "openbsd", + target_os = "netbsd", ))] { mod futex; pub use futex::Parker; From 69f0bcb26def1bccdf3774fc487201258b746fca Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Thu, 28 Apr 2022 11:28:40 +0200 Subject: [PATCH 03/11] Use futex-based locks and thread parker on DragonFlyBSD. --- library/std/src/sys/unix/futex.rs | 30 +++++++++++++++++++ .../std/src/sys/unix/locks/futex_rwlock.rs | 13 +++++++- library/std/src/sys/unix/locks/mod.rs | 1 + .../std/src/sys_common/thread_parker/mod.rs | 1 + 4 files changed, 44 insertions(+), 1 deletion(-) diff --git a/library/std/src/sys/unix/futex.rs b/library/std/src/sys/unix/futex.rs index 64ed146b0367a..cfb2d1f07de25 100644 --- a/library/std/src/sys/unix/futex.rs +++ b/library/std/src/sys/unix/futex.rs @@ -4,6 +4,7 @@ all(target_os = "emscripten", target_feature = "atomics"), target_os = "openbsd", target_os = "netbsd", + target_os = "dragonfly", ))] use crate::sync::atomic::AtomicU32; @@ -158,6 +159,35 @@ pub fn futex_wake_all(futex: &AtomicU32) { } } +#[cfg(target_os = "dragonfly")] +pub fn futex_wait(futex: &AtomicU32, expected: u32, timeout: Option) -> bool { + use crate::convert::TryFrom; + let r = unsafe { + libc::umtx_sleep( + futex as *const AtomicU32 as *const i32, + expected as i32, + // A timeout of 0 means infinite, so we round smaller timeouts up to 1 millisecond. + // Timeouts larger than i32::MAX milliseconds saturate. + timeout.map_or(0, |d| { + i32::try_from(d.as_millis()).map_or(i32::MAX, |millis| millis.max(1)) + }), + ) + }; + + r == 0 || super::os::errno() != libc::ETIMEDOUT +} + +// DragonflyBSD doesn't tell us how many threads are woken up, so this doesn't return a bool. +#[cfg(target_os = "dragonfly")] +pub fn futex_wake(futex: &AtomicU32) { + unsafe { libc::umtx_wakeup(futex as *const AtomicU32 as *const i32, 1) }; +} + +#[cfg(target_os = "dragonfly")] +pub fn futex_wake_all(futex: &AtomicU32) { + unsafe { libc::umtx_wakeup(futex as *const AtomicU32 as *const i32, i32::MAX) }; +} + #[cfg(target_os = "emscripten")] extern "C" { fn emscripten_futex_wake(addr: *const AtomicU32, count: libc::c_int) -> libc::c_int; diff --git a/library/std/src/sys/unix/locks/futex_rwlock.rs b/library/std/src/sys/unix/locks/futex_rwlock.rs index e42edb2585834..8829ed4db2574 100644 --- a/library/std/src/sys/unix/locks/futex_rwlock.rs +++ b/library/std/src/sys/unix/locks/futex_rwlock.rs @@ -283,7 +283,18 @@ impl RwLock { /// writer that was about to go to sleep. fn wake_writer(&self) -> bool { self.writer_notify.fetch_add(1, Release); - futex_wake(&self.writer_notify) + cfg_if::cfg_if! { + if #[cfg(target_os = "dragonfly")] { + // DragonFlyBSD doesn't tell us whether it woke up any threads or not. + // So, we always return `false` here, as that still results in correct behaviour. + // The downside is an extra syscall in case both readers and writers were waiting, + // and unnecessarily waking up readers when a writer is about to attempt to lock the lock. + futex_wake(&self.writer_notify); + false + } else { + futex_wake(&self.writer_notify) + } + } } /// Spin for a while, but stop directly at the given condition. diff --git a/library/std/src/sys/unix/locks/mod.rs b/library/std/src/sys/unix/locks/mod.rs index 61fc0d5841dc9..d39da200dda19 100644 --- a/library/std/src/sys/unix/locks/mod.rs +++ b/library/std/src/sys/unix/locks/mod.rs @@ -5,6 +5,7 @@ cfg_if::cfg_if! { all(target_os = "emscripten", target_feature = "atomics"), target_os = "openbsd", target_os = "netbsd", + target_os = "dragonfly", ))] { mod futex; mod futex_rwlock; diff --git a/library/std/src/sys_common/thread_parker/mod.rs b/library/std/src/sys_common/thread_parker/mod.rs index 4ff76aea23dc4..7fd4d3610caae 100644 --- a/library/std/src/sys_common/thread_parker/mod.rs +++ b/library/std/src/sys_common/thread_parker/mod.rs @@ -5,6 +5,7 @@ cfg_if::cfg_if! { all(target_arch = "wasm32", target_feature = "atomics"), target_os = "openbsd", target_os = "netbsd", + target_os = "dragonfly", ))] { mod futex; pub use futex::Parker; From 04b0bc97bb84915f2487743b7dde7993ddb34e5d Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Thu, 28 Apr 2022 12:22:14 +0200 Subject: [PATCH 04/11] Use futex-based locks and thread parker on FreeBSD. --- library/std/src/sys/unix/futex.rs | 58 ++++++++++++++++++- .../std/src/sys/unix/locks/futex_rwlock.rs | 4 +- library/std/src/sys/unix/locks/mod.rs | 1 + .../std/src/sys_common/thread_parker/mod.rs | 1 + 4 files changed, 60 insertions(+), 4 deletions(-) diff --git a/library/std/src/sys/unix/futex.rs b/library/std/src/sys/unix/futex.rs index cfb2d1f07de25..4f91d48f2f074 100644 --- a/library/std/src/sys/unix/futex.rs +++ b/library/std/src/sys/unix/futex.rs @@ -2,6 +2,7 @@ target_os = "linux", target_os = "android", all(target_os = "emscripten", target_feature = "atomics"), + target_os = "freebsd", target_os = "openbsd", target_os = "netbsd", target_os = "dragonfly", @@ -18,7 +19,12 @@ pub const SYS___futex: i32 = 166; /// Returns directly if the futex doesn't hold the expected value. /// /// Returns false on timeout, and true in all other cases. -#[cfg(any(target_os = "linux", target_os = "android", target_os = "netbsd"))] +#[cfg(any( + target_os = "linux", + target_os = "android", + target_os = "freebsd", + target_os = "netbsd" +))] pub fn futex_wait(futex: &AtomicU32, expected: u32, timeout: Option) -> bool { use super::time::Timespec; use crate::ptr::null; @@ -40,7 +46,26 @@ pub fn futex_wait(futex: &AtomicU32, expected: u32, timeout: Option) - // absolute time rather than a relative time. let r = unsafe { cfg_if::cfg_if! { - if #[cfg(target_os = "netbsd")] { + if #[cfg(target_os = "freebsd")] { + // FreeBSD doesn't have futex(), but it has + // _umtx_op(UMTX_OP_WAIT_UINT_PRIVATE), which is nearly + // identical. It supports absolute timeouts through a flag + // in the _umtx_time struct. + let umtx_timeout = timespec.map(|t| libc::_umtx_time { + _timeout: t.t, + _flags: libc::UMTX_ABSTIME, + _clockid: libc::CLOCK_MONOTONIC as u32, + }); + let umtx_timeout_ptr = umtx_timeout.as_ref().map_or(null(), |t| t as *const _); + let umtx_timeout_size = umtx_timeout.as_ref().map_or(0, |t| crate::mem::size_of_val(t)); + libc::_umtx_op( + futex as *const AtomicU32 as *mut _, + libc::UMTX_OP_WAIT_UINT_PRIVATE, + expected as libc::c_ulong, + crate::ptr::invalid_mut(umtx_timeout_size), + umtx_timeout_ptr as *mut _, + ) + } else if #[cfg(target_os = "netbsd")] { // Netbsd's futex syscall takes addr2 and val2 as separate arguments. // (Both are unused for FUTEX_WAIT[_BITSET].) libc::syscall( @@ -110,6 +135,35 @@ pub fn futex_wake_all(futex: &AtomicU32) { } } +// FreeBSD doesn't tell us how many threads are woken up, so this doesn't return a bool. +#[cfg(target_os = "freebsd")] +pub fn futex_wake(futex: &AtomicU32) { + use crate::ptr::null_mut; + unsafe { + libc::_umtx_op( + futex as *const AtomicU32 as *mut _, + libc::UMTX_OP_WAKE_PRIVATE, + 1, + null_mut(), + null_mut(), + ) + }; +} + +#[cfg(target_os = "freebsd")] +pub fn futex_wake_all(futex: &AtomicU32) { + use crate::ptr::null_mut; + unsafe { + libc::_umtx_op( + futex as *const AtomicU32 as *mut _, + libc::UMTX_OP_WAKE_PRIVATE, + i32::MAX as libc::c_ulong, + null_mut(), + null_mut(), + ) + }; +} + #[cfg(target_os = "openbsd")] pub fn futex_wait(futex: &AtomicU32, expected: u32, timeout: Option) -> bool { use crate::convert::TryInto; diff --git a/library/std/src/sys/unix/locks/futex_rwlock.rs b/library/std/src/sys/unix/locks/futex_rwlock.rs index 8829ed4db2574..9c698f89b0dd7 100644 --- a/library/std/src/sys/unix/locks/futex_rwlock.rs +++ b/library/std/src/sys/unix/locks/futex_rwlock.rs @@ -284,8 +284,8 @@ impl RwLock { fn wake_writer(&self) -> bool { self.writer_notify.fetch_add(1, Release); cfg_if::cfg_if! { - if #[cfg(target_os = "dragonfly")] { - // DragonFlyBSD doesn't tell us whether it woke up any threads or not. + if #[cfg(any(target_os = "freebsd", target_os = "dragonfly"))] { + // FreeBSD and DragonFlyBSD don't tell us whether they woke up any threads or not. // So, we always return `false` here, as that still results in correct behaviour. // The downside is an extra syscall in case both readers and writers were waiting, // and unnecessarily waking up readers when a writer is about to attempt to lock the lock. diff --git a/library/std/src/sys/unix/locks/mod.rs b/library/std/src/sys/unix/locks/mod.rs index d39da200dda19..27d007a1051e5 100644 --- a/library/std/src/sys/unix/locks/mod.rs +++ b/library/std/src/sys/unix/locks/mod.rs @@ -3,6 +3,7 @@ cfg_if::cfg_if! { target_os = "linux", target_os = "android", all(target_os = "emscripten", target_feature = "atomics"), + target_os = "freebsd", target_os = "openbsd", target_os = "netbsd", target_os = "dragonfly", diff --git a/library/std/src/sys_common/thread_parker/mod.rs b/library/std/src/sys_common/thread_parker/mod.rs index 7fd4d3610caae..5305f1f2dee8c 100644 --- a/library/std/src/sys_common/thread_parker/mod.rs +++ b/library/std/src/sys_common/thread_parker/mod.rs @@ -3,6 +3,7 @@ cfg_if::cfg_if! { target_os = "linux", target_os = "android", all(target_arch = "wasm32", target_feature = "atomics"), + target_os = "freebsd", target_os = "openbsd", target_os = "netbsd", target_os = "dragonfly", From 0b4df22f550417d55b32632ccee86a67aca63868 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Fri, 29 Apr 2022 16:31:35 +0200 Subject: [PATCH 05/11] Update libc dependency of std to 0.2.125. --- Cargo.lock | 4 ++-- library/std/Cargo.toml | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index ef4800a226136..be74183628ee9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2070,9 +2070,9 @@ checksum = "830d08ce1d1d941e6b30645f1a0eb5643013d835ce3779a5fc208261dbe10f55" [[package]] name = "libc" -version = "0.2.121" +version = "0.2.125" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "efaa7b300f3b5fe8eb6bf21ce3895e1751d9665086af2d64b42f19701015ff4f" +checksum = "5916d2ae698f6de9bfb891ad7a8d65c09d232dc58cc4ac433c7da3b2fd84bc2b" dependencies = [ "rustc-std-workspace-core", ] diff --git a/library/std/Cargo.toml b/library/std/Cargo.toml index f2d6583206035..d0d7d480fe8e2 100644 --- a/library/std/Cargo.toml +++ b/library/std/Cargo.toml @@ -15,7 +15,7 @@ cfg-if = { version = "0.1.8", features = ['rustc-dep-of-std'] } panic_unwind = { path = "../panic_unwind", optional = true } panic_abort = { path = "../panic_abort" } core = { path = "../core" } -libc = { version = "0.2.116", default-features = false, features = ['rustc-dep-of-std'] } +libc = { version = "0.2.125", default-features = false, features = ['rustc-dep-of-std'] } compiler_builtins = { version = "0.1.71" } profiler_builtins = { path = "../profiler_builtins", optional = true } unwind = { path = "../unwind" } From c4c69143a996a9065a040e7d99c31da7b1ad7a81 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Fri, 29 Apr 2022 16:44:28 +0200 Subject: [PATCH 06/11] Always return false in futex_wake on {Free,DragonFly}BSD. --- library/std/src/sys/unix/futex.rs | 12 ++++++++---- library/std/src/sys/unix/locks/futex_rwlock.rs | 17 +++++------------ 2 files changed, 13 insertions(+), 16 deletions(-) diff --git a/library/std/src/sys/unix/futex.rs b/library/std/src/sys/unix/futex.rs index 4f91d48f2f074..fd046bdf19d6b 100644 --- a/library/std/src/sys/unix/futex.rs +++ b/library/std/src/sys/unix/futex.rs @@ -104,6 +104,8 @@ pub fn futex_wait(futex: &AtomicU32, expected: u32, timeout: Option) - /// /// Returns true if this actually woke up such a thread, /// or false if no thread was waiting on this futex. +/// +/// On some platforms, this always returns false. #[cfg(any(target_os = "linux", target_os = "android", target_os = "netbsd"))] pub fn futex_wake(futex: &AtomicU32) -> bool { let ptr = futex as *const AtomicU32; @@ -135,9 +137,9 @@ pub fn futex_wake_all(futex: &AtomicU32) { } } -// FreeBSD doesn't tell us how many threads are woken up, so this doesn't return a bool. +// FreeBSD doesn't tell us how many threads are woken up, so this always returns false. #[cfg(target_os = "freebsd")] -pub fn futex_wake(futex: &AtomicU32) { +pub fn futex_wake(futex: &AtomicU32) -> bool { use crate::ptr::null_mut; unsafe { libc::_umtx_op( @@ -148,6 +150,7 @@ pub fn futex_wake(futex: &AtomicU32) { null_mut(), ) }; + false } #[cfg(target_os = "freebsd")] @@ -231,10 +234,11 @@ pub fn futex_wait(futex: &AtomicU32, expected: u32, timeout: Option) - r == 0 || super::os::errno() != libc::ETIMEDOUT } -// DragonflyBSD doesn't tell us how many threads are woken up, so this doesn't return a bool. +// DragonflyBSD doesn't tell us how many threads are woken up, so this always returns false. #[cfg(target_os = "dragonfly")] -pub fn futex_wake(futex: &AtomicU32) { +pub fn futex_wake(futex: &AtomicU32) -> bool { unsafe { libc::umtx_wakeup(futex as *const AtomicU32 as *const i32, 1) }; + false } #[cfg(target_os = "dragonfly")] diff --git a/library/std/src/sys/unix/locks/futex_rwlock.rs b/library/std/src/sys/unix/locks/futex_rwlock.rs index 9c698f89b0dd7..57f6d58840db1 100644 --- a/library/std/src/sys/unix/locks/futex_rwlock.rs +++ b/library/std/src/sys/unix/locks/futex_rwlock.rs @@ -283,18 +283,11 @@ impl RwLock { /// writer that was about to go to sleep. fn wake_writer(&self) -> bool { self.writer_notify.fetch_add(1, Release); - cfg_if::cfg_if! { - if #[cfg(any(target_os = "freebsd", target_os = "dragonfly"))] { - // FreeBSD and DragonFlyBSD don't tell us whether they woke up any threads or not. - // So, we always return `false` here, as that still results in correct behaviour. - // The downside is an extra syscall in case both readers and writers were waiting, - // and unnecessarily waking up readers when a writer is about to attempt to lock the lock. - futex_wake(&self.writer_notify); - false - } else { - futex_wake(&self.writer_notify) - } - } + futex_wake(&self.writer_notify) + // Note that FreeBSD and DragonFlyBSD don't tell us whether they woke + // up any threads or not, and always return `false` here. That still + // results in correct behaviour: it just means readers get woken up as + // well in case both readers and writers were waiting. } /// Spin for a while, but stop directly at the given condition. From 1b9c7e6f1a0283c2374760efcac55aad96d10ac6 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Fri, 29 Apr 2022 16:45:01 +0200 Subject: [PATCH 07/11] Disable pthread thread parker on futex platforms. --- library/std/src/sys/unix/thread_parker.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/library/std/src/sys/unix/thread_parker.rs b/library/std/src/sys/unix/thread_parker.rs index fd83f2f73d6d6..0da132337dedf 100644 --- a/library/std/src/sys/unix/thread_parker.rs +++ b/library/std/src/sys/unix/thread_parker.rs @@ -3,7 +3,11 @@ #![cfg(not(any( target_os = "linux", target_os = "android", - all(target_os = "emscripten", target_feature = "atomics") + all(target_os = "emscripten", target_feature = "atomics"), + target_os = "freebsd", + target_os = "openbsd", + target_os = "netbsd", + target_os = "dragonfly", )))] use crate::cell::UnsafeCell; From 7b7d1d6c4835d4e84517cc825aacb817d6435696 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Tue, 3 May 2022 09:08:38 +0200 Subject: [PATCH 08/11] Don't use futexes on netbsd. The latest NetBSD release doesn't include the futex syscall yet. --- library/std/src/sys/unix/futex.rs | 46 ++----------------- library/std/src/sys/unix/locks/mod.rs | 1 - library/std/src/sys/unix/thread_parker.rs | 1 - .../std/src/sys_common/thread_parker/mod.rs | 1 - 4 files changed, 5 insertions(+), 44 deletions(-) diff --git a/library/std/src/sys/unix/futex.rs b/library/std/src/sys/unix/futex.rs index fd046bdf19d6b..f91367da89c74 100644 --- a/library/std/src/sys/unix/futex.rs +++ b/library/std/src/sys/unix/futex.rs @@ -4,27 +4,18 @@ all(target_os = "emscripten", target_feature = "atomics"), target_os = "freebsd", target_os = "openbsd", - target_os = "netbsd", target_os = "dragonfly", ))] use crate::sync::atomic::AtomicU32; use crate::time::Duration; -#[cfg(target_os = "netbsd")] -pub const SYS___futex: i32 = 166; - /// Wait for a futex_wake operation to wake us. /// /// Returns directly if the futex doesn't hold the expected value. /// /// Returns false on timeout, and true in all other cases. -#[cfg(any( - target_os = "linux", - target_os = "android", - target_os = "freebsd", - target_os = "netbsd" -))] +#[cfg(any(target_os = "linux", target_os = "android", target_os = "freebsd"))] pub fn futex_wait(futex: &AtomicU32, expected: u32, timeout: Option) -> bool { use super::time::Timespec; use crate::ptr::null; @@ -65,19 +56,6 @@ pub fn futex_wait(futex: &AtomicU32, expected: u32, timeout: Option) - crate::ptr::invalid_mut(umtx_timeout_size), umtx_timeout_ptr as *mut _, ) - } else if #[cfg(target_os = "netbsd")] { - // Netbsd's futex syscall takes addr2 and val2 as separate arguments. - // (Both are unused for FUTEX_WAIT[_BITSET].) - libc::syscall( - SYS___futex, - futex as *const AtomicU32, - libc::FUTEX_WAIT_BITSET | libc::FUTEX_PRIVATE_FLAG, - expected, - timespec.as_ref().map_or(null(), |t| &t.t as *const libc::timespec), - null::(), // addr2: This argument is unused for FUTEX_WAIT_BITSET. - 0, // val2: This argument is unused for FUTEX_WAIT_BITSET. - !0u32, // val3 / bitmask: A full bitmask, to make it behave like a regular FUTEX_WAIT. - ) } else { libc::syscall( libc::SYS_futex, @@ -106,34 +84,20 @@ pub fn futex_wait(futex: &AtomicU32, expected: u32, timeout: Option) - /// or false if no thread was waiting on this futex. /// /// On some platforms, this always returns false. -#[cfg(any(target_os = "linux", target_os = "android", target_os = "netbsd"))] +#[cfg(any(target_os = "linux", target_os = "android"))] pub fn futex_wake(futex: &AtomicU32) -> bool { let ptr = futex as *const AtomicU32; let op = libc::FUTEX_WAKE | libc::FUTEX_PRIVATE_FLAG; - unsafe { - cfg_if::cfg_if! { - if #[cfg(target_os = "netbsd")] { - libc::syscall(SYS___futex, ptr, op, 1) > 0 - } else { - libc::syscall(libc::SYS_futex, ptr, op, 1) > 0 - } - } - } + unsafe { libc::syscall(libc::SYS_futex, ptr, op, 1) > 0 } } /// Wake up all threads that are waiting on futex_wait on this futex. -#[cfg(any(target_os = "linux", target_os = "android", target_os = "netbsd"))] +#[cfg(any(target_os = "linux", target_os = "android"))] pub fn futex_wake_all(futex: &AtomicU32) { let ptr = futex as *const AtomicU32; let op = libc::FUTEX_WAKE | libc::FUTEX_PRIVATE_FLAG; unsafe { - cfg_if::cfg_if! { - if #[cfg(target_os = "netbsd")] { - libc::syscall(SYS___futex, ptr, op, i32::MAX); - } else { - libc::syscall(libc::SYS_futex, ptr, op, i32::MAX); - } - } + libc::syscall(libc::SYS_futex, ptr, op, i32::MAX); } } diff --git a/library/std/src/sys/unix/locks/mod.rs b/library/std/src/sys/unix/locks/mod.rs index 27d007a1051e5..04c5c489fc9b9 100644 --- a/library/std/src/sys/unix/locks/mod.rs +++ b/library/std/src/sys/unix/locks/mod.rs @@ -5,7 +5,6 @@ cfg_if::cfg_if! { all(target_os = "emscripten", target_feature = "atomics"), target_os = "freebsd", target_os = "openbsd", - target_os = "netbsd", target_os = "dragonfly", ))] { mod futex; diff --git a/library/std/src/sys/unix/thread_parker.rs b/library/std/src/sys/unix/thread_parker.rs index 0da132337dedf..cf37c01598bf3 100644 --- a/library/std/src/sys/unix/thread_parker.rs +++ b/library/std/src/sys/unix/thread_parker.rs @@ -6,7 +6,6 @@ all(target_os = "emscripten", target_feature = "atomics"), target_os = "freebsd", target_os = "openbsd", - target_os = "netbsd", target_os = "dragonfly", )))] diff --git a/library/std/src/sys_common/thread_parker/mod.rs b/library/std/src/sys_common/thread_parker/mod.rs index 5305f1f2dee8c..c789a388e05ad 100644 --- a/library/std/src/sys_common/thread_parker/mod.rs +++ b/library/std/src/sys_common/thread_parker/mod.rs @@ -5,7 +5,6 @@ cfg_if::cfg_if! { all(target_arch = "wasm32", target_feature = "atomics"), target_os = "freebsd", target_os = "openbsd", - target_os = "netbsd", target_os = "dragonfly", ))] { mod futex; From 8ee9b93c4f5b6a7f035540c2e4a151232cd1c6cf Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Tue, 3 May 2022 12:32:41 +0200 Subject: [PATCH 09/11] Add #[cfg] in cfg_if for linux in unix/futex. --- library/std/src/sys/unix/futex.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/library/std/src/sys/unix/futex.rs b/library/std/src/sys/unix/futex.rs index f91367da89c74..6156cc6b7d2ac 100644 --- a/library/std/src/sys/unix/futex.rs +++ b/library/std/src/sys/unix/futex.rs @@ -33,8 +33,6 @@ pub fn futex_wait(futex: &AtomicU32, expected: u32, timeout: Option) - return true; } - // Use FUTEX_WAIT_BITSET rather than FUTEX_WAIT to be able to give an - // absolute time rather than a relative time. let r = unsafe { cfg_if::cfg_if! { if #[cfg(target_os = "freebsd")] { @@ -56,7 +54,9 @@ pub fn futex_wait(futex: &AtomicU32, expected: u32, timeout: Option) - crate::ptr::invalid_mut(umtx_timeout_size), umtx_timeout_ptr as *mut _, ) - } else { + } else if #[cfg(any(target_os = "linux", target_os = "android"))] { + // Use FUTEX_WAIT_BITSET rather than FUTEX_WAIT to be able to give an + // absolute time rather than a relative time. libc::syscall( libc::SYS_futex, futex as *const AtomicU32, @@ -66,6 +66,8 @@ pub fn futex_wait(futex: &AtomicU32, expected: u32, timeout: Option) - null::(), // This argument is unused for FUTEX_WAIT_BITSET. !0u32, // A full bitmask, to make it behave like a regular FUTEX_WAIT. ) + } else { + compile_error!("unknown target_os"); } } }; From 9299e6915d0dec6282dd4a45a4551d94b9ad9790 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Tue, 3 May 2022 12:33:10 +0200 Subject: [PATCH 10/11] Round timeouts up to infinite in futex_wait on DragonFlyBSD. --- library/std/src/sys/unix/futex.rs | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/library/std/src/sys/unix/futex.rs b/library/std/src/sys/unix/futex.rs index 6156cc6b7d2ac..678c6f0d6ead1 100644 --- a/library/std/src/sys/unix/futex.rs +++ b/library/std/src/sys/unix/futex.rs @@ -185,16 +185,15 @@ pub fn futex_wake_all(futex: &AtomicU32) { #[cfg(target_os = "dragonfly")] pub fn futex_wait(futex: &AtomicU32, expected: u32, timeout: Option) -> bool { use crate::convert::TryFrom; + + // A timeout of 0 means infinite. + // We round smaller timeouts up to 1 millisecond. + // Overflows are rounded up to an infinite timeout. + let timeout_ms = + timeout.and_then(|d| Some(i32::try_from(d.as_millis()).ok()?.max(1))).unwrap_or(0); + let r = unsafe { - libc::umtx_sleep( - futex as *const AtomicU32 as *const i32, - expected as i32, - // A timeout of 0 means infinite, so we round smaller timeouts up to 1 millisecond. - // Timeouts larger than i32::MAX milliseconds saturate. - timeout.map_or(0, |d| { - i32::try_from(d.as_millis()).map_or(i32::MAX, |millis| millis.max(1)) - }), - ) + libc::umtx_sleep(futex as *const AtomicU32 as *const i32, expected as i32, timeout_ms) }; r == 0 || super::os::errno() != libc::ETIMEDOUT From 21c5f780f464b27802d0ee0f86c95eb29881096b Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Thu, 5 May 2022 21:47:13 +0200 Subject: [PATCH 11/11] Remove condvar::two_mutexes test. We don't guarantee this panics. On most platforms it doesn't anymore. --- library/std/src/sync/condvar/tests.rs | 21 --------------------- 1 file changed, 21 deletions(-) diff --git a/library/std/src/sync/condvar/tests.rs b/library/std/src/sync/condvar/tests.rs index f7a00676daaed..24f467f0b03d7 100644 --- a/library/std/src/sync/condvar/tests.rs +++ b/library/std/src/sync/condvar/tests.rs @@ -188,24 +188,3 @@ fn wait_timeout_wake() { break; } } - -#[test] -#[should_panic] -#[cfg(all(unix, not(target_os = "linux"), not(target_os = "android")))] -fn two_mutexes() { - let m = Arc::new(Mutex::new(())); - let m2 = m.clone(); - let c = Arc::new(Condvar::new()); - let c2 = c.clone(); - - let mut g = m.lock().unwrap(); - let _t = thread::spawn(move || { - let _g = m2.lock().unwrap(); - c2.notify_one(); - }); - g = c.wait(g).unwrap(); - drop(g); - - let m = Mutex::new(()); - let _ = c.wait(m.lock().unwrap()).unwrap(); -}