From 8d6d68d3d363bb6aa1faec2355b32e3ea290116c Mon Sep 17 00:00:00 2001 From: Olivier Goffart Date: Thu, 16 Feb 2017 10:27:35 +0100 Subject: 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 --- src/corelib/thread/qreadwritelock.cpp | 6 +++--- 1 file 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(quintptr(d) - (1U<<4)); - if (!d_ptr.testAndSetRelease(d, val, d)) + if (!d_ptr.testAndSetOrdered(d, val, d)) continue; return; } -- cgit v1.2.3