From 55c8bb0ce0fe7c0eaa38634f489f56cc24c8bcc4 Mon Sep 17 00:00:00 2001 From: unohee Date: Wed, 7 Oct 2026 08:55:40 +0900 Subject: [PATCH 1/4] test(clap): pin that host GUI show/hide reaches the editor (AUD-2057) `ext_gui_show`/`ext_gui_hide` returned a hardcoded `false` on the assumption that they only applied to free-standing windows, so a macOS host calling `show()` to re-show an embedded editor was told the editor cannot be shown and left the custom GUI invisible. The fix delegates both to `Editor::set_visible`; this pins it. - `gui_show_and_hide_delegate_to_the_editor_and_relay_its_verdict` drives the real `ext_gui_show`/`ext_gui_hide` entry points through `ext_gui_set_parent` with a recording editor fixture and asserts the calls arrive in order (`[true, false, true]`). - `gui_show_reports_false_when_the_editor_cannot_change_visibility` pins the other half: an editor that refuses must produce `false`, so replacing the constant with a blind `true` fails too. - `gui_show_without_an_attached_editor_reports_failure` covers the missing handle path (no panic, no false success). Both first two fail against the previous constant-`false` implementation (verified by reverting `ext_gui_show`/`ext_gui_hide` locally: 2 failed, 1 passed). Test-only; no production source changes. --- src/wrapper/clap/aax_parameter_tests.rs | 223 +++++++++++++++++++++++- 1 file changed, 222 insertions(+), 1 deletion(-) diff --git a/src/wrapper/clap/aax_parameter_tests.rs b/src/wrapper/clap/aax_parameter_tests.rs index 3aca4fbc..19484571 100644 --- a/src/wrapper/clap/aax_parameter_tests.rs +++ b/src/wrapper/clap/aax_parameter_tests.rs @@ -1,6 +1,12 @@ use super::*; -use crate::prelude::{ClapFeature, Enum, EnumParam, FloatParam, FloatRange, Param, ProcessContext}; +use crate::prelude::{ + AsyncExecutor, ClapFeature, Enum, EnumParam, FloatParam, FloatRange, GuiContext, Param, + ProcessContext, +}; +use clap_sys::ext::gui::{clap_window, clap_window_handle, CLAP_WINDOW_API_COCOA}; +use std::any::Any; use std::ptr; +use std::sync::atomic::{AtomicBool, Ordering}; #[derive(Enum, Debug, PartialEq, Clone, Copy)] enum SingleVariant { @@ -583,3 +589,218 @@ fn out_of_range_and_nonfinite_values_reject_the_entire_input_batch() { assert!(received.is_empty()); } } + +// ─── Editor visibility (AUD-2057) ───────────────────────────────────────────── +// +// `ext_gui_show`/`ext_gui_hide` used to return a hardcoded `false`, on the +// assumption that they only applied to free-standing windows. macOS hosts call +// `show()` to re-show an embedded editor after hiding it, so answering `false` +// left the custom GUI invisible. The wrapper must delegate to the editor's +// `set_visible` and report exactly what the editor said — an editor that cannot +// change visibility still has to surface `false`. + +/// Records every visibility request and answers with a configurable verdict, so +/// the test can tell "the wrapper asked" apart from "the wrapper liked the +/// answer". `parking_lot::Mutex` because this is a synchronous single-thread +/// fixture. +struct VisibilityEditor { + calls: parking_lot::Mutex>, + verdict: AtomicBool, +} + +impl VisibilityEditor { + fn record(&self, visible: bool) -> bool { + self.calls.lock().push(visible); + self.verdict.load(Ordering::Relaxed) + } +} + +impl Editor for VisibilityEditor { + fn spawn( + &self, + _parent: ParentWindowHandle, + _context: Arc, + ) -> Box { + Box::new(()) + } + + fn set_visible(&self, _handle: &mut (dyn Any + Send), visible: bool) -> bool { + self.record(visible) + } + + fn size(&self) -> (u32, u32) { + (100, 100) + } + fn set_scale_factor(&self, _factor: f32) -> bool { + false + } + fn param_value_changed(&self, _id: &str, _normalized_value: f32) {} + fn param_modulation_changed(&self, _id: &str, _modulation_offset: f32) {} + fn param_values_changed(&self) {} +} + +struct EditorPlugin { + params: Arc, + /// Shared with every `editor()` clone the wrapper takes, which is the only + /// way for the test to observe the wrapper's own editor instance. + editor: Arc, +} + +impl Default for EditorPlugin { + fn default() -> Self { + Self { + params: Arc::new(TestParams { + value: FloatParam::new("Value", 0.25, FloatRange::Linear { min: 0.0, max: 1.0 }), + single: None, + }), + editor: Arc::new(VisibilityEditor { + calls: parking_lot::Mutex::new(Vec::new()), + verdict: AtomicBool::new(true), + }), + } + } +} + +impl Plugin for EditorPlugin { + const NAME: &'static str = "editor visibility fixture"; + const VENDOR: &'static str = "Intrect"; + const URL: &'static str = "https://intrect.io"; + const EMAIL: &'static str = "test@intrect.io"; + const VERSION: &'static str = "0.0.0"; + const AUDIO_IO_LAYOUTS: &'static [AudioIOLayout] = &[AudioIOLayout { + main_input_channels: NonZeroU32::new(1), + main_output_channels: NonZeroU32::new(1), + ..AudioIOLayout::const_default() + }]; + type SysExMessage = (); + type BackgroundTask = (); + + fn params(&self) -> Arc { + self.params.clone() + } + + fn editor(&mut self, _async_executor: AsyncExecutor) -> Option> { + Some(Box::new(VisibilityEditorHandle { + shared: self.editor.clone(), + })) + } + + fn process( + &mut self, + _: &mut crate::buffer::Buffer, + _: &mut AuxiliaryBuffers, + _: &mut impl ProcessContext, + ) -> ProcessStatus { + ProcessStatus::Normal + } +} + +/// The wrapper owns the editor it gets from `Plugin::editor`, so the fixture +/// hands out a thin proxy over the shared recorder. +struct VisibilityEditorHandle { + shared: Arc, +} + +impl Editor for VisibilityEditorHandle { + fn spawn( + &self, + _parent: ParentWindowHandle, + _context: Arc, + ) -> Box { + Box::new(()) + } + + fn set_visible(&self, _handle: &mut (dyn Any + Send), visible: bool) -> bool { + self.shared.record(visible) + } + + fn size(&self) -> (u32, u32) { + (100, 100) + } + fn set_scale_factor(&self, _factor: f32) -> bool { + false + } + fn param_value_changed(&self, _id: &str, _normalized_value: f32) {} + fn param_modulation_changed(&self, _id: &str, _modulation_offset: f32) {} + fn param_values_changed(&self) {} +} + +impl ClapPlugin for EditorPlugin { + const CLAP_ID: &'static str = "io.intrect.editor-visibility-test"; + const CLAP_DESCRIPTION: Option<&'static str> = None; + const CLAP_MANUAL_URL: Option<&'static str> = None; + const CLAP_SUPPORT_URL: Option<&'static str> = None; + const CLAP_FEATURES: &'static [ClapFeature] = &[ClapFeature::AudioEffect]; +} + +/// Attach an editor the way a host does, then return the wrapper. +unsafe fn wrapper_with_attached_editor() -> (Arc>, clap_window) { + let host = host(); + let wrapper = unsafe { Wrapper::::new(&host) }; + let plugin = wrapper.clap_plugin.borrow(); + let window = clap_window { + api: CLAP_WINDOW_API_COCOA.as_ptr(), + specific: clap_window_handle { + cocoa: ptr::null_mut(), + }, + }; + assert!(unsafe { Wrapper::::ext_gui_set_parent(&*plugin, &window) }); + drop(plugin); + (wrapper, window) +} + +/// The defect was the hardcoded `false`: the editor was never consulted. Assert +/// both directions are forwarded in order and that the wrapper relays the +/// editor's verdict verbatim. +#[test] +fn gui_show_and_hide_delegate_to_the_editor_and_relay_its_verdict() { + let (wrapper, _) = unsafe { wrapper_with_attached_editor() }; + let plugin = wrapper.clap_plugin.borrow(); + + assert!(unsafe { Wrapper::::ext_gui_show(&*plugin) }); + assert!(unsafe { Wrapper::::ext_gui_hide(&*plugin) }); + assert!(unsafe { Wrapper::::ext_gui_show(&*plugin) }); + + assert_eq!( + *wrapper.plugin.lock().editor.calls.lock(), + vec![true, false, true], + "show/hide must reach the editor in call order" + ); +} + +/// An editor that refuses to change visibility must produce `false`, not a +/// wrapper-side success. This is the half of the contract that the old +/// hardcoded `false` accidentally satisfied, so it pins that the fix did not +/// replace a constant with a blind `true`. +#[test] +fn gui_show_reports_false_when_the_editor_cannot_change_visibility() { + let (wrapper, _) = unsafe { wrapper_with_attached_editor() }; + // Flip the fixture before the calls under test. + wrapper + .plugin + .lock() + .editor + .verdict + .store(false, Ordering::Relaxed); + let plugin = wrapper.clap_plugin.borrow(); + + assert!(!unsafe { Wrapper::::ext_gui_show(&*plugin) }); + assert!(!unsafe { Wrapper::::ext_gui_hide(&*plugin) }); + assert_eq!( + *wrapper.plugin.lock().editor.calls.lock(), + vec![true, false] + ); +} + +/// Without an attached editor the extension must answer `false` rather than +/// panic on the missing handle. +#[test] +fn gui_show_without_an_attached_editor_reports_failure() { + let host = host(); + let wrapper = unsafe { Wrapper::::new(&host) }; + let plugin = wrapper.clap_plugin.borrow(); + + assert!(!unsafe { Wrapper::::ext_gui_show(&*plugin) }); + assert!(!unsafe { Wrapper::::ext_gui_hide(&*plugin) }); + assert!(wrapper.plugin.lock().editor.calls.lock().is_empty()); +} From 932dfa875e3fa5958f99bd5a967554df66f91db5 Mon Sep 17 00:00:00 2001 From: unohee Date: Thu, 8 Oct 2026 09:50:21 +0900 Subject: [PATCH 2/4] fix(host): serialize lifecycle against audio mutable state (AUD-2067) --- src/wrapper/au/wrapper.rs | 104 ++++++++++++++--- src/wrapper/clap/aax_parameter_tests.rs | 43 +++++++ src/wrapper/clap/wrapper.rs | 69 +++++++++++- src/wrapper/util.rs | 1 + src/wrapper/util/processing_gate.rs | 68 +++++++++++ src/wrapper/vst3/inner.rs | 13 +++ src/wrapper/vst3/wrapper.rs | 144 +++++++++++++++++++++++- 7 files changed, 419 insertions(+), 23 deletions(-) create mode 100644 src/wrapper/util/processing_gate.rs diff --git a/src/wrapper/au/wrapper.rs b/src/wrapper/au/wrapper.rs index a18724cb..c6fd70fc 100644 --- a/src/wrapper/au/wrapper.rs +++ b/src/wrapper/au/wrapper.rs @@ -210,12 +210,11 @@ pub struct Wrapper { /// /// Wrapped in `UnsafeCell` because `Plugin::initialize` / `process` / /// `reset` / `deactivate` take `&mut self`, but we only ever expose `&Self` - /// from `from_ptr`. AU's threading model guarantees these methods are not - /// concurrent with each other (Initialize/Uninitialize/Reset run on main, - /// process runs on audio thread, and the host serialises lifecycle vs. - /// render via `Initialize`/`Uninitialize` boundaries — render can only - /// happen between them). + /// from `from_ptr`. processing_gate excludes lifecycle mutation and + /// reentrant render before any mutable reference is constructed. plugin: UnsafeCell>, + /// Lifecycle and audio must own the same gate before borrowing mutable state. + processing_gate: crate::wrapper::util::processing_gate::ProcessingGate, /// Parameter handles in declaration order. The AU parameter ID is the /// index into this vec. Built once in `new()`, then read-only. @@ -271,8 +270,7 @@ pub struct Wrapper { /// and the persistent `Buffer<'static>` whose channel slot vector is /// pre-sized in `Initialize`. /// - /// `UnsafeCell` because only `render()` touches it after `Initialize`, - /// and AU does not re-enter render. + /// `UnsafeCell` accesses require processing_gate, including lifecycle calls. render_state: UnsafeCell>, /// The plugin's `Editor` instance, if the plugin provides one. @@ -467,8 +465,8 @@ const DEFAULT_PRESET_NAME: &str = "Untitled"; /// - `plugin` and `input_scratch` are `UnsafeCell`-wrapped, accessed only /// through `&mut` borrows constructed during a single audio-thread render /// call (`input_scratch`) or during main-thread lifecycle calls that AU -/// serialises against render (`plugin`). The host contract forbids -/// `Initialize` / `Uninitialize` / `Reset` concurrent with `Render`. +/// excludes with processing_gate (`plugin`). Lifecycle callbacks and render +/// must acquire the gate before constructing these mutable references. /// - `ParamPtr` in `params_by_id` is built once and read-only thereafter; /// per nih-plug contract, `set_normalized_value` and the value getters /// are themselves thread-safe. @@ -553,6 +551,7 @@ impl Wrapper

