From 73b682d816ec12afbf8064a44e462dd89e4c38df Mon Sep 17 00:00:00 2001 From: Olivier Goffart Date: Thu, 20 Oct 2011 17:11:18 +0200 Subject: [PATCH] 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 --- src/corelib/thread/qmutex.cpp | 44 ++++++++++++++++++++++--------------- src/corelib/thread/qmutex.h | 10 ++++----- src/corelib/thread/qmutex_linux.cpp | 13 ++++------- src/corelib/thread/qmutex_mac.cpp | 3 +-- src/corelib/thread/qmutex_p.h | 25 ++++++++++++--------- src/corelib/thread/qmutex_unix.cpp | 4 ++-- src/corelib/thread/qmutex_win.cpp | 5 ++--- 7 files changed, 55 insertions(+), 49 deletions(-) diff --git a/src/corelib/thread/qmutex.cpp b/src/corelib/thread/qmutex.cpp index 88aee2b..6ba0a8c 100644 --- 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(d_ptr.load()); - else if (d_ptr.load()) { + QMutexData *d = d_ptr.load(); + if (quintptr(d) > 0x3 && d->recursive) { + delete static_cast(d); + } else if (d) { #ifndef Q_OS_LINUX - if (d_ptr.load()->possiblyUnlocked.load() && tryLock()) { unlock(); return; } + if (d != dummyLocked() && static_cast(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(d)->lock(timeout); + } else if (copy->recursive) { + return static_cast(copy)->lock(timeout); } + QMutexPrivate *d = static_cast(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(d)->unlock(); + if (copy->recursive) { + static_cast(copy)->unlock(); return; } + QMutexPrivate *d = reinterpret_cast(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 --- 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 d_ptr; - static inline QMutexPrivate *dummyLocked() { - return reinterpret_cast(quintptr(1)); + QBasicAtomicPointer d_ptr; + static inline QMutexData *dummyLocked() { + return reinterpret_cast(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 --- 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(quintptr(3)); + return reinterpret_cast(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 --- 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 --- 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 --- 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 --- 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() -- 2.7.4