Stop browse thread before deleting the library #11593
Conversation
|
IIUC this isn't stopping the BrowseThread, it just resets the BrowseThreadPointer to prevent further calls from and callbacks to the BrowseFeature/BrowseTableModel. Is real stopping required? (didn't understand the actual bug tbh) |
|
Yes, the code is not obvious. I have added a comment. The issue was that if the browse thread is using the library and when you quit Mixxx in the mean time from the main thread. It is using dangling pointers and crashes Mixxx. Currently the recording feature and the browse feature are using this thread. The recording feature is destructed earlier because it is places in the widget tree of the skin. This asymmetry is not obvious and bites me during development, but a fix would involve a bigger refactoring. Probably required when moving to QML, but not in the stable 2.3 branch. |
|
@mixxxdj/developers Anyone else minds taking a look? |
| @@ -471,3 +473,10 @@ QAbstractItemDelegate* BrowseTableModel::delegateForColumn(const int i, QObject* | |||
| } | |||
| return nullptr; | |||
| } | |||
|
|
|||
| void BrowseTableModel::stopBrowseThread() { | |||
| // The shared browse thread is actually stopped in the desctuctor | |||
| @@ -471,3 +473,10 @@ QAbstractItemDelegate* BrowseTableModel::delegateForColumn(const int i, QObject* | |||
| } | |||
| return nullptr; | |||
| } | |||
|
|
|||
| void BrowseTableModel::stopBrowseThread() { | |||
There was a problem hiding this comment.
The name of this function is inappropriate as the comment tells. It doesn't stop the browse thread, it just drops one of multiple references.
-> resetBrowseThread() or releaseBrowseThread()?
|
Done. |
|
Thank you! |
This will fix #11589