From dc2380b070fab3d80a16d00d0ea2f19e6cafa876 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Sat, 14 Nov 2026 20:08:47 -0500 Subject: [PATCH] fix(ci): stabilize macOS dialog and EXR cache tests (#3704) * fix(ci): stabilize macOS dialog and EXR cache tests * style: format file dialog test helpers --- apps/effectcraft/src/file_dialogs.rs | 20 +++++++++++++++----- crates/media/src/exr_channels.rs | 24 +++++++++++++++++++++++- 2 files changed, 38 insertions(+), 6 deletions(-) diff --git a/apps/effectcraft/src/file_dialogs.rs b/apps/effectcraft/src/file_dialogs.rs index 6a209cb9d..9f3114367 100644 --- a/apps/effectcraft/src/file_dialogs.rs +++ b/apps/effectcraft/src/file_dialogs.rs @@ -88,7 +88,11 @@ struct Errors { /// Run blocking dialog `what`; one that can't open sends why to `errors` and counts as /// cancelled (the blocking hooks can't return an error). fn blocking(errors: &Errors, what: &str, open: impl FnOnce() -> Option) -> Option { - guarded(what, open).unwrap_or_else(|e| { + blocking_on(errors, what, on_main_thread(), open) +} + +fn blocking_on(errors: &Errors, what: &str, main_thread: bool, open: impl FnOnce() -> Option) -> Option { + guarded_on(main_thread, what, open).unwrap_or_else(|e| { let _ = errors.tx.send(e); errors.ctx.request_repaint(); None @@ -175,8 +179,12 @@ fn title(dialog: &FileDialog) -> &'static str { /// Open `dialog` with `open` through [`guarded`] and wait for its answer on a worker thread. /// `done` always gets an answer: the paths, or why the dialog couldn't open or answer. fn show(dialog: FileDialog, done: DialogDone, ctx: egui::Context, open: impl FnOnce(FileDialog) -> Answer) { + show_on(dialog, done, ctx, on_main_thread(), open); +} + +fn show_on(dialog: FileDialog, done: DialogDone, ctx: egui::Context, main_thread: bool, open: impl FnOnce(FileDialog) -> Answer) { let what = title(&dialog); - let answer = match guarded(what, || open(dialog)) { + let answer = match guarded_on(main_thread, what, || open(dialog)) { Ok(answer) => answer, Err(e) => { done(Err(e)); @@ -282,7 +290,7 @@ mod tests { fn answered(dialog: FileDialog, open: impl FnOnce(FileDialog) -> Answer) -> Result, String> { let (tx, rx) = channel(); - show(dialog, Box::new(move |r| tx.send(r).unwrap()), egui::Context::default(), open); + show_on(dialog, Box::new(move |r| tx.send(r).unwrap()), egui::Context::default(), true, open); rx.recv_timeout(Duration::from_secs(10)).expect("the dialog answers") } @@ -328,10 +336,12 @@ mod tests { fn a_blocking_picker_that_cannot_open_reports_why() { let (tx, rx) = channel(); let tx = Errors { tx, ctx: egui::Context::default() }; - assert_eq!(blocking(&tx, "Open", || -> Option { panic!("{NIL_PANEL}") }), None); + assert_eq!(blocking_on(&tx, "Open", true, || -> Option { panic!("{NIL_PANEL}") }), None); assert!(rx.try_recv().is_ok_and(|e| e.contains(NIL_PANEL))); - assert_eq!(blocking(&tx, "Open", || Some(3)), Some(3)); + assert_eq!(blocking_on(&tx, "Open", true, || Some(3)), Some(3)); assert!(rx.try_recv().is_err(), "nothing to report"); + assert_eq!(blocking_on(&tx, "Open", false, || Some(5)), None); + assert!(rx.try_recv().is_ok_and(|e| e.contains("off the main thread"))); } /// #3084: every rfd dialog in the desktop app goes through this module (and so through diff --git a/crates/media/src/exr_channels.rs b/crates/media/src/exr_channels.rs index 96b0173cd..00ab1fc37 100644 --- a/crates/media/src/exr_channels.rs +++ b/crates/media/src/exr_channels.rs @@ -539,8 +539,15 @@ struct Entry { impl Entry { /// Dropping it would free nothing or lose a decode in progress: an effect (or a picture - /// waiting for its effects) still holds its planes, or it is decoding. + /// waiting for its effects) still holds its planes, someone is using the slot, or it is decoding. fn in_use(&self) -> bool { + // `get_or_init` hands an active caller a borrow of the OnceLock value before that caller + // can clone the decoded Arc. Keep the entry while any caller still holds this slot, or a + // concurrent account/eviction can remove it in that gap and make the next request decode + // the same file again. + if Arc::strong_count(&self.slot) > 1 { + return true; + } match self.slot.get() { None => true, Some((None, _)) => false, @@ -967,6 +974,21 @@ mod tests { assert_eq!(reads.load(Ordering::Relaxed), frames as u64); } + #[test] + fn cache_does_not_evict_a_slot_while_a_caller_is_using_it() { + let bytes: Arc<[u8]> = flat(&["R", "G", "B", "A"], (32, 32), 0.25).into(); + let one = 4 * 32 * 32 * 4 + 4; + let cache = ChannelCache::new(one); + drop(cache.get("/active.exr", Use::Channels, || Some(bytes.clone()))); + let active_slot = cache.lock().entries.iter().find(|entry| entry.path == "/active.exr").unwrap().slot.clone(); + drop(cache.get("/next.exr", Use::Channels, || Some(bytes.clone()))); + assert!(cache.cached().contains(&"/active.exr".to_string()), "an active caller pins the slot"); + + drop(active_slot); + drop(cache.get("/newest.exr", Use::Channels, || Some(bytes.clone()))); + assert!(!cache.cached().contains(&"/active.exr".to_string()), "an idle slot is eligible for eviction"); + } + /// #474: a decode that panics doesn't leave its file "decoding" in the cache (never dropped, /// its planes counted): the entry goes, and the next request decodes the file. #[test]