diff --git a/cockatrice/src/interface/widgets/visual_deck_storage/visual_deck_storage_model.cpp b/cockatrice/src/interface/widgets/visual_deck_storage/visual_deck_storage_model.cpp index dc22d4cb9..f9409f5f2 100644 --- a/cockatrice/src/interface/widgets/visual_deck_storage/visual_deck_storage_model.cpp +++ b/cockatrice/src/interface/widgets/visual_deck_storage/visual_deck_storage_model.cpp @@ -10,7 +10,7 @@ #include #include #include -#include +#include #include #include #include @@ -137,39 +137,6 @@ VisualDeckStorageModel::VisualDeckStorageModel(QObject *parent) : QAbstractListM { } -VisualDeckStorageModel::~VisualDeckStorageModel() -{ - // Invalidate every outstanding result and drop queued work. - ++scanGeneration; - pendingLoads.clear(); - drainScheduled = false; - - // Detach and destroy the watchers of still-running futures explicitly, before - // QObject teardown, so none of their finished() lambdas (which capture this) - // can fire into a half-destroyed model. Futures that have not started yet are - // cancelled; ones already running finish on the pool and their result is dropped. - const QList> watchers = std::exchange(activeWatchers, {}); - for (const QPointer &watcher : watchers) { - if (!watcher) { - continue; - } - disconnect(watcher, nullptr, this, nullptr); - watcher->cancel(); - delete watcher.data(); - } -} - -void VisualDeckStorageModel::trackWatcher(QFutureWatcherBase *watcher) -{ - activeWatchers.append(QPointer(watcher)); -} - -void VisualDeckStorageModel::forgetWatcher(QFutureWatcherBase *watcher) -{ - activeWatchers.removeIf( - [watcher](const QPointer &tracked) { return !tracked || tracked.data() == watcher; }); -} - int VisualDeckStorageModel::rowCount(const QModelIndex &parent) const { return parent.isValid() ? 0 : decks.size(); @@ -277,17 +244,15 @@ void VisualDeckStorageModel::startScan() // The scan (directory walk + one stat per file) runs on a worker thread so that // constructing the widget never stalls the UI thread on a large deck folder. - auto *watcher = new QFutureWatcher(this); - trackWatcher(watcher); - connect(watcher, &QFutureWatcher::finished, this, [this, watcher, generation] { - forgetWatcher(watcher); - watcher->deleteLater(); - + // The continuation is bound to this model as its context object: it runs on the UI thread, + // and Qt drops it if the model is destroyed first, so no result is delivered to a dead model. + QFuture scan = + QtConcurrent::run([currentDeckPath]() -> DeckScanResult { return scanDeckDirectory(currentDeckPath); }); + scan.then(this, [this, generation](const DeckScanResult &result) { if (generation != scanGeneration) { return; // A newer scan started while this one was running; drop the stale result. } - const DeckScanResult result = watcher->result(); folderPaths = result.folderPaths; if (result.decks.isEmpty()) { @@ -303,9 +268,6 @@ void VisualDeckStorageModel::startScan() beginLoad(row); } }); - - watcher->setFuture( - QtConcurrent::run([currentDeckPath]() -> DeckScanResult { return scanDeckDirectory(currentDeckPath); })); } void VisualDeckStorageModel::beginLoad(int row) @@ -321,34 +283,7 @@ void VisualDeckStorageModel::beginLoad(int row) const DeckFileFormat::Format fmt = DeckFileFormat::getFormatFromName(filePath); const int generation = scanGeneration; - auto *watcher = new QFutureWatcher>(this); - trackWatcher(watcher); - connect(watcher, &QFutureWatcher>::finished, this, - [this, watcher, filePath, generation] { - forgetWatcher(watcher); - watcher->deleteLater(); - - if (generation != scanGeneration) { - return; // The deck list was re-scanned while this load was running; drop the stale result. - } - - // Queue the result and apply a bounded number per event loop turn so that - // finishing hundreds of loads at once cannot stall the UI thread. - const std::optional result = watcher->result(); - PendingDeckLoad pending; - pending.filePath = filePath; - pending.generation = generation; - if (result) { - pending.ok = true; - pending.deck = std::move(result->deck); - pending.lastModified = result->lastModified; - pending.colorIdentity = std::move(result->colorIdentity); - } - pendingLoads.append(std::move(pending)); - schedulePendingLoadDrain(); - }); - - watcher->setFuture(QtConcurrent::run([filePath, fmt]() -> std::optional { + auto load = QtConcurrent::run([filePath, fmt]() -> std::optional { std::optional deck = DeckLoader::loadFromFile(filePath, fmt, false); if (!deck) { return std::nullopt; @@ -357,7 +292,28 @@ void VisualDeckStorageModel::beginLoad(int row) // keep the completion handler on the UI thread cheap. const QString colorIdentity = getDeckColorIdentity(deck->deckList, CardDatabaseManager::query()); return DeckLoadResult{std::move(*deck), QFileInfo(filePath).lastModified(), colorIdentity}; - })); + }); + + // Bound to this model as context: runs on the UI thread and is dropped if the model is destroyed first. + load.then(this, [this, filePath, generation](std::optional result) { + if (generation != scanGeneration) { + return; // The deck list was re-scanned while this load was running; drop the stale result. + } + + // Queue the result and apply a bounded number per event loop turn so that + // finishing hundreds of loads at once cannot stall the UI thread. + PendingDeckLoad pending; + pending.filePath = filePath; + pending.generation = generation; + if (result) { + pending.ok = true; + pending.deck = std::move(result->deck); + pending.lastModified = result->lastModified; + pending.colorIdentity = std::move(result->colorIdentity); + } + pendingLoads.append(std::move(pending)); + schedulePendingLoadDrain(); + }); } void VisualDeckStorageModel::schedulePendingLoadDrain() diff --git a/cockatrice/src/interface/widgets/visual_deck_storage/visual_deck_storage_model.h b/cockatrice/src/interface/widgets/visual_deck_storage/visual_deck_storage_model.h index 987d151a6..f9af6f318 100644 --- a/cockatrice/src/interface/widgets/visual_deck_storage/visual_deck_storage_model.h +++ b/cockatrice/src/interface/widgets/visual_deck_storage/visual_deck_storage_model.h @@ -17,11 +17,9 @@ #include #include -#include #include #include #include -#include #include #include #include @@ -109,12 +107,6 @@ class VisualDeckStorageModel : public QAbstractListModel public: explicit VisualDeckStorageModel(QObject *parent = nullptr); - /** - * @brief Cancels and detaches every background scan/load still in flight, so no - * QFutureWatcher can deliver a result into (or outlive) a destroyed model. - */ - ~VisualDeckStorageModel() override; - /// @name Qt model overrides ///@{ [[nodiscard]] int rowCount(const QModelIndex &parent = QModelIndex()) const override; @@ -185,8 +177,6 @@ private: static void recomputeDeckMetadata(DeckPreviewData &data, bool recomputeColorIdentity = true); void reindexFilePaths(); void schedulePendingLoadDrain(); - void trackWatcher(QFutureWatcherBase *watcher); - void forgetWatcher(QFutureWatcherBase *watcher); private slots: void drainPendingLoads(); @@ -201,9 +191,6 @@ private: int scanGeneration = 0; ///< Bumped on every scan so stale results are ignored. QVector pendingLoads; ///< Finished background loads waiting to be applied. bool drainScheduled = false; ///< Whether a queued drain pass is already pending. - - /// Scan/load watchers whose future has not finished yet. - QList> activeWatchers; }; #endif // VISUAL_DECK_STORAGE_MODEL_H diff --git a/cockatrice/src/interface/widgets/visual_deck_storage/visual_deck_storage_widget.cpp b/cockatrice/src/interface/widgets/visual_deck_storage/visual_deck_storage_widget.cpp index 724be6cdf..4e1d6293f 100644 --- a/cockatrice/src/interface/widgets/visual_deck_storage/visual_deck_storage_widget.cpp +++ b/cockatrice/src/interface/widgets/visual_deck_storage/visual_deck_storage_widget.cpp @@ -157,36 +157,6 @@ VisualDeckStorageWidget::VisualDeckStorageWidget(QWidget *parent) : QWidget(pare } } -VisualDeckStorageWidget::~VisualDeckStorageWidget() -{ - // Tear down in dependency order instead of relying on QObject child creation order, - // which would destroy the model first while the proxy and the deck preview widgets - // (which hold raw VisualDeckStorageModel pointers) are still alive. - if (refreshTimer) { - refreshTimer->stop(); - } - - // 1. Views: folder and deck preview widgets live inside the scroll area. - delete scrollArea; - scrollArea = nullptr; - folderWidget = nullptr; - - // 2. Proxy: detach it from the model before either is destroyed. - if (storageProxyModel) { - disconnect(storageProxyModel, nullptr, this, nullptr); - storageProxyModel->setSourceModel(nullptr); - delete storageProxyModel; - storageProxyModel = nullptr; - } - - // 3. Model last; its destructor cancels and detaches in-flight background loads. - if (storageModel) { - disconnect(storageModel, nullptr, this, nullptr); - delete storageModel; - storageModel = nullptr; - } -} - void VisualDeckStorageWidget::refreshIfPossible() { if (scrollArea->widget() != databaseLoadIndicator) { diff --git a/cockatrice/src/interface/widgets/visual_deck_storage/visual_deck_storage_widget.h b/cockatrice/src/interface/widgets/visual_deck_storage/visual_deck_storage_widget.h index ad57d4ef8..19d16f107 100644 --- a/cockatrice/src/interface/widgets/visual_deck_storage/visual_deck_storage_widget.h +++ b/cockatrice/src/interface/widgets/visual_deck_storage/visual_deck_storage_widget.h @@ -34,7 +34,6 @@ class VisualDeckStorageWidget final : public QWidget Q_OBJECT public: explicit VisualDeckStorageWidget(QWidget *parent); - ~VisualDeckStorageWidget() override; void refreshIfPossible(); void retranslateUi(); void setShareSelectable(bool selectable);