From 1e1ac6f47178097b9db3a1232599d31e8567591d Mon Sep 17 00:00:00 2001 From: Nika Layzell Date: Sun, 26 Jul 2020 15:18:04 -0400 Subject: [PATCH 1/2] Add support for lock_api Adds Loom mock implementations of the `lock_api::RawMutex` and `lock_api::RawRwLock` traits, as well as re-exports for the relevant concrete types, gated behind a `"lock_api"` feature. The `parking_lot` and `lock_api` crates provide a slightly different interface than the standard library interface to `Mutex` and `RwLock`, which can make them difficult or impossible to test using `sync::Mutex` and `sync::RwLock` mocks. --- Cargo.toml | 8 +++ ci/azure-test-all.yml | 4 ++ src/lib.rs | 3 + src/lock_api.rs | 114 +++++++++++++++++++++++++++++++++ tests/lock_api.rs | 144 ++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 273 insertions(+) create mode 100644 src/lock_api.rs create mode 100644 tests/lock_api.rs diff --git a/Cargo.toml b/Cargo.toml index b5ef12c9..ea6a0471 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -25,6 +25,7 @@ categories = ["concurrency", "data-structures"] default = [] checkpoint = ["serde", "serde_json"] futures = ["futures-util"] +lock_api = ["lock_api_", "once_cell"] [dependencies] cfg-if = "0.1.6" @@ -38,3 +39,10 @@ serde = { version = "1.0.92", features = ["derive"], optional = true } serde_json = { version = "1.0.33", optional = true } futures-util = { version = "0.3.0", optional = true } + +# Provides locking primitives for the `lock_api` crate +lock_api_ = { package = "lock_api", version = "0.4.1", optional = true } +once_cell = { version = "1.4.0", optional = true } + +[package.metadata.docs.rs] +features = ["lock_api"] diff --git a/ci/azure-test-all.yml b/ci/azure-test-all.yml index d1ee03ce..29cd479b 100644 --- a/ci/azure-test-all.yml +++ b/ci/azure-test-all.yml @@ -29,3 +29,7 @@ jobs: # Test with futures feature - script: cargo test --features futures displayName: cargo test --features futures + + # Test with lock_api feature + - script: cargo test --features lock_api + displayName: cargo test --features lock_api diff --git a/src/lib.rs b/src/lib.rs index 922936e4..5b54f88a 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -162,6 +162,9 @@ if_futures! { pub mod future; } +#[cfg(feature = "lock_api")] +pub mod lock_api; + #[doc(hidden)] pub fn __debug_enabled() -> bool { rt::execution(|e| e.log) diff --git a/src/lock_api.rs b/src/lock_api.rs new file mode 100644 index 00000000..771c6a4b --- /dev/null +++ b/src/lock_api.rs @@ -0,0 +1,114 @@ +//! Mock implementation of the `lock_api` and `parking_lot` crates. +//! +//! _These types are only available if loom is built with the `"lock_api"` +//! feature_ + +use crate::rt; +use lock_api_ as lock_api; +use once_cell::sync::OnceCell; + +/// Mock implementation of `lock_api::RawMutex` +#[allow(missing_debug_implementations)] +pub struct RawMutex { + object: OnceCell, +} + +impl RawMutex { + // Unfortunately, we're required to have a `const INIT` in order to + // implement `lock_api::RawMutex`, so we need to lazily create the actual + // `rt::Mutex`. + fn object(&self) -> &rt::Mutex { + self.object.get_or_init(|| rt::Mutex::new(true)) + } +} + +unsafe impl lock_api::RawMutex for RawMutex { + const INIT: RawMutex = RawMutex { + object: OnceCell::new(), + }; + + type GuardMarker = lock_api::GuardNoSend; + + fn lock(&self) { + self.object().acquire_lock(); + } + + fn try_lock(&self) -> bool { + self.object().try_acquire_lock() + } + + unsafe fn unlock(&self) { + self.object().release_lock() + } +} + +/// Mock implementation of `lock_api::RawRwLock` +#[allow(missing_debug_implementations)] +pub struct RawRwLock { + object: OnceCell, +} + +impl RawRwLock { + // Unfortunately we're required to have a `const INIT` in order to implement + // `lock_api::RawRwLock`, so we need to lazily create the actual + // `rt::RwLock`. + fn object(&self) -> &rt::RwLock { + self.object.get_or_init(|| rt::RwLock::new()) + } +} + +unsafe impl lock_api::RawRwLock for RawRwLock { + const INIT: RawRwLock = RawRwLock { + object: OnceCell::new(), + }; + + type GuardMarker = lock_api::GuardNoSend; + + fn lock_shared(&self) { + self.object().acquire_read_lock() + } + + fn try_lock_shared(&self) -> bool { + self.object().try_acquire_read_lock() + } + + unsafe fn unlock_shared(&self) { + self.object().release_read_lock() + } + + fn lock_exclusive(&self) { + self.object().acquire_write_lock() + } + + fn try_lock_exclusive(&self) -> bool { + self.object().try_acquire_write_lock() + } + + unsafe fn unlock_exclusive(&self) { + self.object().release_write_lock() + } +} + +/// Mock implementation of `lock_api::Mutex` +pub type Mutex = lock_api::Mutex; + +/// Mock implementation of `lock_api::MutexGuard` +pub type MutexGuard<'a, T> = lock_api::MutexGuard<'a, RawMutex, T>; + +/// Mock implementation of `lock_api::MappedMutexGuard` +pub type MappedMutexGuard<'a, T> = lock_api::MappedMutexGuard<'a, RawMutex, T>; + +/// Mock implementation of `lock_api::RwLock` +pub type RwLock = lock_api::RwLock; + +/// Mock implementation of `lock_api::RwLockReadGuard` +pub type RwLockReadGuard<'a, T> = lock_api::RwLockReadGuard<'a, RawRwLock, T>; + +/// Mock implementation of `lock_api::RwLockWriteGuard` +pub type RwLockWriteGuard<'a, T> = lock_api::RwLockWriteGuard<'a, RawRwLock, T>; + +/// Mock implementation of `lock_api::MappedRwLockReadGuard` +pub type MappedRwLockReadGuard<'a, T> = lock_api::MappedRwLockReadGuard<'a, RawRwLock, T>; + +/// Mock implementation of `lock_api::MappedRwLockWriteGuard` +pub type MappedRwLockWriteGuard<'a, T> = lock_api::MappedRwLockWriteGuard<'a, RawRwLock, T>; diff --git a/tests/lock_api.rs b/tests/lock_api.rs new file mode 100644 index 00000000..a0478421 --- /dev/null +++ b/tests/lock_api.rs @@ -0,0 +1,144 @@ +#![cfg(feature = "lock_api")] +#![deny(warnings, rust_2018_idioms)] + +use loom::cell::UnsafeCell; +use loom::lock_api::{Mutex, RwLock}; +use loom::sync::atomic::AtomicUsize; +use loom::thread; + +use std::rc::Rc; +use std::sync::atomic::Ordering::SeqCst; +use std::sync::Arc; + +#[test] +fn mutex_enforces_mutal_exclusion() { + loom::model(|| { + let data = Rc::new((Mutex::new(0), AtomicUsize::new(0))); + + let ths: Vec<_> = (0..2) + .map(|_| { + let data = data.clone(); + + thread::spawn(move || { + let mut locked = data.0.lock(); + + let prev = data.1.fetch_add(1, SeqCst); + assert_eq!(prev, *locked); + *locked += 1; + }) + }) + .collect(); + + for th in ths { + th.join().unwrap(); + } + + let locked = data.0.lock(); + + assert_eq!(*locked, data.1.load(SeqCst)); + }); +} + +#[test] +fn mutex_establishes_seq_cst() { + loom::model(|| { + struct Data { + cell: UnsafeCell, + flag: Mutex, + } + + let data = Rc::new(Data { + cell: UnsafeCell::new(0), + flag: Mutex::new(false), + }); + + { + let data = data.clone(); + + thread::spawn(move || { + unsafe { data.cell.with_mut(|v| *v = 1) }; + *data.flag.lock() = true; + }); + } + + let flag = *data.flag.lock(); + + if flag { + let v = unsafe { data.cell.with(|v| *v) }; + assert_eq!(v, 1); + } + }); +} + +#[test] +fn rwlock_read_one() { + loom::model(|| { + let lock = Arc::new(RwLock::new(1)); + let c_lock = lock.clone(); + + let n = lock.read(); + assert_eq!(*n, 1); + + thread::spawn(move || { + let _l = c_lock.read(); + }) + .join() + .unwrap(); + }); +} + +#[test] +fn rwlock_read_two_write_one() { + loom::model(|| { + let lock = Arc::new(RwLock::new(1)); + + for _ in 0..2 { + let lock = lock.clone(); + + thread::spawn(move || { + let _l = lock.read(); + + thread::yield_now(); + }); + } + + let _l = lock.write(); + thread::yield_now(); + }); +} + +#[test] +fn rwlock_try_read() { + loom::model(|| { + let lock = RwLock::new(1); + + match lock.try_read() { + Some(n) => assert_eq!(*n, 1), + None => unreachable!(), + }; + }); +} + +#[test] +fn rwlock_write() { + loom::model(|| { + let lock = RwLock::new(1); + + let mut n = lock.write(); + *n = 2; + + assert!(lock.try_read().is_none()); + }); +} + +#[test] +fn rwlock_try_write() { + loom::model(|| { + let lock = RwLock::new(1); + + let n = lock.read(); + assert_eq!(*n, 1); + + assert!(lock.try_write().is_none()); + }); +} From 9cb0dfb8732779f96a919d3c0fef0bcdd06c5482 Mon Sep 17 00:00:00 2001 From: Nika Layzell Date: Sun, 26 Jul 2020 15:59:28 -0400 Subject: [PATCH 2/2] add specialized is_locked impls --- src/lock_api.rs | 9 +++++++++ src/rt/mutex.rs | 2 +- src/rt/rwlock.rs | 4 ++-- 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/src/lock_api.rs b/src/lock_api.rs index 771c6a4b..5bff4832 100644 --- a/src/lock_api.rs +++ b/src/lock_api.rs @@ -40,6 +40,10 @@ unsafe impl lock_api::RawMutex for RawMutex { unsafe fn unlock(&self) { self.object().release_lock() } + + fn is_locked(&self) -> bool { + self.object().is_locked() + } } /// Mock implementation of `lock_api::RawRwLock` @@ -87,6 +91,11 @@ unsafe impl lock_api::RawRwLock for RawRwLock { unsafe fn unlock_exclusive(&self) { self.object().release_write_lock() } + + fn is_locked(&self) -> bool { + let object = self.object(); + object.is_read_locked() || object.is_write_locked() + } } /// Mock implementation of `lock_api::Mutex` diff --git a/src/rt/mutex.rs b/src/rt/mutex.rs index f939258d..b6ff8b10 100644 --- a/src/rt/mutex.rs +++ b/src/rt/mutex.rs @@ -123,7 +123,7 @@ impl Mutex { } /// Returns `true` if the mutex is currently locked - fn is_locked(&self) -> bool { + pub(crate) fn is_locked(&self) -> bool { super::execution(|execution| self.state.get(&execution.objects).lock.is_some()) } } diff --git a/src/rt/rwlock.rs b/src/rt/rwlock.rs index d37d5eba..72d347a1 100644 --- a/src/rt/rwlock.rs +++ b/src/rt/rwlock.rs @@ -153,7 +153,7 @@ impl RwLock { } /// Returns `true` if RwLock is read locked - fn is_read_locked(&self) -> bool { + pub(crate) fn is_read_locked(&self) -> bool { super::execution( |execution| match self.state.get(&mut execution.objects).lock { Some(Locked::Read(_)) => true, @@ -163,7 +163,7 @@ impl RwLock { } /// Returns `true` if RwLock is write locked. - fn is_write_locked(&self) -> bool { + pub(crate) fn is_write_locked(&self) -> bool { super::execution( |execution| match self.state.get(&mut execution.objects).lock { Some(Locked::Write(_)) => true,