Merge pull request #157 from justanotheranonymoususer/remove-scc-dep

Replace scc/sdd with std::sync::Mutex for Miri strict provenance compatibility

Author: palfrey

.github/workflows/ci.yml

@@ -5,6 +5,7 @@ on:
   pull_request:
   schedule:
     - cron: '0 0 1 * *'
+  workflow_dispatch:
 
 name: Continuous integration
 
@@ -64,6 +65,21 @@ jobs:
           RUST_TEST_THREADS: 3 # So the parallel tests have enough threads
           RUST_LOG: debug
 
+  miri:
+    name: Miri
+    runs-on: ubuntu-24.04
+    steps:
+      - uses: actions/checkout@v3.5.0
+      - uses: dtolnay/rust-toolchain@stable
+        with:
+          toolchain: nightly
+          components: miri
+      - uses: Swatinem/rust-cache@v2.2.1
+      - name: Miri strict provenance
+        run: cargo miri test -p serial_test --all-features -- --skip file_lock
+        env:
+          MIRIFLAGS: -Zmiri-strict-provenance
+
   minimal-versions:
     name: minimal versions check
     runs-on: ubuntu-24.04
@@ -112,4 +128,4 @@ jobs:
       - uses: actions/checkout@v6
       - uses: dtolnay/rust-toolchain@stable
       - run: cargo install cargo-audit --locked
-      - run: cargo audit --deny warnings
+      - run: cargo audit --deny warnings

Cargo.lock

@@ -21,7 +21,6 @@ log = { version = ">=0.4.4", optional = true }
 futures-executor = { version = "^0.3", optional = true, default-features = false, features = ["std"] }
 futures-util = { version = "^0.3", optional = true, default-features = false, features = ["std"] }
 
-scc = { version = "2", default-features = false}
 env_logger = {version=">=0.6.1", optional=true, default-features = false}
 
 [dev-dependencies]

serial_test/Cargo.toml

@@ -1,7 +1,42 @@
 use crate::rwlock::{Locks, MutexGuardWrapper};
 use once_cell::sync::OnceCell;
-use scc::{hash_map::Entry, HashMap};
-use std::sync::atomic::AtomicU32;
+use std::{
+    collections::HashMap,
+    sync::{atomic::AtomicU32, Mutex},
+};
+
+pub(crate) struct ValueRef(UniqueReentrantMutex);
+
+impl ValueRef {
+    pub fn get(&self) -> &UniqueReentrantMutex {
+        &self.0
+    }
+}
+
+pub(crate) struct LockMap {
+    inner: Mutex<HashMap<String, UniqueReentrantMutex>>,
+}
+
+impl LockMap {
+    fn new() -> Self {
+        LockMap {
+            inner: Mutex::new(HashMap::new()),
+        }
+    }
+
+    pub fn get(&self, key: &str) -> Option<ValueRef> {
+        self.inner.lock().unwrap().get(key).cloned().map(ValueRef)
+    }
+
+    fn get_or_insert(
+        &self,
+        key: &str,
+        f: impl FnOnce() -> UniqueReentrantMutex,
+    ) -> UniqueReentrantMutex {
+        let mut map = self.inner.lock().unwrap();
+        map.entry(key.to_owned()).or_insert_with(f).clone()
+    }
+}
 
 #[derive(Clone)]
 pub(crate) struct UniqueReentrantMutex {
@@ -41,11 +76,11 @@ impl UniqueReentrantMutex {
 }
 
 #[inline]
-pub(crate) fn global_locks() -> &'static HashMap<String, UniqueReentrantMutex> {
+pub(crate) fn global_locks() -> &'static LockMap {
     #[cfg(feature = "test_logging")]
     let _ = env_logger::builder().try_init();
-    static LOCKS: OnceCell<HashMap<String, UniqueReentrantMutex>> = OnceCell::new();
-    LOCKS.get_or_init(HashMap::new)
+    static LOCKS: OnceCell<LockMap> = OnceCell::new();
+    LOCKS.get_or_init(LockMap::new)
 }
 
 /// Check if the current thread is holding a serial lock
@@ -117,17 +152,7 @@ impl UniqueReentrantMutex {
 }
 
 pub(crate) fn check_new_key(name: &str) {
-    // Check if a new key is needed. Just need a read lock, which can be done in sync with everyone else
-    if global_locks().contains(name) {
-        return;
-    };
-
-    // This is the rare path, which avoids the multi-writer situation mostly
-    let entry = global_locks().entry(name.to_owned());
-    match entry {
-        Entry::Occupied(o) => o,
-        Entry::Vacant(v) => v.insert_entry(UniqueReentrantMutex::new_mutex(name)),
-    };
+    global_locks().get_or_insert(name, || UniqueReentrantMutex::new_mutex(name));
 }
 
 #[cfg(test)]

serial_test/src/code_lock.rs

@@ -16,7 +16,6 @@ lock_api = { version="^0.4.7", default-features = false }
 wasm-bindgen-test = {version="^0.3.50", optional=true, default-features = false, features=["std"] }
 scoped-tls = { version="1", optional=true, default-features = false }
 log = { version = ">=0.4.4" , default-features = false }
-scc = { version = "2", default-features = false}
 
 [dev-dependencies]
 tokio = { version = "=1.38.2", features = ["macros", "rt", "rt-multi-thread"], default-features = false }

serial_test_test/Cargo.toml

@@ -38,31 +38,36 @@
 
 use log::info;
 use once_cell::sync::OnceCell;
-use scc::HashMap;
 #[cfg(test)]
 use serial_test::{parallel, serial};
 use std::{
+    collections::HashMap,
     convert::TryInto,
     env, fs,
     path::PathBuf,
-    sync::atomic::{AtomicUsize, Ordering},
+    sync::{
+        atomic::{AtomicUsize, Ordering},
+        Arc, Mutex,
+    },
     thread,
     time::Duration,
 };
 
-static LOCKS: OnceCell<HashMap<String, AtomicUsize>> = OnceCell::new();
+static LOCKS: OnceCell<Mutex<HashMap<String, Arc<AtomicUsize>>>> = OnceCell::new();
 
 fn init() {
     let _ = env_logger::builder().is_test(false).try_init();
 }
 
 pub fn test_fn(key: &str, count: usize) {
     init();
-    let local_locks = LOCKS.get_or_init(HashMap::new);
-    let entry = local_locks
-        .entry(key.to_string())
-        .or_insert(AtomicUsize::new(0));
-    let local_lock = entry.get();
+    let local_locks = LOCKS.get_or_init(|| Mutex::new(HashMap::new()));
+    let local_lock = {
+        let mut map = local_locks.lock().unwrap();
+        map.entry(key.to_string())
+            .or_insert_with(|| Arc::new(AtomicUsize::new(0)))
+            .clone()
+    };
     info!("(non-fs) Start {}", count);
     local_lock.store(count, Ordering::Relaxed);
     thread::sleep(Duration::from_millis(1000 * (count as u64)));