From ad74fedfba45fe0f36ff387e586b21d4ede8ca83 Mon Sep 17 00:00:00 2001 From: flow Date: Tue, 17 Jan 2023 22:51:54 -0300 Subject: [PATCH 1/3] feat(tests): add test for stack overflow in ConcurrentTask Signed-off-by: flow --- tests/Task_test.cpp | 57 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/tests/Task_test.cpp b/tests/Task_test.cpp index 80bba02fc..5d9068517 100644 --- a/tests/Task_test.cpp +++ b/tests/Task_test.cpp @@ -1,4 +1,6 @@ #include +#include +#include #include #include @@ -11,6 +13,9 @@ class BasicTask : public Task { friend class TaskTest; + public: + BasicTask(bool show_debug_log = true) : Task(nullptr, show_debug_log) {} + private: void executeTask() override { @@ -30,6 +35,41 @@ class BasicTask_MultiStep : public Task { void executeTask() override {}; }; +class BigConcurrentTask : public QThread { + Q_OBJECT + + ConcurrentTask big_task; + + void run() override + { + QTimer deadline; + deadline.setInterval(10000); + connect(&deadline, &QTimer::timeout, this, [this]{ passed_the_deadline = true; }); + deadline.start(); + + static const unsigned s_num_tasks = 1 << 14; + auto sub_tasks = new BasicTask*[s_num_tasks]; + + for (unsigned i = 0; i < s_num_tasks; i++) { + sub_tasks[i] = new BasicTask(false); + big_task.addTask(sub_tasks[i]); + } + + big_task.run(); + + while (!big_task.isFinished() && !passed_the_deadline) + QCoreApplication::processEvents(); + + emit finished(); + } + + public: + bool passed_the_deadline = false; + + signals: + void finished(); +}; + class TaskTest : public QObject { Q_OBJECT @@ -183,6 +223,23 @@ class TaskTest : public QObject { return t.isFinished(); }, 1000), "Task didn't finish as it should."); } + + void test_stackOverflowInConcurrentTask() + { + QEventLoop loop; + + auto thread = new BigConcurrentTask; + thread->setStackSize(32 * 1024); + + connect(thread, &BigConcurrentTask::finished, &loop, &QEventLoop::quit); + + thread->start(); + + loop.exec(); + + QVERIFY(!thread->passed_the_deadline); + thread->deleteLater(); + } }; QTEST_GUILESS_MAIN(TaskTest) From 00d42d296e6519c92716d377496ba48c348c95b3 Mon Sep 17 00:00:00 2001 From: flow Date: Tue, 17 Jan 2023 16:08:50 -0300 Subject: [PATCH 2/3] fix: call processEvents() before adding new tasks to the task queue This allows the ongoing task to go off the stack before the next one is started. Signed-off-by: flow --- launcher/tasks/ConcurrentTask.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/launcher/tasks/ConcurrentTask.cpp b/launcher/tasks/ConcurrentTask.cpp index a890013ef..190d48d8f 100644 --- a/launcher/tasks/ConcurrentTask.cpp +++ b/launcher/tasks/ConcurrentTask.cpp @@ -110,14 +110,14 @@ void ConcurrentTask::startNext() setStepStatus(next->isMultiStep() ? next->getStepStatus() : next->getStatus()); updateState(); + QCoreApplication::processEvents(); + QMetaObject::invokeMethod(next.get(), &Task::start, Qt::QueuedConnection); // 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) From ec1f73c827c127c1dfc2a8cc1760015336cd8845 Mon Sep 17 00:00:00 2001 From: flow Date: Fri, 20 Jan 2023 12:55:38 -0300 Subject: [PATCH 3/3] fix(tests): add some comments on the stack overflow Task test Signed-off-by: flow --- tests/Task_test.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/Task_test.cpp b/tests/Task_test.cpp index 5d9068517..6649b7248 100644 --- a/tests/Task_test.cpp +++ b/tests/Task_test.cpp @@ -47,6 +47,7 @@ class BigConcurrentTask : public QThread { connect(&deadline, &QTimer::timeout, this, [this]{ passed_the_deadline = true; }); deadline.start(); + // NOTE: Arbitrary value that manages to trigger a problem when there is one. static const unsigned s_num_tasks = 1 << 14; auto sub_tasks = new BasicTask*[s_num_tasks]; @@ -229,6 +230,8 @@ class TaskTest : public QObject { QEventLoop loop; auto thread = new BigConcurrentTask; + // NOTE: This is an arbitrary value, big enough to not cause problems on normal execution, but low enough + // so that the number of tasks that needs to get ran to potentially cause a problem isn't too big. thread->setStackSize(32 * 1024); connect(thread, &BigConcurrentTask::finished, &loop, &QEventLoop::quit);