From a144416f28ff256eed9913edc8453acb00760876 Mon Sep 17 00:00:00 2001 From: "Bradley T. Hughes" Date: Fri, 25 Jun 2010 11:21:21 +0200 Subject: QSemaphore::tryAquire(timeout) -- never times out on an active semaphore If a thread trying to acquire multiple resources is continuously preempted by threads acquiring smaller amounts, the larger consumer would end up waiting forever (instead of for the given timeout). Fix this by keeping track of elapsed time between wakeups using QElapsedTimer. Task-number: QTBUG-11500 Reviewed-by: thiago --- src/corelib/thread/qsemaphore.cpp | 7 ++++- tests/auto/qsemaphore/tst_qsemaphore.cpp | 49 ++++++++++++++++++++++++++++++-- 2 files changed, 53 insertions(+), 3 deletions(-) diff --git a/src/corelib/thread/qsemaphore.cpp b/src/corelib/thread/qsemaphore.cpp index 9dc828d..8e8a88a 100644 --- a/src/corelib/thread/qsemaphore.cpp +++ b/src/corelib/thread/qsemaphore.cpp @@ -44,6 +44,8 @@ #ifndef QT_NO_THREAD #include "qmutex.h" #include "qwaitcondition.h" +#include "qelapsedtimer.h" +#include "qdatetime.h" QT_BEGIN_NAMESPACE @@ -218,8 +220,11 @@ bool QSemaphore::tryAcquire(int n, int timeout) while (n > d->avail) d->cond.wait(locker.mutex()); } else { + QElapsedTimer timer; + timer.start(); while (n > d->avail) { - if (!d->cond.wait(locker.mutex(), timeout)) + if (timer.hasExpired(timeout) + || !d->cond.wait(locker.mutex(), timeout - timer.elapsed())) return false; } } diff --git a/tests/auto/qsemaphore/tst_qsemaphore.cpp b/tests/auto/qsemaphore/tst_qsemaphore.cpp index ace33dc..7cede30 100644 --- a/tests/auto/qsemaphore/tst_qsemaphore.cpp +++ b/tests/auto/qsemaphore/tst_qsemaphore.cpp @@ -63,6 +63,7 @@ private slots: void tryAcquire(); void tryAcquireWithTimeout_data(); void tryAcquireWithTimeout(); + void tryAcquireWithTimeoutStarvation(); void release(); void available(); void producerConsumer(); @@ -232,8 +233,8 @@ void tst_QSemaphore::tryAcquireWithTimeout_data() { QTest::addColumn("timeout"); - QTest::newRow("") << 1000; - QTest::newRow("") << 10000; + QTest::newRow("1s") << 1000; + QTest::newRow("10s") << 10000; } void tst_QSemaphore::tryAcquireWithTimeout() @@ -316,6 +317,50 @@ void tst_QSemaphore::tryAcquireWithTimeout() QCOMPARE(semaphore.available(), 0); } +void tst_QSemaphore::tryAcquireWithTimeoutStarvation() +{ + class Thread : public QThread + { + public: + QSemaphore startup; + QSemaphore *semaphore; + int amountToConsume, timeout; + + void run() + { + startup.release(); + forever { + if (!semaphore->tryAcquire(amountToConsume, timeout)) + break; + semaphore->release(amountToConsume); + } + } + }; + + QSemaphore semaphore; + semaphore.release(1); + + Thread consumer; + consumer.semaphore = &semaphore; + consumer.amountToConsume = 1; + consumer.timeout = 1000; + + // start the thread and wait for it to start consuming + consumer.start(); + consumer.startup.acquire(); + + // try to consume more than the thread we started is, and provide a longer + // timeout... we should timeout, not wait indefinitely + QVERIFY(!semaphore.tryAcquire(consumer.amountToConsume * 2, consumer.timeout * 2)); + + // the consumer should still be running + QVERIFY(consumer.isRunning() && !consumer.isFinished()); + + // acquire, and wait for smallConsumer to timeout + semaphore.acquire(); + QVERIFY(consumer.wait()); +} + void tst_QSemaphore::release() { DEPENDS_ON("acquire"); } -- cgit v0.12