Disconnect controller qt connections #4476
Conversation
Leaving the stale connections can cause segfaults on exit. This fixes https://bugs.launchpad.net/mixxx/+bug/1946581
ywwg
force-pushed
the
disconnect-controllers
branch
from
d728928 to
7b595c0
Compare
| connect(pController, | ||
| m_controllerConnections.append(connect(pController, | ||
| &Controller::openChanged, | ||
| [this, pControllerDlg](bool bOpen) { | ||
| slotHighlightDevice(pControllerDlg, bOpen); | ||
| }); | ||
| })); |
There was a problem hiding this comment.
I think we should always pass a context object as third parameter (this).
If we do that we can probably get rid of the list, and just call disconnect(pController, nullptr, this, nullptr) to remove all connections between a controller and the dialog.
There was a problem hiding this comment.
I did that at first, but I was worried that the call to enumerate the list of controllers would be incorrect at the time of destruction. And since disconnect requires the sender object to be specified, we need to make sure that list is complete. What do you think?
There was a problem hiding this comment.
hm so it looks like because we don't do hotplug, updateControllerList is only called once, so the list of controllers should be static. However once we go to hotplug, we will have big problems with inconsistency between dlgprefcontrollers and the controllermanager. (And these inconsistencies could easily cause crashes). For now, I'll eliminate the list and trust the the controller list doesn't change.
There was a problem hiding this comment.
Updated. This is safe code for now, but may not be in the future. Added some comments as hints to future developers trying to add hotplug so they don't trip over this code. (There is no way to write this code currently that would be future-proof)
There was a problem hiding this comment.
oh I understand your comment now. I don't know how to do what you describe though.
There was a problem hiding this comment.
ahhh I figured out which third parameter you meant. got it
…to disconnect This is safe code for now, but may not be in the future. Added some comments as hints to future developers trying to add hotplug so they don't trip over this code. (There is no way to write this code currently that would be future-proof)
Disconnect connections from controllers when deleting controller prefs
Leaving the stale connections can cause segfaults on exit.
This should fix https://bugs.launchpad.net/mixxx/+bug/1946581