Unify event handling behavior for all cases

This makes the individual plugin host slightly more complex, but now
both individually hosted plugins and plugin groups handle both
dispatcher events and GUI events in the exact same way.

CHANGELOG.md

@@ -20,6 +20,11 @@ Versioning](https://semver.org/spec/v2.0.0.html).
 ### Changed
 
 - Changed architecture to use one less socket.
+- GUI events are now always handled on a steady timer rather than being
+  interleaved as part of the event loop. This change was made to unify the event
+  handling logic for individually hosted plugins and plugin groups. It should
+  have any negative effects, but please let me know if this does cause unwanted
+  behavior.
 
 ### Fixed
 

src/wine-host/bridges/group.cpp

@@ -128,7 +128,7 @@ void GroupBridge::handle_plugin_dispatch(const GroupRequest request) {
 
     // Blocks this thread until the plugin shuts down, handling all events on
     // the main IO context
-    bridge->handle_dispatch_multi(plugin_context);
+    bridge->handle_dispatch();
     logger.log("'" + request.plugin_path + "' has exited");
 
     // After the plugin has exited, we'll remove this thread's plugin from the
@@ -214,8 +214,8 @@ void GroupBridge::accept_requests() {
             logger.log("Received request to host '" + request.plugin_path +
                        "' using socket '" + request.socket_path + "'");
             try {
-                auto bridge = std::make_unique<Vst2Bridge>(request.plugin_path,
-                                                           request.socket_path);
+                auto bridge = std::make_unique<Vst2Bridge>(
+                    plugin_context, request.plugin_path, request.socket_path);
                 logger.log("Finished initializing '" + request.plugin_path +
                            "'");
 
@@ -261,6 +261,8 @@ void GroupBridge::async_handle_events() {
         // TODO: Check if those same weird crashes with Serum are happening
         //       again with these normal threads
         if (!should_skip_message_loop()) {
+            std::lock_guard lock(active_plugins_mutex);
+
             MSG msg;
 
             // Keep the loop responsive by not handling too many events at once

src/wine-host/bridges/group.h

@@ -176,7 +176,7 @@ class GroupBridge {
      * works, all plugins have to be initialized from the same thread, and all
      * event handling and message loop interaction also has to be done from that
      * thread, which is why we initialize the plugin here and use the
-     * `handle_dispatch_multi()` function to run events within the same
+     * `handle_dispatch()` function to run events within the same
      * `plugin_context`.
      *
      * @see handle_plugin_dispatch
@@ -185,8 +185,7 @@ class GroupBridge {
 
     /**
      * Handle both Win32 messages and X11 events on a timer within the IO
-     * context. This is a centralized replacement for the event handling in
-     * `Vst2Bridge::handle_dispatch_single` for plugin groups.
+     * context for all plugins.
      */
     void async_handle_events();
 

src/wine-host/bridges/vst2.cpp

@@ -65,13 +65,14 @@ Vst2Bridge& get_bridge_instance(const AEffect* plugin) {
     return *static_cast<Vst2Bridge*>(plugin->ptr1);
 }
 
-Vst2Bridge::Vst2Bridge(std::string plugin_dll_path,
+Vst2Bridge::Vst2Bridge(boost::asio::io_context& main_context,
+                       std::string plugin_dll_path,
                        std::string socket_endpoint_path)
-    // See `plugin_handle`s docstring for information on why we're leaking
-    // memory here
-    // : plugin_handle(LoadLibrary(plugin_dll_path.c_str()), FreeLibrary),
-    : plugin_handle(LoadLibrary(plugin_dll_path.c_str())),
-      io_context(),
+    : io_context(main_context),
+      // See `plugin_handle`s docstring for information on why we're leaking
+      // memory here
+      // plugin_handle(LoadLibrary(plugin_dll_path.c_str()), FreeLibrary),
+      plugin_handle(LoadLibrary(plugin_dll_path.c_str())),
       socket_endpoint(socket_endpoint_path),
       host_vst_dispatch(io_context),
       host_vst_dispatch_midi_events(io_context),
@@ -133,9 +134,9 @@ Vst2Bridge::Vst2Bridge(std::string plugin_dll_path,
     // `audioMasterIOChanged` host callback.
     write_object(host_vst_dispatch, EventResult{0, *plugin, std::nullopt});
 
-    // This works functionally identically to the `handle_dispatch_single()`
-    // function below, but this socket will only handle MIDI events. This is
-    // needed because of Win32 API limitations.
+    // This works functionally identically to the `handle_dispatch()` function,
+    // but this socket will only handle MIDI events and it will handle them
+    // eagerly. This is needed because of Win32 API limitations.
     dispatch_midi_events_handler =
         Win32Thread(handle_dispatch_midi_events_proxy, this);
 
@@ -149,33 +150,7 @@ bool Vst2Bridge::should_skip_message_loop() {
     return std::holds_alternative<EditorOpening>(editor);
 }
 
-void Vst2Bridge::handle_dispatch_single() {
-    using namespace std::placeholders;
-
-    // For our communication we use simple threads and blocking operations
-    // instead of asynchronous IO since communication has to be handled in
-    // lockstep anyway
-    try {
-        while (true) {
-            receive_event(host_vst_dispatch, std::nullopt,
-                          passthrough_event(
-                              plugin, std::bind(&Vst2Bridge::dispatch_wrapper,
-                                                this, _1, _2, _3, _4, _5, _6)));
-
-            handle_x11_events();
-            handle_win32_events();
-        }
-    } catch (const boost::system::system_error&) {
-        // The plugin has cut off communications, so we can shut down this host
-        // application
-    }
-}
-
-void Vst2Bridge::handle_dispatch_multi(boost::asio::io_context& main_context) {
-    // This works exactly the same as the function above, but execute the
-    // actual event and run the message loop from the main thread that's
-    // also instantiating these plugins. This is required for a few plugins
-    // to run multiple instances in the same process
+void Vst2Bridge::handle_dispatch() {
     try {
         while (true) {
             receive_event(
@@ -184,8 +159,13 @@ void Vst2Bridge::handle_dispatch_multi(boost::asio::io_context& main_context) {
                     plugin,
                     [&](AEffect* plugin, int opcode, int index, intptr_t value,
                         void* data, float option) -> intptr_t {
+                        // Instead of running `plugin->dispatcher()` (or
+                        // `dispatch_wrapper()`) directly, we'll run the
+                        // function within the IO context so all events will be
+                        // executed on the same thread as the one that runs the
+                        // Win32 message loop
                         std::promise<intptr_t> dispatch_result;
-                        boost::asio::dispatch(main_context, [&]() {
+                        boost::asio::dispatch(io_context, [&]() {
                             const intptr_t result = dispatch_wrapper(
                                 plugin, opcode, index, value, data, option);
 

src/wine-host/bridges/vst2.h

@@ -45,16 +45,12 @@ struct EditorOpening {};
  * plugin and provides host callback function for the plugin to talk back.
  *
  * @remark Because of Win32 API limitations, all window handling has to be done
- *   from the same thread. For individually hosted plugins this only means that
- *   this class has to be initialized from the same thread as the one that calls
- *   `handle_dispatch_single()`, and thus also runs the message loop. When using
- *   plugin groups, however, all instantiation, editor event handling and
- *   message loop pumping has to be done from a single thread. Most plugins
- *   won't have any issues when using multiple message loops, but the Melda
- *   plugins for instance will only update their GUIs from the message loop of
- *   the thread that created the first instance. When running multiple plugins
- *   `handle_dispatch_multi()` should be used to make sure all plugins
- *   handle their events on the same thread.
+ *   from the same thread. Most plugins won't have any issues when using
+ *   multiple message loops, but the Melda plugins for instance will only update
+ *   their GUIs from the message loop of the thread that created the first
+ *   instance. This is why we pass an IO context to this class so everything
+ *   that's not performance critical (audio and midi event handling) is handled
+ *   on the same thread, even when hosting multiple plugins.
  */
 class Vst2Bridge {
    public:
@@ -62,18 +58,23 @@ class Vst2Bridge {
      * Initializes the Windows VST plugin and set up communication with the
      * native Linux VST plugin.
      *
+     * @param main_context The main IO context for this application. Most events
+     *   will be dispatched to this context, and the event handling loop should
+     *   also be run from this context.
      * @param plugin_dll_path A (Unix style) path to the VST plugin .dll file to
      *   load.
      * @param socket_endpoint_path A (Unix style) path to the Unix socket
      *   endpoint the native VST plugin created to communicate over.
      *
-     * @note When using plugin groups and `handle_dispatch_multi()`, this
-     *   object has to be constructed from within the IO context.
+     * @note The object has to be constructed from the same thread that calls
+     *   `main_context.run()`.
      *
      * @throw std::runtime_error Thrown when the VST plugin could not be loaded,
      *   or if communication could not be set up.
      */
-    Vst2Bridge(std::string plugin_dll_path, std::string socket_endpoint_path);
+    Vst2Bridge(boost::asio::io_context& main_context,
+               std::string plugin_dll_path,
+               std::string socket_endpoint_path);
 
     /**
      * Returns true if the message loop should be skipped. This happens when the
@@ -89,31 +90,17 @@ class Vst2Bridge {
     bool should_skip_message_loop();
 
     /**
-     * Handle events on the main thread until the plugin quits. This can't be
-     * done on another thread since some plugins (e.g. Melda) expect certain
-     * events to be passed from the same thread it was initiated from. This is
-     * then also the same thread that should handle Win32 GUI events.
-     */
-    void handle_dispatch_single();
-
-    /**
-     * Handle events just like in the function above, but do the actual
-     * execution on the IO context. As explained in this class' docstring, this
-     * is needed because some plugins make the assumption that all of their
-     * instances are handled from the same thread, and that the thread that the
-     * first instance was initiated on will be kept alive until the VST host
-     * terminates.
+     * Handle events until the plugin exits. The actual events are posted to
+     * `main_context` to ensure that all operations to could potentially
+     * interact with Win32 code are run from a single thread, even when hosting
+     * multiple plugins. The message loop should be run on a timer within the
+     * same IO context.
      *
-     * @param main_context The main IO context that's handling the event
-     *   handling for all plugins.
-     *
-     * @note With this approach you'll have to make sure that the object was
-     *   instantiated from the same thread as the one that runs the IO context.
-     * @note This appraoch does _not_ handle any events. This has to be done on
-     *   a timer within the IO context since otherwise things would become very
-     *   messy very quick.
+     * @note Because of the reasons mentioned above, for this to work the plugin
+     *   should be initialized within the same thread that calls
+     *   `main_context.run()`.
      */
-    void handle_dispatch_multi(boost::asio::io_context& main_context);
+    void handle_dispatch();
 
     /**
      * Handle X11 events for the editor window if it is open. This can be run
@@ -121,6 +108,24 @@ class Vst2Bridge {
      */
     void handle_x11_events();
 
+    /**
+     * Run the message loop for this plugin. This is only used for the
+     * individual plugin host. When hosting multiple plugins, a simple central
+     * message loop with a check to `should_skip_message_loop()` should be used
+     * instead. This is run on a timer in the same IO context as the one that
+     * handles the events, i.e. `main_context`.
+     *
+     * Because of the way the Win32 API works we have to process events on the
+     * same thread as the one the window was created on, and that thread is the
+     * thread that's handling dispatcher calls. Some plugins will also rely on
+     * the Win32 message loop to run tasks on a timer and to defer loading, so
+     * we have to make sure to always run this loop. The only exception is a in
+     * specific situation that can cause a race condition in some plugins
+     * because of incorrect assumptions made by the plugin. See the dostring for
+     * `Vst2Bridge::editor` for more information.
+     */
+    void handle_win32_events();
+
     // These functions are the entry points for the `*_handler` threads
     // defined below. They're defined here because we can't use lambdas with
     // WinAPI's `CreateThread` which is needed to support the proper call
@@ -156,21 +161,11 @@ class Vst2Bridge {
                               float option);
 
     /**
-     * Run the message loop for this plugin and potentially also for other
-     * plugins. This is only used in `handle_dispatch_single()`, as this will be
-     * run on a timer when using plugin groups. The caller should first check
-     * whether the event loop can be run through `should_skip_message_loop()`.
-     *
-     * Because of the way the Win32 API works we have to process events on the
-     * same thread as the one the window was created on, and that thread is the
-     * thread that's handling dispatcher calls. Some plugins will also rely on
-     * the Win32 message loop to run tasks on a timer and to defer loading, so
-     * we have to make sure to always run this loop. The only exception is a in
-     * specific situation that can cause a race condition in some plugins
-     * because of incorrect assumptions made by the plugin. See the dostring for
-     * `Vst2Bridge::editor` for more information.
+     * The IO context used for event handling so that all events and window
+     * message handling can be performed from a single thread, even when hosting
+     * multiple plugins.
      */
-    void handle_win32_events();
+    boost::asio::io_context& io_context;
 
     /**
      * The shared library handle of the VST plugin. I sadly could not get
@@ -194,7 +189,10 @@ class Vst2Bridge {
      */
     AEffect* plugin;
 
-    boost::asio::io_context io_context;
+    /**
+     * The UNIX domain socket endpoint used for communicating to this specific
+     * bridged plugin.
+     */
     boost::asio::local::stream_protocol::endpoint socket_endpoint;
 
     // The naming convention for these sockets is `<from>_<to>_<event>`. For

src/wine-host/individual-host.cpp

@@ -14,7 +14,9 @@
 // You should have received a copy of the GNU General Public License
 // along with this program.  If not, see <https://www.gnu.org/licenses/>.
 
+#include <future>
 #include <iostream>
+#include <thread>
 
 // Generated inside of build directory
 #include <src/common/config/config.h>
@@ -22,6 +24,19 @@
 
 #include "bridges/vst2.h"
 
+using namespace std::literals::chrono_literals;
+
+/**
+ * The delay between calls to the event loop at a more than cinematic 30 fps.
+ */
+constexpr std::chrono::duration event_loop_interval = 1000ms / 30;
+
+/**
+ * Handle both Win32 and X11 events on a timer. This is more or less a
+ * simplified version of `GroupBridge::async_handle_events`.
+ */
+void async_handle_events(boost::asio::steady_timer& timer, Vst2Bridge& bridge);
+
 /**
  * This is the default VST host application. It will load the specified VST2
  * plugin, and then connect back to the `libyabridge.so` instace that spawned
@@ -55,17 +70,66 @@ int __cdecl main(int argc, char* argv[]) {
 #endif
               << std::endl;
 
-    try {
-        Vst2Bridge bridge(plugin_dll_path, socket_endpoint_path);
-        std::cerr << "Finished initializing '" << plugin_dll_path << "'"
-                  << std::endl;
+    // As explained in `Vst2Bridge`, the plugin has to be initialized in the
+    // same thread as the one that calls `io_context.run()`. This setup is
+    // slightly more convoluted than it has to be, but doing it this way we
+    // don't need to differentiate between individually hosted plugins and
+    // plugin groups when it comes to event handling.
+    boost::asio::io_context io_context;
+    std::promise<Vst2Bridge&> bridge_instance;
+    std::thread event_handler([&]() {
+        try {
+            Vst2Bridge bridge(io_context, plugin_dll_path,
+                              socket_endpoint_path);
+            std::cerr << "Finished initializing '" << plugin_dll_path << "'"
+                      << std::endl;
 
-        // Blocks the main thread until the plugin shuts down
-        bridge.handle_dispatch_single();
+            bridge_instance.set_value(bridge);
+        } catch (const std::runtime_error&) {
+            bridge_instance.set_exception(std::current_exception());
+            return;
+        }
+
+        io_context.run();
+    });
+
+    try {
+        Vst2Bridge& bridge = bridge_instance.get_future().get();
+
+        // Handle Win32 messages and X11 events on a timer, just like in
+        // `GroupBridge::async_handle_events()``
+        boost::asio::steady_timer events_timer(io_context);
+        async_handle_events(events_timer, bridge);
+
+        // Handle the dispatcher events within the IO context
+        bridge.handle_dispatch();
+
+        io_context.stop();
+        event_handler.join();
     } catch (const std::runtime_error& error) {
         std::cerr << "Error while initializing Wine VST host:" << std::endl;
         std::cerr << error.what() << std::endl;
 
+        io_context.stop();
+        event_handler.join();
+
         return 1;
     }
 }
+
+void async_handle_events(boost::asio::steady_timer& timer, Vst2Bridge& bridge) {
+    // Try to keep a steady framerate, but add in delays to let other events get
+    // handled if the GUI message handling somehow takes very long.
+    timer.expires_at(std::max(timer.expiry() + event_loop_interval,
+                              std::chrono::steady_clock::now() + 5ms));
+    timer.async_wait([&](const boost::system::error_code& error) {
+        if (error.failed()) {
+            return;
+        }
+
+        bridge.handle_x11_events();
+        bridge.handle_win32_events();
+
+        async_handle_events(timer, bridge);
+    });
+}