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 17:02:33 +0000 |
commit | c0ff1786dba467f6709f4a0c27db6c920213d6ae (patch) | |
tree | 6c7ecba1f86d16b2bdd33618fd4e335a1720c022 | |
parent | 8969557673b6a636bdf6e5f963000611d5fa6edd (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>
(cherry picked from commit dcf74bdec83ba450b2016081339f48ac7469e1f5)
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
-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 c465a07487..f4f482027d 100644 --- a/tests/auto/corelib/thread/qthreadpool/tst_qthreadpool.cpp +++ b/tests/auto/corelib/thread/qthreadpool/tst_qthreadpool.cpp @@ -966,14 +966,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(); } @@ -985,8 +992,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 @@ -998,12 +1005,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; |