Merge pull request #628 from flowln/fix_multiple_resource_packs_crash

Fixes https://github.com/PrismLauncher/PrismLauncher/issues/624

7 files changed, 47 insertions, 19 deletions

diff --git a/launcher/minecraft/mod/ResourceFolderModel.cpp b/launcher/minecraft/mod/ResourceFolderModel.cpp
index 0310c8f6..a52c5db3 100644
--- a/launcher/minecraft/mod/ResourceFolderModel.cpp
+++ b/launcher/minecraft/mod/ResourceFolderModel.cpp
@@ -20,6 +20,7 @@ ResourceFolderModel::ResourceFolderModel(QDir dir, QObject* parent) : QAbstractL
     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(); });
 }
 
 ResourceFolderModel::~ResourceFolderModel()
@@ -275,7 +276,11 @@ void ResourceFolderModel::resolveResource(Resource* res)
     connect(
         task, &Task::finished, this, [=] { m_active_parse_tasks.remove(ticket); }, Qt::ConnectionType::QueuedConnection);
 
-    QThreadPool::globalInstance()->start(task);
+    m_helper_thread_task.addTask(task);
+
+    if (!m_helper_thread_task.isRunning()) {
+        QThreadPool::globalInstance()->start(&m_helper_thread_task);
+    }
 }
 
 void ResourceFolderModel::onUpdateSucceeded()
diff --git a/launcher/minecraft/mod/ResourceFolderModel.h b/launcher/minecraft/mod/ResourceFolderModel.h
index fe283b04..f1bc2dd7 100644
--- a/launcher/minecraft/mod/ResourceFolderModel.h
+++ b/launcher/minecraft/mod/ResourceFolderModel.h
@@ -10,6 +10,7 @@
 #include "Resource.h"
 
 #include "tasks/Task.h"
+#include "tasks/ConcurrentTask.h"
 
 class QSortFilterProxyModel;
 
@@ -197,6 +198,7 @@ class ResourceFolderModel : public QAbstractListModel {
     // Represents the relationship between a resource's internal ID and it's row position on the model.
     QMap<QString, int> m_resources_index;
 
+    ConcurrentTask m_helper_thread_task;
     QMap<int, Task::Ptr> m_active_parse_tasks;
     std::atomic<int> m_next_resolution_ticket = 0;
 };
diff --git a/launcher/net/NetJob.cpp b/launcher/net/NetJob.cpp
index 8ced1b7e..9b5d4f1b 100644
--- a/launcher/net/NetJob.cpp
+++ b/launcher/net/NetJob.cpp
@@ -123,7 +123,7 @@ auto NetJob::getFailedFiles() -> QList<QString>
 
 void NetJob::updateState()
 {
-    emit progress(m_done.count(), m_total_size);
+    emit progress(m_done.count(), totalSize());
     setStatus(tr("Executing %1 task(s) (%2 out of %3 are done)")
-                  .arg(QString::number(m_doing.count()), QString::number(m_done.count()), QString::number(m_total_size)));
+                  .arg(QString::number(m_doing.count()), QString::number(m_done.count()), QString::number(totalSize())));
 }
diff --git a/launcher/tasks/ConcurrentTask.cpp b/launcher/tasks/ConcurrentTask.cpp
index ce08a6a2..a890013e 100644
--- a/launcher/tasks/ConcurrentTask.cpp
+++ b/launcher/tasks/ConcurrentTask.cpp
@@ -27,18 +27,13 @@ auto ConcurrentTask::getStepTotalProgress() const -> qint64
 
 void ConcurrentTask::addTask(Task::Ptr task)
 {
-    if (!isRunning())
-        m_queue.append(task);
-    else
-        qWarning() << "Tried to add a task to a running concurrent task!";
+    m_queue.append(task);
 }
 
 void ConcurrentTask::executeTask()
 {
-    m_total_size = m_queue.size();
-
     // Start the least amount of tasks needed, but at least one
-    int num_starts = std::max(1, std::min(m_total_max_size, m_total_size));
+    int num_starts = qMax(1, qMin(m_total_max_size, m_queue.size()));
     for (int i = 0; i < num_starts; i++) {
         QMetaObject::invokeMethod(this, &ConcurrentTask::startNext, Qt::QueuedConnection);
     }
@@ -73,6 +68,20 @@ bool ConcurrentTask::abort()
     return suceedeed;
 }
 
