diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 5c4a751..3dac122 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -270,6 +270,10 @@ pub struct SoundApp { last_meter_poll: f64, /// Movies for the Video window and Video track (decoded in the background). pub video: video_track::VideoPool, + /// Test-only: owns the process-wide palette lock while this app exists, so frames painted + /// and colour assertions in one test can never straddle another test's palette change. + #[cfg(test)] + _palette_lock: theme::TestPaletteGuard, } /// Whether another automation-write pass is due: the transport moved at least @@ -326,6 +330,8 @@ impl SoundApp { last_frame: None, last_meter_poll: f64::NEG_INFINITY, video: video_track::VideoPool::default(), + #[cfg(test)] + _palette_lock: theme::lock_test_palette(), } } diff --git a/crates/ui-egui/src/theme.rs b/crates/ui-egui/src/theme.rs index c667e6f..bb23cda 100644 --- a/crates/ui-egui/src/theme.rs +++ b/crates/ui-egui/src/theme.rs @@ -6,11 +6,64 @@ use std::sync::atomic::{AtomicBool, Ordering}; /// Whether the Light palette is active. Dark is the default (the incumbent is dark-only). static LIGHT_THEME: AtomicBool = AtomicBool::new(false); -/// Unit tests that change the process-wide palette must own it until their assertions finish. +/// Unit tests that change or read the process-wide palette must own it until their assertions +/// finish. The lock is re-entrant per thread, so a test may hold it and still build apps +/// (`SoundApp::new` takes it too) without deadlocking itself. #[cfg(test)] -pub(crate) fn lock_test_palette() -> std::sync::MutexGuard<'static, ()> { - static PALETTE: std::sync::Mutex<()> = std::sync::Mutex::new(()); - PALETTE.lock().unwrap_or_else(std::sync::PoisonError::into_inner) +pub(crate) fn lock_test_palette() -> TestPaletteGuard { + TestPaletteGuard::acquire() +} + +/// RAII handle for the re-entrant, per-thread test palette lock; see [`lock_test_palette`]. +#[cfg(test)] +pub(crate) struct TestPaletteGuard; + +#[cfg(test)] +impl TestPaletteGuard { + fn acquire() -> Self { + palette_lock::acquire(); + Self + } +} + +#[cfg(test)] +impl Drop for TestPaletteGuard { + fn drop(&mut self) { + palette_lock::release(); + } +} + +#[cfg(test)] +mod palette_lock { + use std::cell::{Cell, RefCell}; + use std::sync::{Mutex, MutexGuard, PoisonError}; + + static PALETTE: Mutex<()> = Mutex::new(()); + + thread_local! { + /// How many guards this thread holds; the OS lock is taken on 0→1 and released on 1→0. + static DEPTH: Cell = const { Cell::new(0) }; + static GUARD: RefCell>> = const { RefCell::new(None) }; + } + + pub(super) fn acquire() { + DEPTH.with(|depth| { + if depth.get() == 0 { + GUARD.with(|guard| *guard.borrow_mut() = Some(PALETTE.lock().unwrap_or_else(PoisonError::into_inner))); + } + depth.set(depth.get() + 1); + }); + } + + pub(super) fn release() { + DEPTH.with(|depth| { + let left = depth.get().saturating_sub(1); + depth.set(left); + if left == 0 { + GUARD.with(|guard| *guard.borrow_mut() = None); + } + }); + } } /// The Appearance setting (`ui.theme {mode}`). Dark by default; System and Light are opt-in. @@ -415,6 +468,26 @@ mod tests { use crate::{Services, SoundApp, UiState}; use serde_json::json; + #[test] + fn test_palette_lock_is_reentrant_and_excludes_other_threads() { + // Regression for the parallel-run flakes: the process-wide palette must be owned for a + // whole test, re-entrant on the owning thread (so `SoundApp::new` can take it too), and + // exclusive against other threads (so a frame cannot flip it mid-test). + let outer = lock_test_palette(); + let inner = lock_test_palette(); + let app = SoundApp::new(soundcraft_engine::Engine::default(), None, Services::default()); + let (tx, rx) = std::sync::mpsc::channel(); + std::thread::spawn(move || { + let _held = lock_test_palette(); + let _ = tx.send(()); + }); + assert!(rx.recv_timeout(std::time::Duration::from_millis(100)).is_err(), "another thread must wait for the palette lock"); + drop(inner); + drop(outer); + drop(app); + assert!(rx.recv_timeout(std::time::Duration::from_secs(5)).is_ok(), "the palette lock must be released when every holder drops it"); + } + #[test] fn dark_is_the_default() { assert_eq!(ThemeMode::default(), ThemeMode::Dark);