From 39ac5716c7427d48d8d073d6224a0b26a188d2b4 Mon Sep 17 00:00:00 2001 From: ispyisail Date: Fri, 14 Aug 2026 13:08:45 +1200 Subject: [PATCH] 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. --- .../ElementsCollection/elementscollectionmodel.cpp | 14 ++++++++++++++ .../ElementsCollection/elementscollectionmodel.h | 1 + 2 files changed, 15 insertions(+) diff --git a/sources/ElementsCollection/elementscollectionmodel.cpp b/sources/ElementsCollection/elementscollectionmodel.cpp index 210886e5d..a32eeb674 100644 --- a/sources/ElementsCollection/elementscollectionmodel.cpp +++ b/sources/ElementsCollection/elementscollectionmodel.cpp @@ -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 diff --git a/sources/ElementsCollection/elementscollectionmodel.h b/sources/ElementsCollection/elementscollectionmodel.h index 1bf12e66e..c2a7297ae 100644 --- a/sources/ElementsCollection/elementscollectionmodel.h +++ b/sources/ElementsCollection/elementscollectionmodel.h @@ -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;