[triage:service-04-lock-poisoning-magika-dead-acquire] std::sync::Mutex poisoning on shared caches can wedge routes; plus a pointless magika lock/drop
- Dominant language
- Rust
- Stars
- 72
- Forks
- 13
- Avg merge
- 1d 2h
- Merged PRs (30d)
- 5
Description
Imported from Capsem triage report `service-04-lock-poisoning-magika-dead-acquire.md`.
- Severity: `low`
- Category: `bug`
- Area: `capsem-service`
- Location: `crates/capsem-service/src/main.rs:2737-2739` (dead lock/drop), and the ~96 `*.lock().unwrap()` sites on `ServiceState` mutexes (e.g. `:781`, `:2027`, `:2035`)
- Confidence: `verified`
## Summary
Every shared cache and the `magika`, `instances`, `persistent_registry` maps are `std::sync::Mutex`
accessed with `.lock().unwrap()`. If any thread panics while holding one of these guards, the mutex
is poisoned and every later `.lock().unwrap()` on it panics — permanently wedging the affected
routes for the life of the daemon (axum converts the per-request panic to a 500 but never re-arms the
mutex). `magika::Session::identify_file_sync` runs under such a guard inside `spawn_blocking` (a
likely panic site for malformed input), making the magika mutex the most exposed.
Separately, `handle_list_files` acquires the magika mutex and immediately drops it before doing any
work — a dead acquisition that does nothing but contend the lock.
## Evidence
Dead acquire/drop:
```rust
// main.rs:2737-2739
let magika = state.magika.lock().unwrap();
// We can't send MutexGuard across threads; re-acquire inside spawn_blocking
drop(magika);
```
The guard is taken and dropped with no use; the real work re-locks inside `spawn_blocking` via
`&state_clone.magika`. The two lines are pure overhead (and a no-op comment).
Poisoning exposure: `identify_file_sync` (fs_utils.rs:68) does `magika.lock().unwrap()` then calls
into the `magika` crate on arbitrary uploaded/listed file bytes. A panic there poisons the mutex; all
subsequent `handle_list_files` / `handle_download_file` / `handle_upload_file` calls then panic at
`state.magika.lock().unwrap()`.
## Impact
A single panic under any of these locks turns a transient failure into a permanent per-route outage
until the daemon is restarted. Low severity because it requires a panic while the guard is held and
does not crash the whole process, but it is a latent availability cliff on the file routes.
## Suggested fix
- Delete the dead `magika.lock()/drop` pair at main.rs:2737-2739.
- Recover from poisoning instead of `.unwrap()` on the cache/magika mutexes (e.g.
`.lock().unwrap_or_else(|e| e.into_inner())`), or ensure the work under each guard cannot panic.
For `identify_file_sync`, catching the panic (`catch_unwind`) inside the `spawn_blocking` closure
before it can poison the shared `magika` session is the durable fix.
## Triage
Confirmed from the local reviewed report in `/Users/elie/git/capsem/tmp/bugs/service-04-lock-poisoning-magika-dead-acquire.md`. Track implementation in the triage sprint; add regression coverage before fixing.
Contributor guide
Research direction
Start in crates/capsem-service/src/main.rs:2737-2739 and fs_utils.rs:68, then review the cited ServiceState .lock().unwrap() sites and existing route handlers. Reproduce or add regression coverage for mutex poisoning and the dead magika acquisition. Done means the pointless lock/drop is removed, poisoned locks have a defined recovery or panic-safe path, and file routes remain usable after a guarded operation panics.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- backend
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100