Refactor QMutexLocker to be able to handle recursive mutexes

Since we're going to split QMutex and QRecursiveMutex into
separate classes, make sure QMutexLocker is prepared for that.

Change-Id: Id5e9a955d1db7c8ee663dd3811ad6448dad0aeae
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>

src/corelib/io/qprocess_p.h

@@ -147,7 +147,7 @@ public:
     inline Value prepareValue(const QString &value) const { return value; }
     inline QString valueToString(const Value &value) const { return value; }
 #else
-    struct NameMapMutexLocker : public QMutexLocker
+    struct NameMapMutexLocker : public QMutexLocker<QMutex>
     {
         NameMapMutexLocker(const QProcessEnvironmentPrivate *d) : QMutexLocker(&d->nameMapMutex) {}
     };

src/corelib/kernel/qeventloop.cpp

@@ -179,10 +179,10 @@ int QEventLoop::exec(ProcessEventsFlags flags)
 
     struct LoopReference {
         QEventLoopPrivate *d;
-        QMutexLocker &locker;
+        QMutexLocker<QMutex> &locker;
 
         bool exceptionCaught;
-        LoopReference(QEventLoopPrivate *d, QMutexLocker &locker) : d(d), locker(locker), exceptionCaught(true)
+        LoopReference(QEventLoopPrivate *d, QMutexLocker<QMutex> &locker) : d(d), locker(locker), exceptionCaught(true)
         {
             d->inExec = true;
             d->exit.storeRelease(false);

src/corelib/kernel/qjnihelpers.cpp

@@ -601,7 +601,7 @@ int QtAndroidPrivate::acuqireServiceSetup(int flags)
 
 void QtAndroidPrivate::setOnBindListener(QtAndroidPrivate::OnBindListener *listener)
 {
-    QMutexLocker lock(g_onBindListenerMutex);
+    QMutexLocker lock(g_onBindListenerMutex());
     *g_onBindListener = listener;
     if (!g_serviceSetupLockers->deref())
         g_waitForServiceSetupSemaphore->release();
@@ -609,7 +609,7 @@ void QtAndroidPrivate::setOnBindListener(QtAndroidPrivate::OnBindListener *liste
 
 jobject QtAndroidPrivate::callOnBindListener(jobject intent)
 {
-    QMutexLocker lock(g_onBindListenerMutex);
+    QMutexLocker lock(g_onBindListenerMutex());
     if (*g_onBindListener)
         return (*g_onBindListener)->onBind(intent);
     return nullptr;

src/corelib/thread/qmutex.cpp

@@ -423,7 +423,7 @@ QRecursiveMutex::~QRecursiveMutex()
 
     \ingroup thread
 
-    Locking and unlocking a QMutex in complex functions and
+    Locking and unlocking a QMutex or QRecursiveMutex in complex functions and
     statements or in exception handling code is error-prone and
     difficult to debug. QMutexLocker can be used in such situations
     to ensure that the state of the mutex is always well-defined.

src/corelib/thread/qmutex.h

@@ -174,6 +174,7 @@ public:
 
 private:
     Q_DISABLE_COPY(QMutex)
+    template<typename Mutex>
     friend class QMutexLocker;
     friend class QRecursiveMutex;
     friend class ::tst_QMutex;
@@ -207,6 +208,7 @@ private:
 class QRecursiveMutex : private QMutex
 {
     // ### Qt 6: make it independent of QMutex
+    template<typename Mutex>
     friend class QMutexLocker;
 public:
     Q_CORE_EXPORT QRecursiveMutex();
@@ -222,65 +224,49 @@ public:
 #endif
 };
 
-class Q_CORE_EXPORT QMutexLocker
+template <typename Mutex>
+class QMutexLocker
 {
 public:
-#ifndef Q_CLANG_QDOC
+    inline explicit QMutexLocker(Mutex *mutex) QT_MUTEX_LOCK_NOEXCEPT
-    inline explicit QMutexLocker(QBasicMutex *m) QT_MUTEX_LOCK_NOEXCEPT
     {
-        Q_ASSERT_X((reinterpret_cast<quintptr>(m) & quintptr(1u)) == quintptr(0),
+        m = mutex;
-                   "QMutexLocker", "QMutex pointer is misaligned");
+        if (Q_LIKELY(mutex)) {
-        val = quintptr(m);
+            mutex->lock();
-        if (Q_LIKELY(m)) {
+            isLocked = true;
-            // call QMutex::lock() instead of QBasicMutex::lock()
-            static_cast<QMutex *>(m)->lock();
-            val |= 1;
         }
     }
-    explicit QMutexLocker(QRecursiveMutex *m) QT_MUTEX_LOCK_NOEXCEPT
+    inline ~QMutexLocker() {
-        : QMutexLocker{static_cast<QBasicMutex*>(m)} {}
+        unlock();
-#else
+    }
-    QMutexLocker(QMutex *) { }
-    QMutexLocker(QRecursiveMutex *) {}
-#endif
-    inline ~QMutexLocker() { unlock(); }
 
     inline void unlock() noexcept
     {
-        if ((val & quintptr(1u)) == quintptr(1u)) {
+        if (!isLocked)
-            val &= ~quintptr(1u);
+            return;
-            mutex()->unlock();
+        m->unlock();
-        }
+        isLocked = false;
     }
 
     inline void relock() QT_MUTEX_LOCK_NOEXCEPT
     {
-        if (val) {
+        if (isLocked)
-            if ((val & quintptr(1u)) == quintptr(0u)) {
+            return;
-                mutex()->lock();
+        if (m) {
-                val |= quintptr(1u);
+            m->lock();
-            }
+            isLocked = true;
         }
     }
 
-#if defined(Q_CC_MSVC)
+    Mutex *mutex() const
-#pragma warning( push )
-#pragma warning( disable : 4312 ) // ignoring the warning from /Wp64
-#endif
-
-    inline QMutex *mutex() const
     {
-        return reinterpret_cast<QMutex *>(val & ~quintptr(1u));
+        return m;
     }
-
-#if defined(Q_CC_MSVC)
-#pragma warning( pop )
-#endif
-
 private:
     Q_DISABLE_COPY(QMutexLocker)
 
-    quintptr val;
+    Mutex *m;
+    bool isLocked = false;
 };
 
 #else // !QT_CONFIG(thread) && !Q_CLANG_QDOC
@@ -320,15 +306,16 @@ private:
 
 class QRecursiveMutex : public QMutex {};
 
-class Q_CORE_EXPORT QMutexLocker
+template<typename Mutex>
+class QMutexLocker
 {
 public:
-    inline explicit QMutexLocker(QMutex *) noexcept {}
+    inline explicit QMutexLocker(Mutex *) noexcept {}
     inline ~QMutexLocker() noexcept {}
 
     inline void unlock() noexcept {}
     void relock() noexcept {}
-    inline QMutex *mutex() const noexcept { return nullptr; }
+    inline Mutex *mutex() const noexcept { return nullptr; }
 
 private:
     Q_DISABLE_COPY(QMutexLocker)

src/corelib/thread/qorderedmutexlocker_p.h

@@ -162,7 +162,7 @@ public:
     static bool relock(QBasicMutex *, QBasicMutex *) { return false; }
 };
 
-using QBasicMutexLocker = QMutexLocker;
+using QBasicMutexLocker = QMutexLocker<QBasicMutex>;
 
 #endif
 

src/network/ssl/qsslsocket_mac.cpp

@@ -314,7 +314,7 @@ OSStatus QSslSocketBackendPrivate::WriteCallback(QSslSocketBackendPrivate *socke
 
 void QSslSocketPrivate::ensureInitialized()
 {
-    const QMutexLocker locker(qt_securetransport_mutex);
+    const QMutexLocker locker(qt_securetransport_mutex());
     if (s_loadedCiphersAndCerts)
         return;
 

src/network/ssl/qsslsocket_openssl.cpp

@@ -2233,7 +2233,7 @@ bool QSslSocketPrivate::ensureLibraryLoaded()
     if (!q_resolveOpenSslSymbols())
         return false;
 
-    const QMutexLocker locker(qt_opensslInitMutex);
+    const QMutexLocker locker(qt_opensslInitMutex());
 
     if (!s_libraryLoaded) {
         // Initialize OpenSSL.
@@ -2265,7 +2265,7 @@ bool QSslSocketPrivate::ensureLibraryLoaded()
 
 void QSslSocketPrivate::ensureCiphersAndCertsLoaded()
 {
-    const QMutexLocker locker(qt_opensslInitMutex);
+    const QMutexLocker locker(qt_opensslInitMutex());
 
     if (s_loadedCiphersAndCerts)
         return;

tests/benchmarks/corelib/thread/qreadwritelock/tst_qreadwritelock.cpp

@@ -103,7 +103,7 @@ void tst_QReadWriteLock::uncontended_data()
     QTest::addColumn<FunctionPtrHolder>("holder");
 
     QTest::newRow("nothing") << FunctionPtrHolder(testUncontended<int, FakeLock>);
-    QTest::newRow("QMutex") << FunctionPtrHolder(testUncontended<QMutex, QMutexLocker>);
+    QTest::newRow("QMutex") << FunctionPtrHolder(testUncontended<QMutex, QMutexLocker<QMutex>>);
     QTest::newRow("QReadWriteLock, read")
         << FunctionPtrHolder(testUncontended<QReadWriteLock, QReadLocker>);
     QTest::newRow("QReadWriteLock, write")
@@ -173,7 +173,7 @@ void tst_QReadWriteLock::readOnly_data()
     QTest::addColumn<FunctionPtrHolder>("holder");
 
     QTest::newRow("nothing") << FunctionPtrHolder(testReadOnly<int, FakeLock>);
-    QTest::newRow("QMutex") << FunctionPtrHolder(testReadOnly<QMutex, QMutexLocker>);
+    QTest::newRow("QMutex") << FunctionPtrHolder(testReadOnly<QMutex, QMutexLocker<QMutex>>);
     QTest::newRow("QReadWriteLock") << FunctionPtrHolder(testReadOnly<QReadWriteLock, QReadLocker>);
     QTest::newRow("std::mutex") << FunctionPtrHolder(
         testReadOnly<std::mutex, LockerWrapper<std::unique_lock<std::mutex>>>);
@@ -234,7 +234,7 @@ void tst_QReadWriteLock::writeOnly_data()
     QTest::addColumn<FunctionPtrHolder>("holder");
 
     // QTest::newRow("nothing") << FunctionPtrHolder(testWriteOnly<int, FakeLock>);
-    QTest::newRow("QMutex") << FunctionPtrHolder(testWriteOnly<QMutex, QMutexLocker>);
+    QTest::newRow("QMutex") << FunctionPtrHolder(testWriteOnly<QMutex, QMutexLocker<QMutex>>);
     QTest::newRow("QReadWriteLock") << FunctionPtrHolder(testWriteOnly<QReadWriteLock, QWriteLocker>);
     QTest::newRow("std::mutex") << FunctionPtrHolder(
         testWriteOnly<std::mutex, LockerWrapper<std::unique_lock<std::mutex>>>);

tests/benchmarks/corelib/thread/qwaitcondition/tst_qwaitcondition.cpp

@@ -110,7 +110,7 @@ void tst_QWaitCondition::oscillate_mutex_data()
 void tst_QWaitCondition::oscillate_mutex()
 {
     QFETCH(unsigned long, timeout);
-    oscillate<QMutex, QMutexLocker>(timeout);
+    oscillate<QMutex, QMutexLocker<QMutex>>(timeout);
 }
 
 void tst_QWaitCondition::oscillate_writelock_data()