Don't require Sync for Plugin

In hindsight there's really no reason to, because process() requires
exclusive access and the other functions won't be called during
processing.

BREAKING_CHANGES.md

@@ -8,6 +8,7 @@ code then it will not be listed here.
 
 ## [2022-10-20]
 
+- Similar to the below change, `Plugin` also no longer requires `Sync`.
 - `Editor` and the editor handle returned by `Editor::spawn` now only require
   `Send` and no longer need `Sync`. This is not a breaking change, but it might
   be worth being aware of.

src/plugin.rs

@@ -24,7 +24,7 @@ use crate::wrapper::state::PluginState;
 ///   supported
 /// - Audio thread thread pools (with host integration in CLAP)
 #[allow(unused_variables)]
-pub trait Plugin: Default + Send + Sync + 'static {
+pub trait Plugin: Default + Send + 'static {
     const NAME: &'static str;
     const VENDOR: &'static str;
     const URL: &'static str;

src/wrapper/clap/wrapper.rs

@@ -59,7 +59,7 @@ use clap_sys::stream::{clap_istream, clap_ostream};
 use crossbeam::atomic::AtomicCell;
 use crossbeam::channel::{self, SendTimeoutError};
 use crossbeam::queue::ArrayQueue;
-use parking_lot::{Mutex, RwLock};
+use parking_lot::Mutex;
 use raw_window_handle::RawWindowHandle;
 use std::any::Any;
 use std::cmp;
