From 1afd562b0b0bbba02575aa79601f0fae555cfa19 Mon Sep 17 00:00:00 2001
From: Ievgenii Meshcheriakov <ievgenii.meshcheriakov@qt.io>
Date: Tue, 14 Sep 2021 16:02:26 +0200
Subject: [PATCH] QThreadPool: Fix restarting of expired threads

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
Pick-to: 6.2
Change-Id: I2109b628b8a4e91491115dc56aebf3eb249646b5
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>
Reviewed-by: Edward Welbourne <edward.welbourne@qt.io>
---
 src/corelib/thread/qthreadpool.cpp            |  5 ++++
 .../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 9185554b1fb..4d7d7a22500 100644
--- a/src/corelib/thread/qthreadpool.cpp
+++ b/src/corelib/thread/qthreadpool.cpp
@@ -194,6 +194,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(threadPriority);
         return true;
     }
diff --git a/tests/auto/corelib/thread/qthreadpool/tst_qthreadpool.cpp b/tests/auto/corelib/thread/qthreadpool/tst_qthreadpool.cpp
index bb8513fe1dc..7f36db03e1c 100644
--- a/tests/auto/corelib/thread/qthreadpool/tst_qthreadpool.cpp
+++ b/tests/auto/corelib/thread/qthreadpool/tst_qthreadpool.cpp
@@ -105,6 +105,7 @@ private slots:
     void stressTest();
     void takeAllAndIncreaseMaxThreadCount();
     void waitForDoneAfterTake();
+    void threadReuse();
 
 private:
     QMutex m_functionTestMutex;
@@ -1325,5 +1326,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"