From e6cbf761482bc5608e83fe253a06e0eceeb37b4e Mon Sep 17 00:00:00 2001 From: Zinadin Zidan Date: Thu, 5 Nov 2026 11:45:36 +0600 Subject: [PATCH] fix(clap-host): check for the main thread off macOS too (#901) On Windows and Linux on_main_thread() always said yes, so any thread could open, idle or close a CLAP editor, and an instance dropped on another thread destroyed its editor there. CLAP allows clap.gui calls on the main thread only. It now uses the rule the CLAP host's Windows window code and the VST3 host's module loading already use: the thread Rust names "main", where the app runs its UI. The editor tests check that another thread, named or not, is refused on every platform, and that a plugin dropped on another thread with its editor open warns and leaves the editor instead of destroying it there (the test plugin now allows that, and only that, at destroy). Co-Authored-By: Claude Opus 5.5 --- crates/clap-host/src/ffi/gui.rs | 4 +- crates/clap-host/tests/editor_main_thread.rs | 52 ++++++++++++++++++++ crates/clap-host/tests/fixture/src/lib.rs | 9 +++- crates/clap-host/tests/it/fixture_e2e.rs | 10 ++-- 4 files changed, 67 insertions(+), 8 deletions(-) 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(); }