QReadWriteLocker: Fix race in unlock

An Acquire barrier in QReadWriteLocker::unlock was missing to synchronize
with the testAndSetOrdered on d_ptr in the lock functions.

The race is between the write of d->writerCount in tryLockForWrite,
and the read in unlock. The acquire on d->mutex is not enough because
it is not on the same object. While that race could be fixed by taking
the newly-allocate()ed d's mutex before publishing it with testAndSet,
there's another race, on 'recursive', between a newly-minted Private*
with recursive == false in tryLockForWrite(), and the read of 'recursive'
in unlock().

Task-number: QTBUG-58917
Change-Id: I10ba36573c0e57468d11e9b77d85045711feaea1
Reviewed-by: Marc Mutz <marc.mutz@kdab.com>

1 files changed, 3 insertions, 3 deletions

diff --git a/src/corelib/thread/qreadwritelock.cpp b/src/corelib/thread/qreadwritelock.cpp
index 6302a3a515..42befc4b80 100644
--- a/src/corelib/thread/qreadwritelock.cpp
+++ b/src/corelib/thread/qreadwritelock.cpp
@@ -392,13 +392,13 @@ bool QReadWriteLock::tryLockForWrite(int timeout)
 */
 void QReadWriteLock::unlock()
 {
-    QReadWriteLockPrivate *d = d_ptr.load();
+    QReadWriteLockPrivate *d = d_ptr.loadAcquire();
     while (true) {
         Q_ASSERT_X(d, "QReadWriteLock::unlock()", "Cannot unlock an unlocked lock");
 
         // Fast case: no contention: (no waiters, no other readers)
         if (quintptr(d) <= 2) { // 1 or 2 (StateLockedForRead or StateLockedForWrite)
-            if (!d_ptr.testAndSetRelease(d, nullptr, d))
+            if (!d_ptr.testAndSetOrdered(d, nullptr, d))
                 continue;
             return;
         }
@@ -407,7 +407,7 @@ void QReadWriteLock::unlock()
             Q_ASSERT(quintptr(d) > (1U<<4)); //otherwise that would be the fast case
             // Just decrease the reader's count.
             auto val = reinterpret_cast<QReadWriteLockPrivate *>(quintptr(d) - (1U<<4));
-            if (!d_ptr.testAndSetRelease(d, val, d))
+            if (!d_ptr.testAndSetOrdered(d, val, d))
                 continue;
             return;
         }