From becb73928e75f77b8b4978f03ee891013fb26698 Mon Sep 17 00:00:00 2001
From: Robbert van der Helm <mail@robbertvanderhelm.nl>
Date: Fri, 14 Aug 2020 11:36:55 +0200
Subject: [PATCH] Add a limit to all Win32 message loops #28

This works around Waves plugins causing an infinite message loop. Since
we run the loop 30 times per second anyways splitting the loop up into
chunks of 20 shouldn't be an issue.
---
 CHANGELOG.md                    |  5 +++++
 src/wine-host/bridges/group.cpp | 14 ++++++++------
 src/wine-host/bridges/vst2.cpp  | 31 ++++++++++++++++---------------
 src/wine-host/editor.cpp        |  8 ++++++--
 src/wine-host/editor.h          | 11 +++++++++++
 5 files changed, 46 insertions(+), 23 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 7a5f37bf..de975280 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -12,6 +12,11 @@ Versioning](https://semver.org/spec/v2.0.0.html).
 
 - Don't print calls to `effIdle()` when `YABRIDGE_DEBUG_LEVEL` is set to 1.
 
+### Fixed
+
+- Fix Waves plugins from freezing the plugin process by preventing them from
+  causing an infinite message loop.
+
 ## [1.4.1] - 2020-07-27
 
 ### yabridgectl
diff --git a/src/wine-host/bridges/group.cpp b/src/wine-host/bridges/group.cpp
index f034bcda..bb01b6bb 100644
--- a/src/wine-host/bridges/group.cpp
+++ b/src/wine-host/bridges/group.cpp
@@ -262,12 +262,14 @@ void GroupBridge::async_handle_events() {
             MSG msg;
 
             // Keep the loop responsive by not handling too many events at once
-            // TODO: 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 < 20 && PeekMessage(&msg, nullptr, 0, 0, PM_REMOVE); i++) {
+            //
+            // 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);
             }
diff --git a/src/wine-host/bridges/vst2.cpp b/src/wine-host/bridges/vst2.cpp
index 4f137b85..b85723d1 100644
--- a/src/wine-host/bridges/vst2.cpp
+++ b/src/wine-host/bridges/vst2.cpp
@@ -397,22 +397,23 @@ intptr_t Vst2Bridge::dispatch_wrapper(AEffect* plugin,
 }
 
 void Vst2Bridge::handle_win32_events() {
-    std::visit(
-        overload{[](Editor& editor) { editor.handle_win32_events(); },
-                 [](std::monostate&) {
-                     MSG msg;
+    std::visit(overload{[](Editor& editor) { editor.handle_win32_events(); },
+                        [](std::monostate&) {
+                            MSG msg;
 
-                     while (PeekMessage(&msg, nullptr, 0, 0, PM_REMOVE)) {
-                         TranslateMessage(&msg);
-                         DispatchMessage(&msg);
-                     }
-                 },
-                 [](EditorOpening&) {
-                     // Don't handle any events in this
-                     // particular case as explained in
-                     // `Vst2Bridge::editor`
-                 }},
-        editor);
+                            for (int i = 0;
+                                 i < max_win32_messages &&
+                                 PeekMessage(&msg, nullptr, 0, 0, PM_REMOVE);
+                                 i++) {
+                                TranslateMessage(&msg);
+                                DispatchMessage(&msg);
+                            }
+                        },
+                        [](EditorOpening&) {
+                            // Don't handle any events in this particular case
+                            // as explained in `Vst2Bridge::editor`
+                        }},
+               editor);
 }
 
 void Vst2Bridge::handle_x11_events() {
diff --git a/src/wine-host/editor.cpp b/src/wine-host/editor.cpp
index 7cb10e2b..ad03bdea 100644
--- a/src/wine-host/editor.cpp
+++ b/src/wine-host/editor.cpp
@@ -173,8 +173,12 @@ void Editor::handle_win32_events() const {
     MSG msg;
 
     // The null value for the second argument is needed to handle interaction
-    // with child GUI components
-    while (PeekMessage(&msg, nullptr, 0, 0, PM_REMOVE)) {
+    // with child GUI components. So far limiting this to `max_win32_messages`
+    // messages has only been needed for Waves plugins as they otherwise cause
+    // an infinite message loop.
+    for (int i = 0;
+         i < max_win32_messages && PeekMessage(&msg, nullptr, 0, 0, PM_REMOVE);
+         i++) {
         // This timer would periodically send `effEditIdle` events so the editor
         // remains responsive even during blocking GUI operations such as open
         // dropdowns or message boxes. This is only needed when the GUI is
diff --git a/src/wine-host/editor.h b/src/wine-host/editor.h
index d3c1c769..fd81c788 100644
--- a/src/wine-host/editor.h
+++ b/src/wine-host/editor.h
@@ -35,6 +35,17 @@
 #include "../common/configuration.h"
 #include "utils.h"
 
+/**
+ * The maximum number of Win32 messages to handle per message loop. This is
+ * needed because otherwise some plugins can run into an infinite loop. I've
+ * observed this with:
+ *
+ * - Waves plugins
+ * - Melda plugins when having multiple editor windows open within a single
+ *   plugin group
+ */
+constexpr int max_win32_messages = 20;
+
 /**
  * Used to store the maximum width and height of a screen.
  */