Testcase: ./tst_qreadwritelock heavyLoadLocks When the test run under release mode on arm64, all the spawned threads may block without this fix. When the test run with optimization enabled and assertions enabled and the assertions for !mutex.try_lock() are removed from the entry of QReadWriteLockPrivate:: lockFor{Read,Write}, random assertion failures may happen without this fix. The reason for the race is because when a lock is uncontended locked and being converted into a contended lock, no synchronization happens between the initialization of new allocated QReadWriteLockPrivate object and the use of the existing QReadWriteLockPrivate object in lockFor{Read,Write}. QReadWriteLockPrivate objects are allocated from a statically allocated freelist and it is of high probability that the newly allocated object has just been released. The possible execution order that leads to a data race is described as follows: Suppose there are three threads T1, T2, and T3, and T1 holds the write lock initially. T1 first releases the lock, and then gains the read lock, while T2 tries to gain the write lock, and T3 tries to gain the read lock. The interleaved execution order is as follows, where <- means a normal memory write, <1> means a memory address of a QReadWriteLockPrivate object, : means a return value, #n means a synchronization point. For abberviation, wc denotes writerCount and rc denotes readerCount. The .h/.c and the number in the parentheses denotes the line number in qreadwritelock.h/.cpp. T2 T1 T3 unlock() lockForRead() d = d_ptr.loadRelaxed(): <1> (.h 52) d = d_ptr.loadRelaxed(): <1> (.h 52) <1>->mutex.lock() (.c 393) d = d_ptr.loadAcquire(): <1> (.c 229) <1> <-{wc = 0}(rc should be 0) (.c 397) <1>->mutex.lock() ... (.c 236) d_ptr.storeRelease(null) #1 (.c 409) <1>->release() (.c 410) <1>->mutex.unlock() #2 (.c 412) lockForRead() <1>->mutex.lock() returns #2 d = d_ptr.loadRelaxed(): null (.h 93) lockForWrite() d_ptr.testAndSetAcquire(1) #3 (.h 81) d = d_ptr.loadRelaxed(): 1 (.h 116) val = allocate -> <1> (.c 321) // ^ suppose <1> is reused here <1> <-{rc = 1}(wc should be 0) (.c 325) d_ptr.testAndSetOrdered(<1>) #5 (.c 326) d = d_ptr.loadAcquire(): <1> #6 (.c 335) d_ptr.loadRelaxed(): <1> (.c 237) <1>->mutex.lock() ... (.c 342) // Here T3 sees the d_ptr load result // as <1>, which is the same as // before, thinking it unchanged and // thus continues to execute // d->lockForRead(). // T3 here has no synchronization T2, // but had synchronization with T1 at // #2. So T3 may see the stale data // previous written by T1 to <1>, i.e. // wc = 0, rc = 0 <1> <-{rc = 1} (.c 432) <1>->mutex.unlock() #4 (.c 248) <1>->mutex.lock() returns #4 d_ptr.loadRelaxed(): <1> (.c 343) // The same happens to T2 here, it continues // to execute d->lockForWrite(). // T2 here is synchronized with T3 at #4, // so T2 must see the data written by T3 // to <1>, i.e. wc = 0, rc = 1 <1>->writerCond.wait() (.c 455) After the above interleaved execution, T2 is blocked while T3 and T1 are holding the read lock, but in the QReadWriteLockPrivate object, the readerCount is 1, which is incorrect. This might further lead to deadlock if readerCount becomes -1 after the two readers release the lock or letting a writer to proceed when only one of the readers releases the lock. The fix changes the relaxed load of d_ptr in lockFor{Read,Write} after the acquire of the mutex to an acquire load, to establish synchronization with the release store of d_ptr when converting from an uncontended lock to a contended lock. Fixes: QTBUG-142321 Change-Id: I5a570471b52359dd65f309e644d9aacfd58ce943 Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>