Skip to content

Commit

Permalink
Add loom support to std critical section implementation
Browse files Browse the repository at this point in the history
  • Loading branch information
datdenkikniet committed Mar 10, 2025
1 parent cebd3d7 commit 580ef08
Show file tree
Hide file tree
Showing 5 changed files with 74 additions and 5 deletions.
6 changes: 5 additions & 1 deletion .github/workflows/clippy.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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}}"
8 changes: 7 additions & 1 deletion .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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}}
6 changes: 6 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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)'] }
8 changes: 8 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand Down Expand Up @@ -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.

Expand Down
51 changes: 48 additions & 3 deletions src/std.rs
Original file line number Diff line number Diff line change
@@ -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);
Expand Down Expand Up @@ -64,6 +82,7 @@ unsafe impl crate::Impl for StdCriticalSection {
}

#[cfg(test)]
#[cfg(not(loom))]
mod tests {
use std::thread;

Expand All @@ -85,3 +104,29 @@ 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: `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!");
})
})
}
}

0 comments on commit 580ef08

Please sign in to comment.