diff options
author | Marc Mutz <marc.mutz@qt.io> | 2022-12-16 12:20:08 +0100 |
---|---|---|
committer | Marc Mutz <marc.mutz@qt.io> | 2022-12-16 22:58:10 +0100 |
commit | 26c190f57ea336106aeceffe1191a0314bb4443c (patch) | |
tree | 516c55e0cf9ba76a8acd8fd838f7f097c7aa4844 /src/testlib/qtestcase.cpp | |
parent | 17a7e5cfddcbf33ef0fd1c2b7acc65b85159194e (diff) |
QTest::WatchDog: fix missing timeout resets on test function change
Since e0cad1aab53119a0e47467f2236f019ce8d6da2a, the code suffered
from an ABA problem where the TestFunctionStart expectation is set
in testFinished(), but by the time WatchDog::run() gets around to
examining the state when returning from condition_variable::wait()
in waitFor(), the next beginTest() has already set the expectation
back to TestFunctionEnd.
There are several known solutions for ABA problems. Embedding a
generation count into the expectation state seemed to be the most
straight-forward fix, and can be done without DWCAS support, because
the state is just 2 bits, leaving the other 30 or 62 bits for the
generation counter, so do that.
[ChangeLog][QTestLib] Fixed a bug which caused
QTEST_FUNCTION_TIMEOUT to be applied to the whole test execution,
as opposed to each test function.
Fixes: QTBUG-109466
Pick-to: 6.5 6.4 6.2 5.15
Change-Id: If71ade932330407b85d204d45c74350c651325fe
Reviewed-by: MÃ¥rten Nordheim <marten.nordheim@qt.io>
Diffstat (limited to 'src/testlib/qtestcase.cpp')
-rw-r--r-- | src/testlib/qtestcase.cpp | 25 |
1 files changed, 22 insertions, 3 deletions
diff --git a/src/testlib/qtestcase.cpp b/src/testlib/qtestcase.cpp index 4023ae86eb..cebb934973 100644 --- a/src/testlib/qtestcase.cpp +++ b/src/testlib/qtestcase.cpp @@ -1211,17 +1211,30 @@ void TestMethods::invokeTestOnData(int index) const class WatchDog : public QThread { - enum Expectation { + enum Expectation : std::size_t { + // bits 0..1: state ThreadStart, TestFunctionStart, TestFunctionEnd, ThreadEnd, + + // bits 2..: generation }; + static constexpr auto ExpectationMask = Expectation{ThreadStart | TestFunctionStart | TestFunctionEnd | ThreadEnd}; + static_assert(size_t(ExpectationMask) == 0x3); + static constexpr size_t GenerationShift = 2; + + static constexpr Expectation state(Expectation e) noexcept + { return Expectation{e & ExpectationMask}; } + static constexpr size_t generation(Expectation e) noexcept + { return e >> GenerationShift; } + static constexpr Expectation combine(Expectation e, size_t gen) noexcept + { return Expectation{e | (gen << GenerationShift)}; } bool waitFor(std::unique_lock<QtPrivate::mutex> &m, Expectation e) { auto expectationChanged = [this, e] { return expecting.load(std::memory_order_relaxed) != e; }; - switch (e) { + switch (state(e)) { case TestFunctionEnd: return waitCondition.wait_for(m, defaultTimeout(), expectationChanged); case ThreadStart: @@ -1235,7 +1248,13 @@ class WatchDog : public QThread void setExpectation(Expectation e) { + Q_ASSERT(generation(e) == 0); // no embedded generation allowed const auto locker = qt_scoped_lock(mutex); + auto cur = expecting.load(std::memory_order_relaxed); + auto gen = generation(cur); + if (e == TestFunctionStart) + ++gen; + e = combine(e, gen); expecting.store(e, std::memory_order_relaxed); waitCondition.notify_all(); } @@ -1273,7 +1292,7 @@ public: waitCondition.notify_all(); while (true) { Expectation e = expecting.load(std::memory_order_acquire); - switch (e) { + switch (state(e)) { case ThreadEnd: return; case ThreadStart: |