diff --git a/src/shims/sync.rs b/src/shims/sync.rs index ee2579c22f1..f34799f7425 100644 --- a/src/shims/sync.rs +++ b/src/shims/sync.rs @@ -254,7 +254,8 @@ fn cond_set_clock_id<'mir, 'tcx: 'mir>( set_at_offset(ecx, cond_op, 8, clock_id, ecx.machine.layouts.i32, PTHREAD_COND_T_MIN_SIZE) } -/// Try to reacquire the mutex associated with the condition variable after we were signaled. +/// Try to reacquire the mutex associated with the condition variable after we +/// were signaled. fn reacquire_cond_mutex<'mir, 'tcx: 'mir>( ecx: &mut MiriEvalContext<'mir, 'tcx>, thread: ThreadId, @@ -269,6 +270,17 @@ fn reacquire_cond_mutex<'mir, 'tcx: 'mir>( Ok(()) } +/// Reacquire the conditional variable and remove the timeout callback if any +/// was registered. +fn post_cond_signal<'mir, 'tcx: 'mir>( + ecx: &mut MiriEvalContext<'mir, 'tcx>, + thread: ThreadId, + mutex: MutexId, +) -> InterpResult<'tcx> { + reacquire_cond_mutex(ecx, thread, mutex)?; + ecx.unregister_timeout_callback_if_exists(thread) +} + /// Release the mutex associated with the condition variable because we are /// entering the waiting state. fn release_cond_mutex<'mir, 'tcx: 'mir>( @@ -648,8 +660,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx let this = self.eval_context_mut(); let id = cond_get_or_create_id(this, cond_op)?; if let Some((thread, mutex)) = this.condvar_signal(id) { - reacquire_cond_mutex(this, thread, mutex)?; - this.unregister_timeout_callback_if_exists(thread)?; + post_cond_signal(this, thread, mutex)?; } Ok(0) @@ -660,8 +671,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx let id = cond_get_or_create_id(this, cond_op)?; while let Some((thread, mutex)) = this.condvar_signal(id) { - reacquire_cond_mutex(this, thread, mutex)?; - this.unregister_timeout_callback_if_exists(thread)?; + post_cond_signal(this, thread, mutex)?; } Ok(0) @@ -730,7 +740,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx } else if clock_id == this.eval_libc_i32("CLOCK_MONOTONIC")? { Time::Monotonic(this.machine.time_anchor.checked_add(duration).unwrap()) } else { - throw_ub_format!("Unsupported clock id."); + throw_unsup_format!("Unsupported clock id."); }; // Register the timeout callback. @@ -738,13 +748,14 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx active_thread, timeout_time, Box::new(move |ecx| { - // Try to reacquire the mutex. + // We are not waiting for the condvar any more, wait for the + // mutex instead. reacquire_cond_mutex(ecx, active_thread, mutex_id)?; // Remove the thread from the conditional variable. ecx.condvar_remove_waiter(id, active_thread); - // Set the timeout value. + // Set the return value: we timed out. let timeout = ecx.eval_libc_i32("ETIMEDOUT")?; ecx.write_scalar(Scalar::from_i32(timeout), dest)?; diff --git a/src/sync.rs b/src/sync.rs index a71d4597c66..cbae29bdbb3 100644 --- a/src/sync.rs +++ b/src/sync.rs @@ -179,6 +179,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx /// Put the thread into the queue waiting for the lock. fn mutex_enqueue(&mut self, id: MutexId, thread: ThreadId) { let this = self.eval_context_mut(); + assert!(this.mutex_is_locked(id), "queing on unlocked mutex"); this.machine.threads.sync.mutexes[id].queue.push_back(thread); } diff --git a/tests/run-pass/concurrency/libc_pthread_cond.rs b/tests/run-pass/concurrency/libc_pthread_cond.rs index 9b7a06b431c..39b6a7e4ef8 100644 --- a/tests/run-pass/concurrency/libc_pthread_cond.rs +++ b/tests/run-pass/concurrency/libc_pthread_cond.rs @@ -11,11 +11,11 @@ extern crate libc; use std::mem; use std::time::Instant; -fn test_timed_wait_timeout_monotonic() { +fn test_timed_wait_timeout(clock_id: i32) { unsafe { let mut attr: libc::pthread_condattr_t = mem::zeroed(); assert_eq!(libc::pthread_condattr_init(&mut attr as *mut _), 0); - assert_eq!(libc::pthread_condattr_setclock(&mut attr as *mut _, libc::CLOCK_MONOTONIC), 0); + assert_eq!(libc::pthread_condattr_setclock(&mut attr as *mut _, clock_id), 0); let mut cond: libc::pthread_cond_t = mem::zeroed(); assert_eq!(libc::pthread_cond_init(&mut cond as *mut _, &attr as *const _), 0); @@ -24,7 +24,7 @@ fn test_timed_wait_timeout_monotonic() { let mut mutex: libc::pthread_mutex_t = mem::zeroed(); let mut now: libc::timespec = mem::zeroed(); - assert_eq!(libc::clock_gettime(libc::CLOCK_MONOTONIC, &mut now), 0); + assert_eq!(libc::clock_gettime(clock_id, &mut now), 0); 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); @@ -33,36 +33,8 @@ fn test_timed_wait_timeout_monotonic() { libc::pthread_cond_timedwait(&mut cond as *mut _, &mut mutex as *mut _, &timeout), libc::ETIMEDOUT ); - assert!(current_time.elapsed().as_millis() >= 900); - 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(&mut cond as *mut _), 0); - } -} - -fn test_timed_wait_timeout_realtime() { - unsafe { - let mut attr: libc::pthread_condattr_t = mem::zeroed(); - assert_eq!(libc::pthread_condattr_init(&mut attr as *mut _), 0); - assert_eq!(libc::pthread_condattr_setclock(&mut attr as *mut _, libc::CLOCK_REALTIME), 0); - - let mut cond: libc::pthread_cond_t = mem::zeroed(); - assert_eq!(libc::pthread_cond_init(&mut cond as *mut _, &attr as *const _), 0); - assert_eq!(libc::pthread_condattr_destroy(&mut attr as *mut _), 0); - - let mut mutex: libc::pthread_mutex_t = mem::zeroed(); - - let mut now: libc::timespec = mem::zeroed(); - assert_eq!(libc::clock_gettime(libc::CLOCK_REALTIME, &mut now), 0); - 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(&mut cond as *mut _, &mut mutex as *mut _, &timeout), - libc::ETIMEDOUT - ); - assert!(current_time.elapsed().as_millis() >= 900); + let elapsed_time = current_time.elapsed().as_millis(); + assert!(900 <= elapsed_time && elapsed_time <= 1100); 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(&mut cond as *mut _), 0); @@ -70,6 +42,6 @@ fn test_timed_wait_timeout_realtime() { } fn main() { - test_timed_wait_timeout_monotonic(); - test_timed_wait_timeout_realtime(); + test_timed_wait_timeout(libc::CLOCK_MONOTONIC); + test_timed_wait_timeout(libc::CLOCK_REALTIME); } diff --git a/tests/run-pass/concurrency/sync.rs b/tests/run-pass/concurrency/sync.rs index e3f3a03b11a..5c19eee342f 100644 --- a/tests/run-pass/concurrency/sync.rs +++ b/tests/run-pass/concurrency/sync.rs @@ -35,8 +35,9 @@ fn check_conditional_variables_notify_one() { let pair = Arc::new((Mutex::new(false), Condvar::new())); let pair2 = pair.clone(); - // Inside of our lock, spawn a new thread, and then wait for it to start. + // Spawn a new thread. thread::spawn(move || { + thread::yield_now(); let (lock, cvar) = &*pair2; let mut started = lock.lock().unwrap(); *started = true; @@ -44,7 +45,7 @@ fn check_conditional_variables_notify_one() { cvar.notify_one(); }); - // Wait for the thread to start up. + // Wait for the thread to fully start up. let (lock, cvar) = &*pair; let mut started = lock.lock().unwrap(); while !*started {