From 62efc1c273a378cbd59b5a853bf9a841be2ea80a Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Sun, 2 May 2021 16:34:32 +0200 Subject: [PATCH] Also handle mutual recursion in *::getState() VST3 plugins could freeze in REAPER when the plugin sends `IComponentHandler::performEdit()` followed by `IPlugFrame::resizeView()` when REAPER simultaneously tries to call `*::getState()`. Here `*::getState()` gets called from the GUI thread, while `IPlugFrame::resizeView()` has to be handled on REAPER's GUI thread, resulting in a deadlock unless we use the plugin-side mutual recursion mechanism. I've seen this cause issues with PSPaudioware InfiniStrip. --- CHANGELOG.md | 3 ++ .../bridges/vst3-impls/plugin-proxy.cpp | 31 ++++++----------- src/plugin/bridges/vst3-impls/plugin-proxy.h | 34 +++++++++++++++++++ 3 files changed, 47 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index aa3c1679..de17b830 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -114,6 +114,9 @@ Versioning](https://semver.org/spec/v2.0.0.html). - Fixed _UVI Plugsound Free_ crashing during initialization. - Fixed potential freezing when loading a VST3 preset that tries to resize an open editor window. +- Fixed another potential freezing issue in REAPER that could happen when the + when the plugin resizes itself while sending a parameter change to the host, + if REAPER's 'disable saving full plug-in state' is not disabled. - Because of the new transport information prefetching, the excessive DSP usage in _SWAM Cello_ is now been fixed without requiring any manual compatibility options. diff --git a/src/plugin/bridges/vst3-impls/plugin-proxy.cpp b/src/plugin/bridges/vst3-impls/plugin-proxy.cpp index 60218079..abf5908d 100644 --- a/src/plugin/bridges/vst3-impls/plugin-proxy.cpp +++ b/src/plugin/bridges/vst3-impls/plugin-proxy.cpp @@ -362,32 +362,14 @@ tresult PLUGIN_API Vst3PluginProxyImpl::setState(Steinberg::IBStream* state) { if (state) { // Since both interfaces contain this function, this is used for both // `IComponent::setState()` as well as `IEditController::setState()` - const Vst3PluginProxy::SetState message{.instance_id = instance_id(), - .state = state}; - // NOTE: This will likely be called from the GUI thread, and some // plugins will try to resize as part of setting their new state. // That `IPlugFrame::resizeView()` _also_ has to be handled on the // GUI thread. So if the GUI is active, we'll use the mutual // recursion mechanism to allow this resize call to also be // performed from the GUI thread. - if (last_created_plug_view_active) { - return last_created_plug_view->send_mutually_recursive_message( - std::move(message)); - } else if (connected_instance_id) { - // We should also be able to handle the above situation when a - // `setState()` on a processor triggers a resize coming from the - // edit controller. To do that, we'll also check if the connected - // instance has an active plug view. - Vst3PluginProxyImpl& other_instance = - bridge.plugin_proxies.at(*connected_instance_id).get(); - if (other_instance.last_created_plug_view_active) { - return other_instance.last_created_plug_view - ->send_mutually_recursive_message(std::move(message)); - } - } - - return bridge.send_message(std::move(message)); + return maybe_send_mutually_recursive_message(Vst3PluginProxy::SetState{ + .instance_id = instance_id(), .state = state}); } else { bridge.logger.log( "WARNING: Null pointer passed to " @@ -400,8 +382,15 @@ tresult PLUGIN_API Vst3PluginProxyImpl::getState(Steinberg::IBStream* state) { if (state) { // Since both interfaces contain this function, this is used for both // `IComponent::getState()` as well as `IEditController::getState()` + // NOTE: This will likely be called from the GUI thread, if the plugin + // somehow ends up sending a resize while this happens, we should + // end up in a deadlock. Normally this wouldn't be an issue, but + // REAPER will fetch the complete plugin state from time to time, + // so when changing a parameter also resizes the GUI we can run + // into a situation where we need mutually recursive function + // calls. const GetStateResponse response = - bridge.send_message(Vst3PluginProxy::GetState{ + maybe_send_mutually_recursive_message(Vst3PluginProxy::GetState{ .instance_id = instance_id(), .state = state}); assert(response.state.write_back(state) == Steinberg::kResultOk); diff --git a/src/plugin/bridges/vst3-impls/plugin-proxy.h b/src/plugin/bridges/vst3-impls/plugin-proxy.h index 386abd7b..a5364aa8 100644 --- a/src/plugin/bridges/vst3-impls/plugin-proxy.h +++ b/src/plugin/bridges/vst3-impls/plugin-proxy.h @@ -442,6 +442,40 @@ class Vst3PluginProxyImpl : public Vst3PluginProxy { */ void clear_parameter_cache(); + /** + * If we have an active `IPlugView` instance, try to use the mutual + * recursion mechanism so that callbacks made by the plugin can be handled + * on this same thread. In case this is an audio processor with a separate + * edit controller, we'll also check if the object we're connected to has an + * active `IPlugView` instance. When there's no active `IPlugView` instance, + * we'll just send the event message like normal. This is needed to be able + * to handle function calls made by the host (which is mostly relevant for + * REAPER) on the GUI thread, when the plugin makes a callback to the host + * that should also be handled on that same thread (context menus and + * plugin-driven resizes). + */ + template + typename T::Response maybe_send_mutually_recursive_message( + const T& object) { + if (last_created_plug_view_active) { + return last_created_plug_view->send_mutually_recursive_message( + std::move(object)); + } else if (connected_instance_id) { + // We should also be able to handle the above situation when a + // `setState()` on a processor triggers a resize coming from the + // edit controller. To do that, we'll also check if the connected + // instance has an active plug view. + Vst3PluginProxyImpl& other_instance = + bridge.plugin_proxies.at(*connected_instance_id).get(); + if (other_instance.last_created_plug_view_active) { + return other_instance.last_created_plug_view + ->send_mutually_recursive_message(std::move(object)); + } + } + + return bridge.send_message(std::move(object)); + } + Vst3PluginBridge& bridge; /**