F9 to hide the palette occasionally crashes MuseScore 3

• Aug 19, 2020 - 16:32
Reported version
3.2
Type
Functional
Frequency
Once
Severity
S4 - Minor
Reproducibility
Always
Status
active
Regression
Yes
Workaround
No
Project

Ever since switching from MuseScore 2 to 3 (I’ve seen this in 3.0 and 3.2, but according to git log -p the code hasn’t changed in 3.x since) I get occasional crashes when pressing F9 to hide the palette (not when using it to show it, at least not me).

I’ve installed some dbgsym packages and got a backtrace:

 #0  0x00007f9d86e125b0 in QWidget::setFocus (this=0x561c0cea37a0, reason=Qt::OtherFocusReason)
     at kernel/qwidget.cpp:6296
 #1  0x0000561c07f95a35 in QWidget::setFocus (this=<optimized out>)
     at /usr/include/x86_64-linux-gnu/qt5/QtWidgets/qwidget.h:420
 #2  Ms::MuseScore::cmd (this=0x561c0aed50c0, a=0x561c09c162d0) at ./mscore/musescore.cpp:5803
 []

The code in question is thus:

 5789       if (cmdn == "toggle-palette") {   
 5790             showPalette(a->isChecked());
 5791             PaletteBox* pb = mscore->getPaletteBox();
 5792             QLineEdit* sb = pb->searchBox();
 5793             if (a->isChecked()) {
 5794                   lastFocusWidget = QApplication::focusWidget();
 5795                   sb->setFocus();
 5796                   if (pb->noSelection())
 5797                         pb->setKeyboardNavigation(false);
 5798                   else
 5799                         pb->setKeyboardNavigation(true);
 5800                   }
 5801             else {
 5802                   if (lastFocusWidget)
 5803                         lastFocusWidget->setFocus();
 5804                   }
 5805             return;
 5806             }

I suspect that lastFocusWidget is, while being not NULL (otherwise the line wouldn’t be executed), set to a value that was already free’d.


Comments

Indeed, here is the reliable way to reproduce the issue:
1) Make sure palettes are hidden;
2) Select anything in a score;
3) Focus any widget in Inspector (for example, put a cursor in "Offset" field);
4) Press F9 to show palettes;
5) Deselect everything in a score;
6) Press F9 to hide palettes.
After these steps MuseScore always crashes for me.

The reason seems to be that lastFocusWidget variable is only changed within the toggle-palette action handling code, and if that widget is deleted between showing and hiding palettes this variable does not get updated accordingly.

That’s about the reason I guessed, but thanks for providing a reliable reproducer!

What’s this setFocus call good for, anyway? Can’t we just get rid of it (and the widget caching)?