From 8333615072318b02cd846259a794cf28558b9e86 Mon Sep 17 00:00:00 2001 From: Vanadiae Date: Tue, 5 Aug 2025 14:41:32 +0200 Subject: [PATCH] Fix memory leak when `forget_image` is called while loading (#7380) This is the same issue that was fixed for the http bytes loader in 239ade9a59692f23a6b1d779a9c2631cf774896a * [x] I have followed the instructions in the PR template ---------------- Funnily, [this comment](https://github.com/emilk/egui/issues/3747#issuecomment-1872192997) describes exactly how I encountered this issue: > That assert is wrong if something calls forget between the start of the request and the end of it. I'm displaying lots of images in a scrolling grid (20 or so visible at a time). It seems like image textures are never freed up automatically so it stacks up a lot meaning I have to unload the image textures manually with `egui::Context::forget_image()` in each `eframe::App::update()` call for the images that are no longer shown when scrolling. --------- Co-authored-by: Emil Ernerfeldt --- crates/egui_extras/src/loaders/ehttp_loader.rs | 16 +++++++++++++--- crates/egui_extras/src/loaders/image_loader.rs | 16 +++++++++------- 2 files changed, 22 insertions(+), 10 deletions(-) diff --git a/crates/egui_extras/src/loaders/ehttp_loader.rs b/crates/egui_extras/src/loaders/ehttp_loader.rs index abe5d96f..7310cc6e 100644 --- a/crates/egui_extras/src/loaders/ehttp_loader.rs +++ b/crates/egui_extras/src/loaders/ehttp_loader.rs @@ -94,9 +94,19 @@ impl BytesLoader for EhttpLoader { Err(format!("Failed to load {uri:?}")) } }; - log::trace!("finished loading {uri:?}"); - cache.lock().insert(uri, Poll::Ready(result)); - ctx.request_repaint(); + let mut cache = cache.lock(); + if let std::collections::hash_map::Entry::Occupied(mut entry) = + cache.entry(uri.clone()) + { + let entry = entry.get_mut(); + *entry = Poll::Ready(result); + ctx.request_repaint(); + log::trace!("Finished loading {uri:?}"); + } else { + log::trace!( + "Canceled loading {uri:?}\nNote: This can happen if `forget_image` is called while the image is still loading." + ); + } } }); diff --git a/crates/egui_extras/src/loaders/image_loader.rs b/crates/egui_extras/src/loaders/image_loader.rs index 18e1e483..82df87b3 100644 --- a/crates/egui_extras/src/loaders/image_loader.rs +++ b/crates/egui_extras/src/loaders/image_loader.rs @@ -100,14 +100,16 @@ impl ImageLoader for ImageCrateLoader { let result = crate::image::load_image_bytes(&bytes) .map(Arc::new) .map_err(|err| err.to_string()); - log::trace!("ImageLoader - finished loading {uri:?}"); - let prev = cache.lock().insert(uri, Poll::Ready(result)); - debug_assert!( - matches!(prev, Some(Poll::Pending)), - "Expected previous state to be Pending" - ); + let mut cache = cache.lock(); - ctx.request_repaint(); + if let std::collections::hash_map::Entry::Occupied(mut entry) = cache.entry(uri.clone()) { + let entry = entry.get_mut(); + *entry = Poll::Ready(result); + ctx.request_repaint(); + log::trace!("ImageLoader - finished loading {uri:?}"); + } else { + log::trace!("ImageLoader - canceled loading {uri:?}\nNote: This can happen if `forget_image` is called while the image is still loading."); + } } }) .expect("failed to spawn thread");