Merge pull request #1371 from ZIDAN44/fix/901-clap-main-thread

fix(clap-host): check for the main thread off macOS too (#901)

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.

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<dyn Plugin>) -> Vec<Vec<f32>> {
     io
 }
 
+/// The warnings logged, to tell which way the host went.
+static WARNINGS: Mutex<Vec<String>> = 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");
 }

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<std::thread::ThreadId>,
     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,

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();
 }