{ initialized: AtomicBool::new(false), bypass: AtomicBool::new(false), bypass_param_idx, + processing_gate: crate::wrapper::util::processing_gate::ProcessingGate::new(), plugin: UnsafeCell::new(plugin), params_by_id, host_callbacks: Mutex::new(None), @@ -758,6 +757,10 @@ impl Wrapper

{ unsafe extern "C" fn close(self_ptr: *mut c_void) -> au::OSStatus { // Drop editor handle before the wrapper is destroyed. let this = unsafe { Self::from_ptr(self_ptr) }; + // Close is a non-audio lifecycle call; wait for an in-flight render. + let Some(processing_guard) = this.processing_gate.lock() else { + return au::kAudioUnitErr_CannotDoInCurrentContext; + }; if this.initialized.swap(false, Ordering::AcqRel) { // Hosts are allowed to dispose an initialized AudioUnit without a // preceding Uninitialize call. Keep Plugin's lifecycle balanced. @@ -767,6 +770,8 @@ impl Wrapper

{ let instance = this.instance.swap(0, Ordering::AcqRel) as usize as *mut c_void; cocoaui::close_audio_unit_view(instance); this.editor_handle.clear(); + // Release the guard before freeing its mutex. Hosts must not start new calls after Close. + drop(processing_guard); unsafe { let _ = Box::from_raw(self_ptr as *mut Self); } @@ -841,6 +846,9 @@ impl Wrapper

{ /// from those and forward to `Plugin::initialize`. unsafe extern "C" fn initialize(self_ptr: *mut c_void) -> au::OSStatus { let this = unsafe { Self::from_ptr(self_ptr) }; + let Some(_processing_guard) = this.processing_gate.lock() else { + return au::kAudioUnitErr_CannotDoInCurrentContext; + }; let n_ch = this.n_channels().max(1); let chans = NonZeroU32::new(n_ch).expect("n_ch is clamped to at least one"); @@ -861,7 +869,7 @@ impl Wrapper

{ sink: this.sink.clone(), _marker: PhantomData, }; - // SAFETY: AU forbids Initialize concurrent with Render. + // SAFETY: processing_gate excludes Render and other lifecycle callbacks. let plugin = unsafe { this.plugin_mut() }; let ok = plugin.initialize(&selected_layout, &buffer_config, &mut ctx); if !ok { @@ -905,12 +913,15 @@ impl Wrapper

{ unsafe extern "C" fn uninitialize(self_ptr: *mut c_void) -> au::OSStatus { let this = unsafe { Self::from_ptr(self_ptr) }; + let Some(_processing_guard) = this.processing_gate.lock() else { + return au::kAudioUnitErr_CannotDoInCurrentContext; + }; if this .initialized .compare_exchange(true, false, Ordering::AcqRel, Ordering::Acquire) .is_ok() { - // SAFETY: AU forbids Uninitialize concurrent with Render. + // SAFETY: processing_gate excludes Render and other lifecycle callbacks. unsafe { this.plugin_mut() }.deactivate(); } this.midi_input.clear(); @@ -924,7 +935,10 @@ impl Wrapper

{ _element: au::AudioUnitElement, ) -> au::OSStatus { let this = unsafe { Self::from_ptr(self_ptr) }; - // SAFETY: AU calls Reset on the main thread, serialised against render. + let Some(_processing_guard) = this.processing_gate.try_lock() else { + return au::kAudioUnitErr_CannotDoInCurrentContext; + }; + // SAFETY: processing_gate excludes concurrent mutation. unsafe { this.plugin_mut() }.reset(); this.midi_input.clear(); unsafe { this.render_state_mut() }.midi.clear(); @@ -1656,7 +1670,9 @@ impl Wrapper

{ let accepted = selected.and_then(|layout| { let out_ch = layout.main_output_channels?; match layout_for_output::

(out_ch) { - Some(resolved) if std::ptr::eq(resolved, layout) => Some(out_ch), + Some(resolved) if std::ptr::eq(resolved, layout) => { + Some(out_ch) + } _ => None, } }); @@ -1980,6 +1996,12 @@ impl Wrapper

{ return au::kAudioUnitErr_InvalidParameter; } + let Some(_processing_guard) = this.processing_gate.try_lock() else { + unsafe { zero_buffer_list(io_data, in_number_frames) }; + this.set_last_render_error(au::kAudioUnitErr_CannotDoInCurrentContext); + return au::kAudioUnitErr_CannotDoInCurrentContext; + }; + if !this.is_initialized() { unsafe { zero_buffer_list(io_data, in_number_frames) }; return au::noErr; @@ -2026,8 +2048,8 @@ impl Wrapper

{ }) .flatten(); - // SAFETY: render is not re-entered; we are the sole owner of - // RenderState for the duration of this call. + // SAFETY: processing_gate excludes reentrant render and lifecycle mutation + // for the duration of this call. let rs = unsafe { this.render_state_mut() }; // ── 1) Pull main input from whichever wiring the host installed. ── @@ -3205,6 +3227,45 @@ mod tests { const AU_MANUFACTURER: [u8; 4] = *b"Test"; } + #[test] + fn lifecycle_contention_rejects_mutation_and_silences_render() { + type W = Wrapper>; + let pointer = W::new() as *mut c_void; + let this = unsafe { W::from_ptr(pointer) }; + let guard = this.processing_gate.lock(); + assert_eq!( + unsafe { W::initialize(pointer) }, + au::kAudioUnitErr_CannotDoInCurrentContext + ); + assert_eq!( + unsafe { W::uninitialize(pointer) }, + au::kAudioUnitErr_CannotDoInCurrentContext + ); + assert_eq!( + unsafe { W::reset(pointer, 0, 0) }, + au::kAudioUnitErr_CannotDoInCurrentContext + ); + let mut samples = [1.0f32; 8]; + let mut list = au::AudioBufferList { + mNumberBuffers: 1, + mBuffers: [au::AudioBuffer { + mNumberChannels: 1, + mDataByteSize: 32, + mData: samples.as_mut_ptr().cast(), + }], + }; + let mut flags = 0; + assert_eq!( + unsafe { W::render(pointer, &mut flags, std::ptr::null(), 0, 8, &mut list) }, + au::kAudioUnitErr_CannotDoInCurrentContext + ); + assert_eq!(samples, [0.0; 8]); + assert!(!this.is_initialized()); + drop(guard); + assert_eq!(unsafe { W::initialize(pointer) }, au::noErr); + assert_eq!(unsafe { W::close(pointer) }, au::noErr); + } + fn bypass_sequence(via_parameter: bool) -> (Vec<[f32; 2]>, usize) { type Timestamp = au::AudioTimeStamp; let wrapper = Wrapper::>::new() as *mut c_void; @@ -3627,12 +3688,21 @@ mod tests { use crate::params::range::FloatRange; use crate::params::{FloatParam, Param}; for unit in [ - " ms", "MS", "msec", "millisec", "millisecs", "millisecond", "milliseconds", + " ms", + "MS", + "msec", + "millisec", + "millisecs", + "millisecond", + "milliseconds", ] { let param = FloatParam::new( "Attack", 5.0, - FloatRange::Linear { min: 0.0, max: 200.0 }, + FloatRange::Linear { + min: 0.0, + max: 200.0, + }, ) .with_unit(unit); let entry = ParamEntry { diff --git a/src/wrapper/clap/aax_parameter_tests.rs b/src/wrapper/clap/aax_parameter_tests.rs index 3aca4fbc..c38b7305 100644 --- a/src/wrapper/clap/aax_parameter_tests.rs +++ b/src/wrapper/clap/aax_parameter_tests.rs @@ -583,3 +583,46 @@ fn out_of_range_and_nonfinite_values_reject_the_entire_input_batch() { assert!(received.is_empty()); } } + +#[test] +fn lifecycle_contention_rejects_activation_and_silences_audio_without_borrowing() { + let host = host(); + let wrapper = unsafe { Wrapper::>::new(&host) }; + let plugin = wrapper.clap_plugin.borrow(); + let guard = wrapper.processing_gate.lock(); + // These live borrows used to abort activate() at the C ABI boundary. + let buffers = wrapper.buffer_manager.borrow_mut(); + let events = wrapper.input_events.borrow_mut(); + assert!(!unsafe { Wrapper::>::activate(&*plugin, 48000.0, 1, 16) }); + let mut samples = [1.0f32; 16]; + let mut pointers = [samples.as_mut_ptr()]; + let mut output = clap_sys::audio_buffer::clap_audio_buffer { + data32: pointers.as_mut_ptr(), + data64: ptr::null_mut(), + channel_count: 1, + latency: 0, + constant_mask: 0, + }; + let process = clap_process { + steady_time: 0, + frames_count: 16, + transport: ptr::null(), + audio_inputs: ptr::null(), + audio_outputs: &mut output, + audio_inputs_count: 0, + audio_outputs_count: 1, + in_events: ptr::null(), + out_events: ptr::null(), + }; + assert_eq!( + unsafe { Wrapper::>::process(&*plugin, &process) }, + CLAP_PROCESS_ERROR + ); + assert_eq!(samples, [0.0; 16]); + unsafe { Wrapper::>::reset(&*plugin) }; + assert!(wrapper.reset_pending.load(Ordering::Acquire)); + drop(events); + drop(buffers); + drop(guard); + assert!(unsafe { Wrapper::>::activate(&*plugin, 48000.0, 1, 16) }); +} diff --git a/src/wrapper/clap/wrapper.rs b/src/wrapper/clap/wrapper.rs index 143f92a3..f18f25f5 100644 --- a/src/wrapper/clap/wrapper.rs +++ b/src/wrapper/clap/wrapper.rs @@ -152,6 +152,9 @@ pub struct Wrapper { /// A data structure that helps manage and create buffers for all of the plugin's inputs and /// outputs based on channel pointers provided by the host. buffer_manager: AtomicRefCell, + /// Lifecycle and audio must own the same gate before borrowing mutable state. + processing_gate: crate::wrapper::util::processing_gate::ProcessingGate, + reset_pending: AtomicBool, /// The plugin is able to restore state through a method on the `GuiContext`. To avoid changing /// parameters mid-processing and running into garbled data if the host also tries to load state /// at the same time the restoring happens at the end of each processing call. If this zero @@ -561,6 +564,8 @@ impl Wrapper

{ current_latency: AtomicU32::new(0), // This is initialized just before calling `Plugin::initialize()` so that during the // process call buffers can be initialized without any allocations + processing_gate: crate::wrapper::util::processing_gate::ProcessingGate::new(), + reset_pending: AtomicBool::new(false), buffer_manager: AtomicRefCell::new(BufferManager::for_audio_io_layout( 0, AudioIOLayout::default(), @@ -1758,6 +1763,14 @@ impl Wrapper

{ /// /// `self.plugin` must _not_ be locked while calling this function or it will deadlock. pub fn set_state_inner(&self, state: &mut PluginState) -> bool { + let Some(_processing_guard) = self.processing_gate.lock() else { + return false; + }; + self.set_state_inner_exclusive(state) + } + + // Caller already owns processing_gate, including state restoration at the end of process(). + fn set_state_inner_exclusive(&self, state: &mut PluginState) -> bool { let audio_io_layout = self.current_audio_io_layout.load(); let buffer_config = self.current_buffer_config.load(); @@ -1853,6 +1866,9 @@ impl Wrapper

{ ) -> bool { check_null_ptr!(false, plugin, (*plugin).plugin_data); let wrapper = &*((*plugin).plugin_data as *const Self); + let Some(_processing_guard) = wrapper.processing_gate.lock() else { + return false; + }; let audio_io_layout = wrapper.current_audio_io_layout.load(); let buffer_config = BufferConfig { @@ -1892,6 +1908,9 @@ impl Wrapper

{ check_null_ptr!((), plugin, (*plugin).plugin_data); let wrapper = &*((*plugin).plugin_data as *const Self); + let Some(_processing_guard) = wrapper.processing_gate.lock() else { + return; + }; wrapper.plugin.lock().deactivate(); } @@ -1900,6 +1919,9 @@ impl Wrapper

{ // updating parameters from the GUI while the processing loop isn't running check_null_ptr!(false, plugin, (*plugin).plugin_data); let wrapper = &*((*plugin).plugin_data as *const Self); + let Some(_processing_guard) = wrapper.processing_gate.try_lock() else { + return false; + }; // Always reset the processing status when the plugin gets activated or deactivated wrapper.last_process_status.store(ProcessStatus::Normal); @@ -1907,6 +1929,7 @@ impl Wrapper

{ // To be consistent with the VST3 wrapper, we'll also reset the buffers here in addition to // the dedicated `reset()` function. + wrapper.reset_pending.store(false, Ordering::Release); process_wrapper(|| wrapper.plugin.lock().reset()); true @@ -1923,15 +1946,50 @@ impl Wrapper

{ check_null_ptr!((), plugin, (*plugin).plugin_data); let wrapper = &*((*plugin).plugin_data as *const Self); + let Some(_processing_guard) = wrapper.processing_gate.try_lock() else { + wrapper.reset_pending.store(true, Ordering::Release); + return; + }; + wrapper.reset_pending.store(false, Ordering::Release); process_wrapper(|| wrapper.plugin.lock().reset()); } + /// Clear every host output without touching wrapper-owned buffers on contention. + unsafe fn clear_process_outputs(process: &clap_process) { + if process.audio_outputs.is_null() { + return; + } + for port in 0..process.audio_outputs_count as usize { + let output = &*process.audio_outputs.add(port); + for channel in 0..output.channel_count as usize { + if !output.data32.is_null() { + let samples = *output.data32.add(channel); + if !samples.is_null() { + samples.write_bytes(0, process.frames_count as usize); + } + } else if !output.data64.is_null() { + let samples = *output.data64.add(channel); + if !samples.is_null() { + samples.write_bytes(0, process.frames_count as usize); + } + } + } + } + } + unsafe extern "C" fn process( plugin: *const clap_plugin, process: *const clap_process, ) -> clap_process_status { check_null_ptr!(CLAP_PROCESS_ERROR, plugin, (*plugin).plugin_data, process); let wrapper = &*((*plugin).plugin_data as *const Self); + let Some(_processing_guard) = wrapper.processing_gate.try_lock() else { + Self::clear_process_outputs(&*process); + return CLAP_PROCESS_ERROR; + }; + if wrapper.reset_pending.swap(false, Ordering::AcqRel) { + process_wrapper(|| wrapper.plugin.lock().reset()); + } // Panic on allocations if the `assert_process_allocs` feature has been enabled, and make // sure that FTZ is set up correctly @@ -2276,7 +2334,7 @@ impl Wrapper

{ // doesn't do that let updated_state = permit_alloc(|| wrapper.updated_state_receiver.try_recv()); if let Ok(mut state) = updated_state { - wrapper.set_state_inner(&mut state); + wrapper.set_state_inner_exclusive(&mut state); // We'll pass the state object back to the GUI thread so deallocation can happen // there without potentially blocking the audio thread @@ -3246,6 +3304,15 @@ impl Wrapper

{ ) { check_null_ptr!((), plugin, (*plugin).plugin_data); let wrapper = &*((*plugin).plugin_data as *const Self); + // Inactive flush runs on control; active flush is an audio-thread call. + let guard = if wrapper.is_processing.load(Ordering::Acquire) { + wrapper.processing_gate.try_lock() + } else { + wrapper.processing_gate.lock() + }; + let Some(_processing_guard) = guard else { + return; + }; if !in_.is_null() { wrapper.handle_in_events(&*in_, 0, 0); diff --git a/src/wrapper/util.rs b/src/wrapper/util.rs index 26040fb3..a756cbbd 100644 --- a/src/wrapper/util.rs +++ b/src/wrapper/util.rs @@ -8,6 +8,7 @@ use crate::util::permit_alloc; pub(crate) mod buffer_management; #[cfg(debug_assertions)] pub(crate) mod context_checks; +pub(crate) mod processing_gate; /// The bit that controls flush-to-zero behavior for denormals in 32 and 64-bit floating point /// numbers on x86 family architectures. Rust 1.75 deprecated the built in functions for controlling diff --git a/src/wrapper/util/processing_gate.rs b/src/wrapper/util/processing_gate.rs new file mode 100644 index 00000000..1ec8c03c --- /dev/null +++ b/src/wrapper/util/processing_gate.rs @@ -0,0 +1,68 @@ +//! Exclude lifecycle, audio and event-buffer mutation before borrowing wrapper state. +//! Audio uses try_lock only. Control may wait for an in-flight callback, but +//! synchronous reentry from that callback must fail instead of waiting on itself. +use parking_lot::{ReentrantMutex, ReentrantMutexGuard}; + +pub(crate) struct ProcessingGate(ReentrantMutex<()>); + +impl ProcessingGate { + pub fn new() -> Self { + Self(ReentrantMutex::new(())) + } + + pub fn try_lock(&self) -> Option> { + if self.0.is_owned_by_current_thread() { + return None; + } + self.0.try_lock() + } + + /// Only for non-audio control entrypoints. None means synchronous reentry. + pub fn lock(&self) -> Option> { + if self.0.is_owned_by_current_thread() { + return None; + } + Some(self.0.lock()) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use std::{ + sync::{mpsc, Arc}, + thread, + time::Duration, + }; + #[test] + fn audio_never_waits_control_waits_and_reentry_is_rejected() { + let gate = Arc::new(ProcessingGate::new()); + let held = gate.lock().unwrap(); + assert!(gate.try_lock().is_none()); + assert!(gate.lock().is_none()); + let (audio_tx, audio_rx) = mpsc::channel(); + let audio_gate = gate.clone(); + let audio = thread::spawn(move || audio_tx.send(audio_gate.try_lock().is_none()).unwrap()); + let result = audio_rx.recv_timeout(Duration::from_secs(1)); + if result.is_err() { + drop(held); + audio.join().unwrap(); + panic!("audio waited on control"); + } + assert!(result.unwrap()); + audio.join().unwrap(); + let (entered_tx, entered_rx) = mpsc::channel(); + let (done_tx, done_rx) = mpsc::channel(); + let control_gate = gate.clone(); + let control = thread::spawn(move || { + entered_tx.send(()).unwrap(); + let _held = control_gate.lock().unwrap(); + done_tx.send(()).unwrap(); + }); + entered_rx.recv().unwrap(); + assert!(matches!(done_rx.try_recv(), Err(mpsc::TryRecvError::Empty))); + drop(held); + done_rx.recv_timeout(Duration::from_secs(1)).unwrap(); + control.join().unwrap(); + } +} diff --git a/src/wrapper/vst3/inner.rs b/src/wrapper/vst3/inner.rs index add0a1a1..c471b341 100644 --- a/src/wrapper/vst3/inner.rs +++ b/src/wrapper/vst3/inner.rs @@ -81,6 +81,9 @@ pub(crate) struct WrapperInner { /// A data structure that helps manage and create buffers for all of the plugin's inputs and /// outputs based on channel pointers provided by the host. pub buffer_manager: AtomicRefCell, + /// Lifecycle and audio must own the same gate before borrowing mutable state. + pub processing_gate: crate::wrapper::util::processing_gate::ProcessingGate, + pub reset_pending: AtomicBool, /// The incoming events for the plugin, if `P::ACCEPTS_MIDI` is set. If /// `P::SAMPLE_ACCURATE_AUTOMATION`, this is also read in lockstep with the parameter change /// block splitting. @@ -301,6 +304,8 @@ impl WrapperInner

{ current_latency: AtomicU32::new(0), // This is initialized just before calling `Plugin::initialize()` so that during the // process call buffers can be initialized without any allocations + processing_gate: crate::wrapper::util::processing_gate::ProcessingGate::new(), + reset_pending: AtomicBool::new(false), buffer_manager: AtomicRefCell::new(BufferManager::for_audio_io_layout( 0, AudioIOLayout::default(), @@ -547,6 +552,14 @@ impl WrapperInner

{ /// /// `self.plugin` must _not_ be locked while calling this function or it will deadlock. pub fn set_state_inner(&self, state: &mut PluginState) -> bool { + let Some(_processing_guard) = self.processing_gate.lock() else { + return false; + }; + self.set_state_inner_exclusive(state) + } + + // Caller already owns processing_gate, including audio-thread state restoration. + pub(super) fn set_state_inner_exclusive(&self, state: &mut PluginState) -> bool { let audio_io_layout = self.current_audio_io_layout.load(); let buffer_config = self.current_buffer_config.load(); diff --git a/src/wrapper/vst3/wrapper.rs b/src/wrapper/vst3/wrapper.rs index cab922cf..fc0d5c9e 100644 --- a/src/wrapper/vst3/wrapper.rs +++ b/src/wrapper/vst3/wrapper.rs @@ -10,10 +10,10 @@ use vst3_sys::base::{kInvalidArgument, kNoInterface, kResultFalse, kResultOk, tr use vst3_sys::base::{IBStream, IPluginBase}; use vst3_sys::utils::SharedVstPtr; use vst3_sys::vst::{ - kNoParamId, kNoParentUnitId, kNoProgramListId, kRootUnitId, Event, EventTypes, IAudioProcessor, - IComponent, IEditController, IEventList, IMidiMapping, INoteExpressionController, - IParamValueQueue, IParameterChanges, IProcessContextRequirements, IUnitInfo, - AudioBusBuffers, LegacyMidiCCOutEvent, NoteExpressionTypeInfo, NoteExpressionValueDescription, + kNoParamId, kNoParentUnitId, kNoProgramListId, kRootUnitId, AudioBusBuffers, Event, EventTypes, + IAudioProcessor, IComponent, IEditController, IEventList, IMidiMapping, + INoteExpressionController, IParamValueQueue, IParameterChanges, IProcessContextRequirements, + IUnitInfo, LegacyMidiCCOutEvent, NoteExpressionTypeInfo, NoteExpressionValueDescription, NoteOffEvent, NoteOnEvent, ParameterFlags, PolyPressureEvent, ProgramListInfo, TChar, UnitInfo, }; use vst3_sys::VST3; @@ -55,6 +55,41 @@ impl Wrapper

{ pub fn new() -> Box { Self::allocate(WrapperInner::new()) } + unsafe fn clear_process_outputs(data: &vst3_sys::vst::ProcessData) { + if data.outputs.is_null() || data.num_samples <= 0 { + return; + } + for port in 0..data.num_outputs.max(0) as usize { + let output = &mut *data.outputs.add(port); + for channel in 0..output.num_channels.max(0) as usize { + if data.symbolic_sample_size == vst3_sys::vst::SymbolicSampleSizes::kSample32 as i32 + { + let pointers = output.buffers as *mut *mut f32; + if !pointers.is_null() { + let samples = *pointers.add(channel); + if !samples.is_null() { + samples.write_bytes(0, data.num_samples as usize); + } + } + } else if data.symbolic_sample_size + == vst3_sys::vst::SymbolicSampleSizes::kSample64 as i32 + { + let pointers = output.buffers as *mut *mut f64; + if !pointers.is_null() { + let samples = *pointers.add(channel); + if !samples.is_null() { + samples.write_bytes(0, data.num_samples as usize); + } + } + } + } + output.silence_flags = if output.num_channels >= 64 { + u64::MAX + } else { + (1u64 << output.num_channels.max(0)) - 1 + }; + } + } } impl Drop for Wrapper

{ @@ -368,6 +403,9 @@ impl IComponent for Wrapper

{ } unsafe fn set_active(&self, state: TBool) -> tresult { + let Some(_processing_guard) = self.inner.processing_gate.lock() else { + return kResultFalse; + }; // We could call initialize in `IAudioProcessor::setup_processing()`, but REAPER will set // the bus arrangements between that function and this function. So to be able to handle // custom channel layout overrides we need to initialize here. @@ -905,6 +943,10 @@ impl IAudioProcessor for Wrapper

{ // This function is also used to reset buffers on the plugin, so we should do the same // thing. We don't call `reset()` in `setup_processing()` for that same reason. if state { + let Some(_processing_guard) = self.inner.processing_gate.try_lock() else { + self.inner.reset_pending.store(true, Ordering::Release); + return kResultOk; + }; // HACK: See the comment in `IComponent::setActive()`. This is needed to work around // Ardour bugs. let mut plugin = match self.inner.plugin.try_lock() { @@ -921,6 +963,7 @@ impl IAudioProcessor for Wrapper

{ } }; + self.inner.reset_pending.store(false, Ordering::Release); process_wrapper(|| plugin.reset()); } @@ -932,6 +975,13 @@ impl IAudioProcessor for Wrapper

{ #[allow(clippy::mut_range_bound)] unsafe fn process(&self, data: *mut vst3_sys::vst::ProcessData) -> tresult { check_null_ptr!(data); + let Some(_processing_guard) = self.inner.processing_gate.try_lock() else { + Self::clear_process_outputs(&*data); + return kResultFalse; + }; + if self.inner.reset_pending.swap(false, Ordering::AcqRel) { + process_wrapper(|| self.inner.plugin.lock().reset()); + } // Panic on allocations if the `assert_process_allocs` feature has been enabled, and make // sure that FTZ is set up correctly @@ -1630,7 +1680,7 @@ impl IAudioProcessor for Wrapper

{ // doesn't do that let updated_state = permit_alloc(|| self.inner.updated_state_receiver.try_recv()); if let Ok(mut state) = updated_state { - self.inner.set_state_inner(&mut state); + self.inner.set_state_inner_exclusive(&mut state); // We'll pass the state object back to the GUI thread so deallocation can happen // there without potentially blocking the audio thread @@ -2003,3 +2053,87 @@ mod aux_bus_tests { assert_eq!(channel_pointers[0].map(|p| p.num_channels), Some(0)); } } + +#[cfg(test)] +mod lifecycle_tests { + use super::*; + use crate::prelude::*; + #[derive(Default)] + struct EmptyParams {} + unsafe impl Params for EmptyParams { + fn param_map(&self) -> Vec<(String, crate::params::internals::ParamPtr, String)> { + Vec::new() + } + } + #[derive(Default)] + struct TestPlugin { + params: Arc, + } + impl Plugin for TestPlugin { + const NAME: &'static str = "Lifecycle regression"; + const VENDOR: &'static str = "Intrect"; + const URL: &'static str = "https://intrect.io"; + const EMAIL: &'static str = "test@intrect.io"; + const VERSION: &'static str = "0.0.0"; + const AUDIO_IO_LAYOUTS: &'static [AudioIOLayout] = &[AudioIOLayout { + main_input_channels: NonZeroU32::new(1), + main_output_channels: NonZeroU32::new(1), + ..AudioIOLayout::const_default() + }]; + type SysExMessage = (); + type BackgroundTask = (); + fn params(&self) -> Arc { + self.params.clone() + } + fn process( + &mut self, + _: &mut Buffer, + _: &mut AuxiliaryBuffers, + _: &mut impl ProcessContext, + ) -> ProcessStatus { + ProcessStatus::Normal + } + } + impl Vst3Plugin for TestPlugin { + const VST3_CLASS_ID: [u8; 16] = *b"LifecycleTest001"; + const VST3_SUBCATEGORIES: &'static [Vst3SubCategory] = &[Vst3SubCategory::Fx]; + } + #[test] + fn lifecycle_contention_rejects_activation_and_silences_audio() { + let wrapper = Wrapper::::new(); + wrapper + .inner + .current_buffer_config + .store(Some(BufferConfig { + sample_rate: 48000.0, + min_buffer_size: Some(1), + max_buffer_size: 16, + process_mode: ProcessMode::Realtime, + })); + let guard = wrapper.inner.processing_gate.lock(); + let buffers = wrapper.inner.buffer_manager.borrow_mut(); + let events = wrapper.inner.process_events.borrow_mut(); + assert_eq!(unsafe { wrapper.set_active(1) }, kResultFalse); + let mut samples = [1.0f32; 16]; + let mut pointers = [samples.as_mut_ptr() as *mut c_void]; + let mut output = AudioBusBuffers { + num_channels: 1, + silence_flags: 0, + buffers: pointers.as_mut_ptr(), + }; + let mut process: vst3_sys::vst::ProcessData = unsafe { mem::zeroed() }; + process.num_samples = 16; + process.num_outputs = 1; + process.outputs = &mut output; + process.symbolic_sample_size = vst3_sys::vst::SymbolicSampleSizes::kSample32 as i32; + assert_eq!(unsafe { wrapper.process(&mut process) }, kResultFalse); + assert_eq!(samples, [0.0; 16]); + assert_eq!(output.silence_flags, 1); + assert_eq!(unsafe { wrapper.set_processing(1) }, kResultOk); + assert!(wrapper.inner.reset_pending.load(Ordering::Acquire)); + drop(events); + drop(buffers); + drop(guard); + assert_eq!(unsafe { wrapper.set_active(1) }, kResultOk); + } +} From 1152dc725b8121976a7277aa8dd5b1f152df039e Mon Sep 17 00:00:00 2001 From: unohee Date: Mon, 19 Oct 2026 12:41:27 +0900 Subject: [PATCH 3/4] fix(au): notify set_property listeners before returning e49332fc moved every property notification to a 100 Hz main-run-loop timer. Apple's AU validation (the engine auval and Logic use) and de-artifact's au_qc MiniHost set MaximumFramesPerSlice and check their listener right after the call without running the main loop, so validation failed with 'Changing max frames property did not fire off a property changed notification' (AUD-716 regression, found merging into the de-artifact pin). AUBase::SetProperty notifies before returning. Do the same from set_property only, without the hazard e49332fc avoids: the Arc to the notification state is taken before the change and the drain runs through it, so a listener that disposes the AU never leaves a borrowed Wrapper in use. drain() stays a no-op off the main thread and during another delivery; skipped bits stay pending for the timer. initialize keeps timer delivery because it holds the processing gate, which a reentrant Uninitialize from a listener would need. --- src/wrapper/au/wrapper.rs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/wrapper/au/wrapper.rs b/src/wrapper/au/wrapper.rs index 8f8998ea..d66e3261 100644 --- a/src/wrapper/au/wrapper.rs +++ b/src/wrapper/au/wrapper.rs @@ -1468,8 +1468,16 @@ impl Wrapper

{ in_data_size: au::UInt32, ) -> au::OSStatus { let this = unsafe { Self::from_ptr(self_ptr) }; + // Own the notification state before the change: a listener may dispose the AU, so the + // drain below must not touch `this` (the Wrapper) once host code has run. + let notifications = Arc::clone(&this.property_notifications); let status = Self::set_property_impl(this, id, scope, element, in_data, in_data_size); - // The main-run-loop notifier delivers this after the setter returns. + // AUBase::SetProperty notifies its listeners before returning, and hosts and validators + // (auval: "Changing max frames property did not fire off a property changed + // notification") check right after the call without running the main loop, so the + // timer alone is too late here. `drain` is a no-op off the main thread and while + // another delivery is in progress; whatever it skips stays pending for the timer. + notifications.drain(); status } From bd819abff8ea94198b4c11f1989c710922a914b8 Mon Sep 17 00:00:00 2001 From: unohee Date: Mon, 19 Oct 2026 12:47:02 +0900 Subject: [PATCH 4/4] fix(au): deliver set_property notifications on the calling thread Apple's AU validation calls SetProperty off the main thread, so the previous commit's main-thread-only drain still left 'Changing max frames property did not fire off a property changed notification' failing (MiniHost, which calls from main, already passed). AUBase::PropertyChanged notifies on the calling thread; set_property now does the same through drain_from_control_call(). It is only used from that control call, never from render; the timer keeps its main-thread-only drain. set_property_notifies_its_listener_before_returning fails without this (checked by removing the call) and passes with it. --- src/wrapper/au/property_notifications.rs | 9 ++++ src/wrapper/au/wrapper.rs | 57 ++++++++++++++++++++++-- 2 files changed, 63 insertions(+), 3 deletions(-) diff --git a/src/wrapper/au/property_notifications.rs b/src/wrapper/au/property_notifications.rs index 60dd4bff..91f06738 100644 --- a/src/wrapper/au/property_notifications.rs +++ b/src/wrapper/au/property_notifications.rs @@ -170,11 +170,20 @@ impl PropertyNotifications { pub(super) fn mark(&self, mask: u32) { self.pending.fetch_or(mask, Ordering::Release); } + /// Timer delivery: main thread only. pub(super) fn drain(&self) { // Host callbacks must never run on a render/worker thread. if unsafe { libc::pthread_main_np() } == 0 { return; } + self.dispatch(); + } + /// Delivery from a host control call (`AudioUnitSetProperty`), on whichever thread the host + /// made it, as `AUBase::PropertyChanged` does. Never call this from render. + pub(super) fn drain_from_control_call(&self) { + self.dispatch(); + } + fn dispatch(&self) { if self .dispatching .compare_exchange(0, 1, Ordering::SeqCst, Ordering::SeqCst) diff --git a/src/wrapper/au/wrapper.rs b/src/wrapper/au/wrapper.rs index d66e3261..622aff0a 100644 --- a/src/wrapper/au/wrapper.rs +++ b/src/wrapper/au/wrapper.rs @@ -1475,9 +1475,10 @@ impl Wrapper

{ // AUBase::SetProperty notifies its listeners before returning, and hosts and validators // (auval: "Changing max frames property did not fire off a property changed // notification") check right after the call without running the main loop, so the - // timer alone is too late here. `drain` is a no-op off the main thread and while - // another delivery is in progress; whatever it skips stays pending for the timer. - notifications.drain(); + // timer alone is too late here. Apple's validator makes this call off the main thread, so + // deliver on the calling thread (a control thread, never render). A delivery already in + // progress makes this a no-op; whatever it skips stays pending for the timer. + notifications.drain_from_control_call(); status } @@ -3305,6 +3306,56 @@ mod tests { const AU_MANUFACTURER: [u8; 4] = *b"Test"; } + #[test] + fn set_property_notifies_its_listener_before_returning() { + // Apple's AU validation sets MaximumFramesPerSlice off the main thread and checks its + // listener right after the call; a timer-only delivery fails it (AUD-716). + type W = Wrapper>; + static FIRED: AtomicU32 = AtomicU32::new(0); + unsafe extern "C" fn listener( + _: *mut c_void, + _: au::AudioUnit, + id: au::AudioUnitPropertyID, + _: au::AudioUnitScope, + _: au::AudioUnitElement, + ) { + if id == au::kAudioUnitProperty_MaximumFramesPerSlice { + FIRED.fetch_add(1, Ordering::SeqCst); + } + } + let pointer = W::new() as *mut c_void; + // Any non-null handle: delivery only forwards it to the listener. + let instance = 0x10 as au::AudioUnit; + assert_eq!(unsafe { W::open(pointer, instance) }, au::noErr); + assert_eq!( + unsafe { + W::add_property_listener( + pointer, + au::kAudioUnitProperty_MaximumFramesPerSlice, + listener, + ptr::null_mut(), + ) + }, + au::noErr + ); + let frames: au::UInt32 = 777; + assert_eq!( + unsafe { + W::set_property( + pointer, + au::kAudioUnitProperty_MaximumFramesPerSlice, + au::kAudioUnitScope_Global, + 0, + &frames as *const au::UInt32 as *const c_void, + mem::size_of::() as au::UInt32, + ) + }, + au::noErr + ); + assert_eq!(FIRED.load(Ordering::SeqCst), 1); + assert_eq!(unsafe { W::close(pointer) }, au::noErr); + } + #[test] fn lifecycle_contention_rejects_mutation_and_silences_render() { type W = Wrapper>;