AuroraMiddleware/qt5base-lts
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

QApplicationStatic: enforce acquire-release semantics on creation

Browse Source 
On systems with weak memory ordering, it was possible for the
storeRelaxed(Initialized) to be observed by another thread performing a
loadRelaxed() without observing the contents of the object itself. The
mutex *does* release the contents of the object to memory, but without
the corresponding mutex acquisition, we couldn't guarantee that the
object's contents would be observed. Now we can.

We don't need to fix the load inside the mutex because the mutex will
have acquired everything from either a previous call to pointer() or to
reset(). The store inside reset() need not be storeRelease() either
because the effect of observing the Uninitialized state will be to lock
the mutex.

None of this is used to protect the data as it is being mutated by the
user in multiple threads, or their access simultaneously with reset()
(which is why the load outside the mutex was removed).

Thanks to litb on Slack for noticing this and bringing to my attention.

Pick-to: 6.5
Change-Id: Idd5e1bb52be047d7b4fffffd1752df5b4d9b2e3f
Reviewed-by: Marc Mutz <marc.mutz@qt.io>
This commit is contained in:
Thiago Macieira 2023-04-04 20:33:14 -03:00
parent 207aae5560
commit af95ec4b7b
1 changed files with 8 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
src/corelib/kernel/qapplicationstatic.h
View File

@ -30,7 +30,8 @@ template <typename QAS> struct ApplicationHolder
Q_DISABLE_COPY_MOVE(ApplicationHolder)
~ApplicationHolder()
{
if (guard.loadRelaxed() == QtGlobalStatic::Initialized) {
if (guard.loadAcquire() == QtGlobalStatic::Initialized) {
// No mutex! Up to external code to ensure no race happens.
guard.storeRelease(QtGlobalStatic::Destroyed);
realPointer()->~PlainType();
}
@ -44,24 +45,23 @@ template <typename QAS> struct ApplicationHolder
// called from QGlobalStatic::instance()
PlainType *pointer() noexcept(MutexLockIsNoexcept && ConstructionIsNoexcept)
{
if (guard.loadRelaxed() == QtGlobalStatic::Initialized)
if (guard.loadAcquire() == QtGlobalStatic::Initialized)
return realPointer();
QMutexLocker locker(&mutex);
if (guard.loadRelaxed() == QtGlobalStatic::Uninitialized) {
QAS::innerFunction(&storage);
QObject::connect(QCoreApplication::instance(), &QObject::destroyed, reset);
guard.storeRelaxed(QtGlobalStatic::Initialized);
guard.storeRelease(QtGlobalStatic::Initialized);
}
return realPointer();
}
static void reset()
{
if (guard.loadRelaxed() == QtGlobalStatic::Initialized) {
QMutexLocker locker(&mutex);
realPointer()->~PlainType();
guard.storeRelaxed(QtGlobalStatic::Uninitialized);
}
// we only synchronize using the mutex here, not the guard
QMutexLocker locker(&mutex);
realPointer()->~PlainType();
guard.storeRelaxed(QtGlobalStatic::Uninitialized);
}
};
} // namespace QtGlobalStatic