Short live QRecursiveMutex!

Move the recursive mutex use case out of QMutex into a separate class,
unsurprisingly called QRecursiveMutex. As an immediate benefit, 90% of
the QMutex users now enjoy a constexpr QMutex ctor.

This change prepares for a real split in Qt 6, so that both use-cases
are no longer bundled up in one class.

[ChangeLog][QtCore][QMutex] Added QRecursiveMutex as a replacement of
QMutex(QMutex::Recursive).

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

src/corelib/thread/qmutex.cpp

@@ -147,7 +147,7 @@ public:
     It is constructed and destroyed with almost no overhead,
     which means it is fine to have many mutexes as part of other classes.
 
-    \sa QMutexLocker, QReadWriteLock, QSemaphore, QWaitCondition
+    \sa QRecursiveMutex, QMutexLocker, QReadWriteLock, QSemaphore, QWaitCondition
 */
 
 /*!
@@ -156,12 +156,19 @@ public:
     \value Recursive  In this mode, a thread can lock the same mutex
                       multiple times and the mutex won't be unlocked
                       until a corresponding number of unlock() calls
-                      have been made.
+                      have been made. You should use QRecursiveMutex
+                      for this use-case.
 
     \value NonRecursive  In this mode, a thread may only lock a mutex
                          once.
 
-    \sa QMutex()
+    \sa QMutex(), QRecursiveMutex
+*/
+
+/*!
+    \fn QMutex::QMutex()
+
+    Constructs a new mutex. The mutex is created in an unlocked state.
 */
 
 /*!
@@ -205,13 +212,15 @@ QMutex::~QMutex()
 }
 
 /*! \fn void QMutex::lock()
+    \fn QRecursiveMutex::lock()
+
     Locks the mutex. If another thread has locked the mutex then this
     call will block until that thread has unlocked it.
 
     Calling this function multiple times on the same mutex from the
     same thread is allowed if this mutex is a
-    \l{QMutex::Recursive}{recursive mutex}. If this mutex is a
+    \l{QRecursiveMutex}{recursive mutex}. If this mutex is a
-    \l{QMutex::NonRecursive}{non-recursive mutex}, this function will
+    \l{QMutex}{non-recursive mutex}, this function will
     \e dead-lock when the mutex is locked recursively.
 
     \sa unlock()
@@ -228,6 +237,7 @@ void QMutex::lock() QT_MUTEX_LOCK_NOEXCEPT
 }
 
 /*! \fn bool QMutex::tryLock(int timeout)
+    \fn bool QRecursiveMutex::tryLock(int timeout)
 
     Attempts to lock the mutex. This function returns \c true if the lock
     was obtained; otherwise it returns \c false. If another thread has
@@ -243,8 +253,8 @@ void QMutex::lock() QT_MUTEX_LOCK_NOEXCEPT
 
     Calling this function multiple times on the same mutex from the
     same thread is allowed if this mutex is a
-    \l{QMutex::Recursive}{recursive mutex}. If this mutex is a
+    \l{QRecursiveMutex}{recursive mutex}. If this mutex is a
-    \l{QMutex::NonRecursive}{non-recursive mutex}, this function will
+    \l{QMutex}{non-recursive mutex}, this function will
     \e always return false when attempting to lock the mutex
     recursively.
 
@@ -262,6 +272,7 @@ bool QMutex::tryLock(int timeout) QT_MUTEX_LOCK_NOEXCEPT
 }
 
 /*! \fn bool QMutex::try_lock()
+    \fn bool QRecursiveMutex::try_lock()
     \since 5.8
 
     Attempts to lock the mutex. This function returns \c true if the lock
@@ -275,6 +286,7 @@ bool QMutex::tryLock(int timeout) QT_MUTEX_LOCK_NOEXCEPT
 */
 
 /*! \fn template <class Rep, class Period> bool QMutex::try_lock_for(std::chrono::duration<Rep, Period> duration)
+    \fn template <class Rep, class Period> bool QRecursiveMutex::try_lock_for(std::chrono::duration<Rep, Period> duration)
     \since 5.8
 
     Attempts to lock the mutex. This function returns \c true if the lock
@@ -290,8 +302,8 @@ bool QMutex::tryLock(int timeout) QT_MUTEX_LOCK_NOEXCEPT
 
     Calling this function multiple times on the same mutex from the
     same thread is allowed if this mutex is a
-    \l{QMutex::Recursive}{recursive mutex}. If this mutex is a
+    \l{QRecursiveMutex}{recursive mutex}. If this mutex is a
-    \l{QMutex::NonRecursive}{non-recursive mutex}, this function will
+    \l{QMutex}{non-recursive mutex}, this function will
     \e always return false when attempting to lock the mutex
     recursively.
 
@@ -299,6 +311,7 @@ bool QMutex::tryLock(int timeout) QT_MUTEX_LOCK_NOEXCEPT
 */
 
 /*! \fn template<class Clock, class Duration> bool QMutex::try_lock_until(std::chrono::time_point<Clock, Duration> timePoint)
+    \fn template<class Clock, class Duration> bool QRecursiveMutex::try_lock_until(std::chrono::time_point<Clock, Duration> timePoint)
     \since 5.8
 
     Attempts to lock the mutex. This function returns \c true if the lock
@@ -314,8 +327,8 @@ bool QMutex::tryLock(int timeout) QT_MUTEX_LOCK_NOEXCEPT
 
     Calling this function multiple times on the same mutex from the
     same thread is allowed if this mutex is a
-    \l{QMutex::Recursive}{recursive mutex}. If this mutex is a
+    \l{QRecursiveMutex}{recursive mutex}. If this mutex is a
-    \l{QMutex::NonRecursive}{non-recursive mutex}, this function will
+    \l{QMutex}{non-recursive mutex}, this function will
     \e always return false when attempting to lock the mutex
     recursively.
 
@@ -323,6 +336,8 @@ bool QMutex::tryLock(int timeout) QT_MUTEX_LOCK_NOEXCEPT
 */
 
 /*! \fn void QMutex::unlock()
+    \fn void QRecursiveMutex::unlock()
+
     Unlocks the mutex. Attempting to unlock a mutex in a different
     thread to the one that locked it results in an error. Unlocking a
     mutex that is not locked results in undefined behavior.
@@ -363,6 +378,58 @@ bool QBasicMutex::isRecursive() const noexcept
     return QT_PREPEND_NAMESPACE(isRecursive)(d_ptr.loadAcquire());
 }
 
+/*!
+    \class QRecursiveMutex
+    \inmodule QtCore
+    \since 5.14
+    \brief The QRecursiveMutex class provides access serialization between threads.
+
+    \threadsafe
+
+    \ingroup thread
+
+    The QRecursiveMutex class is a mutex, like QMutex, with which it is
+    API-compatible. It differs from QMutex by accepting lock() calls from
+    the same thread any number of times. QMutex would deadlock in this situation.
+
+    QRecursiveMutex is much more expensive to construct and operate on, so
+    use a plain QMutex whenever you can. Sometimes, one public function,
+    however, calls another public function, and they both need to lock the
+    same mutex. In this case, you have two options:
+
+    \list
+    \li Factor the code that needs mutex protection into private functions,
+    which assume that the mutex is held when they are called, and lock a
+    plain QMutex in the public functions before you call the private
+    implementation ones.
+    \li Or use a recursive mutex, so it doesn't matter that the first public
+    function has already locked the mutex when the second one wishes to do so.
+    \endlist
+
+    \sa QMutex, QMutexLocker, QReadWriteLock, QSemaphore, QWaitCondition
+*/
+
+/*!
+    Constructs a new recursive mutex. The mutex is created in an unlocked state.
+
+    \sa lock(), unlock()
+*/
+QRecursiveMutex::QRecursiveMutex()
+    : QMutex()
+{
+    d_ptr.storeRelaxed(new QRecursiveMutexPrivate);
+}
+
+/*!
+    Destroys the mutex.
+
+    \warning Destroying a locked mutex may result in undefined behavior.
+*/
+QRecursiveMutex::~QRecursiveMutex()
+{
+    delete static_cast<QRecursiveMutexPrivate*>(d_ptr.fetchAndStoreAcquire(nullptr));
+}
+
 /*!
     \class QMutexLocker
     \inmodule QtCore
@@ -426,6 +493,17 @@ bool QBasicMutex::isRecursive() const noexcept
     \sa QMutex::lock()
 */
 
