Fix crash due to concurrent access in MidiController (Alternative)#14159
Fix crash due to concurrent access in MidiController (Alternative)#14159acolombier merged 11 commits intomixxxdj:2.5from
Conversation
|
@mxmilkiib Can you check out his? |
|
@mxmilkiib I have added mutex with debug Information. |
|
@mxmilkiib Does this still crash? |
|
yes maaaybe this is config file related, removing files manually but Mixxx not forgetting can't remember the state from before though |
|
Can you please share the mixxx.log of the crash? |
|
ah this is the older issue I'll see if I can replicate now, not had any recent ones for this issue though, building from this branch that is |
|
Cool, please also share the mixxx.log from a short test run without crash. This will help me polishing. |
|
https://gist.github.com/mxmilkiib/b223874ec20f85b07840b7e0d2a5ba55 I managed to make it crash, on clicking play though.
Otherwise, no problem experienced (except the waveforms were lo res, idk). |
|
The crasher is unrelated and tracked here: |
…ns and assert thread affinity
|
@mxmilkiib Please test if the crasher is fixed now. |
|
I haven't had any further issues so I guess draw a line under it for now. |
|
Thank you for confirming. |
acolombier
left a comment
acolombier
left a comment
There was a problem hiding this comment.
Code's LGTM, but no Midi mapping to test it with. Looks reasonably safe to merge without it, thanks!
| m_pControllerManager.get(), | ||
| &ControllerManager::slotApplyMapping); | ||
| &ControllerManager::slotApplyMapping, | ||
| Qt::BlockingQueuedConnection); |
There was a problem hiding this comment.
This has introduced a deadlock when using with screen controller, which rely on the main thread (GUI) availability (see doc)
3 participants
This is an alternative to fix #13940
I have now "hopfully" managed to wrap my head around the issue. The issue was that the mapping has been cloned from the GUI thread while the Controller thread has applied JS created mappings. This creates a broken copy of the mapping that crashes Mixxx after it was applied back to the Controller thread.
The solution here keeps now a clean mutable copy of the mapping for the GUI thread. The GUI thread waits until the controller thread has cloned the mapping during "Apply" this way we have sorted out all concurrent access to the mappings.
This PR is unfortunately a big bigger than expected, because some related code and comments have not been maintained over the years and was misleading making the issue hard to understand.