mirror of
https://github.com/storytold/soundcraft.git
synced 2026-11-09 03:40:07 -06:00
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 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
1db9563379
commit
e6cbf76148
@@ -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.
|
||||
|
||||
@@ -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");
|
||||
}
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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();
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user