fix(render): cap the failed-image cache at 64 entries (#728)

crates/render/src/images.rs

@@ -3,7 +3,7 @@
 //!
 //! Entries are keyed by the address of the asset's encoded bytes (`Arc<Vec<u8>>`) 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<Vec<u8>>,
     pm: Arc<Pixmap>,
@@ -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<Vec<u8>>>,
+    /// Value: (insertion stamp, the encoded bytes keeping the address reserved).
+    bad: HashMap<(usize, u32), (u64, Arc<Vec<u8>>)>,
+}
+
+impl Cache {
+    /// Remember a failed asset, evicting the oldest failures beyond [`MAX_BAD`].
+    fn mark_bad(&mut self, key: (usize, u32), bytes: &Arc<Vec<u8>>) {
+        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<Option<Cache>> = Mutex::new(None);
@@ -97,7 +113,7 @@ pub fn decoded(bytes: &Arc<Vec<u8>>, page: u32) -> Option<Arc<Pixmap>> {
             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<Arc<Vec<u8>>> = (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)));
+    }
+}