From a84dcc30b096915dc71d8c9696696731dee45c48 Mon Sep 17 00:00:00 2001 From: mread Date: Thu, 17 Feb 2011 11:05:36 +0000 Subject: QMutex symbian review changes Improved documentation Added a trywait(0) test and fixed the bug it found Symbian implementation throws on construction fail Task-number: QTBUG-13990 Reviewed-by: Shane Kearns --- src/corelib/thread/qmutex_p.h | 3 ++ src/corelib/thread/qmutex_symbian.cpp | 4 +- tests/auto/qmutex/tst_qmutex.cpp | 69 +++++++++++++++++++++++++++++++++++ 3 files changed, 75 insertions(+), 1 deletion(-) diff --git a/src/corelib/thread/qmutex_p.h b/src/corelib/thread/qmutex_p.h index c213c80..2588228 100644 --- a/src/corelib/thread/qmutex_p.h +++ b/src/corelib/thread/qmutex_p.h @@ -98,8 +98,11 @@ public: HANDLE event; #elif defined(Q_OS_SYMBIAN) # ifdef QT_SYMBIAN_USE_RFASTLOCK + // RFastLock is a fast semaphone which only calls into the kernel side if there is contention RFastLock lock; # else + // RSemaphore is used on Symbian OS versions that do not have a timed RFastLock wait. + // There is no timed wait on RMutex itself, so RSemaphore has to be used instead. RSemaphore lock; # endif #endif diff --git a/src/corelib/thread/qmutex_symbian.cpp b/src/corelib/thread/qmutex_symbian.cpp index e7487cd..09c59af 100644 --- a/src/corelib/thread/qmutex_symbian.cpp +++ b/src/corelib/thread/qmutex_symbian.cpp @@ -62,6 +62,7 @@ QMutexPrivate::QMutexPrivate(QMutex::RecursionMode mode) #endif if (r != KErrNone) qWarning("QMutex: failed to create lock, error %d", r); + qt_symbian_throwIfError(r); } QMutexPrivate::~QMutexPrivate() @@ -86,7 +87,8 @@ bool QMutexPrivate::wait(int timeout) do { int waitTime = qMin(KMaxTInt / 1000, timeout); timeout -= waitTime; - r = lock.Wait(waitTime * 1000); + // Symbian undocumented feature - 0us means no timeout! Use a minimum of 1 + r = lock.Wait(qMax(1, waitTime * 1000)); } while (r != KErrNone && r != KErrGeneral && r != KErrArgument && timeout > 0); } bool returnValue = (r == KErrNone); diff --git a/tests/auto/qmutex/tst_qmutex.cpp b/tests/auto/qmutex/tst_qmutex.cpp index ea983cb..a8c4b37 100644 --- a/tests/auto/qmutex/tst_qmutex.cpp +++ b/tests/auto/qmutex/tst_qmutex.cpp @@ -129,27 +129,57 @@ void tst_QMutex::tryLock() testsTurn.release(); threadsTurn.acquire(); + QVERIFY(!normalMutex.tryLock(0)); + testsTurn.release(); + + threadsTurn.acquire(); + timer.start(); + QVERIFY(normalMutex.tryLock(0)); + QVERIFY(timer.elapsed() < 1000); + QVERIFY(lockCount.testAndSetRelaxed(0, 1)); + QVERIFY(!normalMutex.tryLock(0)); + QVERIFY(lockCount.testAndSetRelaxed(1, 0)); + normalMutex.unlock(); + testsTurn.release(); + + threadsTurn.acquire(); } }; Thread thread; thread.start(); + // thread can't acquire lock + testsTurn.acquire(); + normalMutex.lock(); + QVERIFY(lockCount.testAndSetRelaxed(0, 1)); + threadsTurn.release(); + + // thread can acquire lock + testsTurn.acquire(); + QVERIFY(lockCount.testAndSetRelaxed(1, 0)); + normalMutex.unlock(); + threadsTurn.release(); + + // thread can't acquire lock, timeout = 1000 testsTurn.acquire(); normalMutex.lock(); QVERIFY(lockCount.testAndSetRelaxed(0, 1)); threadsTurn.release(); + // thread can acquire lock, timeout = 1000 testsTurn.acquire(); QVERIFY(lockCount.testAndSetRelaxed(1, 0)); normalMutex.unlock(); threadsTurn.release(); + // thread can't acquire lock, timeout = 0 testsTurn.acquire(); normalMutex.lock(); QVERIFY(lockCount.testAndSetRelaxed(0, 1)); threadsTurn.release(); + // thread can acquire lock, timeout = 0 testsTurn.acquire(); QVERIFY(lockCount.testAndSetRelaxed(1, 0)); normalMutex.unlock(); @@ -190,6 +220,7 @@ void tst_QMutex::tryLock() timer.start(); QVERIFY(!recursiveMutex.tryLock(1000)); QVERIFY(timer.elapsed() >= 1000); + QVERIFY(!recursiveMutex.tryLock(0)); testsTurn.release(); threadsTurn.acquire(); @@ -206,12 +237,47 @@ void tst_QMutex::tryLock() testsTurn.release(); threadsTurn.acquire(); + QVERIFY(!recursiveMutex.tryLock(0)); + QVERIFY(!recursiveMutex.tryLock(0)); + testsTurn.release(); + + threadsTurn.acquire(); + timer.start(); + QVERIFY(recursiveMutex.tryLock(0)); + QVERIFY(timer.elapsed() < 1000); + QVERIFY(lockCount.testAndSetRelaxed(0, 1)); + QVERIFY(recursiveMutex.tryLock(0)); + QVERIFY(lockCount.testAndSetRelaxed(1, 2)); + QVERIFY(lockCount.testAndSetRelaxed(2, 1)); + recursiveMutex.unlock(); + QVERIFY(lockCount.testAndSetRelaxed(1, 0)); + recursiveMutex.unlock(); + testsTurn.release(); + + threadsTurn.acquire(); } }; Thread thread; thread.start(); + // thread can't acquire lock + testsTurn.acquire(); + recursiveMutex.lock(); + QVERIFY(lockCount.testAndSetRelaxed(0, 1)); + recursiveMutex.lock(); + QVERIFY(lockCount.testAndSetRelaxed(1, 2)); + threadsTurn.release(); + + // thread can acquire lock + testsTurn.acquire(); + QVERIFY(lockCount.testAndSetRelaxed(2, 1)); + recursiveMutex.unlock(); + QVERIFY(lockCount.testAndSetRelaxed(1, 0)); + recursiveMutex.unlock(); + threadsTurn.release(); + + // thread can't acquire lock, timeout = 1000 testsTurn.acquire(); recursiveMutex.lock(); QVERIFY(lockCount.testAndSetRelaxed(0, 1)); @@ -219,6 +285,7 @@ void tst_QMutex::tryLock() QVERIFY(lockCount.testAndSetRelaxed(1, 2)); threadsTurn.release(); + // thread can acquire lock, timeout = 1000 testsTurn.acquire(); QVERIFY(lockCount.testAndSetRelaxed(2, 1)); recursiveMutex.unlock(); @@ -226,6 +293,7 @@ void tst_QMutex::tryLock() recursiveMutex.unlock(); threadsTurn.release(); + // thread can't acquire lock, timeout = 0 testsTurn.acquire(); recursiveMutex.lock(); QVERIFY(lockCount.testAndSetRelaxed(0, 1)); @@ -233,6 +301,7 @@ void tst_QMutex::tryLock() QVERIFY(lockCount.testAndSetRelaxed(1, 2)); threadsTurn.release(); + // thread can acquire lock, timeout = 0 testsTurn.acquire(); QVERIFY(lockCount.testAndSetRelaxed(2, 1)); recursiveMutex.unlock(); -- cgit v0.12