Skip event loop with partially initialized plugins

This should prevent T-RackS 5 from potentially stalling indefinitely
when using plugin groups.

CHANGELOG.md

@@ -107,6 +107,11 @@ TODO: Add an updated screenshot with some fancy VST3-only plugins to the readme
   always executed from the GUI thread. This fixes **EZdrummer** not producing
   any sound because the plugin makes the incorrect assumption that
   `effMainsChanged()` is always called from the GUI thread.
+- Event handling is now temporarily disabled while plugins are in a partially
+  initialized state. The VST2 versions of **T-RackS 5** would have a chance to
+  hang indefinitely if the event loop was being run before those plugins were
+  fully initialized because of a race condition within those plugins. This was
+  issue only noticeable when using plugin groups.
 - Fixed a potential issue where an interaction between _Bitwig Studio_ and
   yabridge's input focus grabbing method could cause delayed mouse events when
   clicking on a plugin's GUI in Bitwig. This issue has not been reported for

src/wine-host/bridges/group.cpp

@@ -117,6 +117,19 @@ GroupBridge::~GroupBridge() {
     stdio_context.stop();
 }
 
+bool GroupBridge::is_event_loop_inhibited() {
+    std::lock_guard lock(active_plugins_mutex);
+
+    for (auto& [parameters, value] : active_plugins) {
+        auto& [thread, bridge] = value;
+        if (bridge->inhibits_event_loop()) {
+            return true;
+        }
+    }
+
+    return false;
+}
+
 void GroupBridge::handle_plugin_run(size_t plugin_id, HostBridge* bridge) {
     // Blocks this thread until the plugin shuts down
     bridge->run();
@@ -256,33 +269,35 @@ void GroupBridge::accept_requests() {
 }
 
 void GroupBridge::async_handle_events() {
-    main_context.async_handle_events([&]() {
-        {
-            // Always handle X11 events
-            std::lock_guard lock(active_plugins_mutex);
-            for (auto& [parameters, value] : active_plugins) {
-                auto& [thread, bridge] = value;
-                bridge->handle_x11_events();
+    main_context.async_handle_events(
+        [&]() {
+            {
+                // Always handle X11 events
+                std::lock_guard lock(active_plugins_mutex);
+                for (auto& [parameters, value] : active_plugins) {
+                    auto& [thread, bridge] = value;
+                    bridge->handle_x11_events();
+                }
             }
-        }
 
-        std::lock_guard lock(active_plugins_mutex);
+            std::lock_guard lock(active_plugins_mutex);
 
-        MSG msg;
+            MSG msg;
 
-        // Keep the loop responsive by not handling too many events at once
-        //
-        // For some reason the Melda plugins run into a seemingly infinite timer
-        // loop for a little while after opening a second editor.  Without this
-        // limit everything will get blocked indefinitely. How could this be
-        // fixed?
-        for (int i = 0; i < max_win32_messages &&
-                        PeekMessage(&msg, nullptr, 0, 0, PM_REMOVE);
-             i++) {
-            TranslateMessage(&msg);
-            DispatchMessage(&msg);
-        }
-    });
+            // Keep the loop responsive by not handling too many events at once
+            //
+            // For some reason the Melda plugins run into a seemingly infinite
+            // timer loop for a little while after opening a second editor.
+            // Without this limit everything will get blocked indefinitely. How
+            // could this be fixed?
+            for (int i = 0; i < max_win32_messages &&
+                            PeekMessage(&msg, nullptr, 0, 0, PM_REMOVE);
+                 i++) {
+                TranslateMessage(&msg);
+                DispatchMessage(&msg);
+            }
+        },
+        [&]() { return !is_event_loop_inhibited(); });
 }
 
 void GroupBridge::async_log_pipe_lines(

src/wine-host/bridges/group.h

@@ -130,6 +130,14 @@ class GroupBridge {
     GroupBridge(const GroupBridge&) = delete;
     GroupBridge& operator=(const GroupBridge&) = delete;
 
+    /**
+     * If this returns `true`, then the group host's event loop should
+     * temporarily be disabled. This simply calls
+     * `HostBridge::inhibits_event_loop()` for all plugins hosted in this group
+     * process.
+     */
+    bool is_event_loop_inhibited();
+
     /**
      * Run a plugin's dispatcher and message loop, processing all events on the
      * main IO context. The plugin will have already been created in

src/wine-host/individual-host.cpp

@@ -132,9 +132,11 @@ __cdecl
 
     // Handle Win32 messages and X11 events on a timer, just like in
     // `GroupBridge::async_handle_events()``
-    main_context.async_handle_events([&]() {
-        bridge->handle_x11_events();
-        bridge->handle_win32_events();
-    });
+    main_context.async_handle_events(
+        [&]() {
+            bridge->handle_x11_events();
+            bridge->handle_win32_events();
+        },
+        [&]() { return !bridge->inhibits_event_loop(); });
     main_context.run();
 }

src/wine-host/utils.h

@@ -41,20 +41,6 @@
  * which can be required because in the Win32 model all GUI related operations
  * have to be handled from the same thread. This will be run from the
  * application's main thread.
- *
- * TODO: Add some point we might have to add some way to inhibit the event loop
- *       from running when a plugin is in a partially initialized state. So far
- *       only the VST2 versions of the T-RackS 5 plugins have this issue (and
- *       only when running them in a plugin group because with individually
- *       hosted plugins the event loop and the plugin's initialization will
- *       align). This will require a fairly significant architectural change to
- *       do this in a clean way (`HostBridge` would need some function that
- *       returns a boolean to indicate whether the event loop should be
- *       inhibited, and then we would need to pass a function object or lambda
- *       to `MainContext` that checks this for all currently hosted plugins so
- *       we can integrate that into `async_handle_events()`). Now that I've
- *       written this out, it's not actually as bad as I thought it would be, so
- *       we'll probably add this sooner rather than later.
  */
 class MainContext {
    public:
@@ -112,33 +98,44 @@ class MainContext {
      * @param handler The function that should be executed in the IO context
      *   when the timer ticks. This should be a function that handles both the
      *   X11 events and the Win32 message loop.
+     * @param predicate A function returning a boolean to indicate whether
+     *   `handler` should be run. If this returns `false`, then the current
+     *   event loop cycle will be skipped. This is used to prevent the Win32
+     *   message loop from being run when there are partially initialized
+     *   plugins. So far the VST2 versions of T-RackS 5 are the only plugins
+     *   where this has been an issue as those plugins have a race condition
+     *   that will cause them to stall indefinitely in this situation, but who
+     *   knows which other plugins exert similar behaviour.
      */
-    template <typename F>
-    void async_handle_events(F handler) {
+    template <typename F, typename P>
+    void async_handle_events(F handler, P predicate) {
         // 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.
         events_timer.expires_at(
             std::max(events_timer.expiry() + timer_interval,
                      std::chrono::steady_clock::now() + timer_interval / 4));
         events_timer.async_wait(
-            [&, handler](const boost::system::error_code& error) {
+            [&, handler, predicate](const boost::system::error_code& error) {
                 if (error.failed()) {
                     return;
                 }
 
-                // NOTE: These periodic callbacks should not be able to
-                //       interrupt other threads that are actively processing
-                //       audio. For me personally having the GUI open makes
-                //       absolutely zero difference on DSP usage (as it should),
-                //       but for some others it does have an impact.
-                // TODO: Benchmark this further on a properly configured system,
-                //       see if it does not increase average load because of the
-                //       rapid scheduling switching.
-                set_realtime_priority(false);
-                handler();
-                set_realtime_priority(true);
+                if (predicate()) {
+                    // NOTE: These periodic callbacks should not be able to
+                    //       interrupt other threads that are actively
+                    //       processing audio. For me personally having the GUI
+                    //       open makes absolutely zero difference on DSP usage
+                    //       (as it should), but for some others it does have an
+                    //       impact.
+                    // TODO: Benchmark this further on a properly configured
+                    //       system, see if it does not increase average load
+                    //       because of the rapid scheduling switching.
+                    set_realtime_priority(false);
+                    handler();
+                    set_realtime_priority(true);
+                }
 
-                async_handle_events(handler);
+                async_handle_events(handler, predicate);
             });
     }