fix: cap the log file by size, and keep LogThrottle usable after poisoning

Rotating on age alone let a single day's file grow without limit, so whoever
can drive a hot log site decided how much disk this uses and no amount of
per-site throttling could bound it. Add a size criterion, which covers every
call site at once — including ones no throttle was added to.

LogThrottle: recover the guard on a poisoned lock rather than returning None.
Poisoning only means another thread panicked while holding it; the guarded
data is two counters that are still usable, and going silent for the rest of
the process is worse than a stale count. AGENTS.md permits handling lock
poisoning directly, and it forbids swallowing the error.

Keep map_or over clippy's is_none_or: that was stabilized in Rust 1.82 and CI
pins 1.75.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ExUfAkYbq8UC9pQCiLy8TQ
This commit is contained in:
rustdesk
2026-08-06 14:21:24 +08:00
parent 9ec46d5cb3
commit b7f79c6954
2 changed files with 30 additions and 6 deletions
+5 -1
View File
@@ -458,7 +458,11 @@ pub fn init_log(_is_async: bool, _name: &str) -> Option<flexi_logger::LoggerHand
})
.format(opt_format)
.rotate(
Criterion::Age(Age::Day),
// Size as well as age: rotating only daily lets one day's file grow
// without limit, so whoever can drive a hot log site — a peer sending
// malformed packets, a socket erroring in a retry loop — decides how
// much disk this uses. Bounding it here covers every call site at once.
Criterion::AgeOrSize(Age::Day, 16 * 1024 * 1024),
Naming::Timestamps,
Cleanup::KeepLogFiles(31),
)
+25 -5
View File
@@ -45,12 +45,17 @@ impl LogThrottle {
/// The first occurrence after a quiet period always reports, so an isolated fault is not
/// delayed by the interval.
pub fn due(&self) -> Option<u64> {
let Ok(mut state) = self.state.lock() else {
// A poisoned mutex means another thread panicked mid-update; the count is not worth
// propagating that, and staying silent is better than logging per call.
return None;
};
// A poisoned lock only means some other thread panicked while holding it; the guarded
// data is two plain counters that are still usable, and going silent for the rest of
// the process would be worse than a stale count.
let mut state = self
.state
.lock()
.unwrap_or_else(|poisoned| poisoned.into_inner());
state.suppressed += 1;
// `map_or(true, ..)` rather than clippy's preferred `is_none_or`: that was stabilized in
// Rust 1.82 and this crate builds on the 1.75 pinned by CI.
#[allow(clippy::unnecessary_map_or)]
let due = state
.last
.map_or(true, |last| last.elapsed() >= self.interval);
@@ -66,6 +71,21 @@ impl LogThrottle {
mod tests {
use super::*;
// Two directions of one socket need two throttles: an ICMP error on a connected socket is
// reported once and cleared, so the steady state alternates (send succeeds, the next recv
// reports it) and anything shared between them is reset by the succeeding side every cycle.
#[test]
fn separate_throttles_do_not_reset_each_other() {
let send = LogThrottle::new(Duration::from_secs(60));
let recv = LogThrottle::new(Duration::from_secs(60));
assert_eq!(recv.due(), Some(1));
for _ in 0..1_000 {
// The send side succeeding must not hand the recv side a fresh emit slot.
assert_eq!(recv.due(), None);
}
assert_eq!(send.due(), Some(1), "the other direction keeps its own slot");
}
#[test]
fn first_call_reports_immediately() {
let t = LogThrottle::new(Duration::from_secs(60));