QRecursiveMutexPrivate should not inherit from QMutexPrivate

QMutexPrivate takes more memory than necessary, and also initialize
platform specific ressources.

Change-Id: I70be1b89b1c21499645785ae47693a6b2514e28b
Reviewed-by: Bradley T. Hughes <bradley.hughes@nokia.com>

diff --git a/src/corelib/thread/qmutex.cpp b/src/corelib/thread/qmutex.cpp
index 88aee2b..6ba0a8c 100644 (file)
--- a/src/corelib/thread/qmutex.cpp
+++ b/src/corelib/thread/qmutex.cpp
@@ -150,11 +150,16 @@ QMutex::QMutex(RecursionMode mode)
 */
 QMutex::~QMutex()
 {
-    if (isRecursive())
-        delete static_cast<QRecursiveMutexPrivate *>(d_ptr.load());
-    else if (d_ptr.load()) {
+    QMutexData *d = d_ptr.load();
+    if (quintptr(d) > 0x3 && d->recursive) {
+        delete static_cast<QRecursiveMutexPrivate *>(d);
+    } else if (d) {
 #ifndef Q_OS_LINUX
-        if (d_ptr.load()->possiblyUnlocked.load() && tryLock()) { unlock(); return; }
+        if (d != dummyLocked() && static_cast<QMutexPrivate *>(d)->possiblyUnlocked.load()
+            && tryLock()) {
+            unlock();
+            return;
+        }
 #endif
         qWarning("QMutex: destroying locked mutex");
     }
@@ -233,7 +238,7 @@ QMutex::~QMutex()
 
 */
 bool QBasicMutex::isRecursive() {
-    QMutexPrivate *d = d_ptr.load();
+    QMutexData *d = d_ptr.load();
     if (quintptr(d) <= 0x3)
         return false;
     return d->recursive;
@@ -342,30 +347,31 @@ bool QBasicMutex::isRecursive() {
 bool QBasicMutex::lockInternal(int timeout)
 {
     while (!fastTryLock()) {
-        QMutexPrivate *d = d_ptr.loadAcquire();
-        if (!d) // if d is 0, the mutex is unlocked
+        QMutexData *copy = d_ptr.loadAcquire();
+        if (!copy) // if d is 0, the mutex is unlocked
             continue;
 
-        if (d == dummyLocked()) {
+        if (copy == dummyLocked()) {
             if (timeout == 0)
                 return false;
             QMutexPrivate *newD = QMutexPrivate::allocate();
-            if (!d_ptr.testAndSetOrdered(d, newD)) {
+            if (!d_ptr.testAndSetOrdered(dummyLocked(), newD)) {
                 //Either the mutex is already unlocked, or another thread already set it.
                 newD->deref();
                 continue;
             }
-            d = newD;
+            copy = newD;
             //the d->refCount is already 1 the deref will occurs when we unlock
-        } else if (d->recursive) {
-             return static_cast<QRecursiveMutexPrivate *>(d)->lock(timeout);
+        } else if (copy->recursive) {
+             return static_cast<QRecursiveMutexPrivate *>(copy)->lock(timeout);
         }
 
+        QMutexPrivate *d = static_cast<QMutexPrivate *>(copy);
         if (timeout == 0 && !d->possiblyUnlocked.load())
             return false;
 
         if (!d->ref())
-            continue; //that QMutexPrivate was already released
+            continue; //that QMutexData was already released
 
         if (d != d_ptr.loadAcquire()) {
             //Either the mutex is already unlocked, or relocked with another mutex
@@ -433,15 +439,17 @@ bool QBasicMutex::lockInternal(int timeout)
 */
 void QBasicMutex::unlockInternal()
 {
-    QMutexPrivate *d = d_ptr.loadAcquire();
-    Q_ASSERT(d); //we must be locked
-    Q_ASSERT(d != dummyLocked()); // testAndSetRelease(dummyLocked(), 0) failed
+    QMutexData *copy = d_ptr.loadAcquire();
+    Q_ASSERT(copy); //we must be locked
+    Q_ASSERT(copy != dummyLocked()); // testAndSetRelease(dummyLocked(), 0) failed
 
-    if (d->recursive) {
-        static_cast<QRecursiveMutexPrivate *>(d)->unlock();
+    if (copy->recursive) {
+        static_cast<QRecursiveMutexPrivate *>(copy)->unlock();
         return;
     }
 
+    QMutexPrivate *d = reinterpret_cast<QMutexPrivate *>(copy);
+
     if (d->waiters.fetchAndAddRelease(-QMutexPrivate::BigNumber) == 0) {
         //there is no one waiting on this mutex anymore, set the mutex as unlocked (d = 0)
         if (d_ptr.testAndSetRelease(d, 0)) {

diff --git a/src/corelib/thread/qmutex.h b/src/corelib/thread/qmutex.h
index 2342892..086a289 100644 (file)
--- a/src/corelib/thread/qmutex.h
+++ b/src/corelib/thread/qmutex.h
@@ -54,7 +54,7 @@ QT_MODULE(Core)
 
 #if !defined(QT_NO_THREAD) && !defined(qdoc)
 
-class QMutexPrivate;
+class QMutexData;
 
 class Q_CORE_EXPORT QBasicMutex
 {
@@ -83,13 +83,13 @@ private:
     bool lockInternal(int timeout = -1);
     void unlockInternal();
 
-    QBasicAtomicPointer<QMutexPrivate> d_ptr;
-    static inline QMutexPrivate *dummyLocked() {
-        return reinterpret_cast<QMutexPrivate *>(quintptr(1));
+    QBasicAtomicPointer<QMutexData> d_ptr;
+    static inline QMutexData *dummyLocked() {
+        return reinterpret_cast<QMutexData *>(quintptr(1));
     }
 
     friend class QMutex;
-    friend class QMutexPrivate;
+    friend class QMutexData;
 };
 
 class Q_CORE_EXPORT QMutex : public QBasicMutex {

diff --git a/src/corelib/thread/qmutex_linux.cpp b/src/corelib/thread/qmutex_linux.cpp
index 3e5b5f2..18ddd23 100644 (file)
--- a/src/corelib/thread/qmutex_linux.cpp
+++ b/src/corelib/thread/qmutex_linux.cpp
@@ -65,16 +65,11 @@ static inline int _q_futex(void *addr, int op, int val, const struct timespec *t
     return syscall(SYS_futex, int_addr, op, val, timeout, addr2, val2);
 }
 
-static inline QMutexPrivate *dummyFutexValue()
+static inline QMutexData *dummyFutexValue()
 {
-    return reinterpret_cast<QMutexPrivate *>(quintptr(3));
+    return reinterpret_cast<QMutexData *>(quintptr(3));
 }
 
-
-QMutexPrivate::~QMutexPrivate() {}
-QMutexPrivate::QMutexPrivate(QMutex::RecursionMode mode)
-    : recursive(mode == QMutex::Recursive) {}
-
 bool QBasicMutex::lockInternal(int timeout)
 {
     QElapsedTimer elapsedTimer;
@@ -82,7 +77,7 @@ bool QBasicMutex::lockInternal(int timeout)
         elapsedTimer.start();
 
     while (!fastTryLock()) {
-        QMutexPrivate *d = d_ptr.load();
+        QMutexData *d = d_ptr.load();
         if (!d) // if d is 0, the mutex is unlocked
             continue;
 
@@ -118,7 +113,7 @@ bool QBasicMutex::lockInternal(int timeout)
 
 void QBasicMutex::unlockInternal()
 {
-    QMutexPrivate *d = d_ptr.load();
+    QMutexData *d = d_ptr.load();
     Q_ASSERT(d); //we must be locked
     Q_ASSERT(d != dummyLocked()); // testAndSetRelease(dummyLocked(), 0) failed
 

diff --git a/src/corelib/thread/qmutex_mac.cpp b/src/corelib/thread/qmutex_mac.cpp
index f63feeb..2fdb2ec 100644 (file)
--- a/src/corelib/thread/qmutex_mac.cpp
+++ b/src/corelib/thread/qmutex_mac.cpp
@@ -53,8 +53,7 @@
 
 QT_BEGIN_NAMESPACE
 
-QMutexPrivate::QMutexPrivate(QMutex::RecursionMode mode)
-    : recursive(mode == QMutex::Recursive)
+QMutexPrivate::QMutexPrivate()
 {
     kern_return_t r = semaphore_create(mach_task_self(), &mach_semaphore, SYNC_POLICY_FIFO, 0);
     if (r != KERN_SUCCESS)

diff --git a/src/corelib/thread/qmutex_p.h b/src/corelib/thread/qmutex_p.h
index 00bda48..e385181 100644 (file)
--- a/src/corelib/thread/qmutex_p.h
+++ b/src/corelib/thread/qmutex_p.h
@@ -65,15 +65,23 @@
 
 QT_BEGIN_NAMESPACE
 
-class QMutexPrivate  {
+class QMutexData
+{
+public:
+    bool recursive;
+    QMutexData(QMutex::RecursionMode mode = QMutex::NonRecursive)
+        : recursive(mode == QMutex::Recursive) {}
+};
+
+#if !defined(Q_OS_LINUX)
+class QMutexPrivate : public QMutexData {
 public:
     ~QMutexPrivate();
-    QMutexPrivate(QMutex::RecursionMode mode = QMutex::NonRecursive);
+    QMutexPrivate();
 
     bool wait(int timeout = -1);
     void wakeUp();
 
-#if !defined(Q_OS_LINUX)
     // Conrol the lifetime of the privates
     QAtomicInt refCount;
     int id;
@@ -102,15 +110,11 @@ public:
     QAtomicInt possiblyUnlocked; //bool saying that a timed wait timed out
     enum { BigNumber = 0x100000 }; //Must be bigger than the possible number of waiters (number of threads)
     void derefWaiters(int value);
-#endif
-
-    // handle recursive mutex
-    bool recursive;
 
     //platform specific stuff
 #if defined(Q_OS_MAC)
     semaphore_t mach_semaphore;
-#elif defined(Q_OS_UNIX) && !defined(Q_OS_LINUX)
+#elif defined(Q_OS_UNIX)
     bool wakeup;
     pthread_mutex_t mutex;
     pthread_cond_t cond;
@@ -118,12 +122,13 @@ public:
     HANDLE event;
 #endif
 };
+#endif //Q_OS_LINUX
 
-class QRecursiveMutexPrivate : public QMutexPrivate
+class QRecursiveMutexPrivate : public QMutexData
 {
 public:
     QRecursiveMutexPrivate()
-        : QMutexPrivate(QMutex::Recursive), owner(0), count(0) {}
+        : QMutexData(QMutex::Recursive), owner(0), count(0) {}
     Qt::HANDLE owner;
     uint count;
     QMutex mutex;

diff --git a/src/corelib/thread/qmutex_unix.cpp b/src/corelib/thread/qmutex_unix.cpp
index 0bccad5..b693088 100644 (file)
--- a/src/corelib/thread/qmutex_unix.cpp
+++ b/src/corelib/thread/qmutex_unix.cpp
@@ -60,8 +60,8 @@ static void report_error(int code, const char *where, const char *what)
         qWarning("%s: %s failure: %s", where, what, qPrintable(qt_error_string(code)));
 }
 
-QMutexPrivate::QMutexPrivate(QMutex::RecursionMode mode)
-    : recursive(mode == QMutex::Recursive), wakeup(false)
+QMutexPrivate::QMutexPrivate()
+    : wakeup(false)
 {
     report_error(pthread_mutex_init(&mutex, NULL), "QMutex", "mutex init");
     report_error(pthread_cond_init(&cond, NULL), "QMutex", "cv init");

diff --git a/src/corelib/thread/qmutex_win.cpp b/src/corelib/thread/qmutex_win.cpp
index f6670f6..781a632 100644 (file)
--- a/src/corelib/thread/qmutex_win.cpp
+++ b/src/corelib/thread/qmutex_win.cpp
@@ -47,12 +47,11 @@
 
 QT_BEGIN_NAMESPACE
 
-QMutexPrivate::QMutexPrivate(QMutex::RecursionMode mode)
-    : recursive(mode)
+QMutexPrivate::QMutexPrivate()
 {
     event = CreateEvent(0, FALSE, FALSE, 0);
     if (!event)
-        qWarning("QMutexPrivate::QMutexPrivate: Cannot create event");
+        qWarning("QMutexData::QMutexData: Cannot create event");
 }
 
 QMutexPrivate::~QMutexPrivate()