summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorMarc Mutz <marc.mutz@kdab.com>2019-06-24 09:41:07 +0200
committerMarc Mutz <marc.mutz@kdab.com>2019-06-30 16:50:38 +0000
commitda38f0d691d9d7eacfac5fbcbd47b887bd59bd39 (patch)
treef051e5417202760ca4307e65a0e0cea8cfb86686
parent51bcc7e07e2bb5b42bb200dcd5269e9e9e2fe240 (diff)
QMutexPool: fix memory order of atomic operations
The array of QAtomicPointer<QMutex> can be initialized using relaxed stores of nullptr, since nullptr is the whole data. But once we store an actual QMutex pointer in the array, we need to publish the indirect data thus created. We did this, with testAndSetRelease(); what was missing was a corresponding acquire fence on load, without which there is no happens-before relationship between the writes performed by the QMutex ctor and the reads performed by a subsequent mutex.lock(), say, on the same data. Fix by adding acquire fences to all loads. That includes the dtor, since mutexes may have been created in different threads, and never been imported into this_thread before the dtor is running. As a drive-by, return a new'ed QMutex that was successfully installed directly to the caller, without again going through a load-acquire. Change-Id: Ia25d205b1127c8c4de0979cef997d1a88123c5c3 Reviewed-by: David Faure <david.faure@kdab.com> Reviewed-by: Giuseppe D'Angelo <giuseppe.dangelo@kdab.com> Reviewed-by: Thiago Macieira <thiago.macieira@intel.com> (cherry picked from commit 65b8f59e045bb41fef99b1a44f462115de65064a)
-rw-r--r--src/corelib/thread/qmutexpool.cpp9
-rw-r--r--src/corelib/thread/qmutexpool_p.h2
2 files changed, 7 insertions, 4 deletions
diff --git a/src/corelib/thread/qmutexpool.cpp b/src/corelib/thread/qmutexpool.cpp
index 90b6989467..9d7c96fd07 100644
--- a/src/corelib/thread/qmutexpool.cpp
+++ b/src/corelib/thread/qmutexpool.cpp
@@ -106,7 +106,7 @@ QMutexPool::QMutexPool(QMutex::RecursionMode recursionMode, int size)
QMutexPool::~QMutexPool()
{
for (int index = 0; index < mutexes.count(); ++index)
- delete mutexes[index].load();
+ delete mutexes[index].loadAcquire();
}
/*!
@@ -131,9 +131,12 @@ QMutex *QMutexPool::createMutex(int index)
{
// mutex not created, create one
QMutex *newMutex = new QMutex(recursionMode);
- if (!mutexes[index].testAndSetRelease(0, newMutex))
+ if (!mutexes[index].testAndSetRelease(nullptr, newMutex)) {
delete newMutex;
- return mutexes[index].load();
+ return mutexes[index].loadAcquire();
+ } else {
+ return newMutex;
+ }
}
/*!
diff --git a/src/corelib/thread/qmutexpool_p.h b/src/corelib/thread/qmutexpool_p.h
index 58d853b0e3..97e698ef75 100644
--- a/src/corelib/thread/qmutexpool_p.h
+++ b/src/corelib/thread/qmutexpool_p.h
@@ -68,7 +68,7 @@ public:
inline QMutex *get(const void *address) {
int index = uint(quintptr(address)) % mutexes.count();
- QMutex *m = mutexes[index].load();
+ QMutex *m = mutexes[index].loadAcquire();
if (m)
return m;
else