fix shared behavior and add tests

src/shims/windows/sync.rs

@@ -12,7 +12,7 @@ const INIT_ONCE_ID_OFFSET: u64 = 0;
 const CONDVAR_ID_OFFSET: u64 = 0;
 
 impl<'mir, 'tcx> EvalContextExtPriv<'mir, 'tcx> for crate::MiriInterpCx<'mir, 'tcx> {}
-pub trait EvalContextExtPriv<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
+trait EvalContextExtPriv<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
     /// Try to reacquire the lock associated with the condition variable after we
     /// were signaled.
     fn reacquire_cond_lock(
@@ -26,13 +26,13 @@ pub trait EvalContextExtPriv<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tc
 
         match mode {
             RwLockMode::Read =>
-                if this.rwlock_is_locked(lock) {
+                if this.rwlock_is_write_locked(lock) {
                     this.rwlock_enqueue_and_block_reader(lock, thread);
                 } else {
                     this.rwlock_reader_lock(lock, thread);
                 },
             RwLockMode::Write =>
-                if this.rwlock_is_write_locked(lock) {
+                if this.rwlock_is_locked(lock) {
                     this.rwlock_enqueue_and_block_writer(lock, thread);
                 } else {
                     this.rwlock_writer_lock(lock, thread);

tests/pass/concurrency/windows_condvar_shared.rs

@@ -0,0 +1,227 @@
+//@only-target-windows: Uses win32 api functions
+// We are making scheduler assumptions here.
+//@compile-flags: -Zmiri-preemption-rate=0
+
+use std::ffi::c_void;
+use std::ptr::null_mut;
+use std::thread;
+
+#[derive(Copy, Clone)]
+struct SendPtr<T>(*mut T);
+
+unsafe impl<T> Send for SendPtr<T> {}
+
+extern "system" {
+    fn SleepConditionVariableSRW(
+        condvar: *mut *mut c_void,
+        lock: *mut *mut c_void,
+        timeout: u32,
+        flags: u32,
+    ) -> i32;
+    fn WakeAllConditionVariable(condvar: *mut *mut c_void);
+
+    fn AcquireSRWLockExclusive(lock: *mut *mut c_void);
+    fn AcquireSRWLockShared(lock: *mut *mut c_void);
+    fn ReleaseSRWLockExclusive(lock: *mut *mut c_void);
+    fn ReleaseSRWLockShared(lock: *mut *mut c_void);
+}
+
+const CONDITION_VARIABLE_LOCKMODE_SHARED: u32 = 1;
+const INFINITE: u32 = u32::MAX;
+
+/// threads should be able to reacquire the lock while it is locked by multiple other threads in shared mode
+fn all_shared() {
+    println!("all_shared");
+
+    let mut lock = null_mut();
+    let mut condvar = null_mut();
+
+    let lock_ptr = SendPtr(&mut lock);
+    let condvar_ptr = SendPtr(&mut condvar);
+
+    let mut handles = Vec::with_capacity(10);
+
+    // waiters
+    for i in 0..5 {
+        handles.push(thread::spawn(move || {
+            unsafe {
+                AcquireSRWLockShared(lock_ptr.0);
+            }
+            println!("exclusive waiter {i} locked");
+
+            let r = unsafe {
+                SleepConditionVariableSRW(
+                    condvar_ptr.0,
+                    lock_ptr.0,
+                    INFINITE,
+                    CONDITION_VARIABLE_LOCKMODE_SHARED,
+                )
+            };
+            assert_ne!(r, 0);
+
+            println!("exclusive waiter {i} reacquired lock");
+
+            // unlocking is unnecessary because the lock is never used again
+        }));
+    }
+
+    // ensures each waiter is waiting by this point
+    thread::yield_now();
+
+    // readers
+    for i in 0..5 {
+        handles.push(thread::spawn(move || {
+            unsafe {
+                AcquireSRWLockShared(lock_ptr.0);
+            }
+            println!("reader {i} locked");
+
+            // switch to next reader or main thread
+            thread::yield_now();
+
+            unsafe {
+                ReleaseSRWLockShared(lock_ptr.0);
+            }
+            println!("reader {i} unlocked");
+        }));
+    }
+
+    // ensures each reader has acquired the lock
+    thread::yield_now();
+
+    unsafe {
+        WakeAllConditionVariable(condvar_ptr.0);
+    }
+
+    for handle in handles {
+        handle.join().unwrap();
+    }
+}
+
+// reacquiring a lock should wait until the lock is not exclusively locked
+fn shared_sleep_and_exclusive_lock() {
+    println!("shared_sleep_and_exclusive_lock");
+
+    let mut lock = null_mut();
+    let mut condvar = null_mut();
+
+    let lock_ptr = SendPtr(&mut lock);
+    let condvar_ptr = SendPtr(&mut condvar);
+
+    let mut waiters = Vec::with_capacity(5);
+    for i in 0..5 {
+        waiters.push(thread::spawn(move || {
+            unsafe {
+                AcquireSRWLockShared(lock_ptr.0);
+            }
+            println!("shared waiter {i} locked");
+
+            let r = unsafe {
+                SleepConditionVariableSRW(
+                    condvar_ptr.0,
+                    lock_ptr.0,
+                    INFINITE,
+                    CONDITION_VARIABLE_LOCKMODE_SHARED,
+                )
+            };
+            assert_ne!(r, 0);
+
+            println!("shared waiter {i} reacquired lock");
+
+            // unlocking is unnecessary because the lock is never used again
+        }));
+    }
+
+    // ensures each waiter is waiting by this point
+    thread::yield_now();
+
+    unsafe {
+        AcquireSRWLockExclusive(lock_ptr.0);
+    }
+    println!("main locked");
+
+    unsafe {
+        WakeAllConditionVariable(condvar_ptr.0);
+    }
+
+    // waiters are now waiting for the lock to be unlocked
+    thread::yield_now();
+
+    unsafe {
+        ReleaseSRWLockExclusive(lock_ptr.0);
+    }
+    println!("main unlocked");
+
+    for handle in waiters {
+        handle.join().unwrap();
+    }
+}
+
+// threads reacquiring locks should wait for all locks to be released first
+fn exclusive_sleep_and_shared_lock() {
+    println!("exclusive_sleep_and_shared_lock");
+
+    let mut lock = null_mut();
+    let mut condvar = null_mut();
+
+    let lock_ptr = SendPtr(&mut lock);
+    let condvar_ptr = SendPtr(&mut condvar);
+
+    let mut handles = Vec::with_capacity(10);
+    for i in 0..5 {
+        handles.push(thread::spawn(move || {
+            unsafe {
+                AcquireSRWLockExclusive(lock_ptr.0);
+            }
+
+            println!("exclusive waiter {i} locked");
+
+            let r = unsafe { SleepConditionVariableSRW(condvar_ptr.0, lock_ptr.0, INFINITE, 0) };
+            assert_ne!(r, 0);
+
+            println!("exclusive waiter {i} reacquired lock");
+
+            // switch to next waiter or main thread
+            thread::yield_now();
+
+            unsafe {
+                ReleaseSRWLockExclusive(lock_ptr.0);
+            }
+            println!("exclusive waiter {i} unlocked");
+        }));
+    }
+
+    for i in 0..5 {
+        handles.push(thread::spawn(move || {
+            unsafe {
+                AcquireSRWLockShared(lock_ptr.0);
+            }
+            println!("reader {i} locked");
+
+            // switch to next reader or main thread
+            thread::yield_now();
+
+            unsafe {
+                ReleaseSRWLockShared(lock_ptr.0);
+            }
+            println!("reader {i} unlocked");
+        }));
+    }
+
+    // ensures each reader has acquired the lock
+    thread::yield_now();
+
+    unsafe {
+        WakeAllConditionVariable(condvar_ptr.0);
+    }
+
+    for handle in handles {
+        handle.join().unwrap();
+    }
+}
+
+fn main() {
+    all_shared();
+    shared_sleep_and_exclusive_lock();
+    exclusive_sleep_and_shared_lock();
+}

tests/pass/concurrency/windows_condvar_shared.stdout

@@ -0,0 +1,60 @@
+all_shared
+exclusive waiter 0 locked
+exclusive waiter 1 locked
+exclusive waiter 2 locked
+exclusive waiter 3 locked
+exclusive waiter 4 locked
+reader 0 locked
+reader 1 locked
+reader 2 locked
+reader 3 locked
+reader 4 locked
+exclusive waiter 0 reacquired lock
+exclusive waiter 1 reacquired lock
+exclusive waiter 2 reacquired lock
+exclusive waiter 3 reacquired lock
+exclusive waiter 4 reacquired lock
+reader 0 unlocked
+reader 1 unlocked
+reader 2 unlocked
+reader 3 unlocked
+reader 4 unlocked
+shared_sleep_and_exclusive_lock
+shared waiter 0 locked
+shared waiter 1 locked
+shared waiter 2 locked
+shared waiter 3 locked
+shared waiter 4 locked
+main locked
+main unlocked
+shared waiter 0 reacquired lock
+shared waiter 1 reacquired lock
+shared waiter 2 reacquired lock
+shared waiter 3 reacquired lock
+shared waiter 4 reacquired lock
+exclusive_sleep_and_shared_lock
+exclusive waiter 0 locked
+exclusive waiter 1 locked
+exclusive waiter 2 locked
+exclusive waiter 3 locked
+exclusive waiter 4 locked
+reader 0 locked
+reader 1 locked
+reader 2 locked
+reader 3 locked
+reader 4 locked
+reader 0 unlocked
+reader 1 unlocked
+reader 2 unlocked
+reader 3 unlocked
+reader 4 unlocked
+exclusive waiter 0 reacquired lock
+exclusive waiter 0 unlocked
+exclusive waiter 1 reacquired lock
+exclusive waiter 1 unlocked
+exclusive waiter 2 reacquired lock
+exclusive waiter 2 unlocked
+exclusive waiter 3 reacquired lock
+exclusive waiter 3 unlocked
+exclusive waiter 4 reacquired lock
+exclusive waiter 4 unlocked