tst_QThread: improve test on multiple threads waiting on wait()

Amends commit 5b5297fe87859f59a7aaf5e86a8915c00714fefa to make it more
reliable against CI timeouts and flakiness. Now, we wait for the threads
that are expected to timeout to actually timeout before releasing the
target thread they were waiting on. It should be impossible now for this
to go wrong. There should also be no problem of their handing off to a
thread that will wait forever.

I've added two more tests that could, possibly, have a problem in the
CI: when timing-out threads hand off to a timed wait that is expected to
succeed (the > 1s wait cases). Though this should be a rare occurrence,
if ever: the target thread's runtime is the longest of the timing out
threads' wait, plus 5 ms. That is 30 ms in the examples I wrote, so the
additional extra time they're waiting on should be more than enough.

An extra benefit is that this test now runs much faster, at 10 to 60 ms
per test row, instead of 800 ms previously. The drawback is that a
failure condition is likely going to be noticed by the QSemaphores
deadlocking.

Change-Id: I360d7e9c7accc1216291fffd743c88a362cf66ac
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>

tests/auto/corelib/thread/qthread/tst_qthread.cpp

@@ -36,6 +36,8 @@
 #include <exception>
 #endif
 
+#include <thread>
+
 #include <QtTest/private/qemulationdetector_p.h>
 
 using namespace std::chrono_literals;
@@ -1226,21 +1228,28 @@ void tst_QThread::multiThreadWait_data()
     // this is probably too fast and the Forever gets in too quickly
     addRow(0, -1);
 
-    addRow(100, -1);
-    addRow(100, 200, -1);
-    addRow(200, 100, -1);
-    addRow(-1, 100, 100, 100);
+    // any time below 100ms (see below) is expected to timeout
+    addRow(25, -1);
+    addRow(25, 50, -1);
+    addRow(50, 25, -1);
+    addRow(-1, 25, 25, 25);
+    addRow(25, 2000);
+    addRow(25, 2000, 25, -1);
 }
 
 void tst_QThread::multiThreadWait()
 {
+    static constexpr auto TimeoutThreshold = 100ms;
+    auto isExpectedToTimeout = [](unsigned value) {
+        return value < TimeoutThreshold.count();
+    };
+
     class TargetThread : public QThread {
     public:
         QSemaphore sync;
         void run() override
         {
             sync.acquire();
-            msleep(Waiting_Thread::WaitTime);
         }
     };
 
@@ -1265,43 +1274,57 @@ void tst_QThread::multiThreadWait()
     QFETCH(QList<int>, deadlines);
     TargetThread target;
     target.start();
+    QElapsedTimer elapsedTimer;
+    elapsedTimer.start();
 
-    QSemaphore startSema, endSema;
+    // we use a QSemaphore to wait on the WaiterThread::run() instead of
+    // QThread::wait() so it's easier to debug when the latter has a problem
+    QSemaphore startSema, timeoutSema, successSema;
     std::array<std::unique_ptr<WaiterThread>, 5> threads;   // 5 threads is enough
+    int expectedTimeoutCount = 0;
     for (int i = 0; i < deadlines.size(); ++i) {
         threads[i] = std::make_unique<WaiterThread>();
         threads[i]->startSema = &startSema;
-        threads[i]->endSema = &endSema;
+        if (isExpectedToTimeout(deadlines.at(i))) {
+            ++expectedTimeoutCount;
+            threads[i]->endSema = &timeoutSema;
+        } else {
+            threads[i]->endSema = &successSema;
+        }
         threads[i]->target = &target;
         threads[i]->deadline = QDeadlineTimer(deadlines.at(i));
         threads[i]->start();
     }
 
-    // release the waiting threads first, then the target thread they're waiting on
+    // release the waiting threads first, so they begin waiting
     startSema.release(deadlines.size());
-    target.sync.release();
 
-    // wait for our waiting threads on a semaphore instead of QThread::wait()
-    // to make debugging easier
-    QVERIFY(endSema.tryAcquire(deadlines.size(), QDeadlineTimer::Forever));
+    // then wait for the threads that are expected to timeout to do so
+    QVERIFY(timeoutSema.tryAcquire(expectedTimeoutCount, QDeadlineTimer::Forever));
+
+    // compute the elapsed time for timing comparisons
+    std::this_thread::sleep_for(5ms);   // short, but long enough to avoid rounding errors
+    auto elapsed = elapsedTimer.durationElapsed();
+    std::this_thread::sleep_for(5ms);
+
+    // cause the target thread to exit, so the successful threads do succeed
+    target.sync.release();
+    int expectedSuccessCount = deadlines.size() - expectedTimeoutCount;
+    QVERIFY(successSema.tryAcquire(expectedSuccessCount, QDeadlineTimer::Forever));
 
     // wait for all the threads to end, before QVERIFY/QCOMPAREs
     for (int i = 0; i < deadlines.size(); ++i)
         threads[i]->wait();
     target.wait();
 
-    std::chrono::milliseconds expectedDuration{Waiting_Thread::WaitTime};
     for (int i = 0; i < deadlines.size(); ++i) {
         auto printI = qScopeGuard([i] { qWarning("i = %i", i); });
-        if (unsigned(deadlines.at(i)) < Waiting_Thread::WaitTime / 2) {
+        if (isExpectedToTimeout(deadlines.at(i))) {
+            QCOMPARE_LT(threads[i]->waitedDuration, elapsed);
             QCOMPARE(threads[i]->result, false);
-            QCOMPARE_LT(threads[i]->waitedDuration, expectedDuration);
-        } else if (unsigned(deadlines.at(i)) > Waiting_Thread::WaitTime * 3 / 2) {
-            QCOMPARE(threads[i]->result, true);
-            QCOMPARE_GE(threads[i]->waitedDuration, expectedDuration);
         } else {
-            qWarning("Wait time %i (index %i) is too close to the target time; test would be flaky",
-                     deadlines.at(i), i);
+            QCOMPARE_GE(threads[i]->waitedDuration, elapsed);
+            QCOMPARE(threads[i]->result, true);
         }
         printI.dismiss();
         threads[i].reset();