+/*!
+    \fn QMutexLocker::QMutexLocker(QRecursiveMutex *mutex)
+    \since 5.14
+
+    Constructs a QMutexLocker and locks \a mutex. The mutex will be
+    unlocked (unlock() called) when the QMutexLocker is destroyed.
+    If \a mutex is \nullptr, QMutexLocker does nothing.
+
+    \sa QMutex::lock()
+*/
+
 /*!
     \fn QMutexLocker::~QMutexLocker()
 

src/corelib/thread/qmutex.h

@@ -62,6 +62,8 @@ QT_BEGIN_NAMESPACE
 # define QT_MUTEX_LOCK_NOEXCEPT
 #endif
 
+class QMutex;
+class QRecursiveMutex;
 class QMutexData;
 
 class Q_CORE_EXPORT QBasicMutex
@@ -120,14 +122,20 @@ private:
     }
 
     friend class QMutex;
+    friend class QRecursiveMutex;
     friend class QMutexData;
 };
 
 class Q_CORE_EXPORT QMutex : public QBasicMutex
 {
 public:
+#if defined(Q_COMPILER_CONSTEXPR)
+    constexpr QMutex() = default;
+#else
+    QMutex() { d_ptr.storeRelaxed(nullptr); }
+#endif
     enum RecursionMode { NonRecursive, Recursive };
-    explicit QMutex(RecursionMode mode = NonRecursive);
+    explicit QMutex(RecursionMode mode);
     ~QMutex();
 
     // BasicLockable concept
@@ -164,6 +172,7 @@ public:
 private:
     Q_DISABLE_COPY(QMutex)
     friend class QMutexLocker;
+    friend class QRecursiveMutex;
     friend class ::tst_QMutex;
 
 #if QT_HAS_INCLUDE(<chrono>)
@@ -192,6 +201,24 @@ private:
 #endif
 };
 
+class QRecursiveMutex : private QMutex
+{
+    // ### Qt 6: make it independent of QMutex
+    friend class QMutexLocker;
+public:
+    Q_CORE_EXPORT QRecursiveMutex();
+    Q_CORE_EXPORT ~QRecursiveMutex();
+
+    using QMutex::lock;
+    using QMutex::tryLock;
+    using QMutex::unlock;
+    using QMutex::try_lock;
+#if QT_HAS_INCLUDE(<chrono>)
+    using QMutex::try_lock_for;
+    using QMutex::try_lock_until;
+#endif
+};
+
 class Q_CORE_EXPORT QMutexLocker
 {
 public:
@@ -207,8 +234,11 @@ public:
             val |= 1;
         }
     }
+    explicit QMutexLocker(QRecursiveMutex *m) QT_MUTEX_LOCK_NOEXCEPT
+        : QMutexLocker{static_cast<QBasicMutex*>(m)} {}
 #else
     QMutexLocker(QMutex *) { }
+    QMutexLocker(QRecursiveMutex *) {}
 #endif
     inline ~QMutexLocker() { unlock(); }
 
@@ -285,6 +315,8 @@ private:
     Q_DISABLE_COPY(QMutex)
 };
 
+class QRecursiveMutex : public QMutex {};
+
 class Q_CORE_EXPORT QMutexLocker
 {
 public:

tests/auto/corelib/thread/qmutex/tst_qmutex.cpp

@@ -69,7 +69,8 @@ private slots:
 static const int iterations = 100;
 
 QAtomicInt lockCount(0);
-QMutex normalMutex, recursiveMutex(QMutex::Recursive);
+QMutex normalMutex;
+QRecursiveMutex recursiveMutex;
 QSemaphore testsTurn;
 QSemaphore threadsTurn;
 
@@ -993,9 +994,9 @@ public:
     QMutex mutex;
     QWaitCondition cond;
 
-    QMutex &test_mutex;
+    QRecursiveMutex &test_mutex;
 
-    inline rmutex_Thread(QMutex &m) : test_mutex(m) { }
+    inline rmutex_Thread(QRecursiveMutex &m) : test_mutex(m) { }
 
     void run()
     {
@@ -1024,7 +1025,7 @@ void tst_QMutex::lock_unlock_locked_tryLock()
     QMutex mutex;
     mutex_Thread thread(mutex);
 
-    QMutex rmutex(QMutex::Recursive);
+    QRecursiveMutex rmutex;
     rmutex_Thread rthread(rmutex);
 
     for (int i = 0; i < iterations; ++i) {