LMMS / lmms

Cross-platform music production software
https://lmms.io
GNU General Public License v2.0
7.81k stars 987 forks source link

ZynAddSubFX crashes when trying to save instrument #4455

Open Jernemies opened 6 years ago

Jernemies commented 6 years ago

If you try to save an .xiz file from the Zyn UI's "save instrument option", the Zyn window rapidly "pings" itself, then crashes. Only the plugin crashes, LMMS is fine even after that. After opening the Zyn UI again, it resets to a default sine sound. 1.2.0RC6 on Windows 7 64-bit

PhysSong commented 6 years ago

Confirmed. It's a Windows-only issue. It happens when the mouse position at the time you click the menu is on any dial knobs. You can work around by selecting the menu by keyboard while mouse is outside the ZynAddSubFX's GUI.

This issue seems like a bug of FLTK as well as ZynAddSubFX(at least our version). FLTK has introduced a Windows-specific code in Fl_Window::hide because of a bug. It hides and re-shows windows owned by it. Showing/hiding a window makes a call to Fl::belowmouse and it calls event handlers for widget which was below mouse with FL_LEAVE.

WidgetPDial::handle calls tipwin->hide() when the event is FL_HIDE or FL_LEAVE. If a WidgetPDial was below mouse, it causes infinite cyclic recursion and ZynAddSubFX crashes. The problem is FLTK has no check for situations like this.

For reference, here's a backtrace from WinDbg using custom debug build of FLTK:

(repeat)
RemoteZynAddSubFx + 0x191ad
libfltk!ZN2Fl10belowmouseEP9Fl_Widget + 0x86
libfltk!ZN8Fl_Group6handleEi + 0x382
libfltk!ZN9Fl_Window6handleEi + 0x124
libfltk!Z12fl_fix_focusv + 0x183
libfltk!ZN4Fl_X4makeEP9Fl_Window + 0xc79
libfltk!ZN9Fl_Window4showEv + 0x91
libfltk!ZN16Fl_Double_Window4showEv + 0x18
libfltk!ZN9Fl_Window4hideEv + 0x529
libfltk!ZN14Fl_Menu_Window4hideEv + 0x21
RemoteZynAddSubFx + 0x191ad
libfltk!ZN2Fl10belowmouseEP9Fl_Widget + 0x86
libfltk!ZN8Fl_Group6handleEi + 0x382
libfltk!ZN9Fl_Window6handleEi + 0x124
libfltk!Z12fl_fix_focusv + 0x183
libfltk!ZN4Fl_X4makeEP9Fl_Window + 0xc79
libfltk!ZN9Fl_Window4showEv + 0x91
libfltk!ZN16Fl_Double_Window4showEv + 0x18
libfltk!ZN9Fl_Window4hideEv + 0x529
libfltk!ZN14Fl_Menu_Window4hideEv + 0x21
RemoteZynAddSubFx + 0x191ad
libfltk!ZN2Fl10belowmouseEP9Fl_Widget + 0x86
libfltk!ZN8Fl_Group6handleEi + 0x382
libfltk!ZN9Fl_Window6handleEi + 0x124
libfltk!Z12fl_fix_focusv + 0x183
libfltk!ZN4Fl_X4makeEP9Fl_Window + 0xc79
libfltk!ZN9Fl_Window4showEv + 0x91
PhysSong commented 6 years ago

I'm going to report to FLTK upstream when possible. It definitely looks like a bug of it. BTW, I think we(downstream) can work around the issue by checking for recursion. I'll try to fix this.

PhysSong commented 6 years ago

It seems like this bug can be worked around. This patch fixes the crash, but FLTK doesn't allow hiding tooltip of the WidgetPDial under mouse. It's an unavoidable side effect unless we patch FLTK.

diff --git a/plugins/zynaddsubfx/zynaddsubfx/src/UI/WidgetPDial.cpp b/plugins/zynaddsubfx/zynaddsubfx/src/UI/WidgetPDial.cpp
index ef752ce70..6de1b1a80 100644
--- a/plugins/zynaddsubfx/zynaddsubfx/src/UI/WidgetPDial.cpp
+++ b/plugins/zynaddsubfx/zynaddsubfx/src/UI/WidgetPDial.cpp
@@ -98,7 +98,7 @@ const char *TipWin::getStr() const
 //static int numobj = 0;

 WidgetPDial::WidgetPDial(int x, int y, int w, int h, const char *label)
-    :Fl_Dial(x, y, w, h, label), oldvalue(0.0f), pos(false), textset(false)
+    :Fl_Dial(x, y, w, h, label), oldvalue(0.0f), pos(false), textset(false), tipHideDepth(0)
 {
     //cout << "[" << label << "] There are now " << ++numobj << endl;
     Fl_Group *save = Fl_Group::current();
@@ -159,8 +159,13 @@ int WidgetPDial::handle(int event)
             return 1;
         case FL_HIDE:
         case FL_LEAVE:
-            tipwin->hide();
-            resetPos();
+            // Workaround for FLTK bug
+            if (tipHideDepth == 0) {
+                ++tipHideDepth;
+                tipwin->hide();
+               resetPos();
+                --tipHideDepth;
+            }
             break;
         case FL_RELEASE:
             tipwin->hide();
diff --git a/plugins/zynaddsubfx/zynaddsubfx/src/UI/WidgetPDial.h b/plugins/zynaddsubfx/zynaddsubfx/src/UI/WidgetPDial.h
index b386b542e..d1925f7a4 100644
--- a/plugins/zynaddsubfx/zynaddsubfx/src/UI/WidgetPDial.h
+++ b/plugins/zynaddsubfx/zynaddsubfx/src/UI/WidgetPDial.h
@@ -20,6 +20,7 @@ class WidgetPDial:public Fl_Dial
         double oldvalue;
         bool   pos;
         bool   textset;
+        int tipHideDepth;
         class TipWin * tipwin;
 };
 #endif
zonkmachine commented 1 month ago

I'm going to report to FLTK upstream when possible. It definitely looks like a bug of it. BTW, I think we(downstream) can work around the issue by checking for recursion. I'll try to fix this.

Is there an upstream bug report for this issue?