From b7f79c69548620ee55496d455466b6a9b9c49de9 Mon Sep 17 00:00:00 2001 From: rustdesk Date: Thu, 6 Aug 2026 14:21:24 +0800 Subject: [PATCH] fix: cap the log file by size, and keep LogThrottle usable after poisoning MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Claude-Session: https://claude.ai/code/session_01ExUfAkYbq8UC9pQCiLy8TQ --- src/lib.rs | 6 +++++- src/log_throttle.rs | 30 +++++++++++++++++++++++++----- 2 files changed, 30 insertions(+), 6 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index c33244dd8..15cc322a0 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -458,7 +458,11 @@ pub fn init_log(_is_async: bool, _name: &str) -> Option Option { - 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));