From e0cad1aab53119a0e47467f2236f019ce8d6da2a Mon Sep 17 00:00:00 2001
From: Marc Mutz <marc.mutz@kdab.com>
Date: Fri, 13 Sep 2019 10:29:49 +0200
Subject: QTestLib: modernize and stream-line WatchDog
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

... by porting from (QWaitCondition,QMutex) to std::{condition_variable,
mutex} and making the code more accessible by introducing explicit
states.

This patch was originally starting out to just replace QWaitCondition
with std::condition_variable, which is faster and more compact, and
the patch still does that, too. The focus, however, has shifted
towards improving the accessibility of the code, in particular its
states and the transitions between them.

Due to the way QWaitCondition prevents spurious wakeups, this code
could use its QWaitCondition as a semaphore, e.g. where it was calling
wait() in the ctor to be released only when the thread it started has
entered run(). That makes the code unnecessarily hard to follow.

Fix by introducing an enum Expectation which tells what the watch-dog
is currently waiting for, and unmistakably determines the state the
watch-dog is in: ThreadStart, TestFunctionStart, ... The timeout value
is now selected based on the Expectation value, so the timeout is no
longer serving as the implicit state, either.

Also port the defaultTimeout() function to return a
std::chrono::milliseconds directly. Elsewhere in Qt, we guard against
lack of <chrono>, so this unguarded use will also serve as a guide to
see whether all supported platforms now, eight Qt releases after we
formally require C++11 for Qt usage, provide it. Without <chrono>,
there's no timed waiting in the standard library.

Change-Id: If97b601c4090a2a2926fa58c903cfe3ec2656324
Reviewed-by: Edward Welbourne <edward.welbourne@qt.io>
Reviewed-by: Mårten Nordheim <marten.nordheim@qt.io>
---
 src/testlib/qtestcase.cpp | 84 ++++++++++++++++++++++++++++++++---------------
 1 file changed, 57 insertions(+), 27 deletions(-)

(limited to 'src/testlib')

diff --git a/src/testlib/qtestcase.cpp b/src/testlib/qtestcase.cpp
index 990fc5679d..e29897abee 100644
--- a/src/testlib/qtestcase.cpp
+++ b/src/testlib/qtestcase.cpp
@@ -58,8 +58,7 @@
 #include <QtCore/qdiriterator.h>
 #include <QtCore/qtemporarydir.h>
 #include <QtCore/qthread.h>
-#include <QtCore/qwaitcondition.h>
-#include <QtCore/qmutex.h>
+#include <QtCore/private/qlocking_p.h>
 
 #include <QtCore/qtestsupport_core.h>
 
@@ -85,6 +84,9 @@
 #include <cmath>
 #include <numeric>
 #include <algorithm>
+#include <condition_variable>
+#include <mutex>
+#include <chrono>
 
 #include <stdarg.h>
 #include <stdio.h>
@@ -406,7 +408,7 @@ int Q_TESTLIB_EXPORT defaultKeyDelay()
     return keyDelay;
 }
 #if QT_CONFIG(thread)
-static int defaultTimeout()
+static std::chrono::milliseconds defaultTimeout()
 {
     if (timeout == -1) {
         bool ok = false;
@@ -415,7 +417,7 @@ static int defaultTimeout()
         if (!ok || timeout <= 0)
             timeout = 5*60*1000;
     }
-    return timeout;
+    return std::chrono::milliseconds{timeout};
 }
 #endif
 
@@ -1008,53 +1010,81 @@ void TestMethods::invokeTestOnData(int index) const
 
 class WatchDog : public QThread
 {
+    enum Expectation {
+        ThreadStart,
+        TestFunctionStart,
+        TestFunctionEnd,
+        ThreadEnd,
+    };
+
+    bool waitFor(std::unique_lock<std::mutex> &m, Expectation e) {
+        auto expectation = [this, e] { return expecting != e; };
+        switch (e) {
+        case TestFunctionEnd:
+            return waitCondition.wait_for(m, defaultTimeout(), expectation);
+        case ThreadStart:
+        case ThreadEnd:
+        case TestFunctionStart:
+            waitCondition.wait(m, expectation);
+            return true;
+        }
+        Q_UNREACHABLE();
+        return false;
+    }
+
 public:
     WatchDog()
     {
-        QMutexLocker locker(&mutex);
-        timeout.storeRelaxed(-1);
+        std::unique_lock<std::mutex> locker(mutex);
+        expecting = ThreadStart;
         start();
-        waitCondition.wait(&mutex);
+        waitFor(locker, ThreadStart);
     }
     ~WatchDog() {
         {
-            QMutexLocker locker(&mutex);
-            timeout.storeRelaxed(0);
-            waitCondition.wakeAll();
+            const auto locker = qt_scoped_lock(mutex);
+            expecting = ThreadEnd;
+            waitCondition.notify_all();
         }
         wait();
     }
 
     void beginTest() {
-        QMutexLocker locker(&mutex);
-        timeout.storeRelaxed(defaultTimeout());
-        waitCondition.wakeAll();
+        const auto locker = qt_scoped_lock(mutex);
+        expecting = TestFunctionEnd;
+        waitCondition.notify_all();
     }
 
     void testFinished() {
-        QMutexLocker locker(&mutex);
-        timeout.storeRelaxed(-1);
-        waitCondition.wakeAll();
+        const auto locker = qt_scoped_lock(mutex);
+        expecting = TestFunctionStart;
+        waitCondition.notify_all();
     }
 
     void run() override {
-        QMutexLocker locker(&mutex);
-        waitCondition.wakeAll();
+        std::unique_lock<std::mutex> locker(mutex);
+        expecting = TestFunctionStart;
+        waitCondition.notify_all();
         while (true) {
-            int t = timeout.loadRelaxed();
-            if (!t)
-                break;
-            if (Q_UNLIKELY(!waitCondition.wait(&mutex, t))) {
-                stackTrace();
-                qFatal("Test function timed out");
+            switch (expecting) {
+            case ThreadEnd:
+                return;
+            case ThreadStart:
+                Q_UNREACHABLE();
+            case TestFunctionStart:
+            case TestFunctionEnd:
+                if (Q_UNLIKELY(!waitFor(locker, expecting))) {
+                    stackTrace();
+                    qFatal("Test function timed out");
+                }
             }
         }
     }
 
 private:
-    QBasicAtomicInt timeout;
-    QMutex mutex;
-    QWaitCondition waitCondition;
+    std::mutex mutex;
+    std::condition_variable waitCondition;
+    Expectation expecting;
 };
 
 #else // !QT_CONFIG(thread)
-- 
cgit v1.2.3