Add loom support to std critical section implementation

Author: datdenkikniet

.github/workflows/clippy.yml

@@ -18,6 +18,10 @@ jobs:
             features: ''
           - rust: 1.63
             features: 'std'
+          - rust: 1.73
+            features: 'std'
+            cfgs: "--cfg loom"
+
     steps:
       - uses: actions/checkout@v2
       - name: Install Rust
@@ -27,4 +31,4 @@ jobs:
             components: clippy
             override: true
       - name: Clippy check
-        run: cargo clippy --features "${{matrix.features}}"
+        run: RUSTFLAGS="${{matrix.cfgs}}" cargo clippy --features "${{matrix.features}}"

.github/workflows/test.yml

@@ -18,6 +18,12 @@ jobs:
             features: ''
           - rust: 1.63
             features: 'std'
+          - rust: 1.73
+            features: 'std'
+            cfgs: "--cfg loom"
+            # Lib tests are the only relevant & working ones
+            # for loom.
+            extra_flags: "--lib"
     steps:
       - uses: actions/checkout@v2
       - name: Install Rust
@@ -27,4 +33,4 @@ jobs:
             components: clippy
             override: true
       - name: Test
-        run: cargo test --features "${{matrix.features}}"
+        run: RUSTFLAGS="${{matrix.cfgs}}" cargo test --features "${{matrix.features}}" ${{matrix.extra_flags}}

Cargo.toml

@@ -29,3 +29,9 @@ restore-state-u16 = []
 restore-state-u32 = []
 restore-state-u64 = []
 restore-state-usize = []
+
+[target.'cfg(loom)'.dependencies]
+loom = "0.7.2"
+
+[lints.rust]
+unexpected_cfgs = { level = "allow", check-cfg = ['cfg(loom)'] }

README.md

@@ -99,6 +99,13 @@ provide an implementation based on `std::sync::Mutex`, so you don't have to add
 critical-section = { version = "1.1", features = ["std"]}
 ```
 
+## Usage in [`loom`] tests
+
+`critical-section` supports [`loom`] by enabling the `std` feature and passing `--cfg loom` as `RUSTFLAGS` (either through `.cargo/config.toml`, or through the environment variable).
+This implementation is identical to the normal `std` implementation, but uses `loom` synchronization primitives instead.
+
+[`loom`]: https://docs.rs/loom/latest/loom/#writing-tests
+
 ## Usage in libraries
 
 If you're writing a library intended to be portable across many targets, simply add a dependency on `critical-section`
@@ -219,6 +226,7 @@ This crate is guaranteed to compile on the following Rust versions:
 
 - If the `std` feature is not enabled: stable Rust 1.54 and up.
 - If the `std` feature is enabled: stable Rust 1.63 and up.
+- If the `std` feature and `--cfg loom` are enabled: stable Rust 1.73 and up.
 
 It might compile with older versions but that may change in any new patch release.
 

src/std.rs

@@ -1,13 +1,31 @@
-use std::cell::Cell;
 use std::mem::MaybeUninit;
-use std::sync::{Mutex, MutexGuard};
 
+#[cfg(not(loom))]
+use std::{
+    cell::Cell,
+    sync::{Mutex, MutexGuard},
+    thread_local,
+};
+
+#[cfg(loom)]
+use loom::{
+    cell::Cell,
+    sync::{Mutex, MutexGuard},
+    thread_local,
+};
+
+#[cfg(not(loom))]
 static GLOBAL_MUTEX: Mutex<()> = Mutex::new(());
 
+#[cfg(loom)]
+loom::lazy_static! {
+    static ref GLOBAL_MUTEX: Mutex<()> = Mutex::new(());
+}
+
 // This is initialized if a thread has acquired the CS, uninitialized otherwise.
 static mut GLOBAL_GUARD: MaybeUninit<MutexGuard<'static, ()>> = MaybeUninit::uninit();
 
-std::thread_local!(static IS_LOCKED: Cell<bool> = Cell::new(false));
+thread_local!(static IS_LOCKED: Cell<bool> = Cell::new(false));
 
 struct StdCriticalSection;
 crate::set_impl!(StdCriticalSection);
@@ -64,6 +82,7 @@ unsafe impl crate::Impl for StdCriticalSection {
 }
 
 #[cfg(test)]
+#[cfg(not(loom))]
 mod tests {
     use std::thread;
 
@@ -85,3 +104,30 @@ mod tests {
         })
     }
 }
+
+#[cfg(test)]
+#[cfg(loom)]
+mod tests {
+    use crate as critical_section;
+
+    #[cfg(feature = "std")]
+    #[test]
+    #[should_panic(expected = "Not a PoisonError!")]
+    fn reusable_after_panic_loom() {
+        loom::model(|| {
+            // IMPORTANT: using `std::thread` here because `loom` is effectively
+            // single-threaded, so panicking in `loom::thread` will panic the
+            // entire test.
+            let _ = std::thread::spawn(|| {
+                critical_section::with(|_| {
+                    panic!("Boom!");
+                });
+            })
+            .join();
+
+            critical_section::with(|_| {
+                panic!("Not a PoisonError!");
+            })
+        })
+    }
+}