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>

2 files changed, 7 insertions, 4 deletions

diff --git a/src/corelib/thread/qmutexpool.cpp b/src/corelib/thread/qmutexpool.cpp
index 2a02197859..3ece30c01c 100644
--- a/src/corelib/thread/qmutexpool.cpp
+++ b/src/corelib/thread/qmutexpool.cpp
@@ -104,7 +104,7 @@ QMutexPool::QMutexPool(QMutex::RecursionMode recursionMode, int size)
 QMutexPool::~QMutexPool()
 {
     for (int index = 0; index < mutexes.count(); ++index)
-        delete mutexes[index].loadRelaxed();
+        delete mutexes[index].loadAcquire();
 }
 
 /*!
@@ -129,9 +129,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].loadRelaxed();
+        return mutexes[index].loadAcquire();
+    } else {
+        return newMutex;
+    }
 }
 
 /*!
diff --git a/src/corelib/thread/qmutexpool_p.h b/src/corelib/thread/qmutexpool_p.h
index 1a47231abc..00710199b8 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].loadRelaxed();
+        QMutex *m = mutexes[index].loadAcquire();
         if (m)
             return m;
         else