Run certain GUI tasks from the host's run loop

This was a bit of a tricky one because it requires simulating mutual
recursion, but it's needed for REAPER as otherwide calls to
`IPlugFrame::resizeView()` and `IContextMenu::popup()` might cause
REAPER to segfault because its GUI is not thread safe.

meson.build

@@ -454,6 +454,7 @@ if with_vst3
       boost_dep,
       boost_filesystem_dep,
       bitsery_dep,
+      function2_dep,
       threads_dep,
       tomlplusplus_dep,
       vst3_sdk_native_dep,

src/plugin/bridges/vst3-impls/plug-view-proxy.cpp

@@ -16,6 +16,73 @@
 
 #include "plug-view-proxy.h"
 
+RunLoopTasks::RunLoopTasks(Steinberg::IPtr<Steinberg::IPlugFrame> plug_frame)
+    : run_loop(plug_frame) {
+    FUNKNOWN_CTOR
+
+    if (!run_loop) {
+        throw std::runtime_error(
+            "The host's 'IPlugFrame' object does not support 'IRunLoop'");
+    }
+
+    // This should be backed by eventfd instead, but Ardour doesn't allow that
+    std::array<int, 2> sockets;
+    if (socketpair(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC | SOCK_NONBLOCK, 0,
+                   sockets.data()) != 0) {
+        throw std::runtime_error("Failed to create a Unix domain socket");
+    }
+
+    socket_read_fd = sockets[0];
+    socket_write_fd = sockets[1];
+    if (run_loop->registerEventHandler(this, socket_read_fd) !=
+        Steinberg::kResultOk) {
+        throw std::runtime_error(
+            "Failed to register an event handler with the host's run loop");
+    }
+}
+
+RunLoopTasks::~RunLoopTasks() {
+    FUNKNOWN_DTOR
+
+    run_loop->unregisterEventHandler(this);
+    close(socket_read_fd);
+    close(socket_write_fd);
+}
+
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wdelete-non-virtual-dtor"
+IMPLEMENT_FUNKNOWN_METHODS(RunLoopTasks,
+                           Steinberg::Linux::IEventHandler,
+                           Steinberg::Linux::IEventHandler::iid)
+#pragma GCC diagnostic pop
+
+void RunLoopTasks::schedule(fu2::unique_function<void()> task) {
+    std::lock_guard eventfd_lock(tasks_mutex);
+    tasks.push_back(std::move(task));
+
+    uint8_t notify_value = 1;
+    write(socket_write_fd, &notify_value, sizeof(notify_value));
+}
+
+void PLUGIN_API
+RunLoopTasks::onFDIsSet(Steinberg::Linux::FileDescriptor /*fd*/) {
+    std::lock_guard lock(tasks_mutex);
+
+    // Run all tasks that have been submitted to our queue from the host's
+    // calling thread (which will be the GUI thread)
+    for (auto& task : tasks) {
+        task();
+
+        // This should in theory stop the host from calling this function, but
+        // REAPER doesn't care. And funnily enough we only have to do all of
+        // this because of REAPER.
+        uint8_t notify_value;
+        read(socket_read_fd, &notify_value, sizeof(notify_value));
+    }
+
+    tasks.clear();
+}
+
 Vst3PlugViewProxyImpl::Vst3PlugViewProxyImpl(
     Vst3PluginBridge& bridge,
     Vst3PlugViewProxy::ConstructArgs&& args)
