mirror of
https://github.com/qelectrotech/qelectrotech-source-mirror.git
synced 2026-08-14 10:34:14 +02:00
Fix crash on cancelling the open-element dialog before collection load finishes
Bugtracker #291: clicking Cancel on the open/save-element dialog before the user collection finishes loading crashes the whole application with an unhandled pointer exception. ElementsCollectionModel::loadCollections() loads collections in the background via QtConcurrent::map(m_items_list_to_setUp, setUpData) - worker threads call setUpData() on each ElementCollectionItem (a QStandardItem), which does setFlags()/setData() on it. ElementDialog::execConfiguredDialog() deletes the dialog immediately after exec() returns: element_dialog->exec(); ... delete element_dialog; That destroys the tree view and its ElementsCollectionModel, which as a QStandardItemModel frees all its items in its destructor. Nothing waited for the QtConcurrent::map() to finish first, so on Cancel before loading completes, background threads were still calling setUpData() on items the main thread had just freed - a use-after-free race. Add an ElementsCollectionModel destructor that waits for the future before QStandardItemModel's destructor runs. QFuture::waitForFinished() on a default-constructed (never-started) future returns immediately, so this is a no-op whenever loading already completed - the crash path is the only one affected.
This commit is contained in:
@@ -38,6 +38,20 @@ ElementsCollectionModel::ElementsCollectionModel(QObject *parent) :
|
||||
{
|
||||
}
|
||||
|
||||
/**
|
||||
@brief ElementsCollectionModel::~ElementsCollectionModel
|
||||
Destructor. loadCollections() may still have background threads
|
||||
(via QtConcurrent::map()) running setUpData() on this model's items
|
||||
when the model is destroyed (e.g. the user cancels the dialog before
|
||||
loading finishes). Wait for them here so QStandardItemModel's
|
||||
destructor doesn't free items out from under them, which used to
|
||||
crash the whole application (bugtracker #291).
|
||||
*/
|
||||
ElementsCollectionModel::~ElementsCollectionModel()
|
||||
{
|
||||
m_future.waitForFinished();
|
||||
}
|
||||
|
||||
/**
|
||||
@brief ElementsCollectionModel::data
|
||||
Reimplemented from QStandardItemModel
|
||||
|
||||
@@ -35,6 +35,7 @@ class ElementsCollectionModel : public QStandardItemModel
|
||||
|
||||
public:
|
||||
ElementsCollectionModel(QObject *parent = Q_NULLPTR);
|
||||
~ElementsCollectionModel() override;
|
||||
|
||||
QVariant data(const QModelIndex &index, int role) const override;
|
||||
QMimeData *mimeData(const QModelIndexList &indexes) const override;
|
||||
|
||||
Reference in New Issue
Block a user