From 9e35230467267691ce745429de8d7e0b18095084 Mon Sep 17 00:00:00 2001 From: flow Date: Fri, 16 Sep 2022 19:22:37 -0300 Subject: fix: memory leak when getting mods from the mods folder friendly reminder to always delete your news. Signed-off-by: flow --- launcher/minecraft/mod/tasks/ModFolderLoadTask.cpp | 2 ++ 1 file changed, 2 insertions(+) (limited to 'launcher/minecraft/mod/tasks') diff --git a/launcher/minecraft/mod/tasks/ModFolderLoadTask.cpp b/launcher/minecraft/mod/tasks/ModFolderLoadTask.cpp index 3a857740..afe892f4 100644 --- a/launcher/minecraft/mod/tasks/ModFolderLoadTask.cpp +++ b/launcher/minecraft/mod/tasks/ModFolderLoadTask.cpp @@ -57,6 +57,8 @@ void ModFolderLoadTask::executeTask() if (mod->enabled()) { if (m_result->mods.contains(mod->internal_id())) { m_result->mods[mod->internal_id()]->setStatus(ModStatus::Installed); + // Delete the object we just created, since a valid one is already in the mods list. + delete mod; } else { m_result->mods[mod->internal_id()] = mod; -- cgit From 10493bd44ab59171ac4f2e3ab7b600bcff8e4af6 Mon Sep 17 00:00:00 2001 From: flow Date: Fri, 16 Sep 2022 19:25:53 -0300 Subject: fix: move newly allocated resources to the main thread This avoids them getting deleted when the worker thread exits, due to thread affinity on the created thread. Signed-off-by: flow --- launcher/minecraft/mod/tasks/BasicFolderLoadTask.h | 12 ++++++++++-- launcher/minecraft/mod/tasks/ModFolderLoadTask.cpp | 16 +++++++++++++++- launcher/minecraft/mod/tasks/ModFolderLoadTask.h | 3 +++ 3 files changed, 28 insertions(+), 3 deletions(-) (limited to 'launcher/minecraft/mod/tasks') diff --git a/launcher/minecraft/mod/tasks/BasicFolderLoadTask.h b/launcher/minecraft/mod/tasks/BasicFolderLoadTask.h index be0e752d..2fce2942 100644 --- a/launcher/minecraft/mod/tasks/BasicFolderLoadTask.h +++ b/launcher/minecraft/mod/tasks/BasicFolderLoadTask.h @@ -3,6 +3,7 @@ #include #include #include +#include #include @@ -23,14 +24,14 @@ class BasicFolderLoadTask : public Task { [[nodiscard]] ResultPtr result() const { return m_result; } public: - BasicFolderLoadTask(QDir dir) : Task(nullptr, false), m_dir(dir), m_result(new Result) + BasicFolderLoadTask(QDir dir) : Task(nullptr, false), m_dir(dir), m_result(new Result), m_thread_to_spawn_into(thread()) { m_create_func = [](QFileInfo const& entry) -> Resource* { return new Resource(entry); }; } BasicFolderLoadTask(QDir dir, std::function create_function) - : Task(nullptr, false), m_dir(dir), m_result(new Result), m_create_func(std::move(create_function)) + : Task(nullptr, false), m_dir(dir), m_result(new Result), m_create_func(std::move(create_function)), m_thread_to_spawn_into(thread()) {} [[nodiscard]] bool canAbort() const override { return true; } @@ -42,9 +43,13 @@ class BasicFolderLoadTask : public Task { void executeTask() override { + if (thread() != m_thread_to_spawn_into) + connect(this, &Task::finished, this->thread(), &QThread::quit); + m_dir.refresh(); for (auto entry : m_dir.entryInfoList()) { auto resource = m_create_func(entry); + resource->moveToThread(m_thread_to_spawn_into); m_result->resources.insert(resource->internal_id(), resource); } @@ -61,4 +66,7 @@ private: std::atomic m_aborted = false; std::function m_create_func; + + /** This is the thread in which we should put new mod objects */ + QThread* m_thread_to_spawn_into; }; diff --git a/launcher/minecraft/mod/tasks/ModFolderLoadTask.cpp b/launcher/minecraft/mod/tasks/ModFolderLoadTask.cpp index afe892f4..40a6ba18 100644 --- a/launcher/minecraft/mod/tasks/ModFolderLoadTask.cpp +++ b/launcher/minecraft/mod/tasks/ModFolderLoadTask.cpp @@ -38,12 +38,23 @@ #include "minecraft/mod/MetadataHandler.h" +#include + ModFolderLoadTask::ModFolderLoadTask(QDir mods_dir, QDir index_dir, bool is_indexed, bool clean_orphan) - : Task(nullptr, false), m_mods_dir(mods_dir), m_index_dir(index_dir), m_is_indexed(is_indexed), m_clean_orphan(clean_orphan), m_result(new Result()) + : Task(nullptr, false) + , m_mods_dir(mods_dir) + , m_index_dir(index_dir) + , m_is_indexed(is_indexed) + , m_clean_orphan(clean_orphan) + , m_result(new Result()) + , m_thread_to_spawn_into(thread()) {} void ModFolderLoadTask::executeTask() { + if (thread() != m_thread_to_spawn_into) + connect(this, &Task::finished, this->thread(), &QThread::quit); + if (m_is_indexed) { // Read metadata first getFromMetadata(); @@ -98,6 +109,9 @@ void ModFolderLoadTask::executeTask() } } + for (auto mod : m_result->mods) + mod->moveToThread(m_thread_to_spawn_into); + if (m_aborted) emit finished(); else diff --git a/launcher/minecraft/mod/tasks/ModFolderLoadTask.h b/launcher/minecraft/mod/tasks/ModFolderLoadTask.h index 0f18b8b9..af5f58a5 100644 --- a/launcher/minecraft/mod/tasks/ModFolderLoadTask.h +++ b/launcher/minecraft/mod/tasks/ModFolderLoadTask.h @@ -79,4 +79,7 @@ private: ResultPtr m_result; std::atomic m_aborted = false; + + /** This is the thread in which we should put new mod objects */ + QThread* m_thread_to_spawn_into; }; -- cgit From c9eb584ac80956730dd56068945f6791e29716b3 Mon Sep 17 00:00:00 2001 From: flow Date: Fri, 16 Sep 2022 20:00:36 -0300 Subject: fix: prevent deletes by shared pointer accidental creation This fixes the launcher crashing when opening the game :iea: Signed-off-by: flow --- launcher/minecraft/mod/tasks/ModFolderLoadTask.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'launcher/minecraft/mod/tasks') diff --git a/launcher/minecraft/mod/tasks/ModFolderLoadTask.cpp b/launcher/minecraft/mod/tasks/ModFolderLoadTask.cpp index 40a6ba18..78ef4386 100644 --- a/launcher/minecraft/mod/tasks/ModFolderLoadTask.cpp +++ b/launcher/minecraft/mod/tasks/ModFolderLoadTask.cpp @@ -99,7 +99,7 @@ void ModFolderLoadTask::executeTask() // Remove orphan metadata to prevent issues // See https://github.com/PolyMC/PolyMC/issues/996 if (m_clean_orphan) { - QMutableMapIterator iter(m_result->mods); + QMutableMapIterator iter(m_result->mods); while (iter.hasNext()) { auto mod = iter.next().value(); if (mod->status() == ModStatus::NotInstalled) { -- cgit