[VisualDeckStorage] Use QFuture continuations instead of QFutureWatcher tracking

Co-authored-by: sentry[bot] <39604003+sentry[bot]@users.noreply.github.com>
This commit is contained in:
rickyrister 2026-11-01 08:51:16 +00:00
parent 75b6031417
commit 1a1d58d3db
4 changed files with 29 additions and 117 deletions

View file

@ -10,7 +10,7 @@
#include <QElapsedTimer> #include <QElapsedTimer>
#include <QFile> #include <QFile>
#include <QFileInfo> #include <QFileInfo>
#include <QFutureWatcher> #include <QFuture>
#include <QMetaObject> #include <QMetaObject>
#include <QMetaType> #include <QMetaType>
#include <QSet> #include <QSet>
@ -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<QPointer<QFutureWatcherBase>> watchers = std::exchange(activeWatchers, {});
for (const QPointer<QFutureWatcherBase> &watcher : watchers) {
if (!watcher) {
continue;
}
disconnect(watcher, nullptr, this, nullptr);
watcher->cancel();
delete watcher.data();
}
}
void VisualDeckStorageModel::trackWatcher(QFutureWatcherBase *watcher)
{
activeWatchers.append(QPointer<QFutureWatcherBase>(watcher));
}
void VisualDeckStorageModel::forgetWatcher(QFutureWatcherBase *watcher)
{
activeWatchers.removeIf(
[watcher](const QPointer<QFutureWatcherBase> &tracked) { return !tracked || tracked.data() == watcher; });
}
int VisualDeckStorageModel::rowCount(const QModelIndex &parent) const int VisualDeckStorageModel::rowCount(const QModelIndex &parent) const
{ {
return parent.isValid() ? 0 : decks.size(); 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 // 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. // constructing the widget never stalls the UI thread on a large deck folder.
auto *watcher = new QFutureWatcher<DeckScanResult>(this); // The continuation is bound to this model as its context object: it runs on the UI thread,
trackWatcher(watcher); // and Qt drops it if the model is destroyed first, so no result is delivered to a dead model.
connect(watcher, &QFutureWatcher<DeckScanResult>::finished, this, [this, watcher, generation] { QFuture<DeckScanResult> scan =
forgetWatcher(watcher); QtConcurrent::run([currentDeckPath]() -> DeckScanResult { return scanDeckDirectory(currentDeckPath); });
watcher->deleteLater(); scan.then(this, [this, generation](const DeckScanResult &result) {
if (generation != scanGeneration) { if (generation != scanGeneration) {
return; // A newer scan started while this one was running; drop the stale result. return; // A newer scan started while this one was running; drop the stale result.
} }
const DeckScanResult result = watcher->result();
folderPaths = result.folderPaths; folderPaths = result.folderPaths;
if (result.decks.isEmpty()) { if (result.decks.isEmpty()) {
@ -303,9 +268,6 @@ void VisualDeckStorageModel::startScan()
beginLoad(row); beginLoad(row);
} }
}); });
watcher->setFuture(
QtConcurrent::run([currentDeckPath]() -> DeckScanResult { return scanDeckDirectory(currentDeckPath); }));
} }
void VisualDeckStorageModel::beginLoad(int row) void VisualDeckStorageModel::beginLoad(int row)
@ -321,34 +283,7 @@ void VisualDeckStorageModel::beginLoad(int row)
const DeckFileFormat::Format fmt = DeckFileFormat::getFormatFromName(filePath); const DeckFileFormat::Format fmt = DeckFileFormat::getFormatFromName(filePath);
const int generation = scanGeneration; const int generation = scanGeneration;
auto *watcher = new QFutureWatcher<std::optional<DeckLoadResult>>(this); auto load = QtConcurrent::run([filePath, fmt]() -> std::optional<DeckLoadResult> {
trackWatcher(watcher);
connect(watcher, &QFutureWatcher<std::optional<DeckLoadResult>>::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<DeckLoadResult> 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<DeckLoadResult> {
std::optional<LoadedDeck> deck = DeckLoader::loadFromFile(filePath, fmt, false); std::optional<LoadedDeck> deck = DeckLoader::loadFromFile(filePath, fmt, false);
if (!deck) { if (!deck) {
return std::nullopt; return std::nullopt;
@ -357,7 +292,28 @@ void VisualDeckStorageModel::beginLoad(int row)
// keep the completion handler on the UI thread cheap. // keep the completion handler on the UI thread cheap.
const QString colorIdentity = getDeckColorIdentity(deck->deckList, CardDatabaseManager::query()); const QString colorIdentity = getDeckColorIdentity(deck->deckList, CardDatabaseManager::query());
return DeckLoadResult{std::move(*deck), QFileInfo(filePath).lastModified(), colorIdentity}; 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<DeckLoadResult> 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() void VisualDeckStorageModel::schedulePendingLoadDrain()

View file

@ -17,11 +17,9 @@
#include <QAbstractListModel> #include <QAbstractListModel>
#include <QDateTime> #include <QDateTime>
#include <QFutureWatcherBase>
#include <QHash> #include <QHash>
#include <QList> #include <QList>
#include <QModelIndex> #include <QModelIndex>
#include <QPointer>
#include <QString> #include <QString>
#include <QStringList> #include <QStringList>
#include <QVector> #include <QVector>
@ -109,12 +107,6 @@ class VisualDeckStorageModel : public QAbstractListModel
public: public:
explicit VisualDeckStorageModel(QObject *parent = nullptr); 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 /// @name Qt model overrides
///@{ ///@{
[[nodiscard]] int rowCount(const QModelIndex &parent = QModelIndex()) const override; [[nodiscard]] int rowCount(const QModelIndex &parent = QModelIndex()) const override;
@ -185,8 +177,6 @@ private:
static void recomputeDeckMetadata(DeckPreviewData &data, bool recomputeColorIdentity = true); static void recomputeDeckMetadata(DeckPreviewData &data, bool recomputeColorIdentity = true);
void reindexFilePaths(); void reindexFilePaths();
void schedulePendingLoadDrain(); void schedulePendingLoadDrain();
void trackWatcher(QFutureWatcherBase *watcher);
void forgetWatcher(QFutureWatcherBase *watcher);
private slots: private slots:
void drainPendingLoads(); void drainPendingLoads();
@ -201,9 +191,6 @@ private:
int scanGeneration = 0; ///< Bumped on every scan so stale results are ignored. int scanGeneration = 0; ///< Bumped on every scan so stale results are ignored.
QVector<PendingDeckLoad> pendingLoads; ///< Finished background loads waiting to be applied. QVector<PendingDeckLoad> pendingLoads; ///< Finished background loads waiting to be applied.
bool drainScheduled = false; ///< Whether a queued drain pass is already pending. bool drainScheduled = false; ///< Whether a queued drain pass is already pending.
/// Scan/load watchers whose future has not finished yet.
QList<QPointer<QFutureWatcherBase>> activeWatchers;
}; };
#endif // VISUAL_DECK_STORAGE_MODEL_H #endif // VISUAL_DECK_STORAGE_MODEL_H

View file

@ -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() void VisualDeckStorageWidget::refreshIfPossible()
{ {
if (scrollArea->widget() != databaseLoadIndicator) { if (scrollArea->widget() != databaseLoadIndicator) {

View file

@ -34,7 +34,6 @@ class VisualDeckStorageWidget final : public QWidget
Q_OBJECT Q_OBJECT
public: public:
explicit VisualDeckStorageWidget(QWidget *parent); explicit VisualDeckStorageWidget(QWidget *parent);
~VisualDeckStorageWidget() override;
void refreshIfPossible(); void refreshIfPossible();
void retranslateUi(); void retranslateUi();
void setShareSelectable(bool selectable); void setShareSelectable(bool selectable);