Skip to content

Commit 59af020

Browse files
authored
Rollup merge of rust-lang#103146 - joboet:cleanup_pthread_condvar, r=Mark-Simulacrum
Cleanup timeouts in pthread condvar
2 parents 32da230 + da0a542 commit 59af020

File tree

3 files changed

+34
-66
lines changed

3 files changed

+34
-66
lines changed

library/std/src/sys/unix/locks/pthread_condvar.rs

+30-63
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ use crate::cell::UnsafeCell;
22
use crate::ptr;
33
use crate::sync::atomic::{AtomicPtr, Ordering::Relaxed};
44
use crate::sys::locks::{pthread_mutex, Mutex};
5+
use crate::sys::time::TIMESPEC_MAX;
56
use crate::sys_common::lazy_box::{LazyBox, LazyInit};
67
use crate::time::Duration;
78

@@ -12,13 +13,6 @@ pub struct Condvar {
1213
mutex: AtomicPtr<libc::pthread_mutex_t>,
1314
}
1415

15-
const TIMESPEC_MAX: libc::timespec =
16-
libc::timespec { tv_sec: <libc::time_t>::MAX, tv_nsec: 1_000_000_000 - 1 };
17-
18-
fn saturating_cast_to_time_t(value: u64) -> libc::time_t {
19-
if value > <libc::time_t>::MAX as u64 { <libc::time_t>::MAX } else { value as libc::time_t }
20-
}
21-
2216
#[inline]
2317
fn raw(c: &Condvar) -> *mut libc::pthread_cond_t {
2418
c.inner.0.get()
@@ -133,26 +127,15 @@ impl Condvar {
133127
target_os = "horizon"
134128
)))]
135129
pub unsafe fn wait_timeout(&self, mutex: &Mutex, dur: Duration) -> bool {
136-
use crate::mem;
130+
use crate::sys::time::Timespec;
137131

138132
let mutex = pthread_mutex::raw(mutex);
139133
self.verify(mutex);
140134

141-
let mut now: libc::timespec = mem::zeroed();
142-
let r = libc::clock_gettime(libc::CLOCK_MONOTONIC, &mut now);
143-
assert_eq!(r, 0);
144-
145-
// Nanosecond calculations can't overflow because both values are below 1e9.
146-
let nsec = dur.subsec_nanos() + now.tv_nsec as u32;
147-
148-
let sec = saturating_cast_to_time_t(dur.as_secs())
149-
.checked_add((nsec / 1_000_000_000) as libc::time_t)
150-
.and_then(|s| s.checked_add(now.tv_sec));
151-
let nsec = nsec % 1_000_000_000;
152-
153-
let timeout =
154-
sec.map(|s| libc::timespec { tv_sec: s, tv_nsec: nsec as _ }).unwrap_or(TIMESPEC_MAX);
155-
135+
let timeout = Timespec::now(libc::CLOCK_MONOTONIC)
136+
.checked_add_duration(&dur)
137+
.and_then(|t| t.to_timespec())
138+
.unwrap_or(TIMESPEC_MAX);
156139
let r = libc::pthread_cond_timedwait(raw(self), mutex, &timeout);
157140
assert!(r == libc::ETIMEDOUT || r == 0);
158141
r == 0
@@ -169,57 +152,41 @@ impl Condvar {
169152
target_os = "espidf",
170153
target_os = "horizon"
171154
))]
172-
pub unsafe fn wait_timeout(&self, mutex: &Mutex, mut dur: Duration) -> bool {
155+
pub unsafe fn wait_timeout(&self, mutex: &Mutex, dur: Duration) -> bool {
156+
use crate::sys::time::SystemTime;
173157
use crate::time::Instant;
174158

175159
let mutex = pthread_mutex::raw(mutex);
176160
self.verify(mutex);
177161

178-
// 1000 years
179-
let max_dur = Duration::from_secs(1000 * 365 * 86400);
180-
181-
if dur > max_dur {
182-
// OSX implementation of `pthread_cond_timedwait` is buggy
183-
// with super long durations. When duration is greater than
184-
// 0x100_0000_0000_0000 seconds, `pthread_cond_timedwait`
185-
// in macOS Sierra return error 316.
186-
//
187-
// This program demonstrates the issue:
188-
// https://gist.github.com/stepancheg/198db4623a20aad2ad7cddb8fda4a63c
189-
//
190-
// To work around this issue, and possible bugs of other OSes, timeout
191-
// is clamped to 1000 years, which is allowable per the API of `wait_timeout`
192-
// because of spurious wakeups.
193-
194-
dur = max_dur;
195-
}
196-
197-
// First, figure out what time it currently is, in both system and
198-
// stable time. pthread_cond_timedwait uses system time, but we want to
199-
// report timeout based on stable time.
200-
let mut sys_now = libc::timeval { tv_sec: 0, tv_usec: 0 };
201-
let stable_now = Instant::now();
202-
let r = libc::gettimeofday(&mut sys_now, ptr::null_mut());
203-
assert_eq!(r, 0, "unexpected error: {:?}", crate::io::Error::last_os_error());
204-
205-
let nsec = dur.subsec_nanos() as libc::c_long + (sys_now.tv_usec * 1000) as libc::c_long;
206-
let extra = (nsec / 1_000_000_000) as libc::time_t;
207-
let nsec = nsec % 1_000_000_000;
208-
let seconds = saturating_cast_to_time_t(dur.as_secs());
209-
210-
let timeout = sys_now
211-
.tv_sec
212-
.checked_add(extra)
213-
.and_then(|s| s.checked_add(seconds))
214-
.map(|s| libc::timespec { tv_sec: s, tv_nsec: nsec })
162+
// OSX implementation of `pthread_cond_timedwait` is buggy
163+
// with super long durations. When duration is greater than
164+
// 0x100_0000_0000_0000 seconds, `pthread_cond_timedwait`
165+
// in macOS Sierra returns error 316.
166+
//
167+
// This program demonstrates the issue:
168+
// https://gist.github.com/stepancheg/198db4623a20aad2ad7cddb8fda4a63c
169+
//
170+
// To work around this issue, and possible bugs of other OSes, timeout
171+
// is clamped to 1000 years, which is allowable per the API of `wait_timeout`
172+
// because of spurious wakeups.
173+
let dur = Duration::min(dur, Duration::from_secs(1000 * 365 * 86400));
174+
175+
// pthread_cond_timedwait uses system time, but we want to report timeout
176+
// based on stable time.
177+
let now = Instant::now();
178+
179+
let timeout = SystemTime::now()
180+
.t
181+
.checked_add_duration(&dur)
182+
.and_then(|t| t.to_timespec())
215183
.unwrap_or(TIMESPEC_MAX);
216184

217-
// And wait!
218185
let r = libc::pthread_cond_timedwait(raw(self), mutex, &timeout);
219186
debug_assert!(r == libc::ETIMEDOUT || r == 0);
220187

221188
// ETIMEDOUT is not a totally reliable method of determining timeout due
222189
// to clock shifts, so do the check ourselves
223-
stable_now.elapsed() < dur
190+
now.elapsed() < dur
224191
}
225192
}

