AuroraMiddleware/qt5base-lts
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix UB (data race) in tst_QThreadPool::cancel()

Browse Source 
Manipulating a simple int from multiple threads is a data race,
thus undefined behavior.

Fix by using QAtomicInt and atomic operations instead.

Change-Id: I5418bc260da57fe353a71b8e5c7c1c97adbe7597
Reviewed-by: David Faure <david.faure@kdab.com>
This commit is contained in:
Marc Mutz 2017-02-10 10:00:53 +01:00
parent af8771867c
commit dcf74bdec8
1 changed files with 17 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

27
tests/auto/corelib/thread/qthreadpool/tst_qthreadpool.cpp
View File

@ -962,14 +962,21 @@ void tst_QThreadPool::cancel()
{ {
public: public:
QSemaphore & sem; QSemaphore & sem;
int & dtorCounter; QAtomicInt &dtorCounter;
int & runCounter; QAtomicInt &runCounter;
int dummy; int dummy;
BlockingRunnable(QSemaphore & s, int & c, int & r) : sem(s), dtorCounter(c), runCounter(r){}
~BlockingRunnable(){dtorCounter++;} explicit BlockingRunnable(QSemaphore &s, QAtomicInt &c, QAtomicInt &r)
: sem(s), dtorCounter(c), runCounter(r){}
~BlockingRunnable()
{
dtorCounter.fetchAndAddRelaxed(1);
}
void run() void run()
{ {
runCounter++; runCounter.fetchAndAddRelaxed(1);
sem.acquire(); sem.acquire();
count.ref(); count.ref();
} }
@ -981,8 +988,8 @@ void tst_QThreadPool::cancel()
int runs = 2 * threadPool.maxThreadCount(); int runs = 2 * threadPool.maxThreadCount();
BlockingRunnablePtr* runnables = new BlockingRunnablePtr[runs]; BlockingRunnablePtr* runnables = new BlockingRunnablePtr[runs];
count.store(0); count.store(0);
int dtorCounter = 0; QAtomicInt dtorCounter = 0;
int runCounter = 0; QAtomicInt runCounter = 0;
for (int i = 0; i < runs; i++) { for (int i = 0; i < runs; i++) {
runnables[i] = new BlockingRunnable(sem, dtorCounter, runCounter); runnables[i] = new BlockingRunnable(sem, dtorCounter, runCounter);
runnables[i]->setAutoDelete(i != 0 && i != (runs-1)); //one which will run and one which will not runnables[i]->setAutoDelete(i != 0 && i != (runs-1)); //one which will run and one which will not
@ -994,12 +1001,12 @@ void tst_QThreadPool::cancel()
} }
runnables[0]->dummy = 0; //valgrind will catch this if cancel() is crazy enough to delete currently running jobs runnables[0]->dummy = 0; //valgrind will catch this if cancel() is crazy enough to delete currently running jobs
runnables[runs-1]->dummy = 0; runnables[runs-1]->dummy = 0;
QCOMPARE(dtorCounter, runs-threadPool.maxThreadCount()-1); QCOMPARE(dtorCounter.load(), runs - threadPool.maxThreadCount() - 1);
sem.release(threadPool.maxThreadCount()); sem.release(threadPool.maxThreadCount());
threadPool.waitForDone(); threadPool.waitForDone();
QCOMPARE(runCounter, threadPool.maxThreadCount()); QCOMPARE(runCounter.load(), threadPool.maxThreadCount());
QCOMPARE(count.load(), threadPool.maxThreadCount()); QCOMPARE(count.load(), threadPool.maxThreadCount());
QCOMPARE(dtorCounter, runs-2); QCOMPARE(dtorCounter.load(), runs - 2);
delete runnables[0]; //if the pool deletes them then we'll get double-free crash delete runnables[0]; //if the pool deletes them then we'll get double-free crash
delete runnables[runs-1]; delete runnables[runs-1];
delete[] runnables; delete[] runnables;