0xf8/pollymc
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix(memory leak): don't give shared pointers out to foldermodels (causes cyclic refrence)

Browse Source 
Signed-off-by: Rachel Powers <508861+Ryex@users.noreply.github.com>
This commit is contained in:
Rachel Powers 2023-05-28 02:15:39 -07:00
parent 37420405c7
commit a04a6f1e0d
17 changed files with 46 additions and 51 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

30
launcher/minecraft/MinecraftInstance.cpp
View File

@ -1109,79 +1109,79 @@ JavaVersion MinecraftInstance::getJavaVersion()
return JavaVersion(settings()->get("JavaVersion").toString()); return JavaVersion(settings()->get("JavaVersion").toString());
} }
std::shared_ptr<ModFolderModel> MinecraftInstance::loaderModList() const std::shared_ptr<ModFolderModel> MinecraftInstance::loaderModList()
{ {
if (!m_loader_mod_list) if (!m_loader_mod_list)
{ {
bool is_indexed = !APPLICATION->settings()->get("ModMetadataDisabled").toBool(); bool is_indexed = !APPLICATION->settings()->get("ModMetadataDisabled").toBool();
m_loader_mod_list.reset(new ModFolderModel(modsRoot(), shared_from_this(), is_indexed)); m_loader_mod_list.reset(new ModFolderModel(modsRoot(), this, is_indexed));
m_loader_mod_list->disableInteraction(isRunning()); m_loader_mod_list->disableInteraction(isRunning());
connect(this, &BaseInstance::runningStatusChanged, m_loader_mod_list.get(), &ModFolderModel::disableInteraction); connect(this, &BaseInstance::runningStatusChanged, m_loader_mod_list.get(), &ModFolderModel::disableInteraction);
} }
return m_loader_mod_list; return m_loader_mod_list;
} }
std::shared_ptr<ModFolderModel> MinecraftInstance::coreModList() const std::shared_ptr<ModFolderModel> MinecraftInstance::coreModList()
{ {
if (!m_core_mod_list) if (!m_core_mod_list)
{ {
bool is_indexed = !APPLICATION->settings()->get("ModMetadataDisabled").toBool(); bool is_indexed = !APPLICATION->settings()->get("ModMetadataDisabled").toBool();
m_core_mod_list.reset(new ModFolderModel(coreModsDir(), shared_from_this(), is_indexed)); m_core_mod_list.reset(new ModFolderModel(coreModsDir(), this, is_indexed));
m_core_mod_list->disableInteraction(isRunning()); m_core_mod_list->disableInteraction(isRunning());
connect(this, &BaseInstance::runningStatusChanged, m_core_mod_list.get(), &ModFolderModel::disableInteraction); connect(this, &BaseInstance::runningStatusChanged, m_core_mod_list.get(), &ModFolderModel::disableInteraction);
} }
return m_core_mod_list; return m_core_mod_list;
} }
std::shared_ptr<ModFolderModel> MinecraftInstance::nilModList() const std::shared_ptr<ModFolderModel> MinecraftInstance::nilModList()
{ {
if (!m_nil_mod_list) if (!m_nil_mod_list)
{ {
bool is_indexed = !APPLICATION->settings()->get("ModMetadataDisabled").toBool(); bool is_indexed = !APPLICATION->settings()->get("ModMetadataDisabled").toBool();
m_nil_mod_list.reset(new ModFolderModel(nilModsDir(), shared_from_this(), is_indexed, false)); m_nil_mod_list.reset(new ModFolderModel(nilModsDir(), this, is_indexed, false));
m_nil_mod_list->disableInteraction(isRunning()); m_nil_mod_list->disableInteraction(isRunning());
connect(this, &BaseInstance::runningStatusChanged, m_nil_mod_list.get(), &ModFolderModel::disableInteraction); connect(this, &BaseInstance::runningStatusChanged, m_nil_mod_list.get(), &ModFolderModel::disableInteraction);
} }
return m_nil_mod_list; return m_nil_mod_list;
} }
std::shared_ptr<ResourcePackFolderModel> MinecraftInstance::resourcePackList() const std::shared_ptr<ResourcePackFolderModel> MinecraftInstance::resourcePackList()
{ {
if (!m_resource_pack_list) if (!m_resource_pack_list)
{ {
m_resource_pack_list.reset(new ResourcePackFolderModel(resourcePacksDir(), shared_from_this())); m_resource_pack_list.reset(new ResourcePackFolderModel(resourcePacksDir(), this));
} }
return m_resource_pack_list; return m_resource_pack_list;
} }
std::shared_ptr<TexturePackFolderModel> MinecraftInstance::texturePackList() const std::shared_ptr<TexturePackFolderModel> MinecraftInstance::texturePackList()
{ {
if (!m_texture_pack_list) if (!m_texture_pack_list)
{ {
m_texture_pack_list.reset(new TexturePackFolderModel(texturePacksDir(), shared_from_this())); m_texture_pack_list.reset(new TexturePackFolderModel(texturePacksDir(), this));
} }
return m_texture_pack_list; return m_texture_pack_list;
} }
std::shared_ptr<ShaderPackFolderModel> MinecraftInstance::shaderPackList() const std::shared_ptr<ShaderPackFolderModel> MinecraftInstance::shaderPackList()
{ {
if (!m_shader_pack_list) if (!m_shader_pack_list)
{ {
m_shader_pack_list.reset(new ShaderPackFolderModel(shaderPacksDir(), shared_from_this())); m_shader_pack_list.reset(new ShaderPackFolderModel(shaderPacksDir(), this));
} }
return m_shader_pack_list; return m_shader_pack_list;
} }
std::shared_ptr<WorldList> MinecraftInstance::worldList() const std::shared_ptr<WorldList> MinecraftInstance::worldList()
{ {
if (!m_world_list) if (!m_world_list)
{ {
m_world_list.reset(new WorldList(worldDir(), shared_from_this())); m_world_list.reset(new WorldList(worldDir(), this));
} }
return m_world_list; return m_world_list;
} }
std::shared_ptr<GameOptions> MinecraftInstance::gameOptionsModel() const std::shared_ptr<GameOptions> MinecraftInstance::gameOptionsModel()
{ {
if (!m_game_options) if (!m_game_options)
{ {

16
launcher/minecraft/MinecraftInstance.h
View File

@ -115,14 +115,14 @@ public:
std::shared_ptr<PackProfile> getPackProfile() const; std::shared_ptr<PackProfile> getPackProfile() const;
////// Mod Lists ////// ////// Mod Lists //////
std::shared_ptr<ModFolderModel> loaderModList() const; std::shared_ptr<ModFolderModel> loaderModList();
std::shared_ptr<ModFolderModel> coreModList() const; std::shared_ptr<ModFolderModel> coreModList();
std::shared_ptr<ModFolderModel> nilModList() const; std::shared_ptr<ModFolderModel> nilModList();
std::shared_ptr<ResourcePackFolderModel> resourcePackList() const; std::shared_ptr<ResourcePackFolderModel> resourcePackList();
std::shared_ptr<TexturePackFolderModel> texturePackList() const; std::shared_ptr<TexturePackFolderModel> texturePackList();
std::shared_ptr<ShaderPackFolderModel> shaderPackList() const; std::shared_ptr<ShaderPackFolderModel> shaderPackList();
std::shared_ptr<WorldList> worldList() const; std::shared_ptr<WorldList> worldList();
std::shared_ptr<GameOptions> gameOptionsModel() const; std::shared_ptr<GameOptions> gameOptionsModel();
////// Launch stuff ////// ////// Launch stuff //////
Task::Ptr createUpdateTask(Net::Mode mode) override; Task::Ptr createUpdateTask(Net::Mode mode) override;

2
launcher/minecraft/WorldList.cpp
View File

@ -45,7 +45,7 @@
#include <QFileSystemWatcher> #include <QFileSystemWatcher>
#include <QDebug> #include <QDebug>
WorldList::WorldList(const QString &dir, std::shared_ptr<const BaseInstance> instance) WorldList::WorldList(const QString &dir, BaseInstance* instance)
: QAbstractListModel(), m_instance(instance), m_dir(dir) : QAbstractListModel(), m_instance(instance), m_dir(dir)
{ {
FS::ensureFolderPathExists(m_dir.absolutePath()); FS::ensureFolderPathExists(m_dir.absolutePath());

4
launcher/minecraft/WorldList.h
View File

@ -50,7 +50,7 @@ public:
IconFileRole IconFileRole
}; };
WorldList(const QString &dir, std::shared_ptr<const BaseInstance> instance); WorldList(const QString &dir, BaseInstance* instance);
virtual QVariant data(const QModelIndex &index, int role = Qt::DisplayRole) const; virtual QVariant data(const QModelIndex &index, int role = Qt::DisplayRole) const;
@ -128,7 +128,7 @@ signals:
void changed(); void changed();
protected: protected:
std::shared_ptr<const BaseInstance> m_instance; BaseInstance* m_instance;
QFileSystemWatcher *m_watcher; QFileSystemWatcher *m_watcher;
bool is_watching; bool is_watching;
QDir m_dir; QDir m_dir;

2
launcher/minecraft/mod/ModFolderModel.cpp
View File

@ -54,7 +54,7 @@
#include "minecraft/mod/tasks/ModFolderLoadTask.h" #include "minecraft/mod/tasks/ModFolderLoadTask.h"
#include "modplatform/ModIndex.h" #include "modplatform/ModIndex.h"
ModFolderModel::ModFolderModel(const QString& dir, std::shared_ptr<const BaseInstance> instance, bool is_indexed, bool create_dir) ModFolderModel::ModFolderModel(const QString& dir, BaseInstance* instance, bool is_indexed, bool create_dir)
: ResourceFolderModel(QDir(dir), instance, nullptr, create_dir), m_is_indexed(is_indexed) : ResourceFolderModel(QDir(dir), instance, nullptr, create_dir), m_is_indexed(is_indexed)
{ {
m_column_sort_keys = { SortType::ENABLED, SortType::NAME, SortType::VERSION, SortType::DATE, SortType::PROVIDER }; m_column_sort_keys = { SortType::ENABLED, SortType::NAME, SortType::VERSION, SortType::DATE, SortType::PROVIDER };

2
launcher/minecraft/mod/ModFolderModel.h
View File

@ -75,7 +75,7 @@ public:
Enable, Enable,
Toggle Toggle
}; };
ModFolderModel(const QString &dir, std::shared_ptr<const BaseInstance> instance, bool is_indexed = false, bool create_dir = true); ModFolderModel(const QString &dir, BaseInstance* instance, bool is_indexed = false, bool create_dir = true);
QVariant data(const QModelIndex &index, int role = Qt::DisplayRole) const override; QVariant data(const QModelIndex &index, int role = Qt::DisplayRole) const override;

2
launcher/minecraft/mod/ResourceFolderModel.cpp
View File

@ -16,7 +16,7 @@
#include "tasks/Task.h" #include "tasks/Task.h"
ResourceFolderModel::ResourceFolderModel(QDir dir, std::shared_ptr<const BaseInstance> instance, QObject* parent, bool create_dir) ResourceFolderModel::ResourceFolderModel(QDir dir, BaseInstance* instance, QObject* parent, bool create_dir)
: QAbstractListModel(parent), m_dir(dir), m_instance(instance), m_watcher(this) : QAbstractListModel(parent), m_dir(dir), m_instance(instance), m_watcher(this)
{ {
if (create_dir) { if (create_dir) {

4
launcher/minecraft/mod/ResourceFolderModel.h
View File

@ -26,7 +26,7 @@ class QSortFilterProxyModel;
class ResourceFolderModel : public QAbstractListModel { class ResourceFolderModel : public QAbstractListModel {
Q_OBJECT Q_OBJECT
public: public:
ResourceFolderModel(QDir, std::shared_ptr<const BaseInstance>, QObject* parent = nullptr, bool create_dir = true); ResourceFolderModel(QDir, BaseInstance* instance, QObject* parent = nullptr, bool create_dir = true);
~ResourceFolderModel() override; ~ResourceFolderModel() override;
/** Starts watching the paths for changes. /** Starts watching the paths for changes.
@ -191,7 +191,7 @@ class ResourceFolderModel : public QAbstractListModel {
bool m_can_interact = true; bool m_can_interact = true;
QDir m_dir; QDir m_dir;
std::shared_ptr<const BaseInstance> m_instance; BaseInstance* m_instance;
QFileSystemWatcher m_watcher; QFileSystemWatcher m_watcher;
bool m_is_watching = false; bool m_is_watching = false;

2
launcher/minecraft/mod/ResourcePackFolderModel.cpp
View File

@ -45,7 +45,7 @@
#include "minecraft/mod/tasks/BasicFolderLoadTask.h" #include "minecraft/mod/tasks/BasicFolderLoadTask.h"
#include "minecraft/mod/tasks/LocalResourcePackParseTask.h" #include "minecraft/mod/tasks/LocalResourcePackParseTask.h"
ResourcePackFolderModel::ResourcePackFolderModel(const QString& dir, std::shared_ptr<const BaseInstance> instance) ResourcePackFolderModel::ResourcePackFolderModel(const QString& dir, BaseInstance* instance)
: ResourceFolderModel(QDir(dir), instance) : ResourceFolderModel(QDir(dir), instance)
{ {
m_column_sort_keys = { SortType::ENABLED, SortType::NAME, SortType::PACK_FORMAT, SortType::DATE }; m_column_sort_keys = { SortType::ENABLED, SortType::NAME, SortType::PACK_FORMAT, SortType::DATE };

2
launcher/minecraft/mod/ResourcePackFolderModel.h
View File

@ -17,7 +17,7 @@ public:
NUM_COLUMNS NUM_COLUMNS
}; };
explicit ResourcePackFolderModel(const QString &dir, std::shared_ptr<const BaseInstance> instance); explicit ResourcePackFolderModel(const QString &dir, BaseInstance* instance);
[[nodiscard]] QVariant data(const QModelIndex &index, int role = Qt::DisplayRole) const override; [[nodiscard]] QVariant data(const QModelIndex &index, int role = Qt::DisplayRole) const override;

2
launcher/minecraft/mod/ShaderPackFolderModel.h
View File

@ -6,7 +6,7 @@ class ShaderPackFolderModel : public ResourceFolderModel {
Q_OBJECT Q_OBJECT
public: public:
explicit ShaderPackFolderModel(const QString& dir, std::shared_ptr<const BaseInstance> instance) explicit ShaderPackFolderModel(const QString& dir, BaseInstance* instance)
: ResourceFolderModel(QDir(dir), instance) : ResourceFolderModel(QDir(dir), instance)
{} {}
}; };

2
launcher/minecraft/mod/TexturePackFolderModel.cpp
View File

@ -39,7 +39,7 @@
#include "minecraft/mod/tasks/BasicFolderLoadTask.h" #include "minecraft/mod/tasks/BasicFolderLoadTask.h"
#include "minecraft/mod/tasks/LocalTexturePackParseTask.h" #include "minecraft/mod/tasks/LocalTexturePackParseTask.h"
TexturePackFolderModel::TexturePackFolderModel(const QString& dir, std::shared_ptr<const BaseInstance> instance) TexturePackFolderModel::TexturePackFolderModel(const QString& dir, BaseInstance* instance)
: ResourceFolderModel(QDir(dir), instance) : ResourceFolderModel(QDir(dir), instance)
{} {}

2
launcher/minecraft/mod/TexturePackFolderModel.h
View File

@ -43,7 +43,7 @@ class TexturePackFolderModel : public ResourceFolderModel
Q_OBJECT Q_OBJECT
public: public:
explicit TexturePackFolderModel(const QString &dir, std::shared_ptr<const BaseInstance> instance); explicit TexturePackFolderModel(const QString &dir, BaseInstance* instance);
[[nodiscard]] Task* createUpdateTask() override; [[nodiscard]] Task* createUpdateTask() override;
[[nodiscard]] Task* createParseTask(Resource&) override; [[nodiscard]] Task* createParseTask(Resource&) override;
}; };

2
launcher/ui/dialogs/NewInstanceDialog.cpp
View File

@ -99,7 +99,7 @@ NewInstanceDialog::NewInstanceDialog(const QString & initialGroup, const QString
// NOTE: m_buttons must be initialized before PageContainer, because it indirectly accesses m_buttons through setSuggestedPack! Do not move this below. // NOTE: m_buttons must be initialized before PageContainer, because it indirectly accesses m_buttons through setSuggestedPack! Do not move this below.
m_buttons = new QDialogButtonBox(QDialogButtonBox::Help | QDialogButtonBox::Ok | QDialogButtonBox::Cancel); m_buttons = new QDialogButtonBox(QDialogButtonBox::Help | QDialogButtonBox::Ok | QDialogButtonBox::Cancel);
m_container = new PageContainer(this); m_container = new PageContainer(this, {}, this);
m_container->setSizePolicy(QSizePolicy::Policy::Preferred, QSizePolicy::Policy::Expanding); m_container->setSizePolicy(QSizePolicy::Policy::Preferred, QSizePolicy::Policy::Expanding);
m_container->layout()->setContentsMargins(0, 0, 0, 0); m_container->layout()->setContentsMargins(0, 0, 0, 0);
ui->verticalLayout->insertWidget(2, m_container); ui->verticalLayout->insertWidget(2, m_container);

2
launcher/ui/dialogs/ResourceDownloadDialog.cpp
View File

@ -89,7 +89,7 @@ void ResourceDownloadDialog::reject()
// won't work with subclasses if we put it in this ctor. // won't work with subclasses if we put it in this ctor.
void ResourceDownloadDialog::initializeContainer() void ResourceDownloadDialog::initializeContainer()
{ {
m_container = new PageContainer(this); m_container = new PageContainer(this, {}, this);
m_container->setSizePolicy(QSizePolicy::Policy::Preferred, QSizePolicy::Policy::Expanding); m_container->setSizePolicy(QSizePolicy::Policy::Preferred, QSizePolicy::Policy::Expanding);
m_container->layout()->setContentsMargins(0, 0, 0, 0); m_container->layout()->setContentsMargins(0, 0, 0, 0);
m_vertical_layout.addWidget(m_container); m_vertical_layout.addWidget(m_container);

4
launcher/ui/widgets/PageContainer.cpp
View File

@ -87,7 +87,9 @@ PageContainer::PageContainer(BasePageProvider *pageProvider, QString defaultId,
auto pages = pageProvider->getPages(); auto pages = pageProvider->getPages();
for (auto page : pages) for (auto page : pages)
{ {
page->stackIndex = m_pageStack->addWidget(dynamic_cast<QWidget *>(page)); auto widget = dynamic_cast<QWidget *>(page);
widget->setParent(this);
page->stackIndex = m_pageStack->addWidget(widget);
page->listIndex = counter; page->listIndex = counter;
page->setParentContainer(this); page->setParentContainer(this);
counter++; counter++;

17
tests/ResourceFolderModel_test.cpp
View File

@ -90,9 +90,7 @@ slots:
QEventLoop loop; QEventLoop loop;
InstancePtr instance; ModFolderModel m(tempDir.path(), nullptr, true);
ModFolderModel m(tempDir.path(), instance, true);
connect(&m, &ModFolderModel::updateFinished, &loop, &QEventLoop::quit); connect(&m, &ModFolderModel::updateFinished, &loop, &QEventLoop::quit);
@ -116,8 +114,7 @@ slots:
QString folder = source + '/'; QString folder = source + '/';
QTemporaryDir tempDir; QTemporaryDir tempDir;
QEventLoop loop; QEventLoop loop;
InstancePtr instance; ModFolderModel m(tempDir.path(), nullptr, true);
ModFolderModel m(tempDir.path(), instance, true);
connect(&m, &ModFolderModel::updateFinished, &loop, &QEventLoop::quit); connect(&m, &ModFolderModel::updateFinished, &loop, &QEventLoop::quit);
@ -140,8 +137,7 @@ slots:
void test_addFromWatch() void test_addFromWatch()
{ {
QString source = QFINDTESTDATA("testdata/ResourceFolderModel"); QString source = QFINDTESTDATA("testdata/ResourceFolderModel");
InstancePtr instance; ModFolderModel model(source, nullptr);
ModFolderModel model(source, instance);
QCOMPARE(model.size(), 0); QCOMPARE(model.size(), 0);
@ -161,9 +157,7 @@ slots:
QString file_mod = QFINDTESTDATA("testdata/ResourceFolderModel/supercoolmod.jar"); QString file_mod = QFINDTESTDATA("testdata/ResourceFolderModel/supercoolmod.jar");
QTemporaryDir tmp; QTemporaryDir tmp;
InstancePtr instance; ResourceFolderModel model(QDir(tmp.path()), nullptr);
ResourceFolderModel model(QDir(tmp.path()), instance);
QCOMPARE(model.size(), 0); QCOMPARE(model.size(), 0);
@ -214,8 +208,7 @@ slots:
QString file_mod = QFINDTESTDATA("testdata/ResourceFolderModel/supercoolmod.jar"); QString file_mod = QFINDTESTDATA("testdata/ResourceFolderModel/supercoolmod.jar");
QTemporaryDir tmp; QTemporaryDir tmp;
InstancePtr instance; ResourceFolderModel model(tmp.path(), nullptr);
ResourceFolderModel model(tmp.path(), instance);
QCOMPARE(model.size(), 0); QCOMPARE(model.size(), 0);