From a57f89251c35f8c210c04ae077fb7a78b3af2a1c Mon Sep 17 00:00:00 2001 From: Marc Mutz Date: Tue, 23 Jan 2024 16:44:23 +0100 Subject: [PATCH] QGlobalStatic: fix atomics in ~Holder Back when we set the guard and _then_ destroyed the object, the atomic_thread_fence was correctly placed between the guard setting and the dtor invocation. We know nothing about the payload type. For all we know it could be some fancy lockless data structure. So in general we need the acquire fence to synchronize the reads done by the dtor with stores performed to the object in other threads. The acquire fence did that for us. But when we reversed the two operations, the fence has moved to after the dtor call, leaving the dtor unprotected. Move it back to before the dtor call. That leaves the store to the guard unordered w.r.t. the execution of the destructor. To fix, make it a store-release instead. Amends e3e2674100b1ecbad7117f15c7aa13a704a7d34e. Pick-to: 6.6 6.5 Change-Id: Ic7adbe17c8b474e82af4e84950c0109984b495d1 Reviewed-by: Thiago Macieira (cherry picked from commit b6537a9513d274866da2f46827f54e6004196097) Reviewed-by: Qt Cherry-pick Bot --- src/corelib/global/qglobalstatic.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/corelib/global/qglobalstatic.h b/src/corelib/global/qglobalstatic.h index 7ed85ba41a7..c7fbba69e81 100644 --- a/src/corelib/global/qglobalstatic.h +++ b/src/corelib/global/qglobalstatic.h @@ -40,9 +40,10 @@ template union Holder ~Holder() { + // import changes to *pointer() by other threads before running ~PlainType(): + std::atomic_thread_fence(std::memory_order_acquire); pointer()->~PlainType(); - std::atomic_thread_fence(std::memory_order_acquire); // avoid mixing stores to guard and *pointer() - guard.storeRelaxed(QtGlobalStatic::Destroyed); + guard.storeRelease(QtGlobalStatic::Destroyed); } PlainType *pointer() noexcept