From 26c190f57ea336106aeceffe1191a0314bb4443c Mon Sep 17 00:00:00 2001
From: Marc Mutz <marc.mutz@qt.io>
Date: Fri, 16 Dec 2022 12:20:08 +0100
Subject: QTest::WatchDog: fix missing timeout resets on test function change
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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>
---
 src/testlib/qtestcase.cpp | 25 ++++++++++++++++++++++---
 1 file changed, 22 insertions(+), 3 deletions(-)

(limited to 'src/testlib')

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:
-- 
cgit v1.2.3