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
closed
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)?

Status PR created fixed

Fixed in branch 3.x, commit 0c907ab822

_fix #309333: crash hiding palettes

Resolves: https://musescore.org/en/node/309333

Crash happens on trying to set the focus upon closing the palette,
we try to restore the previous focused widget.
But it may no longer be valid.

Changed to return focus to the score view if possible,
otherwise let Qt worry about it._

Fix version
3.6.1