QMutex: order reads from QMutexPrivate::waiters and QBasicMutex::d_ptr in QBasicMutex::lockInternal()

Threads that unlock and lock a mutex at the same time perform the following
operations:

Thread 1                               Thread 2
--------                               --------

 QBasicMutex::lockInternal()            QBasicMutex::unlockInternal()

  d_ptr.testAndSetOrdered(..., d)        d = d_ptr.loadAcquire()

  d->waiters.loadRelaxed();         (1)
                                         d->waiters.fetchAndAddRelease() (2)
                                         d_ptr.testAndSetRelease(d, 0)   (3)
                                         d->derefWaiters()               (4)
  d->waiters.testAndSetRelaxed(...) (5)
  if (d != d_ptr.loadAcquire())     (6)

  d->wait()

The operation (1) isn't serialized with the operation (6) so its memory
effect may be observed before the effect of the operation (1). However,
if memory effects are observed in the following order: (6) -> (1) -> (2)
-> (3) -> (4) -> (5) then Thread 1 doesn't notice that Thread 2 updates
d_ptr and goes to sleep with d pointing to a stale object, this object
isn't reachable since d_ptr is zeroed so Thread 1 cannot be woken up.

The patch adds the "acquire" barrier into the operation (1) so that it
cannot be reordered with the operation (6).

Fixes: QTBUG-88247
Pick-to: 5.15 5.12
Change-Id: I1d0c405c0bf5080ec1815d351b9b4b75efeab21a
Reviewed-by: Olivier Goffart (Woboq GmbH) <ogoffart@woboq.com>
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>

1 files changed, 1 insertions, 1 deletions

diff --git a/src/corelib/thread/qmutex.cpp b/src/corelib/thread/qmutex.cpp
index 9c0d498f35..4926eae2e9 100644
--- a/src/corelib/thread/qmutex.cpp
+++ b/src/corelib/thread/qmutex.cpp
@@ -590,7 +590,7 @@ bool QBasicMutex::lockInternal(int timeout) QT_MUTEX_LOCK_NOEXCEPT
         // is set to the BigNumber magic value set in unlockInternal()
         int old_waiters;
         do {
-            old_waiters = d->waiters.loadRelaxed();
+            old_waiters = d->waiters.loadAcquire();
             if (old_waiters == -QMutexPrivate::BigNumber) {
                 // we are unlocking, and the thread that unlocks is about to change d to 0
                 // we try to acquire the mutex by changing to dummyLocked()