@@ -104,7 +104,7 @@ pub struct Wrapper<P: ClapPlugin> {
     this: AtomicRefCell<Weak<Self>>,
 
     /// The wrapped plugin instance.
-    plugin: RwLock<P>,
+    plugin: Mutex<P>,
     /// The plugin's parameters. These are fetched once during initialization. That way the
     /// `ParamPtr`s are guaranteed to live at least as long as this object and we can interact with
     /// the `Params` object without having to acquire a lock on `plugin`.
@@ -541,7 +541,7 @@ impl<P: ClapPlugin> Wrapper<P> {
 
             this: AtomicRefCell::new(Weak::new()),
 
-            plugin: RwLock::new(plugin),
+            plugin: Mutex::new(plugin),
             params,
             editor,
             editor_handle: Mutex::new(None),
@@ -1607,7 +1607,7 @@ impl<P: ClapPlugin> Wrapper<P> {
                 self.notify_param_values_changed();
                 let bus_config = self.current_bus_config.load();
                 if let Some(buffer_config) = self.current_buffer_config.load() {
-                    let mut plugin = self.plugin.write();
+                    let mut plugin = self.plugin.lock();
                     plugin.initialize(&bus_config, &buffer_config, &mut self.make_init_context());
                     process_wrapper(|| plugin.reset());
                 }
@@ -1705,7 +1705,7 @@ impl<P: ClapPlugin> Wrapper<P> {
             param.update_smoother(buffer_config.sample_rate, true);
         }
 
-        let mut plugin = wrapper.plugin.write();
+        let mut plugin = wrapper.plugin.lock();
         if plugin.initialize(
             &bus_config,
             &buffer_config,
@@ -1779,7 +1779,7 @@ impl<P: ClapPlugin> Wrapper<P> {
         check_null_ptr!((), plugin);
         let wrapper = &*(plugin as *const Self);
 
-        wrapper.plugin.write().deactivate();
+        wrapper.plugin.lock().deactivate();
     }
 
     unsafe extern "C" fn start_processing(plugin: *const clap_plugin) -> bool {
@@ -1794,7 +1794,7 @@ impl<P: ClapPlugin> Wrapper<P> {
 
         // To be consistent with the VST3 wrapper, we'll also reset the buffers here in addition to
         // the dedicated `reset()` function.
-        process_wrapper(|| wrapper.plugin.write().reset());
+        process_wrapper(|| wrapper.plugin.lock().reset());
 
         true
     }
@@ -1810,7 +1810,7 @@ impl<P: ClapPlugin> Wrapper<P> {
         check_null_ptr!((), plugin);
         let wrapper = &*(plugin as *const Self);
 
-        process_wrapper(|| wrapper.plugin.write().reset());
+        process_wrapper(|| wrapper.plugin.lock().reset());
     }
 
     unsafe extern "C" fn process(
@@ -2189,7 +2189,7 @@ impl<P: ClapPlugin> Wrapper<P> {
                 }
 
                 let result = if buffer_is_valid {
-                    let mut plugin = wrapper.plugin.write();
+                    let mut plugin = wrapper.plugin.lock();
                     // SAFETY: Shortening these borrows is safe as even if the plugin overwrites the
                     //         slices (which it cannot do without using unsafe code), then they
                     //         would still be reset on the next iteration
@@ -2251,7 +2251,7 @@ impl<P: ClapPlugin> Wrapper<P> {
 
                 let bus_config = wrapper.current_bus_config.load();
                 let buffer_config = wrapper.current_buffer_config.load().unwrap();
-                let mut plugin = wrapper.plugin.write();
+                let mut plugin = wrapper.plugin.lock();
                 // FIXME: This is obviously not realtime-safe, but loading presets without doing
                 //         this could lead to inconsistencies. It's the plugin's responsibility to
                 //         not perform any realtime-unsafe work when the initialize function is
@@ -3154,7 +3154,7 @@ impl<P: ClapPlugin> Wrapper<P> {
 
         let bus_config = wrapper.current_bus_config.load();
         if let Some(buffer_config) = wrapper.current_buffer_config.load() {
-            let mut plugin = wrapper.plugin.write();
+            let mut plugin = wrapper.plugin.lock();
             plugin.initialize(
                 &bus_config,
                 &buffer_config,

src/wrapper/standalone/wrapper.rs

@@ -2,7 +2,7 @@ use atomic_refcell::AtomicRefCell;
 use baseview::{EventStatus, Window, WindowHandler, WindowOpenOptions};
 use crossbeam::channel;
 use crossbeam::queue::ArrayQueue;
-use parking_lot::{Mutex, RwLock};
+use parking_lot::Mutex;
 use raw_window_handle::HasRawWindowHandle;
 use std::any::Any;
 use std::collections::{HashMap, HashSet};
@@ -33,7 +33,7 @@ pub struct Wrapper<P: Plugin, B: Backend> {
     backend: AtomicRefCell<B>,
 
     /// The wrapped plugin instance.
-    plugin: RwLock<P>,
+    plugin: Mutex<P>,
     /// The plugin's parameters. These are fetched once during initialization. That way the
     /// `ParamPtr`s are guaranteed to live at least as long as this object and we can interact with
     /// the `Params` object without having to acquire a lock on `plugin`.
@@ -176,7 +176,7 @@ impl<P: Plugin, B: Backend> Wrapper<P, B> {
         let wrapper = Arc::new(Wrapper {
             backend: AtomicRefCell::new(backend),
 
-            plugin: RwLock::new(plugin),
+            plugin: Mutex::new(plugin),
             params,
             known_parameters: param_map.iter().map(|(_, ptr, _)| *ptr).collect(),
             param_map: param_map
@@ -209,7 +209,7 @@ impl<P: Plugin, B: Backend> Wrapper<P, B> {
         // Right now the IO configuration is fixed in the standalone target, so if the plugin cannot
         // work with this then we cannot initialize the plugin at all.
         {
-            let mut plugin = wrapper.plugin.write();
+            let mut plugin = wrapper.plugin.lock();
             if !plugin.accepts_bus_config(&wrapper.bus_config) {
                 return Err(WrapperError::IncompatibleConfig {
                     input_channels: wrapper.bus_config.num_input_channels,
@@ -309,7 +309,7 @@ impl<P: Plugin, B: Backend> Wrapper<P, B> {
 
         // Some plugins may use this to clean up resources. Should not be needed for the standalone
         // application, but it seems like a good idea to stay consistent.
-        self.plugin.write().deactivate();
+        self.plugin.lock().deactivate();
 
         Ok(())
     }
@@ -386,7 +386,7 @@ impl<P: Plugin, B: Backend> Wrapper<P, B> {
                     }
 
                     let sample_rate = self.buffer_config.sample_rate;
-                    let mut plugin = self.plugin.write();
+                    let mut plugin = self.plugin.lock();
                     if let ProcessStatus::Error(err) = plugin.process(
                         buffer,
                         // TODO: Provide extra inputs and outputs in the JACk backend

src/wrapper/vst3/inner.rs

@@ -29,7 +29,7 @@ use crate::wrapper::util::{hash_param_id, process_wrapper};
 /// its own struct.
 pub(crate) struct WrapperInner<P: Vst3Plugin> {
     /// The wrapped plugin instance.
-    pub plugin: RwLock<P>,
+    pub plugin: Mutex<P>,
     /// The plugin's parameters. These are fetched once during initialization. That way the
     /// `ParamPtr`s are guaranteed to live at least as long as this object and we can interact with
     /// the `Params` object without having to acquire a lock on `plugin`.
@@ -272,7 +272,7 @@ impl<P: Vst3Plugin> WrapperInner<P> {
             .collect();
 
         let wrapper = Self {
-            plugin: RwLock::new(plugin),
+            plugin: Mutex::new(plugin),
             params,
             editor,
 
@@ -477,7 +477,7 @@ impl<P: Vst3Plugin> WrapperInner<P> {
                 self.notify_param_values_changed();
                 let bus_config = self.current_bus_config.load();
                 if let Some(buffer_config) = self.current_buffer_config.load() {
-                    let mut plugin = self.plugin.write();
+                    let mut plugin = self.plugin.lock();
                     plugin.initialize(&bus_config, &buffer_config, &mut self.make_init_context());
                     process_wrapper(|| plugin.reset());
                 }

src/wrapper/vst3/wrapper.rs

@@ -389,7 +389,7 @@ impl<P: Vst3Plugin> IComponent for Wrapper<P> {
                 }
 
                 let bus_config = self.inner.current_bus_config.load();
-                let mut plugin = self.inner.plugin.write();
+                let mut plugin = self.inner.plugin.lock();
                 if plugin.initialize(
                     &bus_config,
                     &buffer_config,
@@ -461,7 +461,7 @@ impl<P: Vst3Plugin> IComponent for Wrapper<P> {
             }
             (true, None) => kResultFalse,
             (false, _) => {
-                self.inner.plugin.write().deactivate();
+                self.inner.plugin.lock().deactivate();
 
                 kResultOk
             }
@@ -519,7 +519,7 @@ impl<P: Vst3Plugin> IComponent for Wrapper<P> {
 
         let bus_config = self.inner.current_bus_config.load();
         if let Some(buffer_config) = self.inner.current_buffer_config.load() {
-            let mut plugin = self.inner.plugin.write();
+            let mut plugin = self.inner.plugin.lock();
             plugin.initialize(
                 &bus_config,
                 &buffer_config,
@@ -864,7 +864,7 @@ impl<P: Vst3Plugin> IAudioProcessor for Wrapper<P> {
         if self
             .inner
             .plugin
-            .read()
+            .lock()
             .accepts_bus_config(&proposed_config)
         {
             self.inner.current_bus_config.store(proposed_config);
@@ -1001,7 +1001,7 @@ impl<P: Vst3Plugin> IAudioProcessor for Wrapper<P> {
         // 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 {
-            process_wrapper(|| self.inner.plugin.write().reset());
+            process_wrapper(|| self.inner.plugin.lock().reset());
         }
 
         // We don't have any special handling for suspending and resuming plugins, yet
@@ -1530,7 +1530,7 @@ impl<P: Vst3Plugin> IAudioProcessor for Wrapper<P> {
                 }
 
                 let result = if buffer_is_valid {
-                    let mut plugin = self.inner.plugin.write();
+                    let mut plugin = self.inner.plugin.lock();
                     // SAFETY: Shortening these borrows is safe as even if the plugin overwrites the
                     //         slices (which it cannot do without using unsafe code), then they
                     //         would still be reset on the next iteration
@@ -1784,7 +1784,7 @@ impl<P: Vst3Plugin> IAudioProcessor for Wrapper<P> {
 
                 let bus_config = self.inner.current_bus_config.load();
                 let buffer_config = self.inner.current_buffer_config.load().unwrap();
-                let mut plugin = self.inner.plugin.write();
+                let mut plugin = self.inner.plugin.lock();
                 // FIXME: This is obviously not realtime-safe, but loading presets without doing
                 //         this could lead to inconsistencies. It's the plugin's responsibility to
                 //         not perform any realtime-unsafe work when the initialize function is