library/std/src/sys/unix/thread_parker/pthread.rs

+1-3
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ use crate::pin::Pin;
66
use crate::ptr::addr_of_mut;
77
use crate::sync::atomic::AtomicUsize;
88
use crate::sync::atomic::Ordering::SeqCst;
9+
use crate::sys::time::TIMESPEC_MAX;
910
use crate::time::Duration;
1011

1112
const EMPTY: usize = 0;
@@ -32,9 +33,6 @@ unsafe fn wait(cond: *mut libc::pthread_cond_t, lock: *mut libc::pthread_mutex_t
3233
debug_assert_eq!(r, 0);
3334
}
3435

35-
const TIMESPEC_MAX: libc::timespec =
36-
libc::timespec { tv_sec: <libc::time_t>::MAX, tv_nsec: 1_000_000_000 - 1 };
37-
3836
unsafe fn wait_timeout(
3937
cond: *mut libc::pthread_cond_t,
4038
lock: *mut libc::pthread_mutex_t,

library/std/src/sys/unix/time.rs

+3
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,9 @@ pub use self::inner::Instant;
55

66
const NSEC_PER_SEC: u64 = 1_000_000_000;
77
pub const UNIX_EPOCH: SystemTime = SystemTime { t: Timespec::zero() };
8+
#[allow(dead_code)] // Used for pthread condvar timeouts
9+
pub const TIMESPEC_MAX: libc::timespec =
10+
libc::timespec { tv_sec: <libc::time_t>::MAX, tv_nsec: 1_000_000_000 - 1 };
811

912
#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
1013
#[repr(transparent)]

0 commit comments

Comments
 (0)