diff options
author | Olivier Goffart <ogoffart@woboq.com> | 2013-04-28 13:50:47 +0200 |
---|---|---|
committer | The Qt Project <gerrit-noreply@qt-project.org> | 2013-04-29 11:15:35 +0200 |
commit | 529b648ece265f30784dadb96624e94b4ced5aba (patch) | |
tree | 26e722ce6e6831b0acd5ca393f9d62bae94c6b20 /src/corelib | |
parent | 7f943968ade6a65321d4a00822f5b3a034a19e0c (diff) |
Fix possible race in QMutex
As described in the QTBUG-30872, there may be a race condition involving
3 threads fighting for a mutex. I am surprised it was not caught
before despite all the Q_ASSERT and the stress test in tst_qmutex.
We do not need to call store(0) because the unlocking thread will
eventually remove the BigNumber flag. And perhaps it even did it
already, and another thread has incremented waiters (hence the Q_ASSERT
is wrong)
Here is a paste of part of the description from the bug report:
---
many threads, one of them is ready to release mutex, while at least two other trying to acquire it
d->waiters is 0
Thread 1 release mutex in unlockInternal:
if (d->waiters.fetchAndAddRelease(-QMutexPrivate::BigNumber) == 0)
d->waiters is now -QMutexPrivate::BigNumber
Thread 2 try to acquire mutex in lockInternal:
old_waiters = d->waiters.load();
if (old_waiters == -QMutexPrivate::BigNumber) {
if (d_ptr.testAndSetAcquire(d, dummyLocked())) {
It acquire 'about to release mutex' by changing d to dummyLocked
Thread 1 continue release procedure:
d->derefWaiters(0);
d->waiters is now back to 0
Thread 3 try to acquire mutex in lockInternal:
while (!d->waiters.testAndSetRelaxed(old_waiters, old_waiters + 1));
d->waiters is now 1
Thread 2 continue its dummy lock:
d->waiters.store(0);
d->waiters is force to 0
Thread 3 continue wait procedure
but it realize that mutex was already unlocked so decrease back waiters
if (d != d_ptr.loadAcquire()) {
if (old_waiters != QMutexPrivate::BigNumber) {
d->waiters.deref();
d->waiters became negative value of -1
Neither thread need internal data so it is released back to pool
The waiters counter in released internal structure is still -1
---
Change-Id: I1b22555db764482775db6e64a8c9ffa9e1ab0cf6
Task-number: QTBUG-30872
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
Diffstat (limited to 'src/corelib')
-rw-r--r-- | src/corelib/thread/qmutex.cpp | 2 |
1 files changed, 0 insertions, 2 deletions
diff --git a/src/corelib/thread/qmutex.cpp b/src/corelib/thread/qmutex.cpp index 45e52d7b85..1ed4a77950 100644 --- a/src/corelib/thread/qmutex.cpp +++ b/src/corelib/thread/qmutex.cpp @@ -467,8 +467,6 @@ bool QBasicMutex::lockInternal(int timeout) QT_MUTEX_LOCK_NOEXCEPT // we try to acquire the mutex by changing to dummyLocked() if (d_ptr.testAndSetAcquire(d, dummyLocked())) { // Mutex acquired - Q_ASSERT(d->waiters.load() == -QMutexPrivate::BigNumber || d->waiters.load() == 0); - d->waiters.store(0); d->deref(); return true; } else { |