Separate mutual recursion on GUI and other threads

I wasn't able to get this to clash, but this way we can be 100% sure
that there aren't any weird issues.

src/wine-host/bridges/vst3-impls/component-handler-proxy.cpp

@@ -61,7 +61,7 @@ Vst3ComponentHandlerProxyImpl::endEdit(Steinberg::Vst::ParamID id) {
 
 tresult PLUGIN_API
 Vst3ComponentHandlerProxyImpl::restartComponent(int32 flags) {
-    return bridge.send_mutually_recursive_message(
+    return bridge.send_mutually_recursive_message<false>(
         YaComponentHandler::RestartComponent{
             .owner_instance_id = owner_instance_id(), .flags = flags});
 }

src/wine-host/bridges/vst3-impls/connection-point-proxy.cpp

@@ -56,7 +56,7 @@ Vst3ConnectionPointProxyImpl::notify(Steinberg::Vst::IMessage* message) {
         // need to use our mutual recursion mechanism. Luckily only Ardour uses
         // connection proxies, so if this ends up breaking something it will
         // only affect Ardour.
-        return bridge.send_mutually_recursive_message(
+        return bridge.send_mutually_recursive_message<true>(
             YaConnectionPoint::Notify{.instance_id = owner_instance_id(),
                                       .message_ptr = YaMessagePtr(*message)});
     } else {

src/wine-host/bridges/vst3-impls/plug-frame-proxy.cpp

@@ -46,8 +46,9 @@ Vst3PlugFrameProxyImpl::resizeView(Steinberg::IPlugView* /*view*/,
         // We have to use this special sending function here so we can handle
         // calls to `IPlugView::onSize()` from this same thread (the UI thread).
         // See the docstring for more information.
-        return bridge.send_mutually_recursive_message(YaPlugFrame::ResizeView{
+        return bridge.send_mutually_recursive_message<true>(
-            .owner_instance_id = owner_instance_id(), .new_size = *newSize});
+            YaPlugFrame::ResizeView{.owner_instance_id = owner_instance_id(),
+                                    .new_size = *newSize});
     } else {
         std::cerr
             << "WARNING: Null pointer passed to 'IPlugFrame::resizeView()'"

src/wine-host/bridges/vst3.cpp

@@ -355,12 +355,11 @@ void Vst3Bridge::run() {
                 //       much slower in Ardour, but there's no other non-hacky
                 //       solution for this (and bypassing Ardour's connection
                 //       proxies sort of goes against the idea behind yabridge)
-                return do_mutual_recursion_or_handle_in_main_context<tresult>(
+                return do_mutual_recursion_on_gui_thread<tresult>([&]() {
-                    [&]() {
+                    return object_instances[request.instance_id]
-                        return object_instances[request.instance_id]
+                        .connection_point->notify(
-                            .connection_point->notify(
+                            request.message_ptr.get_original());
-                                request.message_ptr.get_original());
+                });
-                    });
             },
             [&](YaContextMenuTarget::ExecuteMenuItem& request)
                 -> YaContextMenuTarget::ExecuteMenuItem::Response {
@@ -781,11 +780,10 @@ void Vst3Bridge::run() {
                 // not run from the GUI thread
                 Steinberg::ViewRect size{};
                 const tresult result =
-                    do_mutual_recursion_or_handle_in_main_context<tresult>(
+                    do_mutual_recursion_on_gui_thread<tresult>([&]() {
-                        [&]() {
+                        return object_instances[request.owner_instance_id]
-                            return object_instances[request.owner_instance_id]
+                            .plug_view_instance->plug_view->getSize(&size);
-                                .plug_view_instance->plug_view->getSize(&size);
+                    });
-                        });
 
                 return YaPlugView::GetSizeResponse{.result = result,
                                                    .size = std::move(size)};
@@ -800,12 +798,11 @@ void Vst3Bridge::run() {
                 //       code on the same thread that's currently waiting for a
                 //       response to the message it sent. See the docstring of
                 //       this function for more information on how this works.
-                return do_mutual_recursion_or_handle_in_main_context<tresult>(
+                return do_mutual_recursion_on_gui_thread<tresult>([&]() {
-                    [&]() {
+                    return object_instances[request.owner_instance_id]
-                        return object_instances[request.owner_instance_id]
+                        .plug_view_instance->plug_view->onSize(
-                            .plug_view_instance->plug_view->onSize(
+                            &request.new_size);
-                                &request.new_size);
+                });
-                    });
             },
             [&](const YaPlugView::OnFocus& request)
                 -> YaPlugView::OnFocus::Response {
@@ -848,21 +845,19 @@ void Vst3Bridge::run() {
                 -> YaPlugView::CanResize::Response {
                 // To prevent weird behaviour we'll perform all size related
                 // functions from the GUI thread, including this one
-                return do_mutual_recursion_or_handle_in_main_context<tresult>(
+                return do_mutual_recursion_on_gui_thread<tresult>([&]() {
-                    [&]() {
+                    return object_instances[request.owner_instance_id]
-                        return object_instances[request.owner_instance_id]
+                        .plug_view_instance->plug_view->canResize();
-                            .plug_view_instance->plug_view->canResize();
+                });
-                    });
             },
             [&](YaPlugView::CheckSizeConstraint& request)
                 -> YaPlugView::CheckSizeConstraint::Response {
                 const tresult result =
-                    do_mutual_recursion_or_handle_in_main_context<tresult>(
+                    do_mutual_recursion_on_gui_thread<tresult>([&]() {
-                        [&]() {
+                        return object_instances[request.owner_instance_id]
-                            return object_instances[request.owner_instance_id]
+                            .plug_view_instance->plug_view->checkSizeConstraint(
-                                .plug_view_instance->plug_view
+                                &request.rect);
-                                ->checkSizeConstraint(&request.rect);
+                    });
-                        });
 
                 return YaPlugView::CheckSizeConstraintResponse{
                     .result = result, .updated_rect = std::move(request.rect)};
@@ -1308,14 +1303,11 @@ size_t Vst3Bridge::register_object_instance(
                         //       handled from the same thread to prevent
                         //       deadlocks caused by mutually recursive function
                         //       calls.
-                        // TODO: Check if this causes any issues when activating
+                        return do_mutual_recursion_on_off_thread<tresult>(
-                        //       plugins while simultaneously resizing another
+                            [&]() {
-                        //       instance of the same plugin
+                                return object_instances[request.instance_id]
-                        return do_mutual_recursion_or_handle_in_main_context<
+                                    .component->setActive(request.state);
-                            tresult>([&]() {
+                            });
-                            return object_instances[request.instance_id]
-                                .component->setActive(request.state);
-                        });
                     },
                     [&](const YaPrefetchableSupport::GetPrefetchableSupport&
                             request)

src/wine-host/bridges/vst3.h

@@ -245,18 +245,19 @@ class Vst3Bridge : public HostBridge {
 
     /**
      * Spawn a new thread and call `send_message()` from there, and then handle
-     * functions passed by calls to
+     * functions passed by calls to `do_mutual_recursion_on_gui_thread()` and
-     * `do_mutual_recursion_or_handle_in_main_context()` on this thread until
+     * `do_mutual_recursion_on_off_thread()` on this thread until the original
-     * the original message we're trying to send has succeeded. This is a very
+     * message we're trying to send has successfully returned. This is a very
-     * specific solution to a very specific problem. When a plugin wants to
+     * specific solution to a very specific problem. We need the GUI version for
-     * resize itself, it will call `IPlugFrame::resizeView()` from within the
+     * when a plugin wants to resize itself, it will call
-     * WIn32 message loop. The host will then call `IPlugView::onSize()` on the
+     * `IPlugFrame::resizeView()` from within the WIn32 message loop. The host
-     * plugin's `IPlugView` to actually resize the plugin. The issue is that
+     * will then call `IPlugView::onSize()` on the plugin's `IPlugView` to
-     * that call to `IPlugView::onSize()` has to be handled from the UI thread,
+     * actually resize the plugin. The issue is that that call to
-     * but in this sequence that thread is being blocked by a call to
+     * `IPlugView::onSize()` has to be handled from the UI thread, but in this
+     * sequence that thread is being blocked by a call to
      * `IPlugFrame::resizeView()`.
      *
-     * We also need to use this for when a plugin calls
+     * The off-thread non-GUI version is needed for when a plugin calls
      * `IComponentHandler::restartComponent()` to change the latency, and when
      * the host then calls `IAudioProcessor::setActive()` in response to that to
      * restart the plugin. Otherwise this will lead to an infinite loop.
@@ -266,11 +267,31 @@ class Vst3Bridge : public HostBridge {
      * context.
      *
      * We apply the same trick in `Vst3PlugViewProxyImpl`.
+     *
+     * @tparam on_gui_thread Whether we are currently on the GUI thread. This
+     *   determines which IO contexts we'll use. When this mutual recursion
+     *   sequence _has_ to be executed from the GUI thread, pass `true`, and use
+     *   `do_mutual_recursion_on_gui_thread()` to handle incoming requests that
+     *   should be handled from this same thread. Otherwise pass `false` and use
+     *   `do_mutual_recursion_on_off_thread()` to make sure that these messages
+     *   don't end up on the GUI thread.
      */
-    template <typename T>
+    template <bool on_gui_thread, typename T>
     typename T::Response send_mutually_recursive_message(const T& object) {
         using TResponse = typename T::Response;
 
+        // Keeping mutually recursive tasks that have to be run on the GUI
+        // threads and tasks that cna be run on any thread separate will avoid
+        // any potential clashes, even if I wasn't able to find any problematic
+        // situations when I tested this with only one stack of IO contexts
+        std::vector<std::shared_ptr<boost::asio::io_context>>&
+            mutual_recursion_contexts =
+                on_gui_thread ? gui_thread_mutual_recursion_contexts
+                              : off_thread_mutual_recursion_contexts;
+        std::mutex& mutual_recursion_contexts_mutex =
+            on_gui_thread ? gui_thread_mutual_recursion_contexts_mutex
+                          : off_thread_mutual_recursion_contexts_mutex;
+
         // 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
@@ -316,16 +337,19 @@ class Vst3Bridge : public HostBridge {
 
     /**
      * Crazy functions ask for crazy naming. This is the other part of
-     * `send_mutually_recursive_message()`. If another thread is currently
+     * `send_mutually_recursive_message()`, for executing mutually recursive
-     * calling that function (from the UI thread), then we'll execute `f` from
+     * functions on the GUI thread. If another thread is currently calling that
-     * the UI thread using the IO context started in the above function.
+     * function (from the UI thread), then we'll execute `f` from the UI thread
-     * Otherwise `f` will be run on the UI thread through `main_context` as
+     * using the IO context started in the above function. Otherwise `f` will be
-     * usual.
+     * run on the UI thread through `main_context` as usual.
+     *
+     * You should pass `on_gui_thread = true` to
+     * `Vst3Bridge::send_mutually_recursive_message` when using this.
      *
      * @see Vst3Bridge::send_mutually_recursive_message
      */
     template <typename T, typename F>
-    T do_mutual_recursion_or_handle_in_main_context(F f) {
+    T do_mutual_recursion_on_gui_thread(F f) {
         std::packaged_task<T()> do_call(std::move(f));
         std::future<T> do_call_response = do_call.get_future();
 
@@ -339,10 +363,11 @@ class Vst3Bridge : public HostBridge {
         // `mutual_recursion_contexts` will be blocked until the deeper calls
         // are finished.
         {
-            std::lock_guard lock(mutual_recursion_contexts_mutex);
+            std::lock_guard lock(gui_thread_mutual_recursion_contexts_mutex);
-            if (!mutual_recursion_contexts.empty()) {
+            if (!gui_thread_mutual_recursion_contexts.empty()) {
-                boost::asio::dispatch(*mutual_recursion_contexts.back(),
+                boost::asio::dispatch(
-                                      std::move(do_call));
+                    *gui_thread_mutual_recursion_contexts.back(),
+                    std::move(do_call));
             } else {
                 main_context.schedule_task(std::move(do_call));
             }
@@ -352,19 +377,24 @@ class Vst3Bridge : public HostBridge {
     }
 
     /**
-     * The same as the above function, but we'll just execute the function on
+     * The same as the above function, but for executing a function on some
-     * this thread when the mutual recursion context is not active.
+     * off-thread, that doesn't necessarily have to be the GUI thread. We
+     * separate these to make it impossible for these two things to potentially
+     * block eachother.
+     *
+     * You should pass `on_gui_thread = false` to
+     * `Vst3Bridge::send_mutually_recursive_message` when using this.
      *
      * @see Vst3Bridge::do_mutual_recursion_or_handle_in_main_context
      */
     template <typename T, typename F>
-    T do_mutual_recursion(F f) {
+    T do_mutual_recursion_on_off_thread(F f) {
         std::packaged_task<T()> do_call(std::move(f));
         std::future<T> do_call_response = do_call.get_future();
 
-        if (std::lock_guard lock(mutual_recursion_contexts_mutex);
+        if (std::lock_guard lock(off_thread_mutual_recursion_contexts_mutex);
-            !mutual_recursion_contexts.empty()) {
+            !off_thread_mutual_recursion_contexts.empty()) {
-            boost::asio::dispatch(*mutual_recursion_contexts.back(),
+            boost::asio::dispatch(*off_thread_mutual_recursion_contexts.back(),
                                   std::move(do_call));
         } else {
             do_call();
@@ -478,9 +508,22 @@ class Vst3Bridge : public HostBridge {
      * mutually recursive function calls. See the docstring there for more
      * information. When this doesn't contain an IO context, this function is
      * not being called and `do_mutual_recursion_or_handle_in_main_context()`
-     * should post the task directly to the main IO context.
+     * should post the task directly to the main IO context, on the GUI thread.
      */
     std::vector<std::shared_ptr<boost::asio::io_context>>
-        mutual_recursion_contexts;
+        gui_thread_mutual_recursion_contexts;
-    std::mutex mutual_recursion_contexts_mutex;
+    std::mutex gui_thread_mutual_recursion_contexts_mutex;
+
+    /**
+     * The same as `gui_thread_mutual_recursion_contexts`, but for tasks that
+     * can be executed on any thread as long as they're executed on the same
+     * thread. This is for instance necessary when the plugin tells the host
+     * that its latency has changed, and when the plugin gets reactivates by the
+     * host it will tell the host once again that its latency has changed. This
+     * would otherwise result in deadlocks with latency introducing JUCE VST3
+     * plugins under Ardour and Mixbus.
+     */
+    std::vector<std::shared_ptr<boost::asio::io_context>>
+        off_thread_mutual_recursion_contexts;
+    std::mutex off_thread_mutual_recursion_contexts_mutex;
 };