Significantly clean up mutual recursion workaround

This has much fewer moving parts, and it's probably more understandable.
There was also a race condition in the previous implementation, so
there's that.

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

@@ -47,80 +47,11 @@ Vst3PlugFrameProxyImpl::resizeView(Steinberg::IPlugView* /*view*/,
         //      assume `view` is the `IPlugView*` returned by the last call to
         //      `IEditController::createView()`
 
-        // HACK: This ia bit of a weird one and requires special handling. A
+        // We have to use this special sending function here so we can handle
-        //       plugin will call this function from the Win32 message loop so
+        // calls to `IPlugView::onSize()` from this same thread (the UI thread).
-        //       the call blocks the loop. The host will then check with the
+        // See the docstring for more information.
-        //       plugin if it can actually resize itself to `*newSize`, and it
+        return bridge.send_mutually_recursive_message(YaPlugFrame::ResizeView{
-        //       will then call `IPlugView::onSize()` with the new size. The
+            .owner_instance_id = owner_instance_id(), .new_size = *newSize});
-        //       issue is that the `IPlugView::onSize()` call also has to be
-        //       called from within the UI thread, but that thread is currently
-        //       being blocked by the call to this function.
-        //       As a workaround, we'll send the message for the call to
-        //       `IPlugFrame::resizeView()` on another thread. We then wait for
-        //       either that request to finish immediately (meaning the host
-        //       hasn't resized the window), or for `IPlugView::onSize()` to be
-        //       called by the host. If the host does call `IPlugView::onSize()`
-        //       while the other thread is handling `IPlugFrame::resizeView()`,
-        //       then we'll awaken this thread using a condition variable so we
-        //       can do the actual call to `IPlugView::onSize()` from here, from
-        //       within the Win32 loop.
-        // TODO: Can we someone use Boost.Asio strands to make this cleaner?
-        {
-            std::lock_guard lock(on_size_interrupt_mutex);
-            on_size_interrupt_waiting = true;
-            on_size_interrupt.reset();
-            on_size_interrupt_result.reset();
-        }
-
-        std::promise<tresult> resize_result_promise{};
-        std::future<tresult> resize_result_future =
-            resize_result_promise.get_future();
-        Win32Thread resize_thread([&]() {
-            const tresult result = bridge.send_message(YaPlugFrame::ResizeView{
-                .owner_instance_id = owner_instance_id(),
-                .new_size = *newSize});
-
-            resize_result_promise.set_value(result);
-
-            {
-                std::lock_guard lock(on_size_interrupt_mutex);
-                if (BOOST_LIKELY(!on_size_interrupt_waiting)) {
-                    return;
-                }
-
-                // If the call to `IPlugFrame::resizeView()` finish without the
-                // host calling `IPlugView::onSize()` (e.g. when the call
-                // failed), then we'll have to manually unblock the thread below
-                // by providing a noop function.
-                on_size_interrupt = []() { return Steinberg::kResultOk; };
-            }
-            on_size_interrupt_cv.notify_one();
-
-            // We don't need this result value, but we should still wait for
-            // it
-            std::unique_lock lock(on_size_interrupt_mutex);
-            on_size_interrupt_cv.wait(
-                lock, [&]() { return on_size_interrupt_result.has_value(); });
-        });
-
-        // Wait for `IPlugView::onSize()` to be called by the host and handle
-        // the call here on this thread, or wait for the call to
-        // `IPlugFrame::resizeView()` in the above thread to finish. In the last
-        // case we'll execute a noop function.
-        std::unique_lock lock(on_size_interrupt_mutex);
-        on_size_interrupt_cv.wait(
-            lock, [&]() { return on_size_interrupt.has_value(); });
-
-        // When we get a function through one of the two above means, we'll
-        // store the result and then awake the calling thread again so it can
-        // access the result
-        on_size_interrupt_result = (*on_size_interrupt)();
-        on_size_interrupt_waiting = false;
-
-        lock.unlock();
-        on_size_interrupt_cv.notify_one();
-
-        return resize_result_future.get();
     } else {
         std::cerr
             << "WARNING: Null pointer passed to 'IPlugFrame::resizeView()'"

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

@@ -30,58 +30,10 @@ class Vst3PlugFrameProxyImpl : public Vst3PlugFrameProxy {
     tresult PLUGIN_API queryInterface(const Steinberg::TUID _iid,
                                       void** obj) override;
 
-    /**
-     * This is needed to be able to handle a call to `IPlugView::onSize()` from
-     * the UI thread while the plugin is currently calling
-     * `IPlugFrame::resizeView()` from that same thread.. This is probably the
-     * hackiest (and most error prone, probably) part of this VST3
-     * implementation. The details on how this works and why it is necessary are
-     * explained in the comment in `YaPlugFrameProxyImpl::resizeView()`.
-     *
-     * If there is currently a call to `resizeView()` being processed, then this
-     * will run `on_size` from the same thread that's currently processing a
-     * call to `resizeView()` and return the result from the function call.
-     * Otherwise this will return a nullopt and `on_size` should be passed to
-     * `main_context.run_in_context<tresult>()`.
-     */
-    template <typename F>
-    std::optional<tresult> maybe_run_on_size_from_ui_thread(F on_size) {
-        {
-            std::lock_guard lock(on_size_interrupt_mutex);
-            if (!on_size_interrupt_waiting) {
-                return std::nullopt;
-            }
-
-            on_size_interrupt = std::move(on_size);
-        }
-        on_size_interrupt_cv.notify_one();
-
-        // Since `on_size` is run from another thread, we now have to wait to be
-        // woken up again when the result is ready
-        std::unique_lock lock(on_size_interrupt_mutex);
-        on_size_interrupt_cv.wait(
-            lock, [&]() { return on_size_interrupt_result.has_value(); });
-
-        return *on_size_interrupt_result;
-    }
-
     // From `IPlugFrame`
     tresult PLUGIN_API resizeView(Steinberg::IPlugView* view,
                                   Steinberg::ViewRect* newSize) override;
 
    private:
     Vst3Bridge& bridge;
-
-    /**
-     * A function that will be used to run `IPlugView::onSize()` on the thread
-     * that originally called `IPlugFrame::resizeView()` to work around a Win32
-     * limitation, along with its result, and whether or not we're currently
-     * waiting for this function to be provided by some other thread. See the
-     * comment in `onSize()` for more information.
-     */
-    bool on_size_interrupt_waiting = false;
-    std::optional<fu2::unique_function<tresult()>> on_size_interrupt;
-    std::optional<tresult> on_size_interrupt_result;
-    std::condition_variable on_size_interrupt_cv;
-    std::mutex on_size_interrupt_mutex;
 };

src/wine-host/bridges/vst3.cpp

@@ -69,7 +69,7 @@ void Vst3Bridge::run() {
                 -> Vst3PlugViewProxy::Destruct::Response {
                 // XXX: Not sure if his has to be run form the UI thread
                 main_context
-                    .run_in_context([&]() {
+                    .run_in_context<void>([&]() {
                         // When the pointer gets dropped by the host, we want to
                         // drop it here as well, along with the `IPlugFrame`
                         // proxy object it may have received in
@@ -275,7 +275,7 @@ void Vst3Bridge::run() {
                 -> YaEditController::CreateView::Response {
                 // Instantiate the object from the GUI thread
                 main_context
-                    .run_in_context([&]() {
+                    .run_in_context<void>([&]() {
                         object_instances[request.instance_id].plug_view =
                             Steinberg::owned(
                                 object_instances[request.instance_id]
@@ -409,31 +409,20 @@ void Vst3Bridge::run() {
                     .result = result, .updated_size = request.size};
             },
             [&](YaPlugView::OnSize& request) -> YaPlugView::OnSize::Response {
-                auto call_on_size = [&]() {
+                // HACK: This function has to be run from the UI thread since
-                    const auto result =
+                //       the plugin probably wants to redraw when it gets
-                        object_instances[request.owner_instance_id]
+                //       resized. The issue here is that this function can be
+                //       called in response to a call to
+                //       `IPlugFrame::resizeView()`. That function is always
+                //       called from the UI thread, so we need some way to 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 object_instances[request.owner_instance_id]
                             .plug_view->onSize(&request.new_size);
-                    return result;
+                    });
-                };
-
-                // HACK: As explained in the docstring of
-                //       `YaPlugFrameProxyImpl::resizeView()`, calls to
-                //       `IPlugView::onSize()` have to be handled from the UI
-                //       thread, even if that thread is currently making a call
-                //       to `IPlugFrame::resizeView()`. We use some special
-                //       handling to execute `call_on_size()` on that thread if
-                //       it's currently waiting for a call to
-                //       `IPlugFrame::resizeView()`, and we'll just run in
-                //       within the main context as usual otherwise.
-                if (auto result =
-                        object_instances[request.owner_instance_id]
-                            .plug_frame_proxy_impl
-                            ->maybe_run_on_size_from_ui_thread(call_on_size)) {
-                    return *result;
-                } else {
-                    return main_context.run_in_context<tresult>(call_on_size)
-                        .get();
-                }
             },
             [&](const YaPlugView::OnFocus& request)
                 -> YaPlugView::OnFocus::Response {
@@ -450,15 +439,9 @@ void Vst3Bridge::run() {
                 // pass that to the `setFrame()` function. The lifetime of this
                 // object is tied to that of the actual `IPlugFrame` object
                 // we're passing this proxy to.
-                // We'll also store an unmanaged pointer to the actual
-                // implementation so we can do some sneakiness for
-                // `IPlugView::onSize()`.
-                object_instances[request.owner_instance_id]
-                    .plug_frame_proxy_impl = new Vst3PlugFrameProxyImpl(
-                    *this, std::move(request.plug_frame_args));
                 object_instances[request.owner_instance_id].plug_frame_proxy =
-                    Steinberg::owned(object_instances[request.owner_instance_id]
+                    Steinberg::owned(new Vst3PlugFrameProxyImpl(
-                                         .plug_frame_proxy_impl);
+                        *this, std::move(request.plug_frame_args)));
 
                 // TODO: Does this have to be run from the UI thread? Figure out
                 //       if it does
@@ -711,7 +694,7 @@ void Vst3Bridge::unregister_object_instance(size_t instance_id) {
     //      executed and when the actual host application context on
     //      the plugin side gets deallocated.
     main_context
-        .run_in_context([&, instance_id]() {
+        .run_in_context<void>([&, instance_id]() {
             std::lock_guard lock(object_instances_mutex);
             object_instances.erase(instance_id);
         })

src/wine-host/bridges/vst3.h

@@ -16,6 +16,7 @@
 
 #pragma once
 
+#include <iostream>
 #include <string>
 
 #include <public.sdk/source/vst/hosting/module.h>
@@ -75,13 +76,6 @@ struct InstanceInterfaces {
      */
     Steinberg::IPtr<Vst3PlugFrameProxy> plug_frame_proxy;
 
-    /**
-     * An unmanaged raw pointer for the actual implementation behind
-     * `plug_frame_proxy`. This is needed for some special handling for
-     * `IPlugView::onSize()`.
-     */
-    Vst3PlugFrameProxyImpl* plug_frame_proxy_impl;
-
     /**
      * The base object we cast from.
      */
@@ -160,6 +154,106 @@ class Vst3Bridge : public HostBridge {
         return sockets.vst_host_callback.send_message(object, std::nullopt);
     }
 
+    /**
+     * Spawn a new thread and call `send_message()` from there, and then handle
+     * functions passed by calls to
+     * `do_mutual_recursion_or_handle_in_main_context()` on this thread until
+     * the original message we're trying to send has succeeded. This is a very
+     * specific solution to a very specific problem. When a plugin wants to
+     * resize itself, it will call `IPlugFrame::resizeView()` from within the
+     * WIn32 message loop. The host will then call `IPlugView::onSize()` on the
+     * plugin's `IPlugView` to actually resize the plugin. The issue is that
+     * that 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()`.
+     *
+     * The hacky solution here is to send the message from another thread, and
+     * to then allow this thread to execute other functions submitted to an IO
+     * context.
+     */
+    template <typename T>
+    typename T::Response send_mutually_recursive_message(const T& object) {
+        using TResponse = typename T::Response;
+
+        // This IO context will accept incoming calls from
+        // `do_mutual_recursion_or_handle_in_main_context()` until we receive a
+        // response
+        {
+            std::unique_lock lock(mutual_recursion_context_mutex);
+
+            // In case some other thread is already calling
+            // `send_mutually_recursive_message()` (which should never be the
+            // case since this should only be called from the UI thread), we'll
+            // wait for that function to finish
+            if (mutual_recursion_context) {
+                mutual_recursion_context_cv.wait(lock, [&]() {
+                    return !mutual_recursion_context.has_value();
+                });
+            }
+
+            mutual_recursion_context.emplace();
+        }
+
+        // We will call the function from another thread so we can handle calls
+        // to  from this thread
+        std::promise<TResponse> response_promise{};
+        Win32Thread sending_thread([&]() {
+            const TResponse response = send_message(object);
+
+            // Stop accepting additional work to be run from the calling thread
+            // once we receive a response
+            {
+                std::lock_guard lock(mutual_recursion_context_mutex);
+                mutual_recursion_context->stop();
+                mutual_recursion_context.reset();
+            }
+            mutual_recursion_context_cv.notify_one();
+
+            response_promise.set_value(response);
+        });
+
+        // Accept work from the other thread until we receive a response, at
+        // which point the context will be stopped
+        auto work_guard =
+            boost::asio::make_work_guard(*mutual_recursion_context);
+        mutual_recursion_context->run();
+
+        return response_promise.get_future().get();
+    }
+
+    /**
+     * Crazy functions ask for crazy naming. This is the other part of
+     * `send_mutually_recursive_message()`. If another thread is currently
+     * calling that function (from the UI thread), then we'll execute `f` from
+     * the UI thread using the IO context started in the above function.
+     * Otherwise `f` will be run on the UI thread through `main_context` as
+     * usual.
+     *
+     * @see Vst3Bridge::send_mutually_recursive_message
+     */
+    template <typename T, typename F>
+    T do_mutual_recursion_or_handle_in_main_context(F f) {
+        std::packaged_task<T()> do_call(f);
+        std::future<T> do_call_response = do_call.get_future();
+
+        // If the above function is currently being called from some thread,
+        // then we'll submit the task to the IO context created there so it can
+        // be handled on that same thread. Otherwise we'll just submit it to the
+        // main IO context. Neither of these two functions block until `do_call`
+        // finish executing.
+        {
+            std::lock_guard lock(mutual_recursion_context_mutex);
+            if (mutual_recursion_context) {
+                boost::asio::dispatch(*mutual_recursion_context,
+                                      std::move(do_call));
+            } else {
+                main_context.schedule_task(std::move(do_call));
+            }
+        }
+
+        return do_call_response.get();
+    }
+
    private:
     /**
      * Generate a nique instance identifier using an atomic fetch-and-add. This
@@ -232,4 +326,21 @@ class Vst3Bridge : public HostBridge {
      */
     std::map<size_t, InstanceInterfaces> object_instances;
     std::mutex object_instances_mutex;
+
+    /**
+     * The IO context used in `send_mutually_recursive_message()` to be able to
+     * execute functions from that same calling thread while we're waiting for a
+     * response. 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.
+     */
+    std::optional<boost::asio::io_context> mutual_recursion_context;
+    std::mutex mutual_recursion_context_mutex;
+    /**
+     * Used to make sure only a single call to
+     * `send_mutually_recursive_message()` at a time can be processed (this
+     * should never happen, but better safe tha nsorry).
+     */
+    std::condition_variable mutual_recursion_context_cv;
 };

src/wine-host/utils.h

@@ -71,33 +71,11 @@ class MainContext {
      */
     template <typename T, typename F>
     std::future<T> run_in_context(F fn) {
-        std::promise<T> result{};
+        std::packaged_task<T()> call_fn(std::move(fn));
-        std::future<T> future = result.get_future();
+        std::future<T> response = call_fn.get_future();
-        boost::asio::dispatch(context,
+        boost::asio::dispatch(context, std::move(call_fn));
-                              [result = std::move(result), fn]() mutable {
-                                  result.set_value(fn());
-                              });
 
-        return future;
+        return response;
-    }
-
-    /**
-     * The same as the above, but without returning a value. This allows us to
-     * wait for the task to have been run.
-     *
-     * @overload
-     */
-    template <typename F>
-    std::future<void> run_in_context(F fn) {
-        std::promise<void> result{};
-        std::future<void> future = result.get_future();
-        boost::asio::dispatch(context,
-                              [result = std::move(result), fn]() mutable {
-                                  fn();
-                                  result.set_value();
-                              });
-
-        return future;
     }
 
     /**
@@ -107,7 +85,7 @@ class MainContext {
      */
     template <typename F>
     void schedule_task(F fn) {
-        boost::asio::post(context, fn);
+        boost::asio::post(context, std::move(fn));
     }
 
     /**