@@ -24,7 +91,7 @@ Vst3PlugViewProxyImpl::Vst3PlugViewProxyImpl(
 Vst3PlugViewProxyImpl::~Vst3PlugViewProxyImpl() {
     // Also drop the plug view smart pointer on the Wine side when this gets
     // dropped
-    bridge.send_message(
+    send_mutually_recursive_message(
         Vst3PlugViewProxy::Destruct{.owner_instance_id = owner_instance_id()});
 }
 
@@ -42,8 +109,9 @@ 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 bridge.send_message(YaPlugView::IsPlatformTypeSupported{
+        return send_mutually_recursive_message(
-            .owner_instance_id = owner_instance_id(), .type = type});
+            YaPlugView::IsPlatformTypeSupported{
+                .owner_instance_id = owner_instance_id(), .type = type});
     } else {
         bridge.logger.log(
             "WARNING: Null pointer passed to "
@@ -57,7 +125,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 bridge.send_message(YaPlugView::Attached{
+        return send_mutually_recursive_message(YaPlugView::Attached{
             .owner_instance_id = owner_instance_id(),
             .parent = reinterpret_cast<native_size_t>(parent),
             .type = type});
@@ -69,19 +137,19 @@ tresult PLUGIN_API Vst3PlugViewProxyImpl::attached(void* parent,
 }
 
 tresult PLUGIN_API Vst3PlugViewProxyImpl::removed() {
-    return bridge.send_message(
+    return send_mutually_recursive_message(
         YaPlugView::Removed{.owner_instance_id = owner_instance_id()});
 }
 
 tresult PLUGIN_API Vst3PlugViewProxyImpl::onWheel(float distance) {
-    return bridge.send_message(YaPlugView::OnWheel{
+    return 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 bridge.send_message(
+    return send_mutually_recursive_message(
         YaPlugView::OnKeyDown{.owner_instance_id = owner_instance_id(),
                               .key = key,
                               .key_code = keyCode,
@@ -91,7 +159,7 @@ tresult PLUGIN_API Vst3PlugViewProxyImpl::onKeyDown(char16 key,
 tresult PLUGIN_API Vst3PlugViewProxyImpl::onKeyUp(char16 key,
                                                   int16 keyCode,
                                                   int16 modifiers) {
-    return bridge.send_message(
+    return send_mutually_recursive_message(
         YaPlugView::OnKeyUp{.owner_instance_id = owner_instance_id(),
                             .key = key,
                             .key_code = keyCode,
@@ -101,7 +169,7 @@ tresult PLUGIN_API Vst3PlugViewProxyImpl::onKeyUp(char16 key,
 tresult PLUGIN_API Vst3PlugViewProxyImpl::getSize(Steinberg::ViewRect* size) {
     if (size) {
         const GetSizeResponse response =
-            bridge.send_message(YaPlugView::GetSize{
+            send_mutually_recursive_message(YaPlugView::GetSize{
                 .owner_instance_id = owner_instance_id(), .size = *size});
 
         *size = response.updated_size;
@@ -116,7 +184,7 @@ tresult PLUGIN_API Vst3PlugViewProxyImpl::getSize(Steinberg::ViewRect* size) {
 
 tresult PLUGIN_API Vst3PlugViewProxyImpl::onSize(Steinberg::ViewRect* newSize) {
     if (newSize) {
-        return bridge.send_message(YaPlugView::OnSize{
+        return send_mutually_recursive_message(YaPlugView::OnSize{
             .owner_instance_id = owner_instance_id(), .new_size = *newSize});
     } else {
         bridge.logger.log(
@@ -126,7 +194,7 @@ tresult PLUGIN_API Vst3PlugViewProxyImpl::onSize(Steinberg::ViewRect* newSize) {
 }
 
 tresult PLUGIN_API Vst3PlugViewProxyImpl::onFocus(TBool state) {
-    return bridge.send_message(YaPlugView::OnFocus{
+    return send_mutually_recursive_message(YaPlugView::OnFocus{
         .owner_instance_id = owner_instance_id(), .state = state});
 }
 
@@ -138,7 +206,25 @@ Vst3PlugViewProxyImpl::setFrame(Steinberg::IPlugFrame* frame) {
         // this component handler
         plug_frame = frame;
 
-        return bridge.send_message(YaPlugView::SetFrame{
+        // REAPER's GUI is not thread safe, and if we don't call
+        // `IPlugFrame::resizeView()` or `IContextMenu::popup()` from a thread
+        // owned by REAPER then REAPER will eventually segfault We should thus
+        // try to call those functions from an `IRunLoop` event handler.
+        try {
+            run_loop_tasks = Steinberg::owned(new RunLoopTasks(plug_frame));
+        } catch (const std::runtime_error& e) {
+            // In case the host does not support `IRunLoop` or if we can't
+            // register an event handler, we'll throw during `RunLoopTasks`'
+            // constructor
+            run_loop_tasks = nullptr;
+
+            bridge.logger.log(
+                "The host does not support IRunLoop, falling back to naive GUI "
+                "function execution: " +
+                std::string(e.what()));
+        }
+
+        return send_mutually_recursive_message(YaPlugView::SetFrame{
             .owner_instance_id = owner_instance_id(),
             .plug_frame_args = Vst3PlugFrameProxy::ConstructArgs(
                 plug_frame, owner_instance_id())});
@@ -150,7 +236,7 @@ Vst3PlugViewProxyImpl::setFrame(Steinberg::IPlugFrame* frame) {
 }
 
 tresult PLUGIN_API Vst3PlugViewProxyImpl::canResize() {
-    return bridge.send_message(
+    return send_mutually_recursive_message(
         YaPlugView::CanResize{.owner_instance_id = owner_instance_id()});
 }
 
@@ -158,7 +244,7 @@ tresult PLUGIN_API
 Vst3PlugViewProxyImpl::checkSizeConstraint(Steinberg::ViewRect* rect) {
     if (rect) {
         const CheckSizeConstraintResponse response =
-            bridge.send_message(YaPlugView::CheckSizeConstraint{
+            send_mutually_recursive_message(YaPlugView::CheckSizeConstraint{
                 .owner_instance_id = owner_instance_id(), .rect = *rect});
 
         *rect = response.updated_rect;
@@ -177,7 +263,7 @@ tresult PLUGIN_API Vst3PlugViewProxyImpl::findParameter(
     int32 yPos,
     Steinberg::Vst::ParamID& resultTag /*out*/) {
     const FindParameterResponse response =
-        bridge.send_message(YaParameterFinder::FindParameter{
+        send_mutually_recursive_message(YaParameterFinder::FindParameter{
             .owner_instance_id = owner_instance_id(),
             .x_pos = xPos,
             .y_pos = yPos});
@@ -189,7 +275,7 @@ tresult PLUGIN_API Vst3PlugViewProxyImpl::findParameter(
 
 tresult PLUGIN_API
 Vst3PlugViewProxyImpl::setContentScaleFactor(ScaleFactor factor) {
-    return bridge.send_message(
+    return send_mutually_recursive_message(
         YaPlugViewContentScaleSupport::SetContentScaleFactor{
             .owner_instance_id = owner_instance_id(), .factor = factor});
 }

src/plugin/bridges/vst3-impls/plug-view-proxy.h

@@ -16,8 +16,93 @@
 
 #pragma once
 
+#include <function2/function2.hpp>
+
 #include "../vst3.h"
 
+#include <boost/asio/dispatch.hpp>
+
+/**
+ * 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
+ * GUI drawing (notable `IPlugFrame::resizeView()` and `IContextMenu::popup()`)
+ * have to be run from a thread owned by REAPER. If we don't do this, the
+ * `IPlugFrame::resizeView()` won't resize the actual window and both of these
+ * functions will eventually cause REAPER to segfault.
+ */
+class RunLoopTasks : public Steinberg::Linux::IEventHandler {
+   public:
+    /**
+     * Register an event handler in the host's run loop so we can schedule tasks
+     * to be run from there. This works very much like how we use Boost.Asio IO
+     * contexts everywhere else to run functions on other threads. All of this
+     * is backed by a dummy Unix domain socket, although REAPER will call the
+     * event handler regardless of whether the file descriptor is ready or not.
+     * eventfd would have made much more sense here, but Ardour doesn't support
+     * that.
+     *
+     * @throw std::runtime_error If the host does not support
+     *   `Steinberg::Linux::IRunLoop`, or if registering the event handler was
+     *   not successful. The caller should catch this and call back to not
+     *   relying on the run loop.
+     */
+    RunLoopTasks(Steinberg::IPtr<Steinberg::IPlugFrame> plug_frame);
+
+    /**
+     * Unregister the event handler and close the file descriptor on cleanup.
+     */
+    ~RunLoopTasks();
+
+    DECLARE_FUNKNOWN_METHODS
+
+    /**
+     * Schedule a task to be run from the host's GUI thread in an `IRunLoop`
+     * event handler. This may block if the host is currently calling
+     * `onFDIsSet()`.
+     *
+     * @param task The task to execute. This can be used with
+     *   `std::packaged_task` to run a computation that returns a value from the
+     *   host's GUI thread.
+     */
+    void schedule(fu2::unique_function<void()> task);
+
+    // From `IEventHandler`, required for REAPER because its GUI is not thread
+    // safe
+    void PLUGIN_API onFDIsSet(Steinberg::Linux::FileDescriptor fd) override;
+
+   private:
+    /**
+     * This pointer is cast from `plug_frame` once `IPlugView::setFrame()` has
+     * been called.
+     */
+    Steinberg::FUnknownPtr<Steinberg::Linux::IRunLoop> run_loop;
+
+    /**
+     * Tasks that should be executed in the next `IRunLoop` event handler call.
+     *
+     * @relates Vst3PlugViewProxyImpl::run_gui_task
+     *
+     * @see RunLoopTasks::schedule_task
+     */
+    std::vector<fu2::unique_function<void()>> tasks;
+    std::mutex tasks_mutex;
+
+    /**
+     * A dumy Unix domain socket file descriptor used to signal that there is a
+     * task ready. We'll pass this to the host's `IRunLoop` so it can tell when
+     * we have an event to handle.
+     *
+     * XXX: This should be backed by eventfd instead, but Ardour doesn't support
+     *      that
+     */
+    int socket_read_fd = -1;
+    /**
+     * The other side of `socket_read_fd`. We'll write to this when we want the
+     * hsot to call our event handler.
+     */
+    int socket_write_fd = -1;
+};
+
 class Vst3PlugViewProxyImpl : public Vst3PlugViewProxy {
    public:
     Vst3PlugViewProxyImpl(Vst3PluginBridge& bridge,
@@ -37,6 +122,55 @@ class Vst3PlugViewProxyImpl : public Vst3PlugViewProxy {
     tresult PLUGIN_API queryInterface(const Steinberg::TUID _iid,
                                       void** obj) override;
 
+    /**
+     * Run a task that's supposed to be run from the GUI thread.
+     * `IPlugFrame::resizeView()` and `IContextMenu::popup()` are the likely
+     * candidates here. This is needed for REAPER, as REAPER will segfault if
+     * you run those functions from a thread that's not owned by REAPER itself.
+     * If the `IPlugFrame` object passed to `IPlugView::setFrame()` supports
+     * `IRunLoop`, then we'll schedule `f` to be run from an even handler in the
+     * host's run loop. Otherwise `f` is run directly.
+     *
+     * This works similarly to
+     * `Vst3Bridge::do_mutual_recursion_or_handle_in_main_context`, except that
+     * we can post tasks to `run_loop_tasks` instead of executing them directly
+     * in `main_context` when no mutually recursive function calls are happening
+     * right now.
+     *
+     * @see send_mutually_recursive_message
+     */
+    template <typename T, typename F>
+    T run_gui_task(F f) {
+        std::packaged_task<T()> do_call(std::move(f));
+        std::future<T> 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
+        // currently accepting work on the that thread. 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_context_mutex);
+            if (mutual_recursion_context) {
+                boost::asio::dispatch(*mutual_recursion_context,
+                                      std::move(do_call));
+            } else {
+                mutual_recursion_lock.unlock();
+
+                if (run_loop_tasks) {
+                    run_loop_tasks->schedule(std::move(do_call));
+                } else {
+                    do_call();
+                }
+            }
+        }
+
+        return do_call_response.get();
+    }
+
     // From `IPlugView`
     tresult PLUGIN_API
     isPlatformTypeSupported(Steinberg::FIDString type) override;
@@ -74,5 +208,85 @@ class Vst3PlugViewProxyImpl : public Vst3PlugViewProxy {
     Steinberg::IPtr<Steinberg::IPlugFrame> plug_frame;
 
    private:
+    /**
+     * 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>
+    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
+        {
+            std::unique_lock lock(mutual_recursion_context_mutex);
+
+            // In case some other thread is already calling
+            // `send_mutually_recursive_message()`, we're likely in mutually
+            // recursive calling sequence, and we should thus run the message
+            // from the current thread. This can happen during
+            // `IPlugView::attached() -> IPlugFrame::resizeView() ->
+            // IPlugView::onSize()`.
+            if (mutual_recursion_context) {
+                lock.unlock();
+
+                return bridge.send_message(object);
+            }
+
+            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{};
+        std::jthread sending_thread([&]() {
+            const TResponse response = bridge.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();
+
+            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();
+    }
+
     Vst3PluginBridge& bridge;
+
+    /**
+     * 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
+     * `run_gui_task()` should post the task to `run_loop_tasks`. This works
+     * exactly the same as the mutual recursion handling in `Vst3Bridge`.
+     */
+    std::optional<boost::asio::io_context> mutual_recursion_context;
+    std::mutex mutual_recursion_context_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
+     * `Vst3PlugViewProxyImpl::run_gui_task`.
+     *
+     * _This value is optional_ and it will this be a null pointer of the host
+     * does not support `IRunLoop`. We have to use an `IPtr` instead of a an
+     * `std::optional` in case the host also stores a pointer to this.
+     */
+    Steinberg::IPtr<RunLoopTasks> run_loop_tasks;
 };

src/plugin/bridges/vst3.cpp

@@ -206,20 +206,16 @@ Vst3PluginBridge::Vst3PluginBridge()
                 },
                 [&](const YaContextMenu::Popup& request)
                     -> YaContextMenu::Popup::Response {
-                    // FIXME: In REAPER having the menu open without interacting
+                    // REAPER requires this to be run from its provided event
-                    //        with it causes malloc failures or failing font
+                    // loop or else it will likely segfault at some point
-                    //        drawing calls. Valgrind reports all kinds of
-                    //        memory errors within REAPER when this happens, and
-                    //        I'm not sure if yabridge is to blame here. - As it
-                    //        turns out a lot of stuff in REAPEr, including
-                    //        calls to `IPlugFrame::resizeView()`, are not
-                    //        thread safe. We need to hook into `IRunLoop` and
-                    //        execute `IContextMenu::popup()` and
-                    //        `IPlugFrame::resizeView()` functions from there.
                     return plugin_proxies.at(request.owner_instance_id)
                         .get()
-                        .context_menus.at(request.context_menu_id)
+                        .last_created_plug_view->run_gui_task<tresult>([&]() {
-                        .menu->popup(request.x, request.y);
+                            return plugin_proxies.at(request.owner_instance_id)
+                                .get()
+                                .context_menus.at(request.context_menu_id)
+                                .menu->popup(request.x, request.y);
+                        });
                 },
                 [&](YaConnectionPoint::Notify& request)
                     -> YaConnectionPoint::Notify::Response {
@@ -267,8 +263,12 @@ Vst3PluginBridge::Vst3PluginBridge()
                             .get()
                             .last_created_plug_view;
 
-                    return plug_view->plug_frame->resizeView(plug_view,
+                    // REAPER requires this to be run from its provided event
-                                                             &request.new_size);
+                    // loop or else it will likely segfault at some point
+                    return plug_view->run_gui_task<tresult>([&]() {
+                        return plug_view->plug_frame->resizeView(
+                            plug_view, &request.new_size);
+                    });
                 },
                 [&](const YaPlugInterfaceSupport::IsPlugInterfaceSupported&
                         request) -> YaPlugInterfaceSupport::

src/wine-host/bridges/vst3.h

@@ -244,6 +244,8 @@ class Vst3Bridge : public HostBridge {
      * 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.
+     *
+     * We apply the same trick in `Vst3PlugViewProxyImpl`.
      */
     template <typename T>
     typename T::Response send_mutually_recursive_message(const T& object) {