diff --git a/CHANGELOG.md b/CHANGELOG.md index 29d69e2f..4306ffac 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,11 +24,16 @@ Versioning](https://semver.org/spec/v2.0.0.html). - Redesigned the VST3 audio socket handling to be able to reuse the process data objects on both sides. This greatly reduces the overhead of our VST3 bridging by getting rid of all memory allocations during audio processing. +- Changed the way mutual recursion in VST3 plugins on the plugin side works to + counter any potential GUI related timing issues with VST3 plugins. ### Fixed -- Prevent _DMG_ VST3 plugins from freezing in **REAPER** under certain - circumstances. +- Fixed _DMG_ VST3 plugins freezing in **REAPER** when the plugin resizes itself + while the host passes channel context information to the plugin. +- Also fixed _DMG_ VST3 plugins freezing in **REAPER** when restoring multiple + instances of the plugin at once while the FX window is open and the GUI is + visible. - Fixed builds on Wine 6.8 because of internal changes to Wine's `windows.h` implementation. diff --git a/docs/architecture.md b/docs/architecture.md index 2af63d8d..d7ccfb1c 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -74,7 +74,7 @@ Lastly there are a few specific situations where the above two issues of mutual recursion and functions that can only be called from a single thread are combined. In those cases we need to the send over the socket on a new thread, so that the calling thread can handle other tasks through another IO context. See -`Vst3PlugViewProxyImpl::send_mutually_recursive_message()` and +`Vst3HostBridge::send_mutually_recursive_message()` and `Vst3Bridge::send_mutually_recursive_message()` for the actual implementation with more details. This applies to the functions related to resizing VST3 editors on both the Linux and the Wine sides. diff --git a/src/plugin/bridges/vst3-impls/plug-view-proxy.cpp b/src/plugin/bridges/vst3-impls/plug-view-proxy.cpp index 7f39f80c..2dedaed9 100644 --- a/src/plugin/bridges/vst3-impls/plug-view-proxy.cpp +++ b/src/plugin/bridges/vst3-impls/plug-view-proxy.cpp @@ -85,21 +85,16 @@ RunLoopTasks::onFDIsSet(Steinberg::Linux::FileDescriptor /*fd*/) { Vst3PlugViewProxyImpl::Vst3PlugViewProxyImpl( Vst3PluginBridge& bridge, - std::atomic_bool& is_active, Vst3PlugViewProxy::ConstructArgs&& args) noexcept - : Vst3PlugViewProxy(std::move(args)), bridge(bridge), is_active(is_active) { - is_active = true; -} + : Vst3PlugViewProxy(std::move(args)), bridge(bridge) {} Vst3PlugViewProxyImpl::~Vst3PlugViewProxyImpl() noexcept { - is_active = false; - // Also drop the plug view smart pointer on the Wine side when this gets // dropped // NOTE: This can actually throw (e.g. out of memory or the socket got // closed). But if that were to happen, then we wouldn't be able to // recover from it anyways. - send_mutually_recursive_message( + bridge.send_mutually_recursive_message( Vst3PlugViewProxy::Destruct{.owner_instance_id = owner_instance_id()}); } @@ -117,7 +112,7 @@ Vst3PlugViewProxyImpl::isPlatformTypeSupported(Steinberg::FIDString type) { if (type) { // We'll swap the X11 window ID platform type string for the Win32 HWND // equivalent on the Wine side - return send_mutually_recursive_message( + return bridge.send_mutually_recursive_message( YaPlugView::IsPlatformTypeSupported{ .owner_instance_id = owner_instance_id(), .type = type}); } else { @@ -133,7 +128,7 @@ tresult PLUGIN_API Vst3PlugViewProxyImpl::attached(void* parent, if (parent && type) { // We will embed the Wine Win32 window into the X11 window provided by // the host - return send_mutually_recursive_message(YaPlugView::Attached{ + return bridge.send_mutually_recursive_message(YaPlugView::Attached{ .owner_instance_id = owner_instance_id(), .parent = reinterpret_cast(parent), .type = type}); @@ -145,19 +140,19 @@ tresult PLUGIN_API Vst3PlugViewProxyImpl::attached(void* parent, } tresult PLUGIN_API Vst3PlugViewProxyImpl::removed() { - return send_mutually_recursive_message( + return bridge.send_mutually_recursive_message( YaPlugView::Removed{.owner_instance_id = owner_instance_id()}); } tresult PLUGIN_API Vst3PlugViewProxyImpl::onWheel(float distance) { - return send_mutually_recursive_message(YaPlugView::OnWheel{ + return bridge.send_mutually_recursive_message(YaPlugView::OnWheel{ .owner_instance_id = owner_instance_id(), .distance = distance}); } tresult PLUGIN_API Vst3PlugViewProxyImpl::onKeyDown(char16 key, int16 keyCode, int16 modifiers) { - return send_mutually_recursive_message( + return bridge.send_mutually_recursive_message( YaPlugView::OnKeyDown{.owner_instance_id = owner_instance_id(), .key = key, .key_code = keyCode, @@ -167,7 +162,7 @@ tresult PLUGIN_API Vst3PlugViewProxyImpl::onKeyDown(char16 key, tresult PLUGIN_API Vst3PlugViewProxyImpl::onKeyUp(char16 key, int16 keyCode, int16 modifiers) { - return send_mutually_recursive_message( + return bridge.send_mutually_recursive_message( YaPlugView::OnKeyUp{.owner_instance_id = owner_instance_id(), .key = key, .key_code = keyCode, @@ -176,7 +171,7 @@ tresult PLUGIN_API Vst3PlugViewProxyImpl::onKeyUp(char16 key, tresult PLUGIN_API Vst3PlugViewProxyImpl::getSize(Steinberg::ViewRect* size) { if (size) { - const GetSizeResponse response = send_mutually_recursive_message( + const GetSizeResponse response = bridge.send_mutually_recursive_message( YaPlugView::GetSize{.owner_instance_id = owner_instance_id()}); *size = response.size; @@ -191,7 +186,7 @@ tresult PLUGIN_API Vst3PlugViewProxyImpl::getSize(Steinberg::ViewRect* size) { tresult PLUGIN_API Vst3PlugViewProxyImpl::onSize(Steinberg::ViewRect* newSize) { if (newSize) { - return send_mutually_recursive_message(YaPlugView::OnSize{ + return bridge.send_mutually_recursive_message(YaPlugView::OnSize{ .owner_instance_id = owner_instance_id(), .new_size = *newSize}); } else { bridge.logger.log( @@ -201,7 +196,7 @@ tresult PLUGIN_API Vst3PlugViewProxyImpl::onSize(Steinberg::ViewRect* newSize) { } tresult PLUGIN_API Vst3PlugViewProxyImpl::onFocus(TBool state) { - return send_mutually_recursive_message(YaPlugView::OnFocus{ + return bridge.send_mutually_recursive_message(YaPlugView::OnFocus{ .owner_instance_id = owner_instance_id(), .state = state}); } @@ -232,7 +227,7 @@ Vst3PlugViewProxyImpl::setFrame(Steinberg::IPlugFrame* frame) { std::string(e.what())); } - return send_mutually_recursive_message(YaPlugView::SetFrame{ + return bridge.send_mutually_recursive_message(YaPlugView::SetFrame{ .owner_instance_id = owner_instance_id(), .plug_frame_args = Vst3PlugFrameProxy::ConstructArgs( plug_frame, owner_instance_id())}); @@ -240,7 +235,7 @@ Vst3PlugViewProxyImpl::setFrame(Steinberg::IPlugFrame* frame) { plug_frame.reset(); run_loop_tasks.reset(); - return send_mutually_recursive_message( + return bridge.send_mutually_recursive_message( YaPlugView::SetFrame{.owner_instance_id = owner_instance_id(), .plug_frame_args = std::nullopt}); } @@ -263,7 +258,8 @@ tresult PLUGIN_API Vst3PlugViewProxyImpl::canResize() { } } - const UniversalTResult result = send_mutually_recursive_message(request); + const UniversalTResult result = + bridge.send_mutually_recursive_message(request); { std::lock_guard lock(can_resize_cache_mutex); @@ -277,8 +273,9 @@ tresult PLUGIN_API Vst3PlugViewProxyImpl::checkSizeConstraint(Steinberg::ViewRect* rect) { if (rect) { const CheckSizeConstraintResponse response = - send_mutually_recursive_message(YaPlugView::CheckSizeConstraint{ - .owner_instance_id = owner_instance_id(), .rect = *rect}); + bridge.send_mutually_recursive_message( + YaPlugView::CheckSizeConstraint{ + .owner_instance_id = owner_instance_id(), .rect = *rect}); *rect = response.updated_rect; @@ -296,7 +293,7 @@ tresult PLUGIN_API Vst3PlugViewProxyImpl::findParameter( int32 yPos, Steinberg::Vst::ParamID& resultTag /*out*/) { const FindParameterResponse response = - send_mutually_recursive_message(YaParameterFinder::FindParameter{ + bridge.send_mutually_recursive_message(YaParameterFinder::FindParameter{ .owner_instance_id = owner_instance_id(), .x_pos = xPos, .y_pos = yPos}); @@ -308,7 +305,7 @@ tresult PLUGIN_API Vst3PlugViewProxyImpl::findParameter( tresult PLUGIN_API Vst3PlugViewProxyImpl::setContentScaleFactor(ScaleFactor factor) { - return send_mutually_recursive_message( + return bridge.send_mutually_recursive_message( YaPlugViewContentScaleSupport::SetContentScaleFactor{ .owner_instance_id = owner_instance_id(), .factor = factor}); } diff --git a/src/plugin/bridges/vst3-impls/plug-view-proxy.h b/src/plugin/bridges/vst3-impls/plug-view-proxy.h index 146d2d50..8dc0a506 100644 --- a/src/plugin/bridges/vst3-impls/plug-view-proxy.h +++ b/src/plugin/bridges/vst3-impls/plug-view-proxy.h @@ -20,8 +20,6 @@ #include "../vst3.h" -#include - /** * A RAII wrapper around `IRunLoop`'s event handlers so we can schedule tasks to * be run in it. This is needed for REAPER, because function calls that involve @@ -106,7 +104,6 @@ class RunLoopTasks : public Steinberg::Linux::IEventHandler { class Vst3PlugViewProxyImpl : public Vst3PlugViewProxy { public: Vst3PlugViewProxyImpl(Vst3PluginBridge& bridge, - std::atomic_bool& is_active, Vst3PlugViewProxy::ConstructArgs&& args) noexcept; /** @@ -138,35 +135,26 @@ class Vst3PlugViewProxyImpl : public Vst3PlugViewProxy { * in `main_context` when no mutually recursive function calls are happening * right now. * - * @see send_mutually_recursive_message + * @see Vst3HostBridge::send_mutually_recursive_message */ template T run_gui_task(F f) { std::packaged_task do_call(std::move(f)); std::future do_call_response = do_call.get_future(); - // If `send_mutually_recursive_message()` is currently being called - // (because the host is calling one of `IPlugView`'s methods from its - // UGI thread) then we'll post a message to an IO context that's + // If `Vst3Bridge::send_mutually_recursive_message()` is currently being + // called (because the host is calling one of `IPlugView`'s methods from + // its UGI thread) then we'll post a message to an IO context that's // currently accepting work on the that thread. Since in theory we could // have nested mutual recursion, we need to keep track of a stack of IO // contexts. Great. Otherwise we'll schedule the task to be run from an // event handler registered to the host's run loop. If the host does not // support `IRunLoop`, we'll just run `f` directly. - { - std::unique_lock mutual_recursion_lock( - mutual_recursion_contexts_mutex); - if (!mutual_recursion_contexts.empty()) { - boost::asio::dispatch(*mutual_recursion_contexts.back(), - std::move(do_call)); + if (!bridge.maybe_run_on_mutual_recursion_thread(do_call)) { + if (run_loop_tasks) { + run_loop_tasks->schedule(std::move(do_call)); } else { - mutual_recursion_lock.unlock(); - - if (run_loop_tasks) { - run_loop_tasks->schedule(std::move(do_call)); - } else { - do_call(); - } + do_call(); } } @@ -202,65 +190,6 @@ class Vst3PlugViewProxyImpl : public Vst3PlugViewProxy { // From `IPlugViewContentScaleSupport` tresult PLUGIN_API setContentScaleFactor(ScaleFactor factor) override; - /** - * Send a message from this `IPlugView` instance. This function will be - * called by the host on its GUI thread, so until this function returns - * we'll know that the no `IRunLoop` event handlers will be called. Because - * of this we'll have to use this function to handling mutually recursive - * function calls, such as the calling sequence for resizing views. This - * should be used instead of sending the messages directly. - * - * We use the same trick in `Vst3Bridge`. - */ - template - typename T::Response send_mutually_recursive_message(const T& object) { - using TResponse = typename T::Response; - - // This IO context will accept incoming calls from `run_gui_task()` - // until we receive a response. We keep these on a stack as we need to - // support multiple levels of mutual recursion. This could happen during - // `IPlugView::attached() -> IPlugFrame::resizeView() -> - // IPlugView::onSize()`. - std::shared_ptr current_io_context = - std::make_shared(); - { - std::unique_lock lock(mutual_recursion_contexts_mutex); - mutual_recursion_contexts.push_back(current_io_context); - } - - // Instead of directly stopping the IO context, we'll reset this work - // guard instead. This prevents us from accidentally cancelling any - // outstanding tasks. - auto work_guard = boost::asio::make_work_guard(*current_io_context); - - // We will call the function from another thread so we can handle calls - // to from this thread - std::promise response_promise{}; - std::jthread sending_thread([&]() { - set_realtime_priority(true); - - const TResponse response = bridge.send_message(object); - - // Stop accepting additional work to be run from the calling thread - // once we receive a response. By resetting the work guard we do not - // cancel any pending tasks, but `current_io_context->run()` will - // stop blocking eventually. - std::lock_guard lock(mutual_recursion_contexts_mutex); - work_guard.reset(); - mutual_recursion_contexts.erase( - std::find(mutual_recursion_contexts.begin(), - mutual_recursion_contexts.end(), current_io_context)); - - response_promise.set_value(response); - }); - - // Accept work from the other thread until we receive a response, at - // which point the context will be stopped - current_io_context->run(); - - return response_promise.get_future().get(); - } - /** * The `IPlugFrame` object passed by the host passed to us in * `IPlugView::setFrame()`. When the plugin makes a callback on the @@ -271,28 +200,6 @@ class Vst3PlugViewProxyImpl : public Vst3PlugViewProxy { private: Vst3PluginBridge& bridge; - /** - * We'll use this to signal to the `Vst3PluginProxyImpl` that this object - * has been destroyed. We use this to handle mutual recursion when - * `IEditController::setState()` calls end up calling - * `IPlugFrame::resizeView()`, which should also be handled from the GUI - * thread. - */ - std::atomic_bool& is_active; - - /** - * The IO contexts used in `send_mutually_recursive_message()` to be able to - * execute functions from that same calling thread while we're waiting for a - * response. We need an entire stack of these to support mutual recursion, - * how fun! See the docstring there for more information. When this doesn't - * contain an IO context, this function is not being called and - * `run_gui_task()` should post the task to `run_loop_tasks`. This works - * exactly the same as the mutual recursion handling in `Vst3Bridge`. - */ - std::vector> - mutual_recursion_contexts; - std::mutex mutual_recursion_contexts_mutex; - /** * If the host supports `IRunLoop`, we'll use this to run certain tasks from * the host's GUI thread using a run loop event handler in diff --git a/src/plugin/bridges/vst3-impls/plugin-proxy.cpp b/src/plugin/bridges/vst3-impls/plugin-proxy.cpp index d5b1650c..11f680a3 100644 --- a/src/plugin/bridges/vst3-impls/plugin-proxy.cpp +++ b/src/plugin/bridges/vst3-impls/plugin-proxy.cpp @@ -415,7 +415,7 @@ tresult PLUGIN_API Vst3PluginProxyImpl::setState(Steinberg::IBStream* state) { // 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. - return maybe_send_mutually_recursive_message(Vst3PluginProxy::SetState{ + return bridge.send_mutually_recursive_message(Vst3PluginProxy::SetState{ .instance_id = instance_id(), .state = state}); } else { bridge.logger.log( @@ -437,7 +437,7 @@ tresult PLUGIN_API Vst3PluginProxyImpl::getState(Steinberg::IBStream* state) { // into a situation where we need mutually recursive function // calls. const GetStateResponse response = - maybe_send_mutually_recursive_message(Vst3PluginProxy::GetState{ + bridge.send_mutually_recursive_message(Vst3PluginProxy::GetState{ .instance_id = instance_id(), .state = state}); assert(response.state.write_back(state) == Steinberg::kResultOk); @@ -784,9 +784,8 @@ Vst3PluginProxyImpl::createView(Steinberg::FIDString name) { if (response.plug_view_args) { // The host should manage this. Returning raw pointers feels scary. - auto plug_view_proxy = - new Vst3PlugViewProxyImpl(bridge, last_created_plug_view_active, - std::move(*response.plug_view_args)); + auto plug_view_proxy = new Vst3PlugViewProxyImpl( + bridge, std::move(*response.plug_view_args)); // We also need to store an (unmanaged, since we don't want to // affect the reference counting) pointer to this to be able to @@ -843,7 +842,7 @@ tresult PLUGIN_API Vst3PluginProxyImpl::setChannelContextInfos( // these things need to be handled on the GUI thread on their // receiving sides, resulting in a deadlock without this mutual // recursion. - return maybe_send_mutually_recursive_message( + return bridge.send_mutually_recursive_message( YaInfoListener::SetChannelContextInfos{ .instance_id = instance_id(), .list = YaAttributeList::read_channel_context(list)}); diff --git a/src/plugin/bridges/vst3-impls/plugin-proxy.h b/src/plugin/bridges/vst3-impls/plugin-proxy.h index 5fc6e94e..dd56c612 100644 --- a/src/plugin/bridges/vst3-impls/plugin-proxy.h +++ b/src/plugin/bridges/vst3-impls/plugin-proxy.h @@ -340,12 +340,6 @@ class Vst3PluginProxyImpl : public Vst3PluginProxy { */ Vst3PlugViewProxyImpl* last_created_plug_view = nullptr; - /** - * Whether `last_created_plug_view` is currently active. This field is - * written to from `Vst3PlugViewProxyImpl`'s constructor and destructor. - */ - std::atomic_bool last_created_plug_view_active = false; - /** * A pointer to a context menu returned by the host as a response to a call * to `IComponentHandler3::createContextMenu`, as well as all targets we've @@ -417,40 +411,6 @@ class Vst3PluginProxyImpl : public Vst3PluginProxy { */ void clear_bus_cache() noexcept; - /** - * 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; /** diff --git a/src/plugin/bridges/vst3.h b/src/plugin/bridges/vst3.h index ad935ce0..ac8e7145 100644 --- a/src/plugin/bridges/vst3.h +++ b/src/plugin/bridges/vst3.h @@ -23,6 +23,8 @@ #include "common.h" #include "vst3-impls/plugin-factory-proxy.h" +#include + // Forward declarations class Vst3PluginProxyImpl; @@ -136,6 +138,91 @@ class Vst3PluginBridge : PluginBridge> { std::pair(logger, true)); } + /** + * Send a message, and allow other threads to call functions on _this + * thread_ while we're waiting for a response. This lets us execute + * functions from the host's GUI thread while it is also calling functions + * from that same thread. Because of that, we also know that while this + * function is being called the host won't be able to handle any `IRunLoop` + * events. We need this to support REAPER, because REAPER requires function + * calls involving the GUI to be run from the GUI thread. Grep for + * `run_gui_task` for instances of this. + * + * We use the same trick in `Vst3Bridge`. + */ + template + typename T::Response send_mutually_recursive_message(const T& object) { + using TResponse = typename T::Response; + + // This IO context will accept incoming calls from `run_gui_task()` + // until we receive a response. We keep these on a stack as we need to + // support multiple levels of mutual recursion. This could happen during + // `IPlugView::attached() -> IPlugFrame::resizeView() -> + // IPlugView::onSize()`. + std::shared_ptr current_io_context = + std::make_shared(); + { + std::unique_lock lock(mutual_recursion_contexts_mutex); + mutual_recursion_contexts.push_back(current_io_context); + } + + // Instead of directly stopping the IO context, we'll reset this work + // guard instead. This prevents us from accidentally cancelling any + // outstanding tasks. + auto work_guard = boost::asio::make_work_guard(*current_io_context); + + // We will call the function from another thread so we can handle calls + // to from this thread + std::promise response_promise{}; + std::jthread sending_thread([&]() { + set_realtime_priority(true); + + const TResponse response = send_message(object); + + // Stop accepting additional work to be run from the calling thread + // once we receive a response. By resetting the work guard we do not + // cancel any pending tasks, but `current_io_context->run()` will + // stop blocking eventually. + std::lock_guard lock(mutual_recursion_contexts_mutex); + work_guard.reset(); + mutual_recursion_contexts.erase( + std::find(mutual_recursion_contexts.begin(), + mutual_recursion_contexts.end(), current_io_context)); + + response_promise.set_value(response); + }); + + // Accept work from the other thread until we receive a response, at + // which point the context will be stopped + current_io_context->run(); + + return response_promise.get_future().get(); + } + + /** + * If `send_mutually_recursive_message()` is currently being called, then + * run `cb` on the thread that's currently calling that function. If there's + * currently no mutually recursive function call going on, this will return + * false, and the caller should call `cb` itself. + * + * @return Whether `cb` was scheduled to run on the mutual recursion thread. + * + * @see Vst3PlugViewProxyImpl::run_gui_task + */ + template + bool maybe_run_on_mutual_recursion_thread(F& cb) { + // We're handling an `F&` here because we cannot copy a + // `packged_task()`, and we need to be able to move that actual task + std::unique_lock mutual_recursion_lock(mutual_recursion_contexts_mutex); + if (!mutual_recursion_contexts.empty()) { + boost::asio::dispatch(*mutual_recursion_contexts.back(), + std::move(cb)); + return true; + } else { + return false; + } + } + /** * The logging facility used for this instance of yabridge. Wraps around * `PluginBridge::generic_logger`. @@ -174,4 +261,18 @@ class Vst3PluginBridge : PluginBridge> { private: std::mutex plugin_proxies_mutex; + + /** + * The IO contexts used in `send_mutually_recursive_message()` to be able to + * execute functions from a function's calling thread while we're waiting + * for a response. We need an entire stack of these to support mutual + * recursion, how fun! See the docstring there for more information. When + * this doesn't contain an IO context, this function is not being called and + * `Vst3PlugViewProxyImpl::run_gui_task()` should post the task to + * `Vst3PlugViewProxyImpl::run_loop_tasks`. This works exactly the same as + * the mutual recursion handling in `Vst3Bridge`. + */ + std::vector> + mutual_recursion_contexts; + std::mutex mutual_recursion_contexts_mutex; };