mcallegari / qlcplus

Q Light Controller Plus (QLC+) is a free and cross-platform software to control DMX or analog lighting systems like moving heads, dimmers, scanners etc. This project is a fork of the great QLC project written by Heikki Junnila that aims to continue the QLC development and to introduce new features.
Apache License 2.0
1.02k stars 360 forks source link

Windows Qt UI: Fix segfault when pressing accent keys (^, `, ´) on german QWERTZ keyboard in VCSlider #1628

Closed timholzhey closed 1 month ago

timholzhey commented 1 month ago

With our Windows QLC setup we noticed you can get QLC+ 4 (Qt5 UI) to crash easily on Windows by following these steps:

This usually happens accidentally because the caret symbol is right next to the escape key on the top left. I just now compiled QLC+ from master for Windows MINGW64 with MSYS2 Qt5 debug libraries (v5.15.15-1) and got this stack trace after following the steps to reproduce the crash:

Thread 1 received signal SIGSEGV, Segmentation fault.
QWidget::setStyleSheet (this=0x0, styleSheet=...)
    at C:/M/B/src/qtbase/src/widgets/kernel/qwidget.cpp:2540
warning: 2540   C:/M/B/src/qtbase/src/widgets/kernel/qwidget.cpp: No such file or directory
(gdb) bt
#0  QWidget::setStyleSheet (this=0x0, styleSheet=...)
    at C:/M/B/src/qtbase/src/widgets/kernel/qwidget.cpp:2540
#1  0x00007fff69f14370 in VCSlider::slotResetButtonClicked (this=0x6989ba0)
    at C:/Users/t/Documents/qlcplus/ui/src/virtualconsole/vcslider.cpp:875
#2  0x00007fff69f14775 in VCSlider::slotKeyPressed (this=0x6989ba0, keySequence=...)
    at C:/Users/t/Documents/qlcplus/ui/src/virtualconsole/vcslider.cpp:898
#3  0x00007fff69dd8c57 in VCWidget::qt_static_metacall (_o=0x6989ba0,
    _c=QMetaObject::InvokeMetaMethod, _id=7, _a=0x5fc310)
    at C:/Users/t/Documents/qlcplus/build/ui/src/qlcplusui_autogen/V6OUMXZR2W/moc_vcwidget.cpp:125
#4  0x00007fff73122611 in doActivate<false> (sender=0x65fb860, signal_index=7, argv=0x5fc310)
    at C:/M/B/src/qtbase/src/corelib/kernel/qobject.cpp:3937
#5  0x00007fff7306650c in QMetaObject::activate (sender=<optimized out>, m=<optimized out>,
    local_signal_index=<optimized out>, argv=<optimized out>)
    at C:/M/B/src/qtbase/src/corelib/kernel/qobject.cpp:3985
#6  0x00007fff69dda640 in VirtualConsole::keyPressed (this=0x65fb860, _t1=...)
    at C:/Users/t/Documents/qlcplus/build/ui/src/qlcplusui_autogen/V6OUMXZR2W/moc_virtualconsole.cpp:313
#7  0x00007fff69f594f4 in VirtualConsole::keyPressEvent (this=0x65fb860, event=0x5fc940)
    at C:/Users/t/Documents/qlcplus/ui/src/virtualconsole/virtualconsole.cpp:1609
#8  0x00007fff728b91ca in QWidget::event (this=0x65fb860, event=0x5fc940)
    at C:/M/B/src/qtbase/src/widgets/kernel/qwidget.cpp:8704
#9  0x00007fff7287853e in QApplicationPrivate::notify_helper (this=this@entry=0xcfb250,
    receiver=receiver@entry=0x65fb860, e=e@entry=0x5fc940)
    at C:/M/B/src/qtbase/src/widgets/kernel/qapplication.cpp:3640
#10 0x00007fff7287fe04 in QApplication::notify (this=<optimized out>, receiver=0x65fb860,
    e=0x5fc940) at C:/M/B/src/qtbase/src/widgets/kernel/qapplication.cpp:3002
#11 0x00007fff73033da8 in QCoreApplication::notifyInternal2 (receiver=0x666d300, event=0x5fc940)
    at C:/M/B/src/qtbase/src/corelib/kernel/qcoreapplication.cpp:1064
#12 QCoreApplication::forwardEvent (receiver=0x666d300, event=event@entry=0x5fc940,
    originatingEvent=originatingEvent@entry=0x0)
    at C:/M/B/src/qtbase/src/corelib/kernel/qcoreapplication.cpp:1079
#13 0x00007fff728ce986 in QWidgetWindow::handleKeyEvent (this=this@entry=0xd555f0,
    event=event@entry=0x5fc940) at C:/M/B/src/qtbase/src/widgets/kernel/qwidgetwindow.cpp:724
#14 0x00007fff728d2a64 in QWidgetWindow::event (this=0xd555f0, event=0x5fc940)
    at C:/M/B/src/qtbase/src/widgets/kernel/qwidgetwindow.cpp:293
#15 0x00007fff7287853e in QApplicationPrivate::notify_helper (this=<optimized out>,
    receiver=0xd555f0, e=0x5fc940) at C:/M/B/src/qtbase/src/widgets/kernel/qapplication.cpp:3640
#16 0x00007fff73034298 in QCoreApplication::notifyInternal2 (receiver=0xd555f0, event=0x5fc940)
    at C:/M/B/src/qtbase/src/corelib/kernel/qcoreapplication.cpp:1064
#17 QCoreApplication::sendSpontaneousEvent (receiver=receiver@entry=0xd555f0,
    event=event@entry=0x5fc940) at C:/M/B/src/qtbase/src/corelib/kernel/qcoreapplication.cpp:1474
#18 0x00007fff6973376b in QGuiApplicationPrivate::processKeyEvent (e=0x69968e0)
    at C:/M/B/src/qtbase/src/gui/kernel/qguiapplication.cpp:2417
#19 0x00007fff6970eecc in QWindowSystemInterface::sendWindowSystemEvents (flags=...)
    at C:/M/B/src/qtbase/src/gui/kernel/qwindowsysteminterface.cpp:1169
#20 0x00007fff73090b02 in QEventDispatcherWin32::processEvents (this=<optimized out>,
    this@entry=0xcfb5f0, flags=...)
    at C:/M/B/src/qtbase/src/corelib/kernel/qeventdispatcher_win.cpp:514
#21 0x00007fffc4d84185 in QWindowsGuiEventDispatcher::processEvents (this=0xcfb5f0, flags=...)
    at C:/M/B/src/qtbase/src/platformsupport/eventdispatchers/qwindowsguieventdispatcher.cpp:73
#22 0x00007fff730325a3 in QEventLoop::processEvents (this=0x5ffc80, flags=...)
    at C:/M/B/src/qtbase/src/corelib/kernel/qeventloop.cpp:142
#23 QEventLoop::exec (this=this@entry=0x5ffc80, flags=..., flags@entry=...)
    at C:/M/B/src/qtbase/src/corelib/kernel/qeventloop.cpp:235
#24 0x00007fff7303b3a5 in QCoreApplication::exec ()
    at C:/M/B/src/qtbase/src/corelib/global/qflags.h:121
#25 0x00007fff6972b5ff in QGuiApplication::exec ()
    at C:/M/B/src/qtbase/src/gui/kernel/qguiapplication.cpp:1870
#26 0x00007fff728784b7 in QApplication::exec ()
    at C:/M/B/src/qtbase/src/widgets/kernel/qapplication.cpp:2832
#27 0x00007ff603e42a75 in main (argc=1, argv=0xecef90)
    at C:/Users/t/Documents/qlcplus/main/main.cpp:373

The error originates in the function VCSlider::slotResetButtonClicked(), called from VCSlider::slotKeyPressed.

void VCSlider::slotKeyPressed(const QKeySequence &keySequence)
{
    if (isEnabled() == false)
        return;

    if (m_overrideResetKeySequence == keySequence)
        slotResetButtonClicked();
    else if (m_playbackFlashKeySequence == keySequence)
        flashPlayback(true);
}
void VCSlider::slotResetButtonClicked()
{
    m_isOverriding = false;
    m_resetButton->setStyleSheet(QString("QToolButton{ background: %1; }")
                                 .arg(m_slider->palette().window().color().name()));

    // request to delete all the active fader channels
    foreach (QSharedPointer<GenericFader> fader, m_fadersMap.values())
    {
        if (!fader.isNull())
            fader->removeAll();
    }

    emit monitorDMXValueChanged(m_monitorValue);
}

The function slotResetButtonClicked shouldn't even be called, but here m_overrideResetKeySequence is empty and notified keySequence is empty as well.

With a little debug statement the function VirtualConsole::keyPressEvent can be seen being passed these keyPress events after pressing the ^ key twice:

virtual void VirtualConsole::keyPressEvent(QKeyEvent*) QKeyEvent event: { key:  94 , autorepeat:  false , count:  1 , text:  "^" , mods:  QFlags<Qt::KeyboardModifier>(NoModifier) , nativevirt:  220 , nativemods:  512 , nativescan:  41  }
virtual void VirtualConsole::keyPressEvent(QKeyEvent*) QKeyEvent event: { key:  0 , autorepeat:  false , count:  1 , text:  "^" , mods:  QFlags<Qt::KeyboardModifier>(NoModifier) , nativevirt:  94 , nativemods:  512 , nativescan:  41  }

So Qt5 on Windows notifies a QKeyEvent with key=0 (unknown key) but autorepeating=false for this special key, this is turned into an empty QKeySequence and passed down the widgets. Since none of the components should ever expect to receive an empty QKeySequence, the problematic event should be ignored. The problem now no longer exists for our Windows build.

Other people who had the same problem: https://www.qlcplus.org/forum/viewtopic.php?t=11857 https://www.qlcplus.org/forum/viewtopic.php?t=13885

yestalgia commented 1 month ago

Thanks for the detailed report.

mcallegari commented 1 month ago

Hi, thanks for the accurate report and related fix. Can you please rebase to current master so this builds correctly? Thanks

timholzhey commented 1 month ago

Hi, sure. I merged the upstream master, it triggered request for a CI rerun. Thank you

coveralls commented 1 month ago

Coverage Status

coverage: 31.983%. remained the same when pulling 27d48ff46f55bed2c14f619a25da31d9841c0bd4 on timholzhey:master into a7440182d0975c7797e0a374c1627ab859ef28ca on mcallegari:master.

mcallegari commented 1 month ago

Merged, thanks 👍