From 3c96d5e0d568408e0505841af39aa2d539ebaf36 Mon Sep 17 00:00:00 2001 From: Rachel Powers <508861+Ryex@users.noreply.github.com> Date: Mon, 3 Jul 2023 20:04:24 -0700 Subject: fix: memory leaks in tests Signed-off-by: Rachel Powers <508861+Ryex@users.noreply.github.com> --- tests/Task_test.cpp | 1 + 1 file changed, 1 insertion(+) (limited to 'tests/Task_test.cpp') diff --git a/tests/Task_test.cpp b/tests/Task_test.cpp index dabe5da2..a57102d6 100644 --- a/tests/Task_test.cpp +++ b/tests/Task_test.cpp @@ -80,6 +80,7 @@ class BigConcurrentTaskThread : public QThread { QCoreApplication::processEvents(); emit finished(); + delete[] sub_tasks; } public: -- cgit From 4dbcedd03f6d90a9557cd401c2236f9cda60cf9f Mon Sep 17 00:00:00 2001 From: Rachel Powers <508861+Ryex@users.noreply.github.com> Date: Wed, 5 Jul 2023 11:51:37 -0700 Subject: fix: Task test memory leaks again Signed-off-by: Rachel Powers <508861+Ryex@users.noreply.github.com> --- CMakeLists.txt | 5 ++ tests/Task_test.cpp | 138 +++++++++++++++++++++++++--------------------------- 2 files changed, 72 insertions(+), 71 deletions(-) (limited to 'tests/Task_test.cpp') diff --git a/CMakeLists.txt b/CMakeLists.txt index 4f8befcc..afa67546 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -95,18 +95,23 @@ if (CMAKE_BUILD_TYPE STREQUAL "Debug" AND DEBUG_ADDRESS_SANITIZER) # using clang with clang-cl front end message(STATUS "Address Sanitizer available on Clang MSVC frontend") set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /fsanitize=address /O1 /Oy-") + set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} /fsanitize=address /O1 /Oy-") else() # AppleClang and Clang message(STATUS "Address Sanitizer available on Clang") set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=address -O1 -fno-omit-frame-pointer") + set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -fsanitize=address -O1 -fno-omit-frame-pointer") endif() elseif ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "GNU") # GCC message(STATUS "Address Sanitizer available on GCC") set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=address -O1 -fno-omit-frame-pointer") + set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -fsanitize=address -O1 -fno-omit-frame-pointer") + link_libraries("asan") elseif ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "MSVC") message(STATUS "Address Sanitizer available on MSVC") set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /fsanitize=address /O1 /Oy-") + set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} /fsanitize=address /O1 /Oy-") else() message(STATUS "Address Sanitizer not available on compiler ${CMAKE_CXX_COMPILER_ID}") endif() diff --git a/tests/Task_test.cpp b/tests/Task_test.cpp index a57102d6..b6bd7edf 100644 --- a/tests/Task_test.cpp +++ b/tests/Task_test.cpp @@ -1,6 +1,6 @@ #include -#include #include +#include #include #include @@ -19,10 +19,7 @@ class BasicTask : public Task { BasicTask(bool show_debug_log = true) : Task(nullptr, show_debug_log) {} private: - void executeTask() override - { - emitSucceeded(); - }; + void executeTask() override { emitSucceeded(); }; }; /* Does nothing. Only used for testing. */ @@ -34,7 +31,7 @@ class BasicTask_MultiStep : public Task { private: auto isMultiStep() const -> bool override { return true; } - void executeTask() override {}; + void executeTask() override{}; }; class BigConcurrentTask : public ConcurrentTask { @@ -44,7 +41,7 @@ class BigConcurrentTask : public ConcurrentTask { { // This is here only to help fill the stack a bit more quickly (if there's an issue, of course :^)) // Each tasks thus adds 1024 * 4 bytes to the stack, at the very least. - [[maybe_unused]] volatile std::array some_data_on_the_stack {}; + [[maybe_unused]] volatile std::array some_data_on_the_stack{}; ConcurrentTask::startNext(); } @@ -59,28 +56,28 @@ class BigConcurrentTaskThread : public QThread { { QTimer deadline; deadline.setInterval(10000); - connect(&deadline, &QTimer::timeout, this, [this]{ passed_the_deadline = true; }); + 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. // Considering each tasks, in a problematic state, adds 1024 * 4 bytes to the stack, // this number is enough to fill up 16 MiB of stack, more than enough to cause a problem. static const unsigned s_num_tasks = 1 << 12; - auto sub_tasks = new BasicTask::Ptr[s_num_tasks]; + { + auto sub_tasks = std::array(); - for (unsigned i = 0; i < s_num_tasks; i++) { - auto sub_task = makeShared(false); - sub_tasks[i] = sub_task; - big_task.addTask(sub_task); - } + for (unsigned i = 0; i < s_num_tasks; i++) { + auto sub_task = makeShared(false); + sub_tasks[i] = sub_task; + big_task.addTask(sub_task); + } - big_task.run(); - - while (!big_task.isFinished() && !passed_the_deadline) - QCoreApplication::processEvents(); + big_task.run(); + while (!big_task.isFinished() && !passed_the_deadline) + QCoreApplication::processEvents(); + } // drop before emit emit finished(); - delete[] sub_tasks; } public: @@ -94,9 +91,10 @@ class TaskTest : public QObject { Q_OBJECT private slots: - void test_SetStatus_NoMultiStep(){ + void test_SetStatus_NoMultiStep() + { BasicTask t; - QString status {"test status"}; + QString status{ "test status" }; t.setStatus(status); @@ -104,9 +102,10 @@ class TaskTest : public QObject { QCOMPARE(t.getStepProgress().isEmpty(), TaskStepProgressList{}.isEmpty()); } - void test_SetStatus_MultiStep(){ + void test_SetStatus_MultiStep() + { BasicTask_MultiStep t; - QString status {"test status"}; + QString status{ "test status" }; t.setStatus(status); @@ -116,7 +115,8 @@ class TaskTest : public QObject { QCOMPARE(t.getStepProgress().isEmpty(), TaskStepProgressList{}.isEmpty()); } - void test_SetProgress(){ + void test_SetProgress() + { BasicTask t; int current = 42; int total = 207; @@ -127,17 +127,18 @@ class TaskTest : public QObject { QCOMPARE(t.getTotalProgress(), total); } - void test_basicRun(){ + void test_basicRun() + { BasicTask t; - QObject::connect(&t, &Task::finished, [&]{ QVERIFY2(t.wasSuccessful(), "Task finished but was not successful when it should have been."); }); + QObject::connect(&t, &Task::finished, + [&] { QVERIFY2(t.wasSuccessful(), "Task finished but was not successful when it should have been."); }); t.start(); - QVERIFY2(QTest::qWaitFor([&]() { - return t.isFinished(); - }, 1000), "Task didn't finish as it should."); + QVERIFY2(QTest::qWaitFor([&]() { return t.isFinished(); }, 1000), "Task didn't finish as it should."); } - void test_basicConcurrentRun(){ + void test_basicConcurrentRun() + { auto t1 = makeShared(); auto t2 = makeShared(); auto t3 = makeShared(); @@ -148,21 +149,20 @@ class TaskTest : public QObject { t.addTask(t2); t.addTask(t3); - QObject::connect(&t, &Task::finished, [&]{ - QVERIFY2(t.wasSuccessful(), "Task finished but was not successful when it should have been."); - QVERIFY(t1->wasSuccessful()); - QVERIFY(t2->wasSuccessful()); - QVERIFY(t3->wasSuccessful()); + QObject::connect(&t, &Task::finished, [&t, &t1, &t2, &t3] { + QVERIFY2(t.wasSuccessful(), "Task finished but was not successful when it should have been."); + QVERIFY(t1->wasSuccessful()); + QVERIFY(t2->wasSuccessful()); + QVERIFY(t3->wasSuccessful()); }); t.start(); - QVERIFY2(QTest::qWaitFor([&]() { - return t.isFinished(); - }, 1000), "Task didn't finish as it should."); + QVERIFY2(QTest::qWaitFor([&]() { return t.isFinished(); }, 1000), "Task didn't finish as it should."); } // Tests if starting new tasks after the 6 initial ones is working - void test_moreConcurrentRun(){ + void test_moreConcurrentRun() + { auto t1 = makeShared(); auto t2 = makeShared(); auto t3 = makeShared(); @@ -185,26 +185,25 @@ class TaskTest : public QObject { t.addTask(t8); t.addTask(t9); - QObject::connect(&t, &Task::finished, [&]{ - QVERIFY2(t.wasSuccessful(), "Task finished but was not successful when it should have been."); - QVERIFY(t1->wasSuccessful()); - QVERIFY(t2->wasSuccessful()); - QVERIFY(t3->wasSuccessful()); - QVERIFY(t4->wasSuccessful()); - QVERIFY(t5->wasSuccessful()); - QVERIFY(t6->wasSuccessful()); - QVERIFY(t7->wasSuccessful()); - QVERIFY(t8->wasSuccessful()); - QVERIFY(t9->wasSuccessful()); + QObject::connect(&t, &Task::finished, [&t, &t1, &t2, &t3, &t4, &t5, &t6, &t7, &t8, &t9] { + QVERIFY2(t.wasSuccessful(), "Task finished but was not successful when it should have been."); + QVERIFY(t1->wasSuccessful()); + QVERIFY(t2->wasSuccessful()); + QVERIFY(t3->wasSuccessful()); + QVERIFY(t4->wasSuccessful()); + QVERIFY(t5->wasSuccessful()); + QVERIFY(t6->wasSuccessful()); + QVERIFY(t7->wasSuccessful()); + QVERIFY(t8->wasSuccessful()); + QVERIFY(t9->wasSuccessful()); }); t.start(); - QVERIFY2(QTest::qWaitFor([&]() { - return t.isFinished(); - }, 1000), "Task didn't finish as it should."); + QVERIFY2(QTest::qWaitFor([&]() { return t.isFinished(); }, 1000), "Task didn't finish as it should."); } - void test_basicSequentialRun(){ + void test_basicSequentialRun() + { auto t1 = makeShared(); auto t2 = makeShared(); auto t3 = makeShared(); @@ -215,20 +214,19 @@ class TaskTest : public QObject { t.addTask(t2); t.addTask(t3); - QObject::connect(&t, &Task::finished, [&]{ - QVERIFY2(t.wasSuccessful(), "Task finished but was not successful when it should have been."); - QVERIFY(t1->wasSuccessful()); - QVERIFY(t2->wasSuccessful()); - QVERIFY(t3->wasSuccessful()); + QObject::connect(&t, &Task::finished, [&t, &t1, &t2, &t3] { + QVERIFY2(t.wasSuccessful(), "Task finished but was not successful when it should have been."); + QVERIFY(t1->wasSuccessful()); + QVERIFY(t2->wasSuccessful()); + QVERIFY(t3->wasSuccessful()); }); t.start(); - QVERIFY2(QTest::qWaitFor([&]() { - return t.isFinished(); - }, 1000), "Task didn't finish as it should."); + QVERIFY2(QTest::qWaitFor([&]() { return t.isFinished(); }, 1000), "Task didn't finish as it should."); } - void test_basicMultipleOptionsRun(){ + void test_basicMultipleOptionsRun() + { auto t1 = makeShared(); auto t2 = makeShared(); auto t3 = makeShared(); @@ -239,17 +237,15 @@ class TaskTest : public QObject { t.addTask(t2); t.addTask(t3); - QObject::connect(&t, &Task::finished, [&]{ - QVERIFY2(t.wasSuccessful(), "Task finished but was not successful when it should have been."); - QVERIFY(t1->wasSuccessful()); - QVERIFY(!t2->wasSuccessful()); - QVERIFY(!t3->wasSuccessful()); + QObject::connect(&t, &Task::finished, [&t, &t1, &t2, &t3] { + QVERIFY2(t.wasSuccessful(), "Task finished but was not successful when it should have been."); + QVERIFY(t1->wasSuccessful()); + QVERIFY(!t2->wasSuccessful()); + QVERIFY(!t3->wasSuccessful()); }); t.start(); - QVERIFY2(QTest::qWaitFor([&]() { - return t.isFinished(); - }, 1000), "Task didn't finish as it should."); + QVERIFY2(QTest::qWaitFor([&]() { return t.isFinished(); }, 1000), "Task didn't finish as it should."); } void test_stackOverflowInConcurrentTask() -- cgit From 965ee5687a475aa0a7e3756f0e5a7e1db26c6a16 Mon Sep 17 00:00:00 2001 From: Rachel Powers <508861+Ryex@users.noreply.github.com> Date: Wed, 5 Jul 2023 20:05:18 -0700 Subject: fix(test): task test memory leak *again* - put Big thread on the stack so stack will clean it up. Signed-off-by: Rachel Powers <508861+Ryex@users.noreply.github.com> --- tests/Task_test.cpp | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) (limited to 'tests/Task_test.cpp') diff --git a/tests/Task_test.cpp b/tests/Task_test.cpp index b6bd7edf..ef7042a5 100644 --- a/tests/Task_test.cpp +++ b/tests/Task_test.cpp @@ -252,16 +252,15 @@ class TaskTest : public QObject { { QEventLoop loop; - auto thread = new BigConcurrentTaskThread; + BigConcurrentTaskThread thread{}; - connect(thread, &BigConcurrentTaskThread::finished, &loop, &QEventLoop::quit); + connect(&thread, &BigConcurrentTaskThread::finished, &loop, &QEventLoop::quit); - thread->start(); + thread.start(); loop.exec(); - QVERIFY(!thread->passed_the_deadline); - thread->deleteLater(); + QVERIFY(!thread.passed_the_deadline); } }; -- cgit From 8638076aa1cf3f1a975ac59ddaf0286acc7d39e2 Mon Sep 17 00:00:00 2001 From: Rachel Powers <508861+Ryex@users.noreply.github.com> Date: Wed, 5 Jul 2023 20:19:22 -0700 Subject: fix(test): tasks test memmory leak. don't store local task copy. Signed-off-by: Rachel Powers <508861+Ryex@users.noreply.github.com> --- tests/Task_test.cpp | 2 -- 1 file changed, 2 deletions(-) (limited to 'tests/Task_test.cpp') diff --git a/tests/Task_test.cpp b/tests/Task_test.cpp index ef7042a5..41e51f83 100644 --- a/tests/Task_test.cpp +++ b/tests/Task_test.cpp @@ -64,11 +64,9 @@ class BigConcurrentTaskThread : public QThread { // this number is enough to fill up 16 MiB of stack, more than enough to cause a problem. static const unsigned s_num_tasks = 1 << 12; { - auto sub_tasks = std::array(); for (unsigned i = 0; i < s_num_tasks; i++) { auto sub_task = makeShared(false); - sub_tasks[i] = sub_task; big_task.addTask(sub_task); } -- cgit From 71e73bb6f8c4a695e39f0c0fec901d8a5e5121b3 Mon Sep 17 00:00:00 2001 From: Rachel Powers <508861+Ryex@users.noreply.github.com> Date: Wed, 5 Jul 2023 21:18:49 -0700 Subject: fix(tests): linux big task memory leak. - move big_task into function scope Signed-off-by: Rachel Powers <508861+Ryex@users.noreply.github.com> --- tests/Task_test.cpp | 31 +++++++++++++++++-------------- 1 file changed, 17 insertions(+), 14 deletions(-) (limited to 'tests/Task_test.cpp') diff --git a/tests/Task_test.cpp b/tests/Task_test.cpp index 41e51f83..00d06794 100644 --- a/tests/Task_test.cpp +++ b/tests/Task_test.cpp @@ -50,31 +50,34 @@ class BigConcurrentTask : public ConcurrentTask { class BigConcurrentTaskThread : public QThread { Q_OBJECT - BigConcurrentTask big_task; void run() override { QTimer deadline; + BigConcurrentTask big_task; deadline.setInterval(10000); - connect(&deadline, &QTimer::timeout, this, [this] { passed_the_deadline = true; }); + bool* pt_deadline = &passed_the_deadline; + QMetaObject::Connection conn = connect(&deadline, &QTimer::timeout, this, [pt_deadline] { *pt_deadline = true; }); deadline.start(); // NOTE: Arbitrary value that manages to trigger a problem when there is one. // Considering each tasks, in a problematic state, adds 1024 * 4 bytes to the stack, // this number is enough to fill up 16 MiB of stack, more than enough to cause a problem. static const unsigned s_num_tasks = 1 << 12; - { - - for (unsigned i = 0; i < s_num_tasks; i++) { - auto sub_task = makeShared(false); - big_task.addTask(sub_task); - } - - big_task.run(); - - while (!big_task.isFinished() && !passed_the_deadline) - QCoreApplication::processEvents(); - } // drop before emit + for (unsigned i = 0; i < s_num_tasks; i++) { + auto sub_task = makeShared(false); + big_task.addTask(sub_task); + } + + big_task.run(); + + while (!big_task.isFinished() && !passed_the_deadline) + QCoreApplication::processEvents(); + // don't fire timer after this point + disconnect(conn); + if (deadline.isActive()) + deadline.stop(); + // task finished emit finished(); } -- cgit From b8b8c8d4acab8c794555956fae699d5706e222f3 Mon Sep 17 00:00:00 2001 From: flow Date: Thu, 6 Jul 2023 06:38:36 -0300 Subject: fix(tests): Fix abort of Task test on Linux Not sure exactly what caused the issue, though I suppose using QThread's exec instead of our own thingie is nice. I can't remember why I didn't use that before, so I hope there's no issue with that! :^) Signed-off-by: flow --- tests/Task_test.cpp | 26 ++++++++------------------ 1 file changed, 8 insertions(+), 18 deletions(-) (limited to 'tests/Task_test.cpp') diff --git a/tests/Task_test.cpp b/tests/Task_test.cpp index 00d06794..c59d4bb7 100644 --- a/tests/Task_test.cpp +++ b/tests/Task_test.cpp @@ -50,15 +50,11 @@ class BigConcurrentTask : public ConcurrentTask { class BigConcurrentTaskThread : public QThread { Q_OBJECT - + QTimer m_deadline; void run() override { - QTimer deadline; BigConcurrentTask big_task; - deadline.setInterval(10000); - bool* pt_deadline = &passed_the_deadline; - QMetaObject::Connection conn = connect(&deadline, &QTimer::timeout, this, [pt_deadline] { *pt_deadline = true; }); - deadline.start(); + m_deadline.setInterval(10000); // NOTE: Arbitrary value that manages to trigger a problem when there is one. // Considering each tasks, in a problematic state, adds 1024 * 4 bytes to the stack, @@ -69,23 +65,17 @@ class BigConcurrentTaskThread : public QThread { big_task.addTask(sub_task); } + connect(&big_task, &Task::finished, this, &QThread::quit); + connect(&m_deadline, &QTimer::timeout, this, [&] { passed_the_deadline = true; quit(); }); + + m_deadline.start(); big_task.run(); - while (!big_task.isFinished() && !passed_the_deadline) - QCoreApplication::processEvents(); - // don't fire timer after this point - disconnect(conn); - if (deadline.isActive()) - deadline.stop(); - // task finished - emit finished(); + exec(); } public: bool passed_the_deadline = false; - - signals: - void finished(); }; class TaskTest : public QObject { @@ -253,7 +243,7 @@ class TaskTest : public QObject { { QEventLoop loop; - BigConcurrentTaskThread thread{}; + BigConcurrentTaskThread thread; connect(&thread, &BigConcurrentTaskThread::finished, &loop, &QEventLoop::quit); -- cgit