From dcf74bdec83ba450b2016081339f48ac7469e1f5 Mon Sep 17 00:00:00 2001
From: Marc Mutz <marc.mutz@kdab.com>
Date: Fri, 10 Feb 2017 10:00:53 +0100
Subject: 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>
---
 .../corelib/thread/qthreadpool/tst_qthreadpool.cpp | 27 ++++++++++++++--------
 1 file 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;
-- 
cgit v1.2.3