alex/nih-plug
1
0
Fork 0
Code Activity

Use AtomicRefCell for all uncontested locks

Browse source 
Since it would be a bug if those locks were somehow contested.
This commit is contained in:
Robbert van der Helm 2022-03-03 21:18:57 +01:00
parent 184355a886
commit 80b1bf12f2
5 changed files with 44 additions and 40 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
src/wrapper/clap/context.rs
View file

@ -1,4 +1,4 @@
use parking_lot::RwLockWriteGuard; use atomic_refcell::AtomicRefMut;
use std::collections::VecDeque; use std::collections::VecDeque;
use std::sync::atomic::Ordering; use std::sync::atomic::Ordering;
use std::sync::Arc; use std::sync::Arc;
@ -22,7 +22,7 @@ pub(crate) struct WrapperGuiContext<P: ClapPlugin> {
/// unnecessary atomic operations to lock the uncontested RwLocks. /// unnecessary atomic operations to lock the uncontested RwLocks.
pub(crate) struct WrapperProcessContext<'a, P: ClapPlugin> { pub(crate) struct WrapperProcessContext<'a, P: ClapPlugin> {
pub(super) wrapper: &'a Wrapper<P>, pub(super) wrapper: &'a Wrapper<P>,
pub(super) input_events_guard: RwLockWriteGuard<'a, VecDeque<NoteEvent>>, pub(super) input_events_guard: AtomicRefMut<'a, VecDeque<NoteEvent>>,
} }
impl<P: ClapPlugin> GuiContext for WrapperGuiContext<P> { impl<P: ClapPlugin> GuiContext for WrapperGuiContext<P> {

35
src/wrapper/clap/wrapper.rs
View file

@ -2,7 +2,7 @@
// explicitly pattern match on that unit // explicitly pattern match on that unit
#![allow(clippy::unused_unit)] #![allow(clippy::unused_unit)]
use atomic_refcell::AtomicRefCell; use atomic_refcell::{AtomicRefCell, AtomicRefMut};
use clap_sys::events::{ use clap_sys::events::{
clap_event_header, clap_event_note, clap_event_param_mod, clap_event_param_value, clap_event_header, clap_event_note, clap_event_param_mod, clap_event_param_value,
clap_input_events, clap_output_events, CLAP_CORE_EVENT_SPACE_ID, CLAP_EVENT_MIDI, clap_input_events, clap_output_events, CLAP_CORE_EVENT_SPACE_ID, CLAP_EVENT_MIDI,
@ -35,7 +35,7 @@ use clap_sys::stream::{clap_istream, clap_ostream};
use crossbeam::atomic::AtomicCell; use crossbeam::atomic::AtomicCell;
use crossbeam::queue::ArrayQueue; use crossbeam::queue::ArrayQueue;
use lazy_static::lazy_static; use lazy_static::lazy_static;
use parking_lot::{RwLock, RwLockWriteGuard}; use parking_lot::RwLock;
use raw_window_handle::RawWindowHandle; use raw_window_handle::RawWindowHandle;
use std::any::Any; use std::any::Any;
use std::cmp; use std::cmp;
@ -85,7 +85,7 @@ pub struct Wrapper<P: ClapPlugin> {
pub clap_plugin: clap_plugin, pub clap_plugin: clap_plugin,
/// A reference to this object, upgraded to an `Arc<Self>` for the GUI context. /// A reference to this object, upgraded to an `Arc<Self>` for the GUI context.
this: RwLock<Weak<Self>>, this: AtomicRefCell<Weak<Self>>,
/// The wrapped plugin instance. /// The wrapped plugin instance.
plugin: RwLock<P>, plugin: RwLock<P>,
@ -111,7 +111,7 @@ pub struct Wrapper<P: ClapPlugin> {
/// ///
/// TODO: Maybe load these lazily at some point instead of needing to spool them all to this /// TODO: Maybe load these lazily at some point instead of needing to spool them all to this
/// queue first /// queue first
input_events: RwLock<VecDeque<NoteEvent>>, input_events: AtomicRefCell<VecDeque<NoteEvent>>,
/// The current latency in samples, as set by the plugin through the [ProcessContext]. uses the /// The current latency in samples, as set by the plugin through the [ProcessContext]. uses the
/// latency extnesion /// latency extnesion
pub current_latency: AtomicU32, pub current_latency: AtomicU32,
@ -119,7 +119,7 @@ pub struct Wrapper<P: ClapPlugin> {
/// apointer to pointers, so this needs to be preallocated in the setup call and kept around /// apointer to pointers, so this needs to be preallocated in the setup call and kept around
/// between process calls. This buffer owns the vector, because otherwise it would need to store /// between process calls. This buffer owns the vector, because otherwise it would need to store
/// a mutable reference to the data contained in this mutex. /// a mutable reference to the data contained in this mutex.
pub output_buffer: RwLock<Buffer<'static>>, pub output_buffer: AtomicRefCell<Buffer<'static>>,
/// Needs to be boxed because the plugin object is supposed to contain a static reference to /// Needs to be boxed because the plugin object is supposed to contain a static reference to
/// this. /// this.
@ -305,7 +305,7 @@ impl<P: ClapPlugin> Wrapper<P> {
on_main_thread: Self::on_main_thread, on_main_thread: Self::on_main_thread,
}, },
this: RwLock::new(Weak::new()), this: AtomicRefCell::new(Weak::new()),
plugin, plugin,
editor, editor,
@ -318,9 +318,9 @@ impl<P: ClapPlugin> Wrapper<P> {
}), }),
current_buffer_config: AtomicCell::new(None), current_buffer_config: AtomicCell::new(None),
bypass_state: AtomicBool::new(false), bypass_state: AtomicBool::new(false),
input_events: RwLock::new(VecDeque::with_capacity(512)), input_events: AtomicRefCell::new(VecDeque::with_capacity(512)),
current_latency: AtomicU32::new(0), current_latency: AtomicU32::new(0),
output_buffer: RwLock::new(Buffer::default()), output_buffer: AtomicRefCell::new(Buffer::default()),
plugin_descriptor, plugin_descriptor,
@ -466,7 +466,7 @@ impl<P: ClapPlugin> Wrapper<P> {
// Finally, the wrapper needs to contain a reference to itself so we can create GuiContexts // Finally, the wrapper needs to contain a reference to itself so we can create GuiContexts
// when opening plugin editors // when opening plugin editors
let wrapper = Arc::new(wrapper); let wrapper = Arc::new(wrapper);
*wrapper.this.write() = Arc::downgrade(&wrapper); *wrapper.this.borrow_mut() = Arc::downgrade(&wrapper);
wrapper wrapper
} }
@ -478,7 +478,7 @@ impl<P: ClapPlugin> Wrapper<P> {
fn make_process_context(&self) -> WrapperProcessContext<'_, P> { fn make_process_context(&self) -> WrapperProcessContext<'_, P> {
WrapperProcessContext { WrapperProcessContext {
wrapper: self, wrapper: self,
input_events_guard: self.input_events.write(), input_events_guard: self.input_events.borrow_mut(),
} }
} }
@ -559,7 +559,7 @@ impl<P: ClapPlugin> Wrapper<P> {
/// Handle all incoming events from an event queue. This will clearn `self.input_events` first. /// Handle all incoming events from an event queue. This will clearn `self.input_events` first.
pub unsafe fn handle_in_events(&self, in_: &clap_input_events) { pub unsafe fn handle_in_events(&self, in_: &clap_input_events) {
let mut input_events = self.input_events.write(); let mut input_events = self.input_events.borrow_mut();
input_events.clear(); input_events.clear();
let num_events = ((*in_).size)(&*in_); let num_events = ((*in_).size)(&*in_);
@ -611,7 +611,7 @@ impl<P: ClapPlugin> Wrapper<P> {
pub unsafe fn handle_event( pub unsafe fn handle_event(
&self, &self,
event: *const clap_event_header, event: *const clap_event_header,
input_events: &mut RwLockWriteGuard<VecDeque<NoteEvent>>, input_events: &mut AtomicRefMut<VecDeque<NoteEvent>>,
) { ) {
let raw_event = &*event; let raw_event = &*event;
match (raw_event.space_id, raw_event.type_) { match (raw_event.space_id, raw_event.type_) {
@ -723,9 +723,12 @@ impl<P: ClapPlugin> Wrapper<P> {
) { ) {
// Preallocate enough room in the output slices vector so we can convert a `*mut *mut // Preallocate enough room in the output slices vector so we can convert a `*mut *mut
// f32` to a `&mut [&mut f32]` in the process call // f32` to a `&mut [&mut f32]` in the process call
wrapper.output_buffer.write().with_raw_vec(|output_slices| { wrapper
output_slices.resize_with(bus_config.num_output_channels as usize, || &mut []) .output_buffer
}); .borrow_mut()
.with_raw_vec(|output_slices| {
output_slices.resize_with(bus_config.num_output_channels as usize, || &mut [])
});
// Also store this for later, so we can reinitialize the plugin after restoring state // Also store this for later, so we can reinitialize the plugin after restoring state
wrapper.current_buffer_config.store(Some(buffer_config)); wrapper.current_buffer_config.store(Some(buffer_config));
@ -808,7 +811,7 @@ impl<P: ClapPlugin> Wrapper<P> {
// TODO: The audio buffers have a latency field, should we use those? // TODO: The audio buffers have a latency field, should we use those?
// TODO: Like with VST3, should we expose some way to access or set the silence/constant // TODO: Like with VST3, should we expose some way to access or set the silence/constant
// flags? // flags?
let mut output_buffer = wrapper.output_buffer.write(); let mut output_buffer = wrapper.output_buffer.borrow_mut();
output_buffer.with_raw_vec(|output_slices| { output_buffer.with_raw_vec(|output_slices| {
nih_debug_assert_eq!(num_output_channels, output_slices.len()); nih_debug_assert_eq!(num_output_channels, output_slices.len());
for (output_channel_idx, output_channel_slice) in for (output_channel_idx, output_channel_slice) in

14
src/wrapper/vst3/context.rs
View file

@ -1,4 +1,4 @@
use parking_lot::RwLockWriteGuard; use atomic_refcell::AtomicRefMut;
use std::collections::VecDeque; use std::collections::VecDeque;
use std::sync::atomic::Ordering; use std::sync::atomic::Ordering;
use std::sync::Arc; use std::sync::Arc;
@ -19,17 +19,17 @@ pub(crate) struct WrapperGuiContext<P: Vst3Plugin> {
/// A [ProcessContext] implementation for the wrapper. This is a separate object so it can hold on /// A [ProcessContext] implementation for the wrapper. This is a separate object so it can hold on
/// to lock guards for event queues. Otherwise reading these events would require constant /// to lock guards for event queues. Otherwise reading these events would require constant
/// unnecessary atomic operations to lock the uncontested RwLocks. /// unnecessary atomic operations to lock the uncontested locks.
pub(crate) struct WrapperProcessContext<'a, P: Vst3Plugin> { pub(crate) struct WrapperProcessContext<'a, P: Vst3Plugin> {
pub(super) inner: &'a WrapperInner<P>, pub(super) inner: &'a WrapperInner<P>,
pub(super) input_events_guard: RwLockWriteGuard<'a, VecDeque<NoteEvent>>, pub(super) input_events_guard: AtomicRefMut<'a, VecDeque<NoteEvent>>,
} }
impl<P: Vst3Plugin> GuiContext for WrapperGuiContext<P> { impl<P: Vst3Plugin> GuiContext for WrapperGuiContext<P> {
// All of these functions are supposed to be called from the main thread, so we'll put some // All of these functions are supposed to be called from the main thread, so we'll put some
// trust in the caller and assume that this is indeed the case // trust in the caller and assume that this is indeed the case
unsafe fn raw_begin_set_parameter(&self, param: ParamPtr) { unsafe fn raw_begin_set_parameter(&self, param: ParamPtr) {
match &*self.inner.component_handler.read() { match &*self.inner.component_handler.borrow() {
Some(handler) => match self.inner.param_ptr_to_hash.get(&param) { Some(handler) => match self.inner.param_ptr_to_hash.get(&param) {
Some(hash) => { Some(hash) => {
handler.begin_edit(*hash); handler.begin_edit(*hash);
@ -41,7 +41,7 @@ impl<P: Vst3Plugin> GuiContext for WrapperGuiContext<P> {
} }
unsafe fn raw_set_parameter_normalized(&self, param: ParamPtr, normalized: f32) { unsafe fn raw_set_parameter_normalized(&self, param: ParamPtr, normalized: f32) {
match &*self.inner.component_handler.read() { match &*self.inner.component_handler.borrow() {
Some(handler) => match self.inner.param_ptr_to_hash.get(&param) { Some(handler) => match self.inner.param_ptr_to_hash.get(&param) {
Some(hash) => { Some(hash) => {
// Only update the parameters manually if the host is not processing audio. If // Only update the parameters manually if the host is not processing audio. If
@ -68,7 +68,7 @@ impl<P: Vst3Plugin> GuiContext for WrapperGuiContext<P> {
} }
unsafe fn raw_end_set_parameter(&self, param: ParamPtr) { unsafe fn raw_end_set_parameter(&self, param: ParamPtr) {
match &*self.inner.component_handler.read() { match &*self.inner.component_handler.borrow() {
Some(handler) => match self.inner.param_ptr_to_hash.get(&param) { Some(handler) => match self.inner.param_ptr_to_hash.get(&param) {
Some(hash) => { Some(hash) => {
handler.end_edit(*hash); handler.end_edit(*hash);
@ -95,7 +95,7 @@ impl<P: Vst3Plugin> ProcessContext for WrapperProcessContext<'_, P> {
// Only trigger a restart if it's actually needed // Only trigger a restart if it's actually needed
let old_latency = self.inner.current_latency.swap(samples, Ordering::SeqCst); let old_latency = self.inner.current_latency.swap(samples, Ordering::SeqCst);
if old_latency != samples { if old_latency != samples {
let task_posted = unsafe { self.inner.event_loop.read().assume_init_ref() } let task_posted = unsafe { self.inner.event_loop.borrow().assume_init_ref() }
.do_maybe_async(Task::TriggerRestart( .do_maybe_async(Task::TriggerRestart(
vst3_sys::vst::RestartFlags::kLatencyChanged as i32, vst3_sys::vst::RestartFlags::kLatencyChanged as i32,
)); ));

23
src/wrapper/vst3/inner.rs
View file

@ -1,3 +1,4 @@
use atomic_refcell::AtomicRefCell;
use crossbeam::atomic::AtomicCell; use crossbeam::atomic::AtomicCell;
use parking_lot::RwLock; use parking_lot::RwLock;
use std::collections::{HashMap, VecDeque}; use std::collections::{HashMap, VecDeque};
@ -29,7 +30,7 @@ pub(crate) struct WrapperInner<P: Vst3Plugin> {
/// The host's `IComponentHandler` instance, if passed through /// The host's `IComponentHandler` instance, if passed through
/// `IEditController::set_component_handler`. /// `IEditController::set_component_handler`.
pub component_handler: RwLock<Option<VstPtr<dyn IComponentHandler>>>, pub component_handler: AtomicRefCell<Option<VstPtr<dyn IComponentHandler>>>,
/// Our own [IPlugView] instance. This is set while the editor is actually visible (which is /// Our own [IPlugView] instance. This is set while the editor is actually visible (which is
/// different form the lifetimei of [super::WrapperView] itself). /// different form the lifetimei of [super::WrapperView] itself).
@ -42,7 +43,7 @@ pub(crate) struct WrapperInner<P: Vst3Plugin> {
/// mutably borrow the event loop, so reads will never be contested. /// mutably borrow the event loop, so reads will never be contested.
/// ///
/// TODO: Is there a better type for Send+Sync late initializaiton? /// TODO: Is there a better type for Send+Sync late initializaiton?
pub event_loop: RwLock<MaybeUninit<OsEventLoop<Task, Self>>>, pub event_loop: AtomicRefCell<MaybeUninit<OsEventLoop<Task, Self>>>,
/// Whether the plugin is currently processing audio. In other words, the last state /// Whether the plugin is currently processing audio. In other words, the last state
/// `IAudioProcessor::setActive()` has been called with. /// `IAudioProcessor::setActive()` has been called with.
@ -63,12 +64,12 @@ pub(crate) struct WrapperInner<P: Vst3Plugin> {
/// apointer to pointers, so this needs to be preallocated in the setup call and kept around /// apointer to pointers, so this needs to be preallocated in the setup call and kept around
/// between process calls. This buffer owns the vector, because otherwise it would need to store /// between process calls. This buffer owns the vector, because otherwise it would need to store
/// a mutable reference to the data contained in this mutex. /// a mutable reference to the data contained in this mutex.
pub output_buffer: RwLock<Buffer<'static>>, pub output_buffer: AtomicRefCell<Buffer<'static>>,
/// The incoming events for the plugin, if `P::ACCEPTS_MIDI` is set. /// The incoming events for the plugin, if `P::ACCEPTS_MIDI` is set.
/// ///
/// TODO: Maybe load these lazily at some point instead of needing to spool them all to this /// TODO: Maybe load these lazily at some point instead of needing to spool them all to this
/// queue first /// queue first
pub input_events: RwLock<VecDeque<NoteEvent>>, pub input_events: AtomicRefCell<VecDeque<NoteEvent>>,
/// The keys from `param_map` in a stable order. /// The keys from `param_map` in a stable order.
pub param_hashes: Vec<u32>, pub param_hashes: Vec<u32>,
@ -109,11 +110,11 @@ impl<P: Vst3Plugin> WrapperInner<P> {
plugin, plugin,
editor, editor,
component_handler: RwLock::new(None), component_handler: AtomicRefCell::new(None),
plug_view: RwLock::new(None), plug_view: RwLock::new(None),
event_loop: RwLock::new(MaybeUninit::uninit()), event_loop: AtomicRefCell::new(MaybeUninit::uninit()),
is_processing: AtomicBool::new(false), is_processing: AtomicBool::new(false),
// Some hosts, like the current version of Bitwig and Ardour at the time of writing, // Some hosts, like the current version of Bitwig and Ardour at the time of writing,
@ -128,8 +129,8 @@ impl<P: Vst3Plugin> WrapperInner<P> {
bypass_state: AtomicBool::new(false), bypass_state: AtomicBool::new(false),
last_process_status: AtomicCell::new(ProcessStatus::Normal), last_process_status: AtomicCell::new(ProcessStatus::Normal),
current_latency: AtomicU32::new(0), current_latency: AtomicU32::new(0),
output_buffer: RwLock::new(Buffer::default()), output_buffer: AtomicRefCell::new(Buffer::default()),
input_events: RwLock::new(VecDeque::with_capacity(512)), input_events: AtomicRefCell::new(VecDeque::with_capacity(512)),
param_hashes: Vec::new(), param_hashes: Vec::new(),
param_by_hash: HashMap::new(), param_by_hash: HashMap::new(),
@ -181,7 +182,7 @@ impl<P: Vst3Plugin> WrapperInner<P> {
// serving multiple plugin instances, Arc can't be used because its reference count // serving multiple plugin instances, Arc can't be used because its reference count
// is separate from the internal COM-style reference count. // is separate from the internal COM-style reference count.
let wrapper: Arc<WrapperInner<P>> = wrapper.into(); let wrapper: Arc<WrapperInner<P>> = wrapper.into();
*wrapper.event_loop.write() = *wrapper.event_loop.borrow_mut() =
MaybeUninit::new(OsEventLoop::new_and_spawn(Arc::downgrade(&wrapper))); MaybeUninit::new(OsEventLoop::new_and_spawn(Arc::downgrade(&wrapper)));
wrapper wrapper
@ -194,7 +195,7 @@ impl<P: Vst3Plugin> WrapperInner<P> {
pub fn make_process_context(&self) -> WrapperProcessContext<'_, P> { pub fn make_process_context(&self) -> WrapperProcessContext<'_, P> {
WrapperProcessContext { WrapperProcessContext {
inner: self, inner: self,
input_events_guard: self.input_events.write(), input_events_guard: self.input_events.borrow_mut(),
} }
} }
@ -237,7 +238,7 @@ impl<P: Vst3Plugin> MainThreadExecutor<Task> for WrapperInner<P> {
// function for checking if a to be scheduled task can be handled right ther and // function for checking if a to be scheduled task can be handled right ther and
// then). // then).
match task { match task {
Task::TriggerRestart(flags) => match &*self.component_handler.read() { Task::TriggerRestart(flags) => match &*self.component_handler.borrow() {
Some(handler) => { Some(handler) => {
handler.restart_component(flags); handler.restart_component(flags);
} }

8
src/wrapper/vst3/wrapper.rs
View file

@ -465,7 +465,7 @@ impl<P: Vst3Plugin> IEditController for Wrapper<P> {
&self, &self,
handler: SharedVstPtr<dyn vst3_sys::vst::IComponentHandler>, handler: SharedVstPtr<dyn vst3_sys::vst::IComponentHandler>,
) -> tresult { ) -> tresult {
*self.inner.component_handler.write() = handler.upgrade().map(VstPtr::from); *self.inner.component_handler.borrow_mut() = handler.upgrade().map(VstPtr::from);
kResultOk kResultOk
} }
@ -599,7 +599,7 @@ impl<P: Vst3Plugin> IAudioProcessor for Wrapper<P> {
// f32` to a `&mut [&mut f32]` in the process call // f32` to a `&mut [&mut f32]` in the process call
self.inner self.inner
.output_buffer .output_buffer
.write() .borrow_mut()
.with_raw_vec(|output_slices| { .with_raw_vec(|output_slices| {
output_slices.resize_with(bus_config.num_output_channels as usize, || &mut []) output_slices.resize_with(bus_config.num_output_channels as usize, || &mut [])
}); });
@ -667,7 +667,7 @@ impl<P: Vst3Plugin> IAudioProcessor for Wrapper<P> {
// And also incoming note events if the plugin accepts MDII // And also incoming note events if the plugin accepts MDII
if P::ACCEPTS_MIDI { if P::ACCEPTS_MIDI {
let mut input_events = self.inner.input_events.write(); let mut input_events = self.inner.input_events.borrow_mut();
if let Some(events) = data.input_events.upgrade() { if let Some(events) = data.input_events.upgrade() {
let num_events = events.get_event_count(); let num_events = events.get_event_count();
@ -731,7 +731,7 @@ impl<P: Vst3Plugin> IAudioProcessor for Wrapper<P> {
// This vector has been preallocated to contain enough slices as there are output // This vector has been preallocated to contain enough slices as there are output
// channels // channels
let mut output_buffer = self.inner.output_buffer.write(); let mut output_buffer = self.inner.output_buffer.borrow_mut();
output_buffer.with_raw_vec(|output_slices| { output_buffer.with_raw_vec(|output_slices| {
nih_debug_assert_eq!(num_output_channels, output_slices.len()); nih_debug_assert_eq!(num_output_channels, output_slices.len());
for (output_channel_idx, output_channel_slice) in for (output_channel_idx, output_channel_slice) in