+void ConcurrentTask::clear()
+{
+    Q_ASSERT(!isRunning());
+
+    m_done.clear();
+    m_failed.clear();
+    m_queue.clear();
+
+    m_aborted = false;
+
+    m_progress = 0;
+    m_stepProgress = 0;
+}
+
 void ConcurrentTask::startNext()
 {
     if (m_aborted || m_doing.count() > m_total_max_size)
@@ -101,9 +110,14 @@ void ConcurrentTask::startNext()
     setStepStatus(next->isMultiStep() ? next->getStepStatus() : next->getStatus());
     updateState();
 
-    QCoreApplication::processEvents();
+    QMetaObject::invokeMethod(next.get(), &Task::start, Qt::QueuedConnection);
 
-    next->start();
+    // Allow going up the number of concurrent tasks in case of tasks being added in the middle of a running task.
+    int num_starts = m_total_max_size - m_doing.size();
+    for (int i = 0; i < num_starts; i++)
+        QMetaObject::invokeMethod(this, &ConcurrentTask::startNext, Qt::QueuedConnection);
+
+    QCoreApplication::processEvents();
 }
 
 void ConcurrentTask::subTaskSucceeded(Task::Ptr task)
@@ -145,7 +159,7 @@ void ConcurrentTask::subTaskProgress(qint64 current, qint64 total)
 
 void ConcurrentTask::updateState()
 {
-    setProgress(m_done.count(), m_total_size);
+    setProgress(m_done.count(), totalSize());
     setStatus(tr("Executing %1 task(s) (%2 out of %3 are done)")
-                  .arg(QString::number(m_doing.count()), QString::number(m_done.count()), QString::number(m_total_size)));
+                  .arg(QString::number(m_doing.count()), QString::number(m_done.count()), QString::number(totalSize())));
 }
diff --git a/launcher/tasks/ConcurrentTask.h b/launcher/tasks/ConcurrentTask.h
index f1279d32..b46919fb 100644
--- a/launcher/tasks/ConcurrentTask.h
+++ b/launcher/tasks/ConcurrentTask.h
@@ -24,6 +24,11 @@ public:
 public slots:
     bool abort() override;
 
+    /** Resets the internal state of the task.
+     *  This allows the same task to be re-used.
+     */
+    void clear();
+
 protected
 slots:
     void executeTask() override;
@@ -36,6 +41,9 @@ slots:
     void subTaskProgress(qint64 current, qint64 total);
 
 protected:
+    // NOTE: This is not thread-safe.
+    [[nodiscard]] unsigned int totalSize() const { return m_queue.size() + m_doing.size() + m_done.size(); }
+
     void setStepStatus(QString status) { m_step_status = status; emit stepStatus(status); };
 
     virtual void updateState();
@@ -51,7 +59,6 @@ protected:
     QHash<Task*, Task::Ptr> m_failed;
 
     int m_total_max_size;
-    int m_total_size;
 
     qint64 m_stepProgress = 0;
     qint64 m_stepTotalProgress = 100;
diff --git a/launcher/tasks/MultipleOptionsTask.cpp b/launcher/tasks/MultipleOptionsTask.cpp
index 5ad6181f..034499df 100644
--- a/launcher/tasks/MultipleOptionsTask.cpp
+++ b/launcher/tasks/MultipleOptionsTask.cpp
@@ -22,6 +22,6 @@ void MultipleOptionsTask::startNext()
 
 void MultipleOptionsTask::updateState()
 {
-    setProgress(m_done.count(), m_total_size);
-    setStatus(tr("Attempting task %1 out of %2").arg(QString::number(m_doing.count() + m_done.count()), QString::number(m_total_size)));
+    setProgress(m_done.count(), totalSize());
+    setStatus(tr("Attempting task %1 out of %2").arg(QString::number(m_doing.count() + m_done.count()), QString::number(totalSize())));
 }
diff --git a/launcher/tasks/SequentialTask.cpp b/launcher/tasks/SequentialTask.cpp
index a34137cb..b2f86328 100644
--- a/launcher/tasks/SequentialTask.cpp
+++ b/launcher/tasks/SequentialTask.cpp
@@ -17,6 +17,6 @@ void SequentialTask::startNext()
 
 void SequentialTask::updateState()
 {
-    setProgress(m_done.count(), m_total_size);
-    setStatus(tr("Executing task %1 out of %2").arg(QString::number(m_doing.count() + m_done.count()), QString::number(m_total_size)));
+    setProgress(m_done.count(), totalSize());
+    setStatus(tr("Executing task %1 out of %2").arg(QString::number(m_doing.count() + m_done.count()), QString::number(totalSize())));
 }