fix: recover the auth rate limiter from a poisoned mutex
try_acquire used .expect on lock(), so a single panic while holding either limiter mutex would poison it and make every subsequent auth request re-panic — one blip became a persistent auth outage. Recover the guard with unwrap_or_else(|e| e.into_inner()) so the limiter keeps serving. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -141,10 +141,10 @@ impl AuthRateLimiter {
|
||||
return self
|
||||
.global
|
||||
.lock()
|
||||
.expect("rate limiter mutex poisoned")
|
||||
.unwrap_or_else(|e| e.into_inner())
|
||||
.try_take(&self.cfg, now);
|
||||
};
|
||||
let mut map = self.per_ip.lock().expect("rate limiter mutex poisoned");
|
||||
let mut map = self.per_ip.lock().unwrap_or_else(|e| e.into_inner());
|
||||
if map.len() >= MAX_TRACKED_IPS && !map.contains_key(&ip) {
|
||||
map.retain(|_, b| !b.is_idle(&self.cfg, now));
|
||||
if map.len() >= MAX_TRACKED_IPS {
|
||||
@@ -155,7 +155,7 @@ impl AuthRateLimiter {
|
||||
return self
|
||||
.global
|
||||
.lock()
|
||||
.expect("rate limiter mutex poisoned")
|
||||
.unwrap_or_else(|e| e.into_inner())
|
||||
.try_take(&self.cfg, now);
|
||||
}
|
||||
}
|
||||
@@ -287,4 +287,35 @@ mod tests {
|
||||
}
|
||||
assert!(rl.per_ip.lock().unwrap().len() <= MAX_TRACKED_IPS);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn survives_a_poisoned_mutex() {
|
||||
// If any thread ever panics while holding a limiter mutex, the lock
|
||||
// becomes poisoned. With the old `.expect(...)` every later auth request
|
||||
// would re-panic — one blip turned into a permanent auth outage. Recover
|
||||
// the guard via `into_inner()` instead so the limiter keeps serving.
|
||||
let rl = AuthRateLimiter::new(RateLimitConfig {
|
||||
per_sec: 5,
|
||||
burst: 5,
|
||||
});
|
||||
|
||||
// Poison per_ip by panicking while holding its guard.
|
||||
let poisoned = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| {
|
||||
let _g = rl.per_ip.lock().unwrap();
|
||||
panic!("poison the per-IP mutex");
|
||||
}));
|
||||
assert!(poisoned.is_err(), "the panic must unwind");
|
||||
assert!(rl.per_ip.is_poisoned(), "the mutex must now be poisoned");
|
||||
|
||||
// The per-IP path (Some(ip)) must still work despite the poison.
|
||||
assert_eq!(rl.try_acquire(ip("198.51.100.9")), AcquireResult::Allowed);
|
||||
|
||||
// And poison the global bucket too — the None-key path must recover.
|
||||
let _ = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| {
|
||||
let _g = rl.global.lock().unwrap();
|
||||
panic!("poison the global mutex");
|
||||
}));
|
||||
assert!(rl.global.is_poisoned());
|
||||
assert_eq!(rl.try_acquire(None), AcquireResult::Allowed);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user