From 605647f2b8a6c8d4f700732a7fcdb1dec3459231 Mon Sep 17 00:00:00 2001 From: David Faure Date: Fri, 21 Jun 2013 10:32:17 +0200 Subject: QThreadPool: fix counting of waiting threads QTBUG-21051 has a testcase where activeThreadCount() could actually end up at -1 (converted to an autotest in this commit). The reason was: start() calls tryStart() which returns false due to too many active threads (reserveThread() causes this), so it calls enqueueTask() - which actually wakes up the waiting thread, but it didn't decrement the number of waiting threads. Note that tryStart() is "if I can grab a waiting thread, enqueue task and wake it" while start(), in case tryStart() fails, wants to "enqueue, and then if I can grab a waiting thread, wake it". This is why enqueue shouldn't wake; waking must happen only if we can grab a thread (d->waitingThreads > 0). Task-number: QTBUG-21051 Backport from qtbase/dacf9961da86751a59da0e84bc943fe0d1c8d95b Change-Id: I1e437da27b733a72b48ff1b6f2b78f81a7ed129b Reviewed-by: Thiago Macieira --- tests/auto/qthreadpool/tst_qthreadpool.cpp | 74 ++++++++++++++++++++++++++++++ 1 file changed, 74 insertions(+) (limited to 'tests') diff --git a/tests/auto/qthreadpool/tst_qthreadpool.cpp b/tests/auto/qthreadpool/tst_qthreadpool.cpp index 5d134ab0e1..61343c1cd3 100644 --- a/tests/auto/qthreadpool/tst_qthreadpool.cpp +++ b/tests/auto/qthreadpool/tst_qthreadpool.cpp @@ -44,6 +44,24 @@ #include #include +// From Qt5 + +#define QTRY_COMPARE_WITH_TIMEOUT(__expr, __expected, __timeout) \ + do { \ + const int __step = 50; \ + const int __timeoutValue = __timeout; \ + if ((__expr) != (__expected)) { \ + QTest::qWait(0); \ + } \ + for (int __i = 0; __i < __timeoutValue && ((__expr) != (__expected)); __i+=__step) { \ + QTest::qWait(__step); \ + } \ + QCOMPARE(__expr, __expected); \ + } while (0) + +#define QTRY_COMPARE(__expr, __expected) QTRY_COMPARE_WITH_TIMEOUT(__expr, __expected, 5000) + + typedef void (*FunctionPointer)(); class FunctionPointerTask : public QRunnable @@ -90,6 +108,7 @@ private slots: void reserveThread(); void releaseThread_data(); void releaseThread(); + void reserveAndStart(); void start(); void tryStart(); void tryStartPeakThreadCount(); @@ -647,6 +666,61 @@ void tst_QThreadPool::releaseThread() threadpool->setMaxThreadCount(savedLimit); } +void tst_QThreadPool::reserveAndStart() // QTBUG-21051 +{ + class WaitingTask : public QRunnable + { + public: + QAtomicInt count; + QSemaphore waitForStarted; + + WaitingTask() { setAutoDelete(false); } + + void run() + { + count.ref(); + waitForStarted.release(); + } + }; + + // Set up + QThreadPool *threadpool = QThreadPool::globalInstance(); + int savedLimit = threadpool->maxThreadCount(); + threadpool->setMaxThreadCount(1); + QCOMPARE(threadpool->activeThreadCount(), 0); + + // reserve + threadpool->reserveThread(); + QCOMPARE(threadpool->activeThreadCount(), 1); + + // start a task, to get a running thread + WaitingTask *task = new WaitingTask; + threadpool->start(task); + QCOMPARE(threadpool->activeThreadCount(), 2); + task->waitForStarted.acquire(); + QTRY_COMPARE(int(task->count), 1); + QTRY_COMPARE(threadpool->activeThreadCount(), 1); + + // now the thread is waiting, but tryStart() will fail since activeThreadCount() >= maxThreadCount() + QVERIFY(!threadpool->tryStart(task)); + QTRY_COMPARE(threadpool->activeThreadCount(), 1); + + // start() will therefore do a failing tryStart(), followed by enqueueTask() + // which will actually wake up the waiting thread. + threadpool->start(task); + QTRY_COMPARE(threadpool->activeThreadCount(), 2); + task->waitForStarted.acquire(); + QTRY_COMPARE(int(task->count), 2); + QTRY_COMPARE(threadpool->activeThreadCount(), 1); + + threadpool->releaseThread(); + QTRY_COMPARE(threadpool->activeThreadCount(), 0); + + delete task; + + threadpool->setMaxThreadCount(savedLimit); +} + QAtomicInt count; class CountingRunnable : public QRunnable { -- cgit v1.2.3