From e400ba64a39703a87bbdbd213504bd384997a5a2 Mon Sep 17 00:00:00 2001 From: Fabian Kosmale Date: Fri, 4 Feb 2022 11:50:45 +0100 Subject: [PATCH] Use acq_rel ordering semantics in qatomic_cxx11.h ref/deref Change ref() and deref() in qatomic_cxx11.h to use acq_rel ordered semantics as documented instead of the stricter sequential consistency that was previously used implicitly. It was pointed out in the discussion on atomic reference counts that acq_rel semantics should be sufficient for reference counts. It also turned out that that is all that the definition of "Ordered" in the documentation really guarantees. Original-patch-by: Kevin Kofler Change-Id: I6cd50b34bdc699ff8b7c8fe6d303b28d71494731 Reviewed-by: Oswald Buddenhagen Reviewed-by: Thiago Macieira Reviewed-by: Marc Mutz --- src/corelib/thread/qatomic_cxx11.h | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/src/corelib/thread/qatomic_cxx11.h b/src/corelib/thread/qatomic_cxx11.h index bf487bbf1f6..0d4817249e7 100644 --- a/src/corelib/thread/qatomic_cxx11.h +++ b/src/corelib/thread/qatomic_cxx11.h @@ -278,13 +278,27 @@ template struct QAtomicOps template static inline bool ref(std::atomic &_q_value) { - return ++_q_value != 0; + /* Conceptually, we want to + * return ++_q_value != 0; + * However, that would be sequentially consistent, and thus stronger + * than what we need. Based on + * http://eel.is/c++draft/atomics.types.memop#6, we know that + * pre-increment is equivalent to fetch_add(1) + 1. Unlike + * pre-increment, fetch_add takes a memory order argument, so we can get + * the desired acquire-release semantics. + * One last gotcha is that fetch_add(1) + 1 would need to be converted + * back to T, because it's susceptible to integer promotion. To sidestep + * this issue and to avoid UB on signed overflow, we rewrite the + * expression to: + */ + return _q_value.fetch_add(1, std::memory_order_acq_rel) != T(-1); } template static inline bool deref(std::atomic &_q_value) noexcept { - return --_q_value != 0; + // compare with ref + return _q_value.fetch_sub(1, std::memory_order_acq_rel) != T(1); } static inline bool isTestAndSetNative() noexcept