diff options
-rw-r--r-- | src/corelib/concurrent/qthreadpool.cpp | 28 | ||||
-rw-r--r-- | src/corelib/concurrent/qthreadpool_p.h | 3 | ||||
-rw-r--r-- | tests/auto/qthreadpool/tst_qthreadpool.cpp | 25 |
3 files changed, 34 insertions, 22 deletions
diff --git a/src/corelib/concurrent/qthreadpool.cpp b/src/corelib/concurrent/qthreadpool.cpp index 26a5337334..fb90f4e72a 100644 --- a/src/corelib/concurrent/qthreadpool.cpp +++ b/src/corelib/concurrent/qthreadpool.cpp @@ -68,6 +68,7 @@ public: void run(); void registerThreadInactive(); + QWaitCondition runnableReady; QThreadPoolPrivate *manager; QRunnable *runnable; }; @@ -135,14 +136,13 @@ void QThreadPoolThread::run() // if too many threads are active, expire this thread bool expired = manager->tooManyThreadsActive(); if (!expired) { - ++manager->waitingThreads; + manager->waitingThreads.enqueue(this); registerThreadInactive(); // wait for work, exiting after the expiry timeout is reached - expired = !manager->runnableReady.wait(locker.mutex(), manager->expiryTimeout); + runnableReady.wait(locker.mutex(), manager->expiryTimeout); ++manager->activeThreads; - - if (expired) - --manager->waitingThreads; + if (manager->waitingThreads.removeOne(this)) + expired = true; } if (expired) { manager->expiredThreads.enqueue(this); @@ -167,7 +167,6 @@ QThreadPoolPrivate:: QThreadPoolPrivate() expiryTimeout(30000), maxThreadCount(qAbs(QThread::idealThreadCount())), reservedThreads(0), - waitingThreads(0), activeThreads(0) { } @@ -183,11 +182,10 @@ bool QThreadPoolPrivate::tryStart(QRunnable *task) if (activeThreadCount() >= maxThreadCount) return false; - if (waitingThreads > 0) { + if (waitingThreads.count() > 0) { // recycle an available thread - --waitingThreads; enqueueTask(task); - runnableReady.wakeOne(); + waitingThreads.takeFirst()->runnableReady.wakeOne(); return true; } @@ -225,7 +223,7 @@ int QThreadPoolPrivate::activeThreadCount() const { return (allThreads.count() - expiredThreads.count() - - waitingThreads + - waitingThreads.count() + reservedThreads); } @@ -266,7 +264,6 @@ void QThreadPoolPrivate::reset() { QMutexLocker locker(&mutex); isExiting = true; - runnableReady.wakeAll(); do { // make a copy of the set so that we can iterate without the lock @@ -275,6 +272,7 @@ void QThreadPoolPrivate::reset() locker.unlock(); foreach (QThreadPoolThread *thread, allThreadsCopy) { + thread->runnableReady.wakeAll(); thread->wait(); delete thread; } @@ -283,7 +281,7 @@ void QThreadPoolPrivate::reset() // repeat until all newly arrived threads have also completed } while (!allThreads.isEmpty()); - waitingThreads = 0; + waitingThreads.clear(); expiredThreads.clear(); isExiting = false; @@ -474,10 +472,8 @@ void QThreadPool::start(QRunnable *runnable, int priority) if (!d->tryStart(runnable)) { d->enqueueTask(runnable, priority); - if (d->waitingThreads > 0) { - --d->waitingThreads; - d->runnableReady.wakeOne(); - } + if (!d->waitingThreads.isEmpty()) + d->waitingThreads.takeFirst()->runnableReady.wakeOne(); } } diff --git a/src/corelib/concurrent/qthreadpool_p.h b/src/corelib/concurrent/qthreadpool_p.h index 67d36068ed..3846333521 100644 --- a/src/corelib/concurrent/qthreadpool_p.h +++ b/src/corelib/concurrent/qthreadpool_p.h @@ -87,8 +87,8 @@ public: void stealRunnable(QRunnable *); mutable QMutex mutex; - QWaitCondition runnableReady; QSet<QThreadPoolThread *> allThreads; + QQueue<QThreadPoolThread *> waitingThreads; QQueue<QThreadPoolThread *> expiredThreads; QList<QPair<QRunnable *, int> > queue; QWaitCondition noActiveThreads; @@ -97,7 +97,6 @@ public: int expiryTimeout; int maxThreadCount; int reservedThreads; - int waitingThreads; int activeThreads; }; diff --git a/tests/auto/qthreadpool/tst_qthreadpool.cpp b/tests/auto/qthreadpool/tst_qthreadpool.cpp index 61343c1cd3..c40bd4038c 100644 --- a/tests/auto/qthreadpool/tst_qthreadpool.cpp +++ b/tests/auto/qthreadpool/tst_qthreadpool.cpp @@ -98,6 +98,7 @@ private slots: void destruction(); void threadRecycling(); void expiryTimeout(); + void expiryTimeoutRace(); void exceptions(); void maxThreadCount(); void setMaxThreadCount_data(); @@ -333,7 +334,7 @@ class ExpiryTimeoutTask : public QRunnable { public: QThread *thread; - int runCount; + QAtomicInt runCount; QSemaphore semaphore; ExpiryTimeoutTask() @@ -345,7 +346,7 @@ public: void run() { thread = QThread::currentThread(); - ++runCount; + runCount.ref(); semaphore.release(); } }; @@ -364,7 +365,7 @@ void tst_QThreadPool::expiryTimeout() // run the task threadPool.start(&task); QVERIFY(task.semaphore.tryAcquire(1, 10000)); - QCOMPARE(task.runCount, 1); + QCOMPARE(int(task.runCount), 1); QVERIFY(!task.thread->wait(100)); // thread should expire QThread *firstThread = task.thread; @@ -373,7 +374,7 @@ void tst_QThreadPool::expiryTimeout() // run task again, thread should be restarted threadPool.start(&task); QVERIFY(task.semaphore.tryAcquire(1, 10000)); - QCOMPARE(task.runCount, 2); + QCOMPARE(int(task.runCount), 2); QVERIFY(!task.thread->wait(100)); // thread should expire again QVERIFY(task.thread->wait(10000)); @@ -386,6 +387,22 @@ void tst_QThreadPool::expiryTimeout() QCOMPARE(threadPool.expiryTimeout(), expiryTimeout); } +void tst_QThreadPool::expiryTimeoutRace() // QTBUG-3786 +{ + ExpiryTimeoutTask task; + + QThreadPool threadPool; + threadPool.setMaxThreadCount(1); + threadPool.setExpiryTimeout(50); + const int numTasks = 20; + for (int i = 0; i < numTasks; ++i) { + threadPool.start(&task); + QTest::qSleep(50); // exactly the same as the expiry timeout + } + QCOMPARE(int(task.runCount), numTasks); + QVERIFY(threadPool.waitForDone(2000)); +} + #ifndef QT_NO_EXCEPTIONS class ExceptionTask : public QRunnable { |