std: improve handling of timed condition variable waits on macOS #146503
+187
−26
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The Miri subtree was changed cc @rust-lang/miri |
rustbot
added
O-unix
|
r? @ibraheemdev rustbot has assigned @ibraheemdev. Use |
RalfJung
reviewed
Sep 13, 2025
| let (clock, anchor) = if relative { | ||
| // `pthread_cond_timedwait_relative_np` always measures time against the | ||
| // monotonic clock, regardless of the condvar clock. | ||
| (TimeoutClock::Monotonic, TimeoutAnchor::Relative) |
There was a problem hiding this comment.
So the argument named relative really is very specific for this macos operation, and also swaps out the clock. Please give it a name that more accurately reflects what it does (e.g. macos_relative_np), and document it in the doc comment for this function.
| libc::ETIMEDOUT | ||
| ); | ||
| let elapsed_time = current_time.elapsed().as_millis(); | ||
| assert!(50 <= elapsed_time && elapsed_time <= 150); |
There was a problem hiding this comment.
Suggested change
| assert!(50 <= elapsed_time && elapsed_time <= 150); | |
| // This is actually deterministic (since isolation remains enabled), but can change slightly with Rust updates. | |
| assert!(90 <= elapsed_time && elapsed_time <= 110); |
| "pthread_cond_timedwait_relative_np" => { | ||
| let [cond, mutex, reltime] = | ||
| this.check_shim_sig_lenient(abi, CanonAbi::C, link_name, args)?; | ||
| this.pthread_cond_timedwait(cond, mutex, reltime, dest, true)?; |
There was a problem hiding this comment.
Suggested change
| this.pthread_cond_timedwait(cond, mutex, reltime, dest, true)?; | |
| this.pthread_cond_timedwait(cond, mutex, reltime, dest, /* macos_relative_np */ true)?; |
| @@ -815,7 +815,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { | |||
| "pthread_cond_timedwait" => { | |||
| let [cond, mutex, abstime] = | |||
| this.check_shim_sig_lenient(abi, CanonAbi::C, link_name, args)?; | |||
| this.pthread_cond_timedwait(cond, mutex, abstime, dest)?; | |||
| this.pthread_cond_timedwait(cond, mutex, abstime, dest, false)?; | |||
There was a problem hiding this comment.
Suggested change
| this.pthread_cond_timedwait(cond, mutex, abstime, dest, false)?; | |
| this.pthread_cond_timedwait(cond, mutex, abstime, dest, /* macos_relative_np */ false)?; |
RalfJung
reviewed
Sep 13, 2025
| // 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 timeouts). |
There was a problem hiding this comment.
Suggested change
| // even if the "ulock" variant is used (which does guard against timeouts). | |
| // even if the "ulock" variant is used (which does guard against overflow). |
joboet
force-pushed
the
macos-condvar-timeout
branch
from
205907b to
95efab6
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #37440 (for good).
This fixes two issues with
Condvar::wait_timeouton macOS:Apple's implementation of
pthread_cond_timedwaitinternally converts the absolute timeout to a relative one, measured in nanoseconds, but fails to consider overflow when doing so. This results inwait_timeoutreturning much earlier than anticipated when passed a duration that is slightly longer thanu64::MAXnanoseconds (around 584 years). The existing clamping introduced by #42604 to address #37440 unfortunately used a maximum duration of 1000 years and thus still runs into the bug when run on older macOS versions (or withPTHREAD_MUTEX_USE_ULOCKset to a value other than "1"). See #37440 (comment) for context.Reducing the maximum duration alone however would not be enough to make the implementation completely correct. As macOS does not support$2^{64}\ \textrm{ns}$ . Luckily however, macOS supports the non-standard, tongue-twisting
pthread_condattr_setclock, the deadline passed topthread_cond_timedwaitis measured against the wall-time clock.stdcurrently calculates the deadline by retrieving the current time and adding the duration to that, only for macOS to convert the deadline back to a relative duration by retrieving the current time itself (this conversion is performed before the aforementioned problematic one). Thus, if the wall-time clock is adjusted between thestdlookup and the system lookup, the relative duration could have changed, possibly even to a value larger thanpthread_cond_timedwait_relative_npfunction which avoids the wall-clock-time roundtrip by taking a relative timeout. Even apart from that, this function is perfectly suited forstd's purposes: it is public (albeit badly-documented) API, available since macOS 10.4 (that's way below our minimum of 10.12) and completely resilient against wall-time changes as all timeouts are measured against the monotonic clock inside the kernel.Thus, this PR switches$2^{64} - 1 \ \textrm{ns}$ . I've added a miri shim as well, so the only thing missing is a definition of
Condvar::wait_timeouttopthread_cond_timedwait_relative_np, making sure to clamp the duration to a maximum ofpthread_cond_timedwait_relative_npinsidelibc.