std: improve handling of timed condition variable waits on macOS
This commit is contained in:
parent
fb24b04b09
commit
8a145efc70
4 changed files with 115 additions and 22 deletions
|
|
@ -139,9 +139,9 @@ dependencies = [
|
|||
|
||||
[[package]]
|
||||
name = "libc"
|
||||
version = "0.2.175"
|
||||
version = "0.2.177"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "6a82ae493e598baaea5209805c49bbf2ea7de956d50d7da0da1164f9c6d28543"
|
||||
checksum = "2874a2af47a2325c2001a6e6fad9b16a53b802102b528163885171cf92b15976"
|
||||
dependencies = [
|
||||
"rustc-std-workspace-core",
|
||||
]
|
||||
|
|
|
|||
|
|
@ -33,7 +33,7 @@ miniz_oxide = { version = "0.8.0", optional = true, default-features = false }
|
|||
addr2line = { version = "0.25.0", optional = true, default-features = false }
|
||||
|
||||
[target.'cfg(not(all(windows, target_env = "msvc")))'.dependencies]
|
||||
libc = { version = "0.2.172", default-features = false, features = [
|
||||
libc = { version = "0.2.177", default-features = false, features = [
|
||||
'rustc-dep-of-std',
|
||||
], public = true }
|
||||
|
||||
|
|
|
|||
|
|
@ -1,11 +1,6 @@
|
|||
use super::Mutex;
|
||||
use crate::cell::UnsafeCell;
|
||||
use crate::pin::Pin;
|
||||
#[cfg(not(target_os = "nto"))]
|
||||
use crate::sys::pal::time::TIMESPEC_MAX;
|
||||
#[cfg(target_os = "nto")]
|
||||
use crate::sys::pal::time::TIMESPEC_MAX_CAPPED;
|
||||
use crate::sys::pal::time::Timespec;
|
||||
use crate::time::Duration;
|
||||
|
||||
pub struct Condvar {
|
||||
|
|
@ -47,27 +42,29 @@ impl Condvar {
|
|||
let r = unsafe { libc::pthread_cond_wait(self.raw(), mutex.raw()) };
|
||||
debug_assert_eq!(r, 0);
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(not(target_vendor = "apple"))]
|
||||
impl Condvar {
|
||||
/// # Safety
|
||||
/// * `init` must have been called on this instance.
|
||||
/// * `mutex` must be locked by the current thread.
|
||||
/// * This condition variable may only be used with the same mutex.
|
||||
pub unsafe fn wait_timeout(&self, mutex: Pin<&Mutex>, dur: Duration) -> bool {
|
||||
#[cfg(not(target_os = "nto"))]
|
||||
use crate::sys::pal::time::TIMESPEC_MAX;
|
||||
#[cfg(target_os = "nto")]
|
||||
use crate::sys::pal::time::TIMESPEC_MAX_CAPPED;
|
||||
use crate::sys::pal::time::Timespec;
|
||||
|
||||
let mutex = mutex.raw();
|
||||
|
||||
// OSX implementation of `pthread_cond_timedwait` is buggy
|
||||
// with super long durations. When duration is greater than
|
||||
// 0x100_0000_0000_0000 seconds, `pthread_cond_timedwait`
|
||||
// in macOS Sierra returns error 316.
|
||||
//
|
||||
// This program demonstrates the issue:
|
||||
// https://gist.github.com/stepancheg/198db4623a20aad2ad7cddb8fda4a63c
|
||||
//
|
||||
// To work around this issue, the timeout is clamped to 1000 years.
|
||||
//
|
||||
// Cygwin implementation is based on NT API and a super large timeout
|
||||
// makes the syscall block forever.
|
||||
#[cfg(any(target_vendor = "apple", target_os = "cygwin"))]
|
||||
// Cygwin's implementation is based on the NT API, which measures time
|
||||
// in units of 100 ns. Unfortunately, Cygwin does not properly guard
|
||||
// against overflow when converting the time, hence we clamp the interval
|
||||
// to 1000 years, which will only become a problem in around 27000 years,
|
||||
// when the next rollover is less than 1000 years away...
|
||||
#[cfg(target_os = "cygwin")]
|
||||
let dur = Duration::min(dur, Duration::from_secs(1000 * 365 * 86400));
|
||||
|
||||
let timeout = Timespec::now(Self::CLOCK).checked_add_duration(&dur);
|
||||
|
|
@ -84,6 +81,57 @@ impl Condvar {
|
|||
}
|
||||
}
|
||||
|
||||
// Apple platforms (since macOS version 10.4 and iOS version 2.0) have
|
||||
// `pthread_cond_timedwait_relative_np`, a non-standard extension that
|
||||
// measures timeouts based on the monotonic clock and is thus resilient
|
||||
// against wall-clock changes.
|
||||
#[cfg(target_vendor = "apple")]
|
||||
impl Condvar {
|
||||
/// # Safety
|
||||
/// * `init` must have been called on this instance.
|
||||
/// * `mutex` must be locked by the current thread.
|
||||
/// * This condition variable may only be used with the same mutex.
|
||||
pub unsafe fn wait_timeout(&self, mutex: Pin<&Mutex>, dur: Duration) -> bool {
|
||||
let mutex = mutex.raw();
|
||||
|
||||
// The macOS implementation of `pthread_cond_timedwait` internally
|
||||
// converts the timeout passed to `pthread_cond_timedwait_relative_np`
|
||||
// to nanoseconds. Unfortunately, the "psynch" variant of condvars does
|
||||
// not guard against overflow during the conversion[^1], which means
|
||||
// that `pthread_cond_timedwait_relative_np` will return `ETIMEDOUT`
|
||||
// much earlier than expected if the relative timeout is longer than
|
||||
// `u64::MAX` nanoseconds.
|
||||
//
|
||||
// This can be observed even on newer platforms (by setting the environment
|
||||
// variable PTHREAD_MUTEX_USE_ULOCK to a value other than "1") by calling e.g.
|
||||
// ```
|
||||
// condvar.wait_timeout(..., Duration::from_secs(u64::MAX.div_ceil(1_000_000_000));
|
||||
// ```
|
||||
// (see #37440, especially
|
||||
// https://github.com/rust-lang/rust/issues/37440#issuecomment-3285958326).
|
||||
//
|
||||
// To work around this issue, always clamp the timeout to u64::MAX nanoseconds,
|
||||
// even if the "ulock" variant is used (which does guard against overflow).
|
||||
//
|
||||
// [^1]: https://github.com/apple-oss-distributions/libpthread/blob/1ebf56b3a702df53213c2996e5e128a535d2577e/kern/kern_synch.c#L1269
|
||||
const MAX_DURATION: Duration = Duration::from_nanos(u64::MAX);
|
||||
|
||||
let (dur, clamped) = if dur <= MAX_DURATION { (dur, false) } else { (MAX_DURATION, true) };
|
||||
|
||||
let timeout = libc::timespec {
|
||||
// This cannot overflow because of the clamping above.
|
||||
tv_sec: dur.as_secs() as i64,
|
||||
tv_nsec: dur.subsec_nanos() as i64,
|
||||
};
|
||||
|
||||
let r = unsafe { libc::pthread_cond_timedwait_relative_np(self.raw(), mutex, &timeout) };
|
||||
assert!(r == libc::ETIMEDOUT || r == 0);
|
||||
// Report clamping as a spurious wakeup. Who knows, maybe some
|
||||
// interstellar space probe will rely on this ;-).
|
||||
r == 0 || clamped
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(not(any(
|
||||
target_os = "android",
|
||||
target_vendor = "apple",
|
||||
|
|
@ -125,10 +173,23 @@ impl Condvar {
|
|||
}
|
||||
}
|
||||
|
||||
#[cfg(target_vendor = "apple")]
|
||||
impl Condvar {
|
||||
// `pthread_cond_timedwait_relative_np` measures the timeout
|
||||
// based on the monotonic clock.
|
||||
pub const PRECISE_TIMEOUT: bool = true;
|
||||
|
||||
/// # Safety
|
||||
/// May only be called once per instance of `Self`.
|
||||
pub unsafe fn init(self: Pin<&mut Self>) {
|
||||
// `PTHREAD_COND_INITIALIZER` is fully supported and we don't need to
|
||||
// change clocks, so there's nothing to do here.
|
||||
}
|
||||
}
|
||||
|
||||
// `pthread_condattr_setclock` is unfortunately not supported on these platforms.
|
||||
#[cfg(any(
|
||||
target_os = "android",
|
||||
target_vendor = "apple",
|
||||
target_os = "espidf",
|
||||
target_os = "horizon",
|
||||
target_os = "l4re",
|
||||
|
|
|
|||
|
|
@ -267,3 +267,35 @@ nonpoison_and_poison_unwrap_test!(
|
|||
}
|
||||
}
|
||||
);
|
||||
|
||||
// Some platforms internally cast the timeout duration into nanoseconds.
|
||||
// If they fail to consider overflow during the conversion (I'm looking
|
||||
// at you, macOS), `wait_timeout` will return immediately and indicate a
|
||||
// timeout for durations that are slightly longer than u64::MAX nanoseconds.
|
||||
// `std` should guard against this by clamping the timeout.
|
||||
// See #37440 for context.
|
||||
nonpoison_and_poison_unwrap_test!(
|
||||
name: timeout_nanoseconds,
|
||||
test_body: {
|
||||
use locks::Mutex;
|
||||
use locks::Condvar;
|
||||
|
||||
let sent = Mutex::new(false);
|
||||
let cond = Condvar::new();
|
||||
|
||||
thread::scope(|s| {
|
||||
s.spawn(|| {
|
||||
thread::sleep(Duration::from_secs(2));
|
||||
maybe_unwrap(sent.set(true));
|
||||
cond.notify_all();
|
||||
});
|
||||
|
||||
let guard = maybe_unwrap(sent.lock());
|
||||
// If there is internal overflow, this call will return almost
|
||||
// immediately, before the other thread has reached the `notify_all`
|
||||
let (guard, res) = maybe_unwrap(cond.wait_timeout(guard, Duration::from_secs(u64::MAX.div_ceil(1_000_000_000))));
|
||||
assert!(!res.timed_out());
|
||||
assert!(*guard);
|
||||
})
|
||||
}
|
||||
);
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue