Skip to content

Commit 2aec3ed

Browse files
Replace scc/sdd with std::sync::Mutex for Miri strict provenance compatibility
The sdd crate (transitive dependency of scc) uses integer-to-pointer casts that are rejected by Miri's strict provenance mode. Replace scc::HashMap with std::sync::Mutex<HashMap> wrapped in a LockMap type that preserves the existing API surface. Add Miri CI job. Fixes #156
1 parent df3121e commit 2aec3ed

7 files changed

Lines changed: 82 additions & 37 deletions

File tree

.github/workflows/ci.yml

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,21 @@ jobs:
6464
RUST_TEST_THREADS: 3 # So the parallel tests have enough threads
6565
RUST_LOG: debug
6666

67+
miri:
68+
name: Miri
69+
runs-on: ubuntu-24.04
70+
steps:
71+
- uses: actions/checkout@v3.5.0
72+
- uses: dtolnay/rust-toolchain@stable
73+
with:
74+
toolchain: nightly
75+
components: miri
76+
- uses: Swatinem/rust-cache@v2.2.1
77+
- name: Miri strict provenance
78+
run: cargo miri test -p serial_test --no-default-features --test tests
79+
env:
80+
MIRIFLAGS: -Zmiri-strict-provenance
81+
6782
minimal-versions:
6883
name: minimal versions check
6984
runs-on: ubuntu-24.04

Cargo.lock

Lines changed: 0 additions & 17 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

serial_test/Cargo.toml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ log = { version = ">=0.4.4", optional = true }
2121
futures-executor = { version = "^0.3", optional = true, default-features = false, features = ["std"] }
2222
futures-util = { version = "^0.3", optional = true, default-features = false, features = ["std"] }
2323

24-
scc = { version = "2", default-features = false}
2524
env_logger = {version=">=0.6.1", optional=true, default-features = false}
2625

2726
[dev-dependencies]

serial_test/src/code_lock.rs

Lines changed: 43 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,45 @@
11
use crate::rwlock::{Locks, MutexGuardWrapper};
22
use once_cell::sync::OnceCell;
3-
use scc::{hash_map::Entry, HashMap};
3+
use std::collections::HashMap;
44
use std::sync::atomic::AtomicU32;
5+
use std::sync::Mutex;
6+
7+
pub(crate) struct ValueRef(UniqueReentrantMutex);
8+
9+
impl ValueRef {
10+
pub fn get(&self) -> &UniqueReentrantMutex {
11+
&self.0
12+
}
13+
}
14+
15+
pub(crate) struct LockMap {
16+
inner: Mutex<HashMap<String, UniqueReentrantMutex>>,
17+
}
18+
19+
impl LockMap {
20+
fn new() -> Self {
21+
LockMap {
22+
inner: Mutex::new(HashMap::new()),
23+
}
24+
}
25+
26+
pub fn get(&self, key: &str) -> Option<ValueRef> {
27+
self.inner.lock().unwrap().get(key).cloned().map(ValueRef)
28+
}
29+
30+
pub fn contains(&self, key: &str) -> bool {
31+
self.inner.lock().unwrap().contains_key(key)
32+
}
33+
34+
fn get_or_insert(
35+
&self,
36+
key: &str,
37+
f: impl FnOnce() -> UniqueReentrantMutex,
38+
) -> UniqueReentrantMutex {
39+
let mut map = self.inner.lock().unwrap();
40+
map.entry(key.to_owned()).or_insert_with(f).clone()
41+
}
42+
}
543

644
#[derive(Clone)]
745
pub(crate) struct UniqueReentrantMutex {
@@ -41,11 +79,11 @@ impl UniqueReentrantMutex {
4179
}
4280

4381
#[inline]
44-
pub(crate) fn global_locks() -> &'static HashMap<String, UniqueReentrantMutex> {
82+
pub(crate) fn global_locks() -> &'static LockMap {
4583
#[cfg(feature = "test_logging")]
4684
let _ = env_logger::builder().try_init();
47-
static LOCKS: OnceCell<HashMap<String, UniqueReentrantMutex>> = OnceCell::new();
48-
LOCKS.get_or_init(HashMap::new)
85+
static LOCKS: OnceCell<LockMap> = OnceCell::new();
86+
LOCKS.get_or_init(LockMap::new)
4987
}
5088

5189
/// Check if the current thread is holding a serial lock
@@ -123,11 +161,7 @@ pub(crate) fn check_new_key(name: &str) {
123161
};
124162

125163
// This is the rare path, which avoids the multi-writer situation mostly
126-
let entry = global_locks().entry(name.to_owned());
127-
match entry {
128-
Entry::Occupied(o) => o,
129-
Entry::Vacant(v) => v.insert_entry(UniqueReentrantMutex::new_mutex(name)),
130-
};
164+
global_locks().get_or_insert(name, || UniqueReentrantMutex::new_mutex(name));
131165
}
132166

133167
#[cfg(test)]

serial_test/tests/tests.rs

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,18 @@
1-
use serial_test::local_serial_core;
1+
use serial_test::{local_parallel_core, local_serial_core};
22

33
#[test]
44
fn test_empty_serial_call() {
55
local_serial_core(vec!["beta"], None, || {
66
println!("Bar");
77
});
88
}
9+
10+
// Verify that serial_test is compatible with Miri's strict provenance mode.
11+
//
12+
// Run with:
13+
// MIRIFLAGS="-Zmiri-strict-provenance" cargo +nightly miri test -p serial_test --no-default-features --test tests test_miri_strict_provenance
14+
#[test]
15+
fn test_miri_strict_provenance() {
16+
local_serial_core(vec!["miri_provenance_test"], None, || {});
17+
local_parallel_core(vec!["miri_provenance_test"], None, || {});
18+
}

serial_test_test/Cargo.toml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ lock_api = { version="^0.4.7", default-features = false }
1616
wasm-bindgen-test = {version="^0.3.50", optional=true, default-features = false, features=["std"] }
1717
scoped-tls = { version="1", optional=true, default-features = false }
1818
log = { version = ">=0.4.4" , default-features = false }
19-
scc = { version = "2", default-features = false}
2019

2120
[dev-dependencies]
2221
tokio = { version = "=1.38.2", features = ["macros", "rt", "rt-multi-thread"], default-features = false }

serial_test_test/src/lib.rs

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -38,31 +38,36 @@
3838
3939
use log::info;
4040
use once_cell::sync::OnceCell;
41-
use scc::HashMap;
4241
#[cfg(test)]
4342
use serial_test::{parallel, serial};
4443
use std::{
44+
collections::HashMap,
4545
convert::TryInto,
4646
env, fs,
4747
path::PathBuf,
48-
sync::atomic::{AtomicUsize, Ordering},
48+
sync::{
49+
atomic::{AtomicUsize, Ordering},
50+
Arc, Mutex,
51+
},
4952
thread,
5053
time::Duration,
5154
};
5255

53-
static LOCKS: OnceCell<HashMap<String, AtomicUsize>> = OnceCell::new();
56+
static LOCKS: OnceCell<Mutex<HashMap<String, Arc<AtomicUsize>>>> = OnceCell::new();
5457

5558
fn init() {
5659
let _ = env_logger::builder().is_test(false).try_init();
5760
}
5861

5962
pub fn test_fn(key: &str, count: usize) {
6063
init();
61-
let local_locks = LOCKS.get_or_init(HashMap::new);
62-
let entry = local_locks
63-
.entry(key.to_string())
64-
.or_insert(AtomicUsize::new(0));
65-
let local_lock = entry.get();
64+
let local_locks = LOCKS.get_or_init(|| Mutex::new(HashMap::new()));
65+
let local_lock = {
66+
let mut map = local_locks.lock().unwrap();
67+
map.entry(key.to_string())
68+
.or_insert_with(|| Arc::new(AtomicUsize::new(0)))
69+
.clone()
70+
};
6671
info!("(non-fs) Start {}", count);
6772
local_lock.store(count, Ordering::Relaxed);
6873
thread::sleep(Duration::from_millis(1000 * (count as u64)));

0 commit comments

Comments
 (0)