mixxxdj / mixxx

Mixxx is Free DJ software that gives you everything you need to perform live mixes.
http://mixxx.org
Other
4.54k stars 1.28k forks source link

qhash.h:790 ASSERT on multiple MIDI messages in quick succession #13940

Open mxmilkiib opened 3 days ago

mxmilkiib commented 3 days ago

Bug Description

When my script sends multiple MIDI sysex messages in quick succession, it often makes Mixxx trigger an assert;

 [Controller] fatal /usr/include/qt6/QtCore/qhash.h:790 ASSERT: "!isUnused()" in file /usr/include/qt6/QtCore/qhash.h, line 790

Version

No response

OS

No response

Swiftb0y commented 3 days ago

Thank you for reporting your issue. Please understand that in order for us to be able to properly debug this, we need a backtrace. Have a look at the wiki for finding out how to create one: https://github.com/mixxxdj/mixxx/wiki/Creating%20Backtraces

mxmilkiib commented 1 day ago

https://gist.github.com/mxmilkiib/9bee33b0ed7113243c9e1a3a14d5ae01 https://gist.github.com/mxmilkiib/6fe50aeaee3200dd1ff2d346f1b2f978#file-launchpadpromk3-js

daschuer commented 1 day ago

The assertion violation happens here: https://github.com/qt/qtbase/blob/4059ea60e52291ca7e2c9086663c3c3e10839090/src/corelib/tools/qhash.h#L790

Probably a race condition?

daschuer commented 1 day ago

Which version of Mixxx is affected?

Please create a Backtraces: https://github.com/mixxxdj/mixxx/wiki/Creating-Backtraces

Does Mixxx crash?

mxmilkiib commented 1 day ago

I should have said, the first link in the last post is what I got from following those instructions (which fwiw could mention gdb -ex "set height 0")

n yes, crashes

I pulled n built before testing, r2.6-alpha-138-g0eccb1b2db (main)

daschuer commented 1 day ago

https://github.com/daschuer/mixxx/blob/8b81a37302b68e3b072c96cc1539fe578c34976e/src/controllers/midi/midicontroller.cpp#L290

So the race may happen around: LegacyMidiControllerMapping::m_inputMappings However, there is noting obvious.

Is this only a side effect and we are using a dangling pointer, because the m_pMapping is already down? Or is m_pMapping still null?

Al least there are a couple of race conditions happening possible with m_pMapping. std::shared_ptr is thread safe, but the pointer may change between two calls.

This means m_pMapping->getInputMappings().constEnd() point to a new mapping while it still points to the old. The function needs to become an shared owner first before using the m_pMappings.

Please try to alter the code to:

    // Function becomes temorary shared owner 
    std::shared_ptr<LegacyMidiControllerMapping> pMapping = m_pMapping; 
    if (!pMapping) {
        return; 
    }
    auto it = pMapping->getInputMappings().constFind(mappingKey.key);
    for (; it != pMapping->getInputMappings().constEnd() && it.key() == mappingKey.key; ++it) {
mxmilkiib commented 1 day ago

Yes, that change seems to solve the problem, thank you!

edit; what exactly is/was the problem? what part of the process is tripping up?