summaryrefslogtreecommitdiffstats
path: root/tests
diff options
context:
space:
mode:
authorDavid Faure <david.faure@kdab.com>2013-11-01 09:49:30 (GMT)
committerThe Qt Project <gerrit-noreply@qt-project.org>2013-11-15 11:42:27 (GMT)
commit92a4fcff80cf7a2e3a71c0ff8a2a378866b79739 (patch)
tree489cea2b8b4bbfd0a7a2cf35e4a5ba48ff435438 /tests
parent605647f2b8a6c8d4f700732a7fcdb1dec3459231 (diff)
downloadQt-92a4fcff80cf7a2e3a71c0ff8a2a378866b79739.zip
Qt-92a4fcff80cf7a2e3a71c0ff8a2a378866b79739.tar.gz
Qt-92a4fcff80cf7a2e3a71c0ff8a2a378866b79739.tar.bz2
QThreadPool: fix race at time of thread expiry.
The current synchronization mechanism was racy: decrementing waitingThreads and then hoping that the wakeOne will wake a thread before its expiry timeout happens. In other words, on timeout, a just-assigned task would never run. And then no other task would run, if maxThreadCount is reached. Fixed by using a queue of waiting threads (rather than just a count), and by moving the wait condition into the thread itself, so we know precisely which one we're waking up, and we can remove it from the set of waiting threads before waking it up, and therefore it can determine on wakeup whether it has work to do (caller removed it from the queue) or it expired (it's still in the queue). This is reliable, whereas the return value from QWaitCondition::wait isn't reliable, when the main thread has already decided that this thread has work to do. Task-number: QTBUG-3786 Backport from qtbase/a9b6a78e54670a70b96c122b10ad7bd64d166514 Change-Id: Ic766ff67dea7a8bb8f1bc893943060ee5428d782 Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
Diffstat (limited to 'tests')
-rw-r--r--tests/auto/qthreadpool/tst_qthreadpool.cpp25
1 files changed, 21 insertions, 4 deletions
diff --git a/tests/auto/qthreadpool/tst_qthreadpool.cpp b/tests/auto/qthreadpool/tst_qthreadpool.cpp
index 61343c1..c40bd40 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
{