QTest::WatchDog: fix missing timeout resets on test function change

Since e0cad1aab5, 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>
This commit is contained in:
Marc Mutz 2022-12-16 12:20:08 +01:00
parent 17a7e5cfdd
commit 26c190f57e

View File

@ -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: