diff --git a/src/libs/solutions/tasking/tasktree.cpp b/src/libs/solutions/tasking/tasktree.cpp index 71dece59855..bae7c2e17c5 100644 --- a/src/libs/solutions/tasking/tasktree.cpp +++ b/src/libs/solutions/tasking/tasktree.cpp @@ -2672,46 +2672,51 @@ struct TimerData TimeoutCallback m_callback; }; -QMutex s_mutex; -std::atomic_int s_timerId = 0; -QHash s_timerIdToTimerData = {}; -QMultiMap s_deadlineToTimerId = {}; +struct TimerThreadData +{ + Q_DISABLE_COPY_MOVE(TimerThreadData) + + QHash m_timerIdToTimerData = {}; + QMultiMap m_deadlineToTimerId = {}; + int m_timerIdCounter = 0; +}; + +// Please note the thread_local keyword below guarantees a separate instance per thread. +static thread_local TimerThreadData s_threadTimerData = {}; static QList prepareForActivation(int timerId) { - QMutexLocker lock(&s_mutex); - const auto it = s_timerIdToTimerData.constFind(timerId); - if (it == s_timerIdToTimerData.cend()) + const auto it = s_threadTimerData.m_timerIdToTimerData.constFind(timerId); + if (it == s_threadTimerData.m_timerIdToTimerData.cend()) return {}; // the timer was already activated const system_clock::time_point deadline = it->m_deadline; QList toActivate; - auto itMap = s_deadlineToTimerId.cbegin(); - while (itMap != s_deadlineToTimerId.cend()) { + auto itMap = s_threadTimerData.m_deadlineToTimerId.cbegin(); + while (itMap != s_threadTimerData.m_deadlineToTimerId.cend()) { if (itMap.key() > deadline) break; - const auto it = s_timerIdToTimerData.constFind(itMap.value()); - if (it != s_timerIdToTimerData.cend()) { + const auto it = s_threadTimerData.m_timerIdToTimerData.constFind(itMap.value()); + if (it != s_threadTimerData.m_timerIdToTimerData.cend()) { toActivate.append(it.value()); - s_timerIdToTimerData.erase(it); + s_threadTimerData.m_timerIdToTimerData.erase(it); } - itMap = s_deadlineToTimerId.erase(itMap); + itMap = s_threadTimerData.m_deadlineToTimerId.erase(itMap); } return toActivate; } static void removeTimerId(int timerId) { - QMutexLocker lock(&s_mutex); - const auto it = s_timerIdToTimerData.constFind(timerId); - QT_ASSERT(it != s_timerIdToTimerData.cend(), + const auto it = s_threadTimerData.m_timerIdToTimerData.constFind(timerId); + QT_ASSERT(it != s_threadTimerData.m_timerIdToTimerData.cend(), qWarning("Removing active timerId failed."); return); const system_clock::time_point deadline = it->m_deadline; - s_timerIdToTimerData.erase(it); + s_threadTimerData.m_timerIdToTimerData.erase(it); - const int removedCount = s_deadlineToTimerId.remove(deadline, timerId); + const int removedCount = s_threadTimerData.m_deadlineToTimerId.remove(deadline, timerId); QT_ASSERT(removedCount == 1, qWarning("Removing active timerId failed."); return); } @@ -2720,18 +2725,17 @@ static void handleTimeout(int timerId) const QList toActivate = prepareForActivation(timerId); for (const TimerData &timerData : toActivate) { if (timerData.m_context) - QMetaObject::invokeMethod(timerData.m_context.get(), timerData.m_callback); + timerData.m_callback(); } } static int scheduleTimeout(milliseconds timeout, QObject *context, const TimeoutCallback &callback) { - const int timerId = s_timerId.fetch_add(1) + 1; + const int timerId = ++s_threadTimerData.m_timerIdCounter; const system_clock::time_point deadline = system_clock::now() + timeout; QTimer::singleShot(timeout, context, [timerId] { handleTimeout(timerId); }); - QMutexLocker lock(&s_mutex); - s_timerIdToTimerData.emplace(timerId, TimerData{deadline, context, callback}); - s_deadlineToTimerId.insert(deadline, timerId); + s_threadTimerData.m_timerIdToTimerData.emplace(timerId, TimerData{deadline, context, callback}); + s_threadTimerData.m_deadlineToTimerId.insert(deadline, timerId); return timerId; } diff --git a/tests/auto/solutions/tasking/tst_tasking.cpp b/tests/auto/solutions/tasking/tst_tasking.cpp index da9929bff1f..108654190d5 100644 --- a/tests/auto/solutions/tasking/tst_tasking.cpp +++ b/tests/auto/solutions/tasking/tst_tasking.cpp @@ -394,7 +394,7 @@ static Handler toTweakDoneHandler(DoneResult result) return result == DoneResult::Success ? Handler::TweakDoneToSuccess : Handler::TweakDoneToError; } -static TestData storageShadowing() +static TestData storageShadowingData() { // This test check if storage shadowing works OK. @@ -471,6 +471,70 @@ static TestData storageShadowing() return {storage, root, log, 0, DoneWith::Success}; } +static TestData parallelData() +{ + TreeStorage storage; + + const auto setupTask = [storage](int taskId, milliseconds timeout) { + return [storage, taskId, timeout](TaskObject &taskObject) { + taskObject = timeout; + storage->m_log.append({taskId, Handler::Setup}); + }; + }; + + const auto setupDone = [storage](int taskId, DoneResult result = DoneResult::Success) { + return [storage, taskId, result](DoneWith doneWith) { + const Handler handler = doneWith == DoneWith::Cancel ? Handler::Canceled + : result == DoneResult::Success ? Handler::Success : Handler::Error; + storage->m_log.append({taskId, handler}); + return doneWith == DoneWith::Cancel ? DoneResult::Error + : result == DoneResult::Success ? DoneResult::Success : DoneResult::Error; + }; + }; + + const auto createTask = [storage, setupTask, setupDone]( + int taskId, DoneResult result, milliseconds timeout = 0ms) { + return TestTask(setupTask(taskId, timeout), setupDone(taskId, result)); + }; + + const auto createSuccessTask = [createTask](int taskId, milliseconds timeout = 0ms) { + return createTask(taskId, DoneResult::Success, timeout); + }; + + const auto groupDone = [storage](int taskId) { + return onGroupDone([storage, taskId](DoneWith result) { + storage->m_log.append({taskId, resultToGroupHandler(result)}); + }); + }; + + const Group root { + Storage(storage), + parallel, + createSuccessTask(1), + createSuccessTask(2), + createSuccessTask(3), + createSuccessTask(4), + createSuccessTask(5), + groupDone(0) + }; + + const Log log { + {1, Handler::Setup}, // Setup order is determined in parallel mode + {2, Handler::Setup}, + {3, Handler::Setup}, + {4, Handler::Setup}, + {5, Handler::Setup}, + {1, Handler::Success}, + {2, Handler::Success}, + {3, Handler::Success}, + {4, Handler::Success}, + {5, Handler::Success}, + {0, Handler::GroupSuccess} + }; + + return {storage, root, log, 5, DoneWith::Success}; +} + void tst_Tasking::testTree_data() { QTest::addColumn("testData"); @@ -792,32 +856,7 @@ void tst_Tasking::testTree_data() QTest::newRow("Nested") << TestData{storage, root, log, 1, DoneWith::Success}; } - { - const Group root { - Storage(storage), - parallel, - createSuccessTask(1), - createSuccessTask(2), - createSuccessTask(3), - createSuccessTask(4), - createSuccessTask(5), - groupDone(0) - }; - const Log log { - {1, Handler::Setup}, // Setup order is determined in parallel mode - {2, Handler::Setup}, - {3, Handler::Setup}, - {4, Handler::Setup}, - {5, Handler::Setup}, - {1, Handler::Success}, - {2, Handler::Success}, - {3, Handler::Success}, - {4, Handler::Success}, - {5, Handler::Success}, - {0, Handler::GroupSuccess} - }; - QTest::newRow("Parallel") << TestData{storage, root, log, 5, DoneWith::Success}; - } + QTest::newRow("Parallel") << parallelData(); { auto setupSubTree = [storage, createSuccessTask](TaskTree &taskTree) { @@ -2592,10 +2631,8 @@ void tst_Tasking::testTree_data() DoneWith::Success}; } - { - // This test check if storage shadowing works OK. - QTest::newRow("StorageShadowing") << storageShadowing(); - } + // This test check if storage shadowing works OK. + QTest::newRow("StorageShadowing") << storageShadowingData(); } void tst_Tasking::testTree() @@ -2622,13 +2659,15 @@ void tst_Tasking::testTree() void tst_Tasking::testInThread_data() { QTest::addColumn("testData"); - QTest::newRow("StorageShadowing") << storageShadowing(); + QTest::newRow("StorageShadowing") << storageShadowingData(); + QTest::newRow("Parallel") << parallelData(); } struct TestResult { int executeCount = 0; ThreadResult threadResult = ThreadResult::Success; + Log actualLog = {}; }; static const int s_loopCount = 1000; @@ -2664,7 +2703,7 @@ static void runInThread(QPromise &promise, const TestData &testData) return; } if (actualLog != testData.expectedLog) { - promise.addResult(TestResult{i, ThreadResult::FailOnLogCheck}); + promise.addResult(TestResult{i, ThreadResult::FailOnLogCheck, actualLog}); return; } if (result != testData.onDone) { @@ -2672,7 +2711,7 @@ static void runInThread(QPromise &promise, const TestData &testData) return; } } - promise.addResult(TestResult{s_loopCount, ThreadResult::Success}); + promise.addResult(TestResult{s_loopCount, ThreadResult::Success, testData.expectedLog}); } void tst_Tasking::testInThread() @@ -2685,8 +2724,9 @@ void tst_Tasking::testInThread() const auto onDone = [testData](const ConcurrentCall &task) { QVERIFY(task.future().resultCount()); const TestResult result = task.result(); - QCOMPARE(result.executeCount, s_loopCount); + QCOMPARE(result.actualLog, testData.expectedLog); QCOMPARE(result.threadResult, ThreadResult::Success); + QCOMPARE(result.executeCount, s_loopCount); }; QList tasks = { parallel };