From ce08318a46164172eaa72f4436cddf7f69ce9e1c Mon Sep 17 00:00:00 2001
From: Svenn-Arne Dragly <svenn-arne.dragly@qt.io>
Date: Wed, 20 Sep 2017 09:28:27 +0200
Subject: [PATCH] Add QThreadPool autotest to detect stale threads after
 tryTake

This test makes sure that we do not introduce a regression where the
threads exited the inner loop over the queue before the queue was
empty. This was triggered by calling tryTake at least maxThreadCount
times, which left the same number of null pointers in the queue
and caused the inner loop to exit too soon for all the threads.

Change-Id: I3a9d800149b88d09510ddc424667670b60f06a33
Reviewed-by: Lars Knoll <lars.knoll@qt.io>
---
 .../thread/qthreadpool/tst_qthreadpool.cpp    | 68 +++++++++++++++++++
 1 file changed, 68 insertions(+)

diff --git a/tests/auto/corelib/thread/qthreadpool/tst_qthreadpool.cpp b/tests/auto/corelib/thread/qthreadpool/tst_qthreadpool.cpp
index 66853a88d8..0eaf8a4b77 100644
--- a/tests/auto/corelib/thread/qthreadpool/tst_qthreadpool.cpp
+++ b/tests/auto/corelib/thread/qthreadpool/tst_qthreadpool.cpp
@@ -94,6 +94,7 @@ private slots:
     void destroyingWaitsForTasksToFinish();
     void stressTest();
     void takeAllAndIncreaseMaxThreadCount();
+    void waitForDoneAfterTake();
 
 private:
     QMutex m_functionTestMutex;
@@ -1263,5 +1264,72 @@ void tst_QThreadPool::takeAllAndIncreaseMaxThreadCount() {
     delete task3;
 }
 
+void tst_QThreadPool::waitForDoneAfterTake()
+{
+    class Task : public QRunnable
+    {
+    public:
+        Task(QSemaphore *mainBarrier, QSemaphore *threadBarrier)
+            : m_mainBarrier(mainBarrier)
+            , m_threadBarrier(threadBarrier)
+        {}
+
+        void run()
+        {
+            m_mainBarrier->release();
+            m_threadBarrier->acquire();
+        }
+
+    private:
+        QSemaphore *m_mainBarrier = nullptr;
+        QSemaphore *m_threadBarrier = nullptr;
+    };
+
+    int threadCount = 4;
+
+    // Blocks the main thread from releasing the threadBarrier before all run() functions have started
+    QSemaphore mainBarrier;
+    // Blocks the tasks from completing their run function
+    QSemaphore threadBarrier;
+
+    QThreadPool manager;
+    manager.setMaxThreadCount(threadCount);
+
+    // Fill all the threads with runnables that wait for the threadBarrier
+    for (int i = 0; i < threadCount; i++) {
+        auto *task = new Task(&mainBarrier, &threadBarrier);
+        manager.start(task);
+    }
+
+    QVERIFY(manager.activeThreadCount() == manager.maxThreadCount());
+
+    // Add runnables that are immediately removed from the pool queue.
+    // This sets the queue elements to nullptr in QThreadPool and we want to test that
+    // the threads keep going through the queue after encountering a nullptr.
+    for (int i = 0; i < threadCount; i++) {
+        QRunnable *runnable = createTask(emptyFunct);
+        manager.start(runnable);
+        QVERIFY(manager.tryTake(runnable));
+    }
+
+    // Add another runnable that will not be removed
+    manager.start(createTask(emptyFunct));
+
+    // Wait for the first runnables to start
+    mainBarrier.acquire(threadCount);
+
+    QVERIFY(mainBarrier.available() == 0);
+    QVERIFY(threadBarrier.available() == 0);
+
+    // Release runnables that are waiting and expect all runnables to complete
+    threadBarrier.release(threadCount);
+
+    // Using qFatal instead of QVERIFY to force exit if threads are still running after timeout.
+    // Otherwise, QCoreApplication will still wait for the stale threads and never exit the test.
+    if (!manager.waitForDone(5 * 60 * 1000))
+        qFatal("waitForDone returned false. Aborting to stop background threads.");
+
+}
+
 QTEST_MAIN(tst_QThreadPool);
 #include "tst_qthreadpool.moc"