summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorAlexander Kartashov <regmka@gmail.com>2020-11-06 16:37:11 +0200
committerAlexander Kartashov <regmka@gmail.com>2020-11-18 07:07:54 +0200
commit940414aff1de8b963edf064b864a05c5cdfb5094 (patch)
treecbb1b5abf6f57775d3d42d625a0a48433b70ce16
parenta9922920c6cb0c0c96079d7b917ab15da04f1cad (diff)
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 Change-Id: I1d0c405c0bf5080ec1815d351b9b4b75efeab21a Reviewed-by: Olivier Goffart (Woboq GmbH) <ogoffart@woboq.com> Reviewed-by: Thiago Macieira <thiago.macieira@intel.com> (cherry picked from commit d08e3b6de16118becaada17a58aed4042f400a5a)
-rw-r--r--src/corelib/thread/qmutex.cpp2
1 files changed, 1 insertions, 1 deletions
diff --git a/src/corelib/thread/qmutex.cpp b/src/corelib/thread/qmutex.cpp
index 3881ac017e..c338ebc907 100644
--- a/src/corelib/thread/qmutex.cpp
+++ b/src/corelib/thread/qmutex.cpp
@@ -541,7 +541,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.load();
+ 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()