diff options
| -rw-r--r-- | src/corelib/thread/qmutex.cpp | 49 | ||||
| -rw-r--r-- | src/corelib/thread/qmutex_p.h | 9 |
2 files changed, 54 insertions, 4 deletions
diff --git a/src/corelib/thread/qmutex.cpp b/src/corelib/thread/qmutex.cpp index 2bf3176787..745455dc3f 100644 --- a/src/corelib/thread/qmutex.cpp +++ b/src/corelib/thread/qmutex.cpp @@ -2,6 +2,7 @@ ** ** Copyright (C) 2012 Nokia Corporation and/or its subsidiary(-ies). ** Copyright (C) 2012 Intel Corporation +** Copyright (C) 2012 Olivier Goffart <ogoffart@woboq.com> ** Contact: http://www.qt-project.org/ ** ** This file is part of the QtCore module of the Qt Toolkit. @@ -371,6 +372,27 @@ bool QBasicMutex::isRecursive() */ #ifndef QT_LINUX_FUTEX //linux implementation is in qmutex_linux.cpp + +/* + For a rough introduction on how this works, refer to + http://woboq.com/blog/internals-of-qmutex-in-qt5.html + which explains a slightly simplified version of it. + The differences are that here we try to work with timeout (requires the + possiblyUnlocked flag) and that we only wake one thread when unlocking + (requires maintaining the waiters count) + We also support recursive mutexes which always have a valid d_ptr. + + The waiters flag represents the number of threads that are waiting or about + to wait on the mutex. There are two tricks to keep in mind: + We don't want to increment waiters after we checked no threads are waiting + (waiters == 0). That's why we atomically set the BigNumber flag on waiters when + we check waiters. Similarily, if waiters is decremented right after we checked, + the mutex would be unlocked (d->wakeUp() has (or will) be called), but there is + no thread waiting. This is only happening if there was a timeout in tryLock at the + same time as the mutex is unlocked. So when there was a timeout, we set the + possiblyUnlocked flag. +*/ + /*! \internal helper for lock() */ @@ -394,6 +416,8 @@ bool QBasicMutex::lockInternal(int timeout) QT_MUTEX_LOCK_NOEXCEPT if (copy == dummyLocked()) { if (timeout == 0) return false; + // The mutex is locked but does not have a QMutexPrivate yet. + // we need to allocate a QMutexPrivate QMutexPrivate *newD = QMutexPrivate::allocate(); if (!d_ptr.testAndSetOrdered(dummyLocked(), newD)) { //Either the mutex is already unlocked, or another thread already set it. @@ -408,15 +432,25 @@ bool QBasicMutex::lockInternal(int timeout) QT_MUTEX_LOCK_NOEXCEPT if (timeout == 0 && !d->possiblyUnlocked.load()) return false; + // At this point we have a pointer to a QMutexPrivate. But the other thread + // may unlock the mutex at any moment and release the QMutexPrivate to the pool. + // We will try to reference it to avoid unlock to release it to the pool to make + // sure it won't be released. But if the refcount is already 0 it has been released. if (!d->ref()) continue; //that QMutexData was already released + // We now hold a reference to the QMutexPrivate. It won't be released and re-used. + // But it is still possible that it was already re-used by another QMutex right before + // we did the ref(). So check if we still hold a pointer to the right mutex. if (d != d_ptr.loadAcquire()) { //Either the mutex is already unlocked, or relocked with another mutex d->deref(); continue; } + // In this part, we will try to increment the waiters count. + // We just need to take care of the case in which the old_waiters + // is set to the BigNumber magic value set in unlockInternal() int old_waiters; do { old_waiters = d->waiters.load(); @@ -440,7 +474,7 @@ bool QBasicMutex::lockInternal(int timeout) QT_MUTEX_LOCK_NOEXCEPT } while (!d->waiters.testAndSetRelaxed(old_waiters, old_waiters + 1)); if (d != d_ptr.loadAcquire()) { - // Mutex was unlocked. + // The mutex was unlocked before we incremented waiters. if (old_waiters != QMutexPrivate::BigNumber) { //we did not break the previous loop Q_ASSERT(d->waiters.load() >= 1); @@ -451,6 +485,7 @@ bool QBasicMutex::lockInternal(int timeout) QT_MUTEX_LOCK_NOEXCEPT } if (d->wait(timeout)) { + // reset the possiblyUnlocked flag if needed (and deref its corresponding reference) if (d->possiblyUnlocked.load() && d->possiblyUnlocked.testAndSetRelaxed(true, false)) d->deref(); d->derefWaiters(1); @@ -463,8 +498,12 @@ bool QBasicMutex::lockInternal(int timeout) QT_MUTEX_LOCK_NOEXCEPT d->derefWaiters(1); //There may be a race in which the mutex is unlocked right after we timed out, // and before we deref the waiters, so maybe the mutex is actually unlocked. - if (!d->possiblyUnlocked.testAndSetRelaxed(false, true)) + // Set the possiblyUnlocked flag to indicate this possibility. + if (!d->possiblyUnlocked.testAndSetRelaxed(false, true)) { + // We keep a reference when possiblyUnlocked is true. + // but if possiblyUnlocked was already true, we don't need to keep the reference. d->deref(); + } return false; } } @@ -484,9 +523,15 @@ void QBasicMutex::unlockInternal() Q_DECL_NOTHROW QMutexPrivate *d = reinterpret_cast<QMutexPrivate *>(copy); + // If no one is waiting for the lock anymore, we shoud reset d to 0x0. + // Using fetchAndAdd, we atomically check that waiters was equal to 0, and add a flag + // to the waiters variable (BigNumber). That way, we avoid the race in which waiters is + // incremented right after we checked, because we won't increment waiters if is + // equal to -BigNumber 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)) { + // reset the possiblyUnlocked flag if needed (and deref its corresponding reference) if (d->possiblyUnlocked.load() && d->possiblyUnlocked.testAndSetRelaxed(true, false)) d->deref(); } diff --git a/src/corelib/thread/qmutex_p.h b/src/corelib/thread/qmutex_p.h index 6d84732751..5ae1ab7c80 100644 --- a/src/corelib/thread/qmutex_p.h +++ b/src/corelib/thread/qmutex_p.h @@ -2,6 +2,7 @@ ** ** Copyright (C) 2012 Nokia Corporation and/or its subsidiary(-ies). ** Copyright (C) 2012 Intel Corporation +** Copyright (C) 2012 Olivier Goffart <ogoffart@woboq.com> ** Contact: http://www.qt-project.org/ ** ** This file is part of the QtCore module of the Qt Toolkit. @@ -113,8 +114,12 @@ public: void release(); static QMutexPrivate *allocate(); - QAtomicInt waiters; //number of thread waiting - QAtomicInt possiblyUnlocked; //bool saying that a timed wait timed out + QAtomicInt waiters; // Number of threads waiting on this mutex. (may be offset by -BigNumber) + QAtomicInt possiblyUnlocked; /* Boolean indicating that a timed wait timed out. + When it is true, a reference is held. + It is there to avoid a race that happens if unlock happens right + when the mutex is unlocked. + */ enum { BigNumber = 0x100000 }; //Must be bigger than the possible number of waiters (number of threads) void derefWaiters(int value) Q_DECL_NOTHROW; |