Small changes.

Author: vakaras

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,21 +740,22 @@ 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.
         this.register_timeout_callback(
             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)?;
 

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);
     }
 

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,43 +33,15 @@ 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);
     }
 }
 
 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);
 }

tests/run-pass/concurrency/sync.rs

@@ -35,16 +35,17 @@ 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;
         // We notify the condvar that the value has changed.
         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 {