From d1462ec5a3e083e8e47dfa5075deb1575509e8cb Mon Sep 17 00:00:00 2001
From: David Faure <david.faure@kdab.com>
Date: Wed, 5 Mar 2025 17:45:50 +0100
Subject: [PATCH] tst_qthread: use QSemaphore to fix a race and a harmless TSAN
 warning

The current code with two QWaitConditions, a mutex and a bool lead
to a TSAN warning about lock order inversion: the first for loop locks
mutexes M0, M1, M2 etc. and then the second for loop calls QWaitCondition::wait(M0)
which unlocks+relocks M0, while M1 is locked.

It was also racy, in my opinion: with QWaitCondition there's the risk of
calling wakeOne before the other thread calls wait, and then the
notification is missed.

Ported to QSemaphore which doesn't have either problem.

Change-Id: I41574e944a2631cf42a81d3923abb658c862c25d
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
---
 .../corelib/thread/qthread/tst_qthread.cpp    | 33 ++++++-------------
 1 file changed, 10 insertions(+), 23 deletions(-)

diff --git a/tests/auto/corelib/thread/qthread/tst_qthread.cpp b/tests/auto/corelib/thread/qthread/tst_qthread.cpp
index e055d49e2fb..c5837133d34 100644
--- a/tests/auto/corelib/thread/qthread/tst_qthread.cpp
+++ b/tests/auto/corelib/thread/qthread/tst_qthread.cpp
@@ -716,19 +716,18 @@ void noop(void*) { }
 class NativeThreadWrapper
 {
 public:
-    NativeThreadWrapper() : qthread(nullptr), waitForStop(false) {}
+    NativeThreadWrapper() : qthread(nullptr), stopSemaphore(1) {}
     void start(FunctionPointer functionPointer = noop, void *data = nullptr);
     void startAndWait(FunctionPointer functionPointer = noop, void *data = nullptr);
     void join();
-    void setWaitForStop() { waitForStop = true; }
+    void setWaitForStop() { stopSemaphore.acquire(); }
     void stop();
 
     ThreadHandle nativeThreadHandle;
     QThread *qthread;
-    QWaitCondition startCondition;
-    QMutex mutex;
-    bool waitForStop;
-    QWaitCondition stopCondition;
+    QSemaphore startSemaphore;
+    QSemaphore stopSemaphore;
+
 protected:
     static void *runUnix(void *data);
     static unsigned WIN_FIX_STDCALL runWin(void *data);
@@ -752,9 +751,8 @@ void NativeThreadWrapper::start(FunctionPointer functionPointer, void *data)
 
 void NativeThreadWrapper::startAndWait(FunctionPointer functionPointer, void *data)
 {
-    QMutexLocker locker(&mutex);
     start(functionPointer, data);
-    startCondition.wait(locker.mutex());
+    startSemaphore.acquire();
 }
 
 void NativeThreadWrapper::join()
@@ -775,20 +773,13 @@ void *NativeThreadWrapper::runUnix(void *that)
     nativeThreadWrapper->qthread = QThread::currentThread();
 
     // Release main thread.
-    {
-        QMutexLocker lock(&nativeThreadWrapper->mutex);
-        nativeThreadWrapper->startCondition.wakeOne();
-    }
+    nativeThreadWrapper->startSemaphore.release();
 
     // Run function.
     nativeThreadWrapper->functionPointer(nativeThreadWrapper->data);
 
     // Wait for stop.
-    {
-        QMutexLocker lock(&nativeThreadWrapper->mutex);
-        if (nativeThreadWrapper->waitForStop)
-            nativeThreadWrapper->stopCondition.wait(lock.mutex());
-    }
+    nativeThreadWrapper->stopSemaphore.acquire();
 
     return nullptr;
 }
@@ -801,9 +792,7 @@ unsigned WIN_FIX_STDCALL NativeThreadWrapper::runWin(void *data)
 
 void NativeThreadWrapper::stop()
 {
-    QMutexLocker lock(&mutex);
-    waitForStop = false;
-    stopCondition.wakeOne();
+    stopSemaphore.release();
 }
 
 static bool threadAdoptedOk = false;
@@ -993,13 +982,11 @@ void tst_QThread::adoptMultipleThreadsOverlap()
     for (int i = 0; i < numThreads; ++i) {
         nativeThreads.append(new NativeThreadWrapper());
         nativeThreads.at(i)->setWaitForStop();
-        nativeThreads.at(i)->mutex.lock();
         nativeThreads.at(i)->start();
     }
     for (int i = 0; i < numThreads; ++i) {
-        nativeThreads.at(i)->startCondition.wait(&nativeThreads.at(i)->mutex);
+        nativeThreads.at(i)->startSemaphore.acquire();
         QObject::connect(nativeThreads.at(i)->qthread, SIGNAL(finished()), &recorder, SLOT(slot()));
-        nativeThreads.at(i)->mutex.unlock();
     }
 
     QObject::connect(nativeThreads.at(numThreads - 1)->qthread, SIGNAL(finished()), &QTestEventLoop::instance(), SLOT(exitLoop()));