fix(memory leak): IndexedPack too large to live inside a qlist without pointers ()

Signed-off-by: Rachel Powers <508861+Ryex@users.noreply.github.com>
This commit is contained in:
Rachel Powers 2023-05-26 19:21:07 -07:00
parent e61d8e4dc8
commit aae892dfd1
7 changed files with 28 additions and 24 deletions

View File

@ -23,6 +23,7 @@
#include <QString> #include <QString>
#include <QVariant> #include <QVariant>
#include <QVector> #include <QVector>
#include <memory>
class QIODevice; class QIODevice;
@ -83,6 +84,8 @@ struct ExtraPackData {
}; };
struct IndexedPack { struct IndexedPack {
using Ptr = std::shared_ptr<IndexedPack>;
QVariant addonId; QVariant addonId;
ResourceProvider provider; ResourceProvider provider;
QString name; QString name;

View File

@ -36,7 +36,7 @@ ResourceAPI::SearchArgs ModModel::createSearchArguments()
ResourceAPI::VersionSearchArgs ModModel::createVersionsArguments(QModelIndex& entry) ResourceAPI::VersionSearchArgs ModModel::createVersionsArguments(QModelIndex& entry)
{ {
auto& pack = m_packs[entry.row()]; auto& pack = *m_packs[entry.row()];
auto profile = static_cast<MinecraftInstance const&>(m_base_instance).getPackProfile(); auto profile = static_cast<MinecraftInstance const&>(m_base_instance).getPackProfile();
Q_ASSERT(profile); Q_ASSERT(profile);
@ -51,7 +51,7 @@ ResourceAPI::VersionSearchArgs ModModel::createVersionsArguments(QModelIndex& en
ResourceAPI::ProjectInfoArgs ModModel::createInfoArguments(QModelIndex& entry) ResourceAPI::ProjectInfoArgs ModModel::createInfoArguments(QModelIndex& entry)
{ {
auto& pack = m_packs[entry.row()]; auto& pack = *m_packs[entry.row()];
return { pack }; return { pack };
} }

View File

@ -9,6 +9,7 @@
#include <QMessageBox> #include <QMessageBox>
#include <QPixmapCache> #include <QPixmapCache>
#include <QUrl> #include <QUrl>
#include <memory>
#include "Application.h" #include "Application.h"
#include "BuildConfig.h" #include "BuildConfig.h"
@ -45,16 +46,16 @@ auto ResourceModel::data(const QModelIndex& index, int role) const -> QVariant
auto pack = m_packs.at(pos); auto pack = m_packs.at(pos);
switch (role) { switch (role) {
case Qt::ToolTipRole: { case Qt::ToolTipRole: {
if (pack.description.length() > 100) { if (pack->description.length() > 100) {
// some magic to prevent to long tooltips and replace html linebreaks // some magic to prevent to long tooltips and replace html linebreaks
QString edit = pack.description.left(97); QString edit = pack->description.left(97);
edit = edit.left(edit.lastIndexOf("<br>")).left(edit.lastIndexOf(" ")).append("..."); edit = edit.left(edit.lastIndexOf("<br>")).left(edit.lastIndexOf(" ")).append("...");
return edit; return edit;
} }
return pack.description; return pack->description;
} }
case Qt::DecorationRole: { case Qt::DecorationRole: {
if (auto icon_or_none = const_cast<ResourceModel*>(this)->getIcon(const_cast<QModelIndex&>(index), pack.logoUrl); if (auto icon_or_none = const_cast<ResourceModel*>(this)->getIcon(const_cast<QModelIndex&>(index), pack->logoUrl);
icon_or_none.has_value()) icon_or_none.has_value())
return icon_or_none.value(); return icon_or_none.value();
@ -64,16 +65,16 @@ auto ResourceModel::data(const QModelIndex& index, int role) const -> QVariant
return QSize(0, 58); return QSize(0, 58);
case Qt::UserRole: { case Qt::UserRole: {
QVariant v; QVariant v;
v.setValue(pack); v.setValue(*pack);
return v; return v;
} }
// Custom data // Custom data
case UserDataTypes::TITLE: case UserDataTypes::TITLE:
return pack.name; return pack->name;
case UserDataTypes::DESCRIPTION: case UserDataTypes::DESCRIPTION:
return pack.description; return pack->description;
case UserDataTypes::SELECTED: case UserDataTypes::SELECTED:
return pack.isAnyVersionSelected(); return pack->isAnyVersionSelected();
default: default:
break; break;
} }
@ -102,7 +103,7 @@ bool ResourceModel::setData(const QModelIndex& index, const QVariant& value, int
if (pos >= m_packs.size() || pos < 0 || !index.isValid()) if (pos >= m_packs.size() || pos < 0 || !index.isValid())
return false; return false;
m_packs[pos] = value.value<ModPlatform::IndexedPack>(); m_packs[pos] = std::make_shared<ModPlatform::IndexedPack>(value.value<ModPlatform::IndexedPack>());
emit dataChanged(index, index); emit dataChanged(index, index);
return true; return true;
@ -161,7 +162,7 @@ void ResourceModel::loadEntry(QModelIndex& entry)
if (!hasActiveInfoJob()) if (!hasActiveInfoJob())
m_current_info_job.clear(); m_current_info_job.clear();
if (!pack.versionsLoaded) { if (!pack->versionsLoaded) {
auto args{ createVersionsArguments(entry) }; auto args{ createVersionsArguments(entry) };
auto callbacks{ createVersionsCallbacks(entry) }; auto callbacks{ createVersionsCallbacks(entry) };
@ -177,7 +178,7 @@ void ResourceModel::loadEntry(QModelIndex& entry)
runInfoJob(job); runInfoJob(job);
} }
if (!pack.extraDataLoaded) { if (!pack->extraDataLoaded) {
auto args{ createInfoArguments(entry) }; auto args{ createInfoArguments(entry) };
auto callbacks{ createInfoCallbacks(entry) }; auto callbacks{ createInfoCallbacks(entry) };
@ -326,15 +327,15 @@ void ResourceModel::loadIndexedPackVersions(ModPlatform::IndexedPack&, QJsonArra
void ResourceModel::searchRequestSucceeded(QJsonDocument& doc) void ResourceModel::searchRequestSucceeded(QJsonDocument& doc)
{ {
QList<ModPlatform::IndexedPack> newList; QList<ModPlatform::IndexedPack::Ptr> newList;
auto packs = documentToArray(doc); auto packs = documentToArray(doc);
for (auto packRaw : packs) { for (auto packRaw : packs) {
auto packObj = packRaw.toObject(); auto packObj = packRaw.toObject();
ModPlatform::IndexedPack pack; ModPlatform::IndexedPack::Ptr pack = std::make_shared<ModPlatform::IndexedPack>();
try { try {
loadIndexedPack(pack, packObj); loadIndexedPack(*pack, packObj);
newList.append(pack); newList.append(pack);
} catch (const JSONValidationError& e) { } catch (const JSONValidationError& e) {
qWarning() << "Error while loading resource from " << debugName() << ": " << e.cause(); qWarning() << "Error while loading resource from " << debugName() << ": " << e.cause();

View File

@ -123,7 +123,7 @@ class ResourceModel : public QAbstractListModel {
QSet<QUrl> m_currently_running_icon_actions; QSet<QUrl> m_currently_running_icon_actions;
QSet<QUrl> m_failed_icon_actions; QSet<QUrl> m_failed_icon_actions;
QList<ModPlatform::IndexedPack> m_packs; QList<ModPlatform::IndexedPack::Ptr> m_packs;
// HACK: We need this to prevent callbacks from calling the model after it has already been deleted. // HACK: We need this to prevent callbacks from calling the model after it has already been deleted.
// This leaks a tiny bit of memory per time the user has opened a resource dialog. How to make this better? // This leaks a tiny bit of memory per time the user has opened a resource dialog. How to make this better?

View File

@ -22,13 +22,13 @@ ResourceAPI::SearchArgs ResourcePackResourceModel::createSearchArguments()
ResourceAPI::VersionSearchArgs ResourcePackResourceModel::createVersionsArguments(QModelIndex& entry) ResourceAPI::VersionSearchArgs ResourcePackResourceModel::createVersionsArguments(QModelIndex& entry)
{ {
auto& pack = m_packs[entry.row()]; auto& pack = m_packs[entry.row()];
return { pack }; return { *pack };
} }
ResourceAPI::ProjectInfoArgs ResourcePackResourceModel::createInfoArguments(QModelIndex& entry) ResourceAPI::ProjectInfoArgs ResourcePackResourceModel::createInfoArguments(QModelIndex& entry)
{ {
auto& pack = m_packs[entry.row()]; auto& pack = m_packs[entry.row()];
return { pack }; return { *pack };
} }
void ResourcePackResourceModel::searchWithTerm(const QString& term, unsigned int sort) void ResourcePackResourceModel::searchWithTerm(const QString& term, unsigned int sort)

View File

@ -22,13 +22,13 @@ ResourceAPI::SearchArgs ShaderPackResourceModel::createSearchArguments()
ResourceAPI::VersionSearchArgs ShaderPackResourceModel::createVersionsArguments(QModelIndex& entry) ResourceAPI::VersionSearchArgs ShaderPackResourceModel::createVersionsArguments(QModelIndex& entry)
{ {
auto& pack = m_packs[entry.row()]; auto& pack = m_packs[entry.row()];
return { pack }; return { *pack };
} }
ResourceAPI::ProjectInfoArgs ShaderPackResourceModel::createInfoArguments(QModelIndex& entry) ResourceAPI::ProjectInfoArgs ShaderPackResourceModel::createInfoArguments(QModelIndex& entry)
{ {
auto& pack = m_packs[entry.row()]; auto& pack = m_packs[entry.row()];
return { pack }; return { *pack };
} }
void ShaderPackResourceModel::searchWithTerm(const QString& term, unsigned int sort) void ShaderPackResourceModel::searchWithTerm(const QString& term, unsigned int sort)

View File

@ -75,9 +75,9 @@ class ResourceModelTest : public QObject {
auto search_json = DummyResourceAPI::searchRequestResult(); auto search_json = DummyResourceAPI::searchRequestResult();
auto processed_response = model->documentToArray(search_json).first().toObject(); auto processed_response = model->documentToArray(search_json).first().toObject();
QVERIFY(processed_pack.addonId.toString() == Json::requireString(processed_response, "project_id")); QVERIFY(processed_pack->addonId.toString() == Json::requireString(processed_response, "project_id"));
QVERIFY(processed_pack.description == Json::requireString(processed_response, "description")); QVERIFY(processed_pack->description == Json::requireString(processed_response, "description"));
QVERIFY(processed_pack.authors.first().name == Json::requireString(processed_response, "author")); QVERIFY(processed_pack->authors.first().name == Json::requireString(processed_response, "author"));
} }
}; };