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

3 files changed, 75 insertions, 1 deletions

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();