From 14b0d9a50f1679daabb0f5a64368006f7a64a189 Mon Sep 17 00:00:00 2001
From: Ievgenii Meshcheriakov <ievgenii.meshcheriakov@qt.io>
Date: Tue, 14 Sep 2021 16:02:26 +0200
Subject: QThreadPool: Fix restarting of expired threads
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Ensure that expired threads have actually finished before attempting
to restart them. Calling start() on a thread that is not yet finished
does nothing.

Add a regression test into tst_qthreadpool that attempts to trigger
reuse of expired threads and verifies that all submitted tasks
execute.

Fixes: QTBUG-72872
Change-Id: I2109b628b8a4e91491115dc56aebf3eb249646b5
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>
Reviewed-by: Edward Welbourne <edward.welbourne@qt.io>
(cherry picked from commit 1afd562b0b0bbba02575aa79601f0fae555cfa19)
Reviewed-by: Mårten Nordheim <marten.nordheim@qt.io>
---
 src/corelib/thread/qthreadpool.cpp                 |  5 +++++
 .../corelib/thread/qthreadpool/tst_qthreadpool.cpp | 25 ++++++++++++++++++++++
 2 files changed, 30 insertions(+)

diff --git a/src/corelib/thread/qthreadpool.cpp b/src/corelib/thread/qthreadpool.cpp
index 14d7acffb2..282119a5c5 100644
--- a/src/corelib/thread/qthreadpool.cpp
+++ b/src/corelib/thread/qthreadpool.cpp
@@ -195,6 +195,11 @@ bool QThreadPoolPrivate::tryStart(QRunnable *task)
         ++activeThreads;
 
         thread->runnable = task;
+
+        // Ensure that the thread has actually finished, otherwise the following
+        // start() has no effect.
+        thread->wait();
+        Q_ASSERT(thread->isFinished());
         thread->start();
         return true;
     }
diff --git a/tests/auto/corelib/thread/qthreadpool/tst_qthreadpool.cpp b/tests/auto/corelib/thread/qthreadpool/tst_qthreadpool.cpp
index bf313c27ef..e85fb5ea88 100644
--- a/tests/auto/corelib/thread/qthreadpool/tst_qthreadpool.cpp
+++ b/tests/auto/corelib/thread/qthreadpool/tst_qthreadpool.cpp
@@ -104,6 +104,7 @@ private slots:
     void stressTest();
     void takeAllAndIncreaseMaxThreadCount();
     void waitForDoneAfterTake();
+    void threadReuse();
 
 private:
     QMutex m_functionTestMutex;
@@ -1385,5 +1386,29 @@ void tst_QThreadPool::waitForDoneAfterTake()
 
 }
 
+/*
+    Try trigger reuse of expired threads and check that all tasks execute.
+
+    This is a regression test for QTBUG-72872.
+*/
+void tst_QThreadPool::threadReuse()
+{
+    QThreadPool manager;
+    manager.setExpiryTimeout(-1);
+    manager.setMaxThreadCount(1);
+
+    constexpr int repeatCount = 10000;
+    constexpr int timeoutMs = 1000;
+    QSemaphore sem;
+
+    for (int i = 0; i < repeatCount; i++) {
+        manager.start([&sem]() { sem.release(); });
+        manager.start([&sem]() { sem.release(); });
+        manager.releaseThread();
+        QVERIFY(sem.tryAcquire(2, timeoutMs));
+        manager.reserveThread();
+    }
+}
+
 QTEST_MAIN(tst_QThreadPool);
 #include "tst_qthreadpool.moc"
-- 
cgit v1.2.3