1berry/qtbase
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

tst_qthread: use QSemaphore to fix a race and a harmless TSAN warning

Browse Source 
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>
This commit is contained in:
David Faure 2025-03-05 17:45:50 +01:00
parent 10affadd12
commit d1462ec5a3
1 changed files with 10 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

33
tests/auto/corelib/thread/qthread/tst_qthread.cpp
View File

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