From 2eb07a05a3fa1e604f4243ddd5659f294d6b69b9 Mon Sep 17 00:00:00 2001 From: DrMeepster <19316085+DrMeepster@users.noreply.github.com> Date: Sun, 30 Oct 2022 19:14:45 -0700 Subject: [PATCH] fix shared behavior and add tests --- src/tools/miri/src/shims/windows/sync.rs | 6 +- .../concurrency/windows_condvar_shared.rs | 227 ++++++++++++++++++ .../concurrency/windows_condvar_shared.stdout | 60 +++++ 3 files changed, 290 insertions(+), 3 deletions(-) create mode 100644 src/tools/miri/tests/pass/concurrency/windows_condvar_shared.rs create mode 100644 src/tools/miri/tests/pass/concurrency/windows_condvar_shared.stdout diff --git a/src/tools/miri/src/shims/windows/sync.rs b/src/tools/miri/src/shims/windows/sync.rs index a34d142f4a5..8f414d98dba 100644 --- a/src/tools/miri/src/shims/windows/sync.rs +++ b/src/tools/miri/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); diff --git a/src/tools/miri/tests/pass/concurrency/windows_condvar_shared.rs b/src/tools/miri/tests/pass/concurrency/windows_condvar_shared.rs new file mode 100644 index 00000000000..d89320bfe59 --- /dev/null +++ b/src/tools/miri/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(*mut T); + +unsafe impl Send for SendPtr {} + +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(); +} diff --git a/src/tools/miri/tests/pass/concurrency/windows_condvar_shared.stdout b/src/tools/miri/tests/pass/concurrency/windows_condvar_shared.stdout new file mode 100644 index 00000000000..918b54668f2 --- /dev/null +++ b/src/tools/miri/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