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

@@ -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;
     }

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"