S04: Arc<Mutex<T>> where no sharing across threads exists

S04Severity: Low

Thread-safe wrappers in code that only one thread uses.

Clippy: no lint finds this. Look for it in review.

What it looks like

A page view counter, used by one thread:

pub struct PageViews {
    counts: Arc<Mutex<HashMap<String, u64>>>,
}

impl PageViews {
    pub fn new() -> Self {
        Self {
            counts: Arc::new(Mutex::new(HashMap::new())),
        }
    }

    pub fn record(&self, path: &str) {
        let mut counts = self.counts.lock().unwrap();
        *counts.entry(path.to_string()).or_insert(0) += 1;
    }

    pub fn get(&self, path: &str) -> u64 {
        self.counts.lock().unwrap().get(path).copied().unwrap_or(0)
    }
}

Why AI agents produce it

“Thread-safe to be safe.” In Java, a shared service is often synchronized or uses ConcurrentHashMap, because the framework may call it from many threads. The agent copies the habit.
It makes &self methods compile. record must change the map. With &self, that needs interior mutability. Arc<Mutex<_>> is the pattern the agent knows best, so it uses it instead of changing the signature to &mut self.
It prepares for threads that do not exist. “This might be used from several threads later.”

Why it is a problem

It misleads the reader. Arc<Mutex<_>> says “several threads share this”. A reviewer looks for the threads, and for deadlocks, and finds none.
Every access has an unwrap(). lock() returns a Result because of lock poisoning. In single-thread code, that error cannot happen in a useful way, but every reader must check each unwrap().
&self hides the change. record(&self) looks like a read. The signature no longer tells you which methods change the counter.
It costs a little at run time. An atomic reference count and a lock on every call. The cost is small, but it buys nothing.
It hides real design work. If threads are added later, the code may need a different design (per-thread counts, a channel, or a concurrent map). The Mutex makes it look as if that work is done.

The better version

WorseAn Arc<Mutex<HashMap>>, lock().unwrap() on every access, and record(&self).

pub struct PageViews {
    counts: Arc<Mutex<HashMap<String, u64>>>,
}

impl PageViews {
    pub fn new() -> Self {
        Self {
            counts: Arc::new(Mutex::new(HashMap::new())),
        }
    }

    pub fn record(&self, path: &str) {
        let mut counts = self.counts.lock().unwrap();
        *counts.entry(path.to_string()).or_insert(0) += 1;
    }

    pub fn get(&self, path: &str) -> u64 {
        self.counts.lock().unwrap().get(path).copied().unwrap_or(0)
    }
}

BetterA plain HashMap, and record(&mut self). Default is derived.

#[derive(Default)]
pub struct PageViews {
    counts: HashMap<String, u64>,
}

impl PageViews {
    pub fn record(&mut self, path: &str) {
        *self.counts.entry(path.to_string()).or_insert(0) += 1;
    }

    pub fn get(&self, path: &str) -> u64 {
        self.counts.get(path).copied().unwrap_or(0)
    }
}

The “better” version is shorter, has no unwrap(), and says in its signatures which method changes the data.

When it is acceptable

When several threads really change the same data. Here, worker threads update one map:

/// Real sharing: several worker threads update the same counts.
pub fn count_in_parallel(batches: Vec<Vec<String>>) -> HashMap<String, u64> {
    let counts = Arc::new(Mutex::new(HashMap::new()));
    let workers: Vec<_> = batches
        .into_iter()
        .map(|batch| {
            let counts = Arc::clone(&counts);
            thread::spawn(move || {
                for path in batch {
                    let mut counts = counts
                        .lock()
                        .expect("no worker panics while holding the lock");
                    *counts.entry(path).or_insert(0) += 1;
                }
            })
        })
        .collect();
    for worker in workers {
        worker.join().expect("worker panicked");
    }
    Arc::into_inner(counts)
        .expect("all workers are done")
        .into_inner()
        .expect("no worker panicked")
}

The test acceptable_shares_across_threads runs it. Even here, ask if each thread can count its own batch into its own map, and the maps be merged at the end. That needs no lock at all.

In async code, the shared state of a web server (for example, an axum application state) is often Arc<...>, because the runtime runs handlers on many threads. That is real sharing. See S19: std::sync::Mutex guard held across .await for the lock rules there.

How to detect it

No Clippy lint reports this pattern. (Clippy arc_with_non_send_sync reports a different problem: an Arc around a type that cannot be shared.)
Search for Arc<Mutex< and Arc<RwLock<. For each one, search for thread::spawn, rayon, tokio::spawn, or an async runtime that uses the value. If there is none, it is the smell.
Look for Arc::clone calls. If nothing clones the Arc, nothing shares it.

What to tell your agent

This code runs in one thread. Replace Arc<Mutex<T>> with T, change the
methods that modify the data from &self to &mut self, and remove the
lock().unwrap() calls. If a caller needs shared mutation in one thread,
use RefCell and say why in a comment. Run the tests.