diff --git a/crates/clap-host/src/ffi/gui.rs b/crates/clap-host/src/ffi/gui.rs index 92619742..1a4f3b27 100644 --- a/crates/clap-host/src/ffi/gui.rs +++ b/crates/clap-host/src/ffi/gui.rs @@ -17,9 +17,11 @@ pub(crate) fn on_main_thread() -> bool { unsafe { pthread_main_np() == 1 } } +/// True on the process's main thread: Rust names it "main", and the app runs its UI there (the +/// rule `hostwin` and the VST3 host use). CLAP makes `clap.gui` main-thread only on every platform. #[cfg(not(target_os = "macos"))] pub(crate) fn on_main_thread() -> bool { - true + std::thread::current().name() == Some("main") } /// The windowing API of this platform. diff --git a/crates/clap-host/tests/editor_main_thread.rs b/crates/clap-host/tests/editor_main_thread.rs index e46ca4ab..8ddf3d8c 100644 --- a/crates/clap-host/tests/editor_main_thread.rs +++ b/crates/clap-host/tests/editor_main_thread.rs @@ -5,6 +5,7 @@ mod common; use soundcraft_dsp::Plugin; +use std::sync::Mutex; const GAIN: &str = "clap:org.soundcraft.test.gain"; @@ -14,12 +15,31 @@ fn block(p: &mut Box) -> Vec> { io } +/// The warnings logged, to tell which way the host went. +static WARNINGS: Mutex> = Mutex::new(Vec::new()); + +struct Warnings; + +impl log::Log for Warnings { + fn enabled(&self, m: &log::Metadata) -> bool { + m.level() <= log::Level::Warn + } + fn log(&self, r: &log::Record) { + if self.enabled(r.metadata()) { + WARNINGS.lock().unwrap().push(r.args().to_string()); + } + } + fn flush(&self) {} +} + fn main() { // nextest lists custom-harness tests with `--list`; each binary here is one test. if std::env::args().any(|a| a == "--list") { println!("editor_main_thread: test"); return; } + log::set_logger(&Warnings).expect("logger"); + log::set_max_level(log::LevelFilter::Warn); common::fixture_dir(); let mut p = soundcraft_clap_host::create(GAIN).expect("fixture"); p.prepare(48_000.0, 64, 2); @@ -32,6 +52,26 @@ fn main() { // block and is reported (in SoundCraft units) by idle. let io = block(&mut p); assert!((io[0][10] - 0.25).abs() < 1e-6, "{}", io[0][10]); + // Only the main thread drives the editor, on every platform (#901). Another thread, named or + // not, is refused: it neither takes the edit idle reports nor closes or reopens the editor. + for name in [None, Some("soundcraft-ui")] { + let mut other = p.editor().expect("editor handle"); + let thread = match name { + Some(n) => std::thread::Builder::new().name(n.to_string()), + None => std::thread::Builder::new(), + }; + let seen = thread + .spawn(move || { + let edits = other.idle(); + other.close(); + (edits, other.open().map_err(|e| e.contains("main thread"))) + }) + .expect("spawn") + .join() + .expect("no panic"); + assert_eq!(seen, (Vec::new(), Err(true)), "{name:?}"); + assert!(ed.is_open(), "{name:?}"); + } assert_eq!(ed.idle(), vec![("7".to_string(), 0.25)]); assert!(ed.idle().is_empty(), "drained"); ed.close(); @@ -49,5 +89,17 @@ fn main() { assert!(ed.open().is_err()); assert!(ed.idle().is_empty()); ed.close(); + // #901: a plugin dropped on another thread with its editor open (an audio callback dropped + // by the device's thread) does not destroy the editor there, clap.gui being main-thread only: + // it warns and leaves it. + let mut p = soundcraft_clap_host::create(GAIN).expect("fixture"); + p.prepare(48_000.0, 64, 2); + let mut ed = p.editor().expect("editor handle"); + ed.open().expect("open on the main thread"); + std::thread::Builder::new().name("audio".into()).spawn(move || drop(p)).expect("spawn").join().expect("no panic"); + let left = "clap: editor still open while its plugin is destroyed off the main thread"; + assert!(WARNINGS.lock().unwrap().iter().any(|w| w == left), "{:?}", WARNINGS.lock().unwrap()); + assert!(!ed.is_open()); + assert!(ed.open().is_err()); println!("editor_main_thread: ok"); } diff --git a/crates/clap-host/tests/fixture/src/lib.rs b/crates/clap-host/tests/fixture/src/lib.rs index b31028cc..2422c4f9 100644 --- a/crates/clap-host/tests/fixture/src/lib.rs +++ b/crates/clap-host/tests/fixture/src/lib.rs @@ -102,6 +102,8 @@ struct Fx { active: bool, processing: bool, gui_created: bool, + /// The thread that created the GUI (the host's main thread). + gui_thread: Option, gui_visible: bool, gui_edit_pending: bool, /// `clap.render` set the offline mode (wide fixtures only). @@ -130,7 +132,10 @@ unsafe extern "C" fn init(p: *const clap_plugin) -> bool { } unsafe extern "C" fn destroy(p: *const clap_plugin) { - assert!(!unsafe { fx(p) }.gui_created, "plugin destroyed with its GUI alive"); + let f = unsafe { fx(p) }; + // A host destroying the plugin off the GUI's thread can't destroy the GUI there (clap.gui is + // main-thread only), so it may leave it; on the GUI's thread it must destroy it first. + assert!(!f.gui_created || f.gui_thread != Some(std::thread::current().id()), "plugin destroyed with its GUI alive"); let data = unsafe { (*p).plugin_data } as *mut Fx; drop(unsafe { Box::from_raw(data) }); } @@ -492,6 +497,7 @@ unsafe extern "C" fn gui_create(p: *const clap_plugin, _api: *const c_char, floa let f = unsafe { fx(p) }; assert!(floating && !f.gui_created, "create"); f.gui_created = true; + f.gui_thread = Some(std::thread::current().id()); true } @@ -638,6 +644,7 @@ unsafe extern "C" fn factory_create(_f: *const clap_plugin_factory, host: *const active: false, processing: false, gui_created: false, + gui_thread: None, gui_visible: false, gui_edit_pending: false, offline: false, diff --git a/crates/clap-host/tests/it/fixture_e2e.rs b/crates/clap-host/tests/it/fixture_e2e.rs index 1c2fcfea..309aa79a 100644 --- a/crates/clap-host/tests/it/fixture_e2e.rs +++ b/crates/clap-host/tests/it/fixture_e2e.rs @@ -215,14 +215,12 @@ fn state_reader_reads_while_another_thread_processes() { #[test] fn editors_refuse_to_open_off_the_main_thread() { fixture_dir(); - // Test threads are never the process main thread. + // Test threads are never the process main thread, on any platform (#901). let mut p = soundcraft_clap_host::create(GAIN).unwrap(); let mut ed = p.editor().expect("the gain fixture has clap.gui"); - if cfg!(target_os = "macos") { - assert!(ed.open().unwrap_err().contains("main thread")); - assert!(p.open_editor().is_err()); - assert!(!ed.is_open()); - } + assert!(ed.open().unwrap_err().contains("main thread")); + assert!(p.open_editor().is_err()); + assert!(!ed.is_open()); ed.close(); p.close_editor(); }