diff options
author | Marc Mutz <marc.mutz@kdab.com> | 2017-02-10 10:00:53 +0100 |
---|---|---|
committer | Marc Mutz <marc.mutz@kdab.com> | 2017-02-15 11:25:24 +0000 |
commit | dcf74bdec83ba450b2016081339f48ac7469e1f5 (patch) | |
tree | 3b2686b85588b2c52fcabe53b03566c5ffe0e8c0 /tests/auto | |
parent | af8771867c3bca9b1bc34d6d82b6603749938d22 (diff) |
Fix UB (data race) in tst_QThreadPool::cancel()
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>
Diffstat (limited to 'tests/auto')
-rw-r--r-- | tests/auto/corelib/thread/qthreadpool/tst_qthreadpool.cpp | 27 |
1 files changed, 17 insertions, 10 deletions
diff --git a/tests/auto/corelib/thread/qthreadpool/tst_qthreadpool.cpp b/tests/auto/corelib/thread/qthreadpool/tst_qthreadpool.cpp index 3bc3f8a759..49e1177e79 100644 --- a/tests/auto/corelib/thread/qthreadpool/tst_qthreadpool.cpp +++ b/tests/auto/corelib/thread/qthreadpool/tst_qthreadpool.cpp @@ -962,14 +962,21 @@ void tst_QThreadPool::cancel() { public: QSemaphore & sem; - int & dtorCounter; - int & runCounter; + QAtomicInt &dtorCounter; + QAtomicInt &runCounter; 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() { - runCounter++; + runCounter.fetchAndAddRelaxed(1); sem.acquire(); count.ref(); } @@ -981,8 +988,8 @@ void tst_QThreadPool::cancel() int runs = 2 * threadPool.maxThreadCount(); BlockingRunnablePtr* runnables = new BlockingRunnablePtr[runs]; count.store(0); - int dtorCounter = 0; - int runCounter = 0; + QAtomicInt dtorCounter = 0; + QAtomicInt runCounter = 0; for (int i = 0; i < runs; i++) { runnables[i] = new BlockingRunnable(sem, dtorCounter, runCounter); 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[runs-1]->dummy = 0; - QCOMPARE(dtorCounter, runs-threadPool.maxThreadCount()-1); + QCOMPARE(dtorCounter.load(), runs - threadPool.maxThreadCount() - 1); sem.release(threadPool.maxThreadCount()); threadPool.waitForDone(); - QCOMPARE(runCounter, threadPool.maxThreadCount()); + QCOMPARE(runCounter.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[runs-1]; delete[] runnables; |