The deadlock I fixed three times

My bot has duels: two users' pigs fight, and both sides' state changes at once. That is the only place in it where two requests touch the same rows, and predictably it is the only place that ever hung.

Looking back through the history, I fixed it four times over two months. The first three did not work. The commit messages are an honest record of how little I understood:

Try avoid deadlocks
Possible avoid deadlocks
Try to fix rare deadlocks
Fixed duel deadlocks

The shape of the bug

Each duel took a per-user lock so that two duels involving the same person could not interleave. The locks lived in a concurrent map:

static DUEL_LOCKS: Lazy<DashMap<u64, Mutex<Vec<u64>>>>

That looks reasonable. DashMap gives you a concurrent hash map without wrapping the whole thing in one lock; the inner Mutex protects the individual entry. Two different users hit two different entries and never contend.

The problem is what DashMap::get returns. It hands you a guard, and that guard holds the lock on the map shard for as long as it lives. So this sequence:

let entry = DUEL_LOCKS.get(&key);      // shard lock acquired here
let mut list = entry.lock().await;      // ...and still held across this await

holds a shard lock across an await point. If another task needs any key that hashes to the same shard while the first task is suspended, it waits — and if the task it is waiting on is waiting for it, that is the hang.

It was rare because it needs two duels whose user IDs land in the same shard, overlapping in time. On a bot with a few hundred people that is a once-a-week event. Rare enough that each of my fixes looked like it had worked.

The three fixes that were not fixes

is_locked() plus a log line. If the entry looks busy, log an error and bail out. This is checking a lock's state and then acting on it, which is a race by construction — the answer can change before you use it. What it did give me was evidence the problem was real, which is why it survived a version.

A one-line reordering. I no longer remember what I believed. It changed nothing.

try_get instead of get. Take the entry only if the shard is uncontended, otherwise skip. This did reduce the frequency, which was the worst possible outcome: it made a reproducible-once-a-week bug into a reproducible-once-a-month bug, and moved it further from the change that caused it.

Each of those treats the symptom — "sometimes we are stuck on this lock" — rather than the cause, which is that the lock was being held at all.

The fix

static DUEL_LOCKS: Lazy<RwLock<HashMap<u64, Arc<Mutex<Vec<u64>>>>>>

Take a read lock on the map, clone the Arc out, drop the map guard, then await on the inner mutex. The map is only locked while you look something up, never while you wait for anything. It is more typing and it is not clever, and it has not hung since.

Why the compiler did not save me

This is the part worth internalising. Rust will stop you sharing state unsafely across threads, and it will refuse to send a future holding a non-Send guard. It will not stop you holding a perfectly Send guard across an await and deadlocking yourself. The type system's question is "is this safe to move between threads", not "should this still be locked while you wait for the network".

So the rule I run on now: any guard that is alive across an await is a bug until proven otherwise. Look up, clone out what you need, drop the guard, then await.

The postscript

Three years later I was building an unrelated thing — a WASM plugin host — and one of the first commits replaces dashmap with a plain RwLock<HashMap<_, _>>. Not because dashmap is bad; it is good at the thing it is for. Because the shape of that bug is now something I recognise from a distance, and in a host process where instances come and go per request, I did not want to spend another two months rediscovering it.