From 9ac0314d7ac39bdf93e5d80bf054ffce4434fe3e Mon Sep 17 00:00:00 2001 From: Octol1ttle Date: Wed, 21 Jan 2026 17:10:14 +0500 Subject: [PATCH 1/5] Add asserts to invalid Task states Signed-off-by: Octol1ttle --- launcher/tasks/Task.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/launcher/tasks/Task.cpp b/launcher/tasks/Task.cpp index 92b345c8d..d59660146 100644 --- a/launcher/tasks/Task.cpp +++ b/launcher/tasks/Task.cpp @@ -98,6 +98,7 @@ void Task::start() case State::Running: { if (m_show_debug) qCWarning(taskLogC) << "The launcher tried to start task" << describe() << "while it was already running!"; + Q_ASSERT(!isRunning()); return; } } @@ -112,6 +113,7 @@ void Task::emitFailed(QString reason) // Don't fail twice. if (!isRunning()) { qCCritical(taskLogC) << "Task" << describe() << "failed while not running!!!!: " << reason; + Q_ASSERT(!isRunning()); return; } m_state = State::Failed; @@ -126,6 +128,7 @@ void Task::emitAborted() // Don't abort twice. if (!isRunning()) { qCCritical(taskLogC) << "Task" << describe() << "aborted while not running!!!!"; + Q_ASSERT(!isRunning()); return; } m_state = State::AbortedByUser; @@ -141,6 +144,7 @@ void Task::emitSucceeded() // Don't succeed twice. if (!isRunning()) { qCCritical(taskLogC) << "Task" << describe() << "succeeded while not running!!!!"; + Q_ASSERT(!isRunning()); return; } m_state = State::Succeeded; From 490df18fd58758865b68ccfdbe5b568069216846 Mon Sep 17 00:00:00 2001 From: Octol1ttle Date: Wed, 21 Jan 2026 17:45:12 +0500 Subject: [PATCH 2/5] Introduce macro to assert and return the assertion condition Signed-off-by: Octol1ttle --- launcher/Assert.h | 23 +++++++++++++++++++++++ launcher/CMakeLists.txt | 7 +++++-- launcher/tasks/Task.cpp | 14 ++++++-------- 3 files changed, 34 insertions(+), 10 deletions(-) create mode 100644 launcher/Assert.h diff --git a/launcher/Assert.h b/launcher/Assert.h new file mode 100644 index 000000000..df15ed389 --- /dev/null +++ b/launcher/Assert.h @@ -0,0 +1,23 @@ +// SPDX-License-Identifier: GPL-3.0-only +/* + * Prism Launcher - Minecraft Launcher + * Copyright (C) 2025 Octol1ttle + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, version 3. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +#pragma once + +#if !defined(ASSERT_NEVER) +#define ASSERT_NEVER(cond) (Q_ASSERT(cond == false), cond) +#endif diff --git a/launcher/CMakeLists.txt b/launcher/CMakeLists.txt index b89a49186..1ecea9d1d 100644 --- a/launcher/CMakeLists.txt +++ b/launcher/CMakeLists.txt @@ -102,6 +102,9 @@ set(CORE_SOURCES MMCTime.cpp MTPixmapCache.h + + # Assertion helper + Assert.h ) if (UNIX AND NOT CYGWIN AND NOT APPLE) set(CORE_SOURCES @@ -1614,7 +1617,7 @@ if(WIN32 OR (UNIX AND APPLE)) ) # FIXME: remove this crap once we stop using msys2 if(MINGW) - # i've not found a solution better than injecting the config vars like this... + # i've not found a solution better than injecting the config vars like this... # with install(CODE" for everything everything just breaks install(CODE " set(QT_PLUGINS_DIR \"${QT_PLUGINS_DIR}\") @@ -1622,7 +1625,7 @@ if(WIN32 OR (UNIX AND APPLE)) set(QT_LIBEXECS_DIR \"${QT_LIBEXECS_DIR}\") set(CMAKE_SYSTEM_LIBRARY_PATH \"${CMAKE_SYSTEM_LIBRARY_PATH}\") set(CMAKE_INSTALL_PREFIX \"${CMAKE_INSTALL_PREFIX}\") - " + " COMPONENT bundle) install(CODE [[ diff --git a/launcher/tasks/Task.cpp b/launcher/tasks/Task.cpp index d59660146..3032521c8 100644 --- a/launcher/tasks/Task.cpp +++ b/launcher/tasks/Task.cpp @@ -38,6 +38,8 @@ #include +#include "Assert.h" + Q_LOGGING_CATEGORY(taskLogC, "launcher.task") Task::Task(bool show_debug) : m_show_debug(show_debug) @@ -96,9 +98,8 @@ void Task::start() break; } case State::Running: { - if (m_show_debug) + if (ASSERT_NEVER(isRunning()) && m_show_debug) qCWarning(taskLogC) << "The launcher tried to start task" << describe() << "while it was already running!"; - Q_ASSERT(!isRunning()); return; } } @@ -111,9 +112,8 @@ void Task::start() void Task::emitFailed(QString reason) { // Don't fail twice. - if (!isRunning()) { + if (ASSERT_NEVER(!isRunning())) { qCCritical(taskLogC) << "Task" << describe() << "failed while not running!!!!: " << reason; - Q_ASSERT(!isRunning()); return; } m_state = State::Failed; @@ -126,9 +126,8 @@ void Task::emitFailed(QString reason) void Task::emitAborted() { // Don't abort twice. - if (!isRunning()) { + if (ASSERT_NEVER(!isRunning())) { qCCritical(taskLogC) << "Task" << describe() << "aborted while not running!!!!"; - Q_ASSERT(!isRunning()); return; } m_state = State::AbortedByUser; @@ -142,9 +141,8 @@ void Task::emitAborted() void Task::emitSucceeded() { // Don't succeed twice. - if (!isRunning()) { + if (ASSERT_NEVER(!isRunning())) { qCCritical(taskLogC) << "Task" << describe() << "succeeded while not running!!!!"; - Q_ASSERT(!isRunning()); return; } m_state = State::Succeeded; From 1cd78bf94a80acc6a9e9fe11860462d3b3096f9d Mon Sep 17 00:00:00 2001 From: Octol1ttle Date: Wed, 21 Jan 2026 19:15:12 +0500 Subject: [PATCH 3/5] code review Signed-off-by: Octol1ttle --- launcher/Assert.h | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/launcher/Assert.h b/launcher/Assert.h index df15ed389..0b1cdb742 100644 --- a/launcher/Assert.h +++ b/launcher/Assert.h @@ -18,6 +18,8 @@ #pragma once -#if !defined(ASSERT_NEVER) -#define ASSERT_NEVER(cond) (Q_ASSERT(cond == false), cond) +#if defined(ASSERT_NEVER) +#error ASSERT_NEVER already defined +#else +#define ASSERT_NEVER(cond) (Q_ASSERT((cond) == false), (cond)) #endif From 6cb07e203ba88667daf5248bb59842d36b1a222c Mon Sep 17 00:00:00 2001 From: Octol1ttle Date: Thu, 22 Jan 2026 22:10:13 +0500 Subject: [PATCH 4/5] fix(ResourceFolderModel): don't read state from off-thread task Signed-off-by: Octol1ttle --- launcher/minecraft/mod/ResourceFolderModel.cpp | 14 +++++++++----- launcher/minecraft/mod/ResourceFolderModel.h | 5 ++++- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/launcher/minecraft/mod/ResourceFolderModel.cpp b/launcher/minecraft/mod/ResourceFolderModel.cpp index b0082653d..3473379e3 100644 --- a/launcher/minecraft/mod/ResourceFolderModel.cpp +++ b/launcher/minecraft/mod/ResourceFolderModel.cpp @@ -38,9 +38,12 @@ ResourceFolderModel::ResourceFolderModel(const QDir& dir, BaseInstance* instance m_dir.setSorting(QDir::Name | QDir::IgnoreCase | QDir::LocaleAware); connect(&m_watcher, &QFileSystemWatcher::directoryChanged, this, &ResourceFolderModel::directoryChanged); - connect(&m_helper_thread_task, &ConcurrentTask::finished, this, [this] { m_helper_thread_task.clear(); }); + connect(&m_resourceResolver, &ConcurrentTask::finished, this, [this] { + m_resourceResolver.clear(); + m_resourceResolverRunning = false; + }); if (APPLICATION_DYN) { // in tests the application macro doesn't work - m_helper_thread_task.setMaxConcurrent(APPLICATION->settings()->get("NumberOfConcurrentTasks").toInt()); + m_resourceResolver.setMaxConcurrent(APPLICATION->settings()->get("NumberOfConcurrentTasks").toInt()); } } @@ -382,10 +385,11 @@ void ResourceFolderModel::resolveResource(Resource::Ptr res) }, Qt::ConnectionType::QueuedConnection); - m_helper_thread_task.addTask(task); + m_resourceResolver.addTask(task); - if (!m_helper_thread_task.isRunning()) { - QThreadPool::globalInstance()->start(&m_helper_thread_task); + if (!m_resourceResolverRunning) { + QThreadPool::globalInstance()->start(&m_resourceResolver); + m_resourceResolverRunning = true; } } diff --git a/launcher/minecraft/mod/ResourceFolderModel.h b/launcher/minecraft/mod/ResourceFolderModel.h index 0526b5bbf..b6343a807 100644 --- a/launcher/minecraft/mod/ResourceFolderModel.h +++ b/launcher/minecraft/mod/ResourceFolderModel.h @@ -261,7 +261,10 @@ class ResourceFolderModel : public QAbstractListModel { // Represents the relationship between a resource's internal ID and it's row position on the model. QMap m_resources_index; - ConcurrentTask m_helper_thread_task; + // Runs off-thread + ConcurrentTask m_resourceResolver; + bool m_resourceResolverRunning = false; + QMap m_active_parse_tasks; std::atomic m_next_resolution_ticket = 0; }; From fa040fc9596484c69a3153752619d161b3eda5e2 Mon Sep 17 00:00:00 2001 From: Octol1ttle Date: Thu, 22 Jan 2026 23:44:17 +0500 Subject: [PATCH 5/5] rename Assert.h because it causes conflicts??? Signed-off-by: Octol1ttle --- launcher/{Assert.h => AssertHelpers.h} | 0 launcher/CMakeLists.txt | 2 +- launcher/tasks/Task.cpp | 2 +- 3 files changed, 2 insertions(+), 2 deletions(-) rename launcher/{Assert.h => AssertHelpers.h} (100%) diff --git a/launcher/Assert.h b/launcher/AssertHelpers.h similarity index 100% rename from launcher/Assert.h rename to launcher/AssertHelpers.h diff --git a/launcher/CMakeLists.txt b/launcher/CMakeLists.txt index 1ecea9d1d..9b4d0d130 100644 --- a/launcher/CMakeLists.txt +++ b/launcher/CMakeLists.txt @@ -104,7 +104,7 @@ set(CORE_SOURCES MTPixmapCache.h # Assertion helper - Assert.h + AssertHelpers.h ) if (UNIX AND NOT CYGWIN AND NOT APPLE) set(CORE_SOURCES diff --git a/launcher/tasks/Task.cpp b/launcher/tasks/Task.cpp index 3032521c8..4c809c651 100644 --- a/launcher/tasks/Task.cpp +++ b/launcher/tasks/Task.cpp @@ -38,7 +38,7 @@ #include -#include "Assert.h" +#include "AssertHelpers.h" Q_LOGGING_CATEGORY(taskLogC, "launcher.task")