From fe36d47b371b71ad5fec30d4b5d7bf0baa0205ea Mon Sep 17 00:00:00 2001
From: Allan Sandfeld Jensen <allan.jensen@qt.io>
Date: Thu, 1 Oct 2020 09:56:16 +0200
Subject: [PATCH] Fix race condition in QThreadPool::clear

Since we drop the lock while deleting threads, we need to handle
the queue possibly being accessed and changed by the pool threads
while clear() is running.

Pick-to: 5.15
Fixes: QTBUG-87092
Change-Id: I7611edab90520454278502a58621e299f9cd1f6e
Reviewed-by: Sona Kurazyan <sona.kurazyan@qt.io>
---
 src/corelib/thread/qthreadpool.cpp            |  6 ++---
 .../thread/qthreadpool/tst_qthreadpool.cpp    | 26 +++++++++++++++++++
 2 files changed, 29 insertions(+), 3 deletions(-)

diff --git a/src/corelib/thread/qthreadpool.cpp b/src/corelib/thread/qthreadpool.cpp
index 1b417aa8be6..5765bf6b092 100644
--- a/src/corelib/thread/qthreadpool.cpp
+++ b/src/corelib/thread/qthreadpool.cpp
@@ -325,7 +325,8 @@ bool QThreadPoolPrivate::waitForDone(int msecs)
 void QThreadPoolPrivate::clear()
 {
     QMutexLocker locker(&mutex);
-    for (QueuePage *page : qAsConst(queue)) {
+    while (!queue.isEmpty()) {
+        auto *page = queue.takeLast();
         while (!page->isFinished()) {
             QRunnable *r = page->pop();
             if (r && r->autoDelete()) {
@@ -335,9 +336,8 @@ void QThreadPoolPrivate::clear()
                 locker.relock();
             }
         }
+        delete page;
     }
-    qDeleteAll(queue);
-    queue.clear();
 }
 
 /*!
diff --git a/tests/auto/corelib/thread/qthreadpool/tst_qthreadpool.cpp b/tests/auto/corelib/thread/qthreadpool/tst_qthreadpool.cpp
index 9b536aec00d..3054e3c5a44 100644
--- a/tests/auto/corelib/thread/qthreadpool/tst_qthreadpool.cpp
+++ b/tests/auto/corelib/thread/qthreadpool/tst_qthreadpool.cpp
@@ -93,6 +93,7 @@ private slots:
     void priorityStart();
     void waitForDone();
     void clear();
+    void clearWithAutoDelete();
     void tryTake();
     void waitForDoneTimeout();
     void destroyingWaitsForTasksToFinish();
@@ -974,6 +975,31 @@ void tst_QThreadPool::clear()
     QCOMPARE(count.loadRelaxed(), threadPool.maxThreadCount());
 }
 
+void tst_QThreadPool::clearWithAutoDelete()
+{
+    class MyRunnable : public QRunnable
+    {
+    public:
+        MyRunnable() {}
+        void run() override { QThread::usleep(30); }
+    };
+
+    QThreadPool threadPool;
+    threadPool.setMaxThreadCount(4);
+    const int loopCount = 20;
+    const int batchSize = 500;
+    // Should not crash see QTBUG-87092
+    for (int i = 0; i < loopCount; i++) {
+        threadPool.clear();
+        for (int j = 0; j < batchSize; j++) {
+            auto *runnable = new MyRunnable();
+            runnable->setAutoDelete(true);
+            threadPool.start(runnable);
+        }
+    }
+    QVERIFY(threadPool.waitForDone());
+}
+
 void tst_QThreadPool::tryTake()
 {
     QSemaphore sem(0);