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>
This commit is contained in:
Olivier Goffart 2013-04-28 13:50:47 +02:00 committed by The Qt Project
parent 7f943968ad
commit 529b648ece

View File

@ -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 {