Skip to content

Commit

Permalink
Auto merge of rust-lang#2631 - RalfJung:futex-wait-no-isolate, r=Ralf…
Browse files Browse the repository at this point in the history
…Jung

Support timeouts with monotonic clocks even when isolation is enabled

With the deterministic monotonic clock support we now have, we can allow some synchronization primitives with timeouts even under isolation:
- Linux futex waiting (when set to the monotonic clock)
- pthread_cond_timedwait (when set to the monotonic clock)
- Windows WaitOnAddress

Unfortunately none of these exist on macOS -- the standard library always uses the system clock for timeouts on macOS, so that will still require `-Zmiri-disable-isolation`.
  • Loading branch information
bors committed Oct 29, 2022
2 parents a0fbf0d + c8a5d4b commit e729db1
Show file tree
Hide file tree
Showing 5 changed files with 100 additions and 7 deletions.
8 changes: 5 additions & 3 deletions src/tools/miri/src/shims/unix/linux/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,9 +90,11 @@ pub fn futex<'tcx>(
let timeout_time = if this.ptr_is_null(timeout.ptr)? {
None
} else {
this.check_no_isolation(
"`futex` syscall with `op=FUTEX_WAIT` and non-null timeout",
)?;
if op & futex_realtime != 0 {
this.check_no_isolation(
"`futex` syscall with `op=FUTEX_WAIT` and non-null timeout with `FUTEX_CLOCK_REALTIME`",
)?;
}
let duration = match this.read_timespec(&timeout)? {
Some(duration) => duration,
None => {
Expand Down
3 changes: 1 addition & 2 deletions src/tools/miri/src/shims/unix/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -743,8 +743,6 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
) -> InterpResult<'tcx> {
let this = self.eval_context_mut();

this.check_no_isolation("`pthread_cond_timedwait`")?;

let id = this.condvar_get_or_create_id(cond_op, CONDVAR_ID_OFFSET)?;
let mutex_id = this.mutex_get_or_create_id(mutex_op, MUTEX_ID_OFFSET)?;
let active_thread = this.get_active_thread();
Expand All @@ -761,6 +759,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
};

let timeout_time = if clock_id == this.eval_libc_i32("CLOCK_REALTIME")? {
this.check_no_isolation("`pthread_cond_timedwait` with `CLOCK_REALTIME`")?;
Time::RealTime(SystemTime::UNIX_EPOCH.checked_add(duration).unwrap())
} else if clock_id == this.eval_libc_i32("CLOCK_MONOTONIC")? {
Time::Monotonic(this.machine.clock.anchor().checked_add(duration).unwrap())
Expand Down
2 changes: 0 additions & 2 deletions src/tools/miri/src/shims/windows/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -254,8 +254,6 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
let timeout_time = if timeout_ms == this.eval_windows("c", "INFINITE")?.to_u32()? {
None
} else {
this.check_no_isolation("`WaitOnAddress` with non-infinite timeout")?;

let duration = Duration::from_millis(timeout_ms.into());
Some(Time::Monotonic(this.machine.clock.now().checked_add(duration).unwrap()))
};
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
//@ignore-target-windows: No libc on Windows
//@ignore-target-apple: pthread_condattr_setclock is not supported on MacOS.

/// Test that conditional variable timeouts are working properly
/// with monotonic clocks even under isolation.
use std::mem::MaybeUninit;
use std::time::Instant;

fn test_timed_wait_timeout(clock_id: i32) {
unsafe {
let mut attr: MaybeUninit<libc::pthread_condattr_t> = MaybeUninit::uninit();
assert_eq!(libc::pthread_condattr_init(attr.as_mut_ptr()), 0);
assert_eq!(libc::pthread_condattr_setclock(attr.as_mut_ptr(), clock_id), 0);

let mut cond: MaybeUninit<libc::pthread_cond_t> = MaybeUninit::uninit();
assert_eq!(libc::pthread_cond_init(cond.as_mut_ptr(), attr.as_ptr()), 0);
assert_eq!(libc::pthread_condattr_destroy(attr.as_mut_ptr()), 0);

let mut mutex: libc::pthread_mutex_t = libc::PTHREAD_MUTEX_INITIALIZER;

let mut now_mu: MaybeUninit<libc::timespec> = MaybeUninit::uninit();
assert_eq!(libc::clock_gettime(clock_id, now_mu.as_mut_ptr()), 0);
let now = now_mu.assume_init();
// Waiting for a second... mostly because waiting less requires mich more tricky arithmetic.
// FIXME: wait less.
let timeout = libc::timespec { tv_sec: now.tv_sec + 1, tv_nsec: now.tv_nsec };

assert_eq!(libc::pthread_mutex_lock(&mut mutex as *mut _), 0);
let current_time = Instant::now();
assert_eq!(
libc::pthread_cond_timedwait(cond.as_mut_ptr(), &mut mutex as *mut _, &timeout),
libc::ETIMEDOUT
);
let elapsed_time = current_time.elapsed().as_millis();
assert!(900 <= elapsed_time && elapsed_time <= 1300);

// Test calling `pthread_cond_timedwait` again with an already elapsed timeout.
assert_eq!(
libc::pthread_cond_timedwait(cond.as_mut_ptr(), &mut mutex as *mut _, &timeout),
libc::ETIMEDOUT
);

// Test that invalid nanosecond values (above 10^9 or negative) are rejected with the
// correct error code.
let invalid_timeout_1 = libc::timespec { tv_sec: now.tv_sec + 1, tv_nsec: 1_000_000_000 };
assert_eq!(
libc::pthread_cond_timedwait(
cond.as_mut_ptr(),
&mut mutex as *mut _,
&invalid_timeout_1
),
libc::EINVAL
);
let invalid_timeout_2 = libc::timespec { tv_sec: now.tv_sec + 1, tv_nsec: -1 };
assert_eq!(
libc::pthread_cond_timedwait(
cond.as_mut_ptr(),
&mut mutex as *mut _,
&invalid_timeout_2
),
libc::EINVAL
);
// Test that invalid second values (negative) are rejected with the correct error code.
let invalid_timeout_3 = libc::timespec { tv_sec: -1, tv_nsec: 0 };
assert_eq!(
libc::pthread_cond_timedwait(
cond.as_mut_ptr(),
&mut mutex as *mut _,
&invalid_timeout_3
),
libc::EINVAL
);

assert_eq!(libc::pthread_mutex_unlock(&mut mutex as *mut _), 0);
assert_eq!(libc::pthread_mutex_destroy(&mut mutex as *mut _), 0);
assert_eq!(libc::pthread_cond_destroy(cond.as_mut_ptr()), 0);
}
}

fn main() {
test_timed_wait_timeout(libc::CLOCK_MONOTONIC);
}
12 changes: 12 additions & 0 deletions src/tools/miri/tests/pass/concurrency/thread_park_isolated.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
//@ignore-target-apple: park_timeout on macOS uses the system clock
use std::thread;
use std::time::{Duration, Instant};

fn main() {
let start = Instant::now();

thread::park_timeout(Duration::from_millis(200));

// Thanks to deterministic execution, this will wiat *exactly* 200ms (rounded to 1ms).
assert!((200..201).contains(&start.elapsed().as_millis()));
}

0 comments on commit e729db1

Please sign in to comment.