From b8e7e8ede681db7abfd90d4f9ba7f93d42394744 Mon Sep 17 00:00:00 2001 From: MarekSpodBiedry <73122968+MarekSpodBiedry@users.noreply.github.com> Date: Tue, 13 Oct 2026 18:19:22 +0200 Subject: [PATCH] fix(render): cap the failed-image cache at 64 entries (#728) --- crates/render/src/images.rs | 42 ++++++++++++++++++++++++++++++++++--- 1 file changed, 39 insertions(+), 3 deletions(-) diff --git a/crates/render/src/images.rs b/crates/render/src/images.rs index 117a0fa11..465867e82 100644 --- a/crates/render/src/images.rs +++ b/crates/render/src/images.rs @@ -3,7 +3,7 @@ //! //! Entries are keyed by the address of the asset's encoded bytes (`Arc>`) and hold a //! clone of that `Arc`, so the address can't be reused while cached. Least recently used levels -//! are evicted beyond a memory budget. +//! are evicted beyond a memory budget; at most `MAX_BAD` failed assets are remembered. use std::collections::HashMap; use std::sync::{Arc, Mutex}; @@ -15,6 +15,9 @@ use crate::{decode_pixmap_page, halve}; /// Decoded pixels kept (bytes). const BUDGET: usize = 768 << 20; +/// Failed assets remembered; the oldest is forgotten (and retried if drawn again) beyond this. +const MAX_BAD: usize = 64; + struct Entry { _bytes: Arc>, pm: Arc, @@ -28,7 +31,20 @@ struct Cache { bytes: usize, clock: u64, /// Assets that failed to decode (don't retry every frame). - bad: HashMap<(usize, u32), Arc>>, + /// Value: (insertion stamp, the encoded bytes keeping the address reserved). + bad: HashMap<(usize, u32), (u64, Arc>)>, +} + +impl Cache { + /// Remember a failed asset, evicting the oldest failures beyond [`MAX_BAD`]. + fn mark_bad(&mut self, key: (usize, u32), bytes: &Arc>) { + self.clock += 1; + self.bad.insert(key, (self.clock, bytes.clone())); + while self.bad.len() > MAX_BAD { + let Some(oldest) = self.bad.iter().min_by_key(|(_, (stamp, _))| *stamp).map(|(k, _)| *k) else { break }; + self.bad.remove(&oldest); + } + } } static CACHE: Mutex> = Mutex::new(None); @@ -97,7 +113,7 @@ pub fn decoded(bytes: &Arc>, page: u32) -> Option> { Some(pm) } None => { - with(|c| c.bad.insert((id, page), bytes.clone())); + with(|c| c.mark_bad((id, page), bytes)); None } } @@ -133,3 +149,23 @@ pub fn clear() { c.bytes = 0; }); } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn failed_assets_are_capped_and_oldest_is_dropped() { + let mut c = Cache::default(); + let buffers: Vec>> = (0..MAX_BAD + 10).map(|i| Arc::new(vec![i as u8; 4])).collect(); + for b in &buffers { + c.mark_bad((Arc::as_ptr(b) as usize, 0), b); + } + assert_eq!(c.bad.len(), MAX_BAD); + let first = &buffers[0]; + assert!(!c.bad.contains_key(&(Arc::as_ptr(first) as usize, 0))); + assert_eq!(Arc::strong_count(first), 1, "evicted buffer must be released"); + let last = &buffers[MAX_BAD + 9]; + assert!(c.bad.contains_key(&(Arc::as_ptr(last) as usize, 0))); + } +}