From 4a8abc948ef15664dbde196ba77daccdf3d623ad Mon Sep 17 00:00:00 2001 From: flow Date: Mon, 1 Aug 2022 18:34:15 -0300 Subject: [PATCH] fix: prevent segfault due to callbacks into deleted objects Since network requests are, for the most part, asynchronous, there's a chance a request only comes through after the request sender has already been deleted. This adds a global (read static) hash table relating models for the mod downloader to their status (true = alive, false = destroyed). It is a bit of a hack, but I couldn't come up with a better way of doing this. To reproduce the issue before this commit: scroll really quickly through CF mods, to trigger network requests for their versions and description. Then, in the middle of it close the mod downloader. Sometimes this will create a crash. Signed-off-by: flow --- launcher/ui/pages/modplatform/ModModel.cpp | 24 ++++++++++++++++++---- launcher/ui/pages/modplatform/ModModel.h | 2 +- 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/launcher/ui/pages/modplatform/ModModel.cpp b/launcher/ui/pages/modplatform/ModModel.cpp index 9d7a9970..9cf9ec29 100644 --- a/launcher/ui/pages/modplatform/ModModel.cpp +++ b/launcher/ui/pages/modplatform/ModModel.cpp @@ -13,7 +13,16 @@ namespace ModPlatform { -ListModel::ListModel(ModPage* parent) : QAbstractListModel(parent), m_parent(parent) {} +// HACK: We need this to prevent callbacks from calling the ListModel after it has already been deleted. +// This leaks a tiny bit of memory per time the user has opened the mod dialog. How to make this better? +static QHash s_running; + +ListModel::ListModel(ModPage* parent) : QAbstractListModel(parent), m_parent(parent) { s_running.insert(this, true); } + +ListModel::~ListModel() +{ + s_running.find(this).value() = false; +} auto ListModel::debugName() const -> QString { @@ -101,7 +110,11 @@ void ListModel::requestModVersions(ModPlatform::IndexedPack const& current, QMod auto profile = (dynamic_cast((dynamic_cast(parent()))->m_instance))->getPackProfile(); m_parent->apiProvider()->getVersions({ current.addonId.toString(), getMineVersions(), profile->getModLoaders() }, - [this, current, index](QJsonDocument& doc, QString addonId) { versionRequestSucceeded(doc, addonId, index); }); + [this, current, index](QJsonDocument& doc, QString addonId) { + if (!s_running.constFind(this).value()) + return; + versionRequestSucceeded(doc, addonId, index); + }); } void ListModel::performPaginatedSearch() @@ -114,8 +127,11 @@ void ListModel::performPaginatedSearch() void ListModel::requestModInfo(ModPlatform::IndexedPack& current, QModelIndex index) { - m_parent->apiProvider()->getModInfo( - current, [this, index](QJsonDocument& doc, ModPlatform::IndexedPack& pack) { infoRequestFinished(doc, pack, index); }); + m_parent->apiProvider()->getModInfo(current, [this, index](QJsonDocument& doc, ModPlatform::IndexedPack& pack) { + if (!s_running.constFind(this).value()) + return; + infoRequestFinished(doc, pack, index); + }); } void ListModel::refresh() diff --git a/launcher/ui/pages/modplatform/ModModel.h b/launcher/ui/pages/modplatform/ModModel.h index d338b8c0..a58c7c55 100644 --- a/launcher/ui/pages/modplatform/ModModel.h +++ b/launcher/ui/pages/modplatform/ModModel.h @@ -18,7 +18,7 @@ class ListModel : public QAbstractListModel { public: ListModel(ModPage* parent); - ~ListModel() override = default; + ~ListModel() override; inline auto rowCount(const QModelIndex& parent) const -> int override { return modpacks.size(); }; inline auto columnCount(const QModelIndex& parent) const -> int override { return 1; };