src/fcatomic.h | 34 +++++++++++++++++++--------------- src/fccache.c | 4 +++- src/fccfg.c | 5 +++-- 3 files changed, 25 insertions(+), 18 deletions(-) New commits: commit 447b9ccc7d03bf953d1f1c4708ca16c56c1511ec Author: Ben Wagner <bungeman@xxxxxxxxxxxx> Date: Thu Oct 29 22:29:00 2020 -0400 Fix fc_atomic_ptr_get and use. Before this change building with ThreadSanitizer and running test/test-pthread generated a large number of threading issues. These mostly stemmed from fc_atomic_ptr_get not doing an atomic load and using "acquire load" instead of "load acquire". After making these changes it was still necessary to use fc_atomic_ptr_get where it was needed. This also documents the current memory barrier requirements for the atomic primitives. diff --git a/src/fcatomic.h b/src/fcatomic.h index ba45b6c..27bf6ca 100644 --- a/src/fcatomic.h +++ b/src/fcatomic.h @@ -43,28 +43,23 @@ #if 0 +typedef <type> fc_atomic_int_t; +#define FC_ATOMIC_INT_FORMAT "<printf format for fc_atomic_int_t>" +#define fc_atomic_int_add(AI, V) o = (AI), (AI) += (V), o // atomic acquire/release + +#define fc_atomic_ptr_get(P) *(P) // atomic acquire +#define fc_atomic_ptr_cmpexch(P,O,N) *(P) == (O) ? (*(P) = (N), FcTrue) : FcFalse // atomic release + #elif !defined(FC_NO_MT) && defined(_MSC_VER) || defined(__MINGW32__) #include "fcwindows.h" -/* MinGW has a convoluted history of supporting MemoryBarrier - * properly. As such, define a function to wrap the whole - * thing. */ -static inline void _FCMemoryBarrier (void) { -#if !defined(MemoryBarrier) - long dummy = 0; - InterlockedExchange (&dummy, 1); -#else - MemoryBarrier (); -#endif -} - typedef LONG fc_atomic_int_t; #define FC_ATOMIC_INT_FORMAT "ld" #define fc_atomic_int_add(AI, V) InterlockedExchangeAdd (&(AI), (V)) -#define fc_atomic_ptr_get(P) (_FCMemoryBarrier (), (void *) *(P)) +#define fc_atomic_ptr_get(P) (InterlockedCompareExchangePointerAcquire ((void **) (P), NULL, NULL)) #define fc_atomic_ptr_cmpexch(P,O,N) (InterlockedCompareExchangePointer ((void **) (P), (void *) (N), (void *) (O)) == (void *) (O)) @@ -77,20 +72,29 @@ typedef int fc_atomic_int_t; #define FC_ATOMIC_INT_FORMAT "d" #define fc_atomic_int_add(AI, V) (OSAtomicAdd32Barrier ((V), &(AI)) - (V)) -#define fc_atomic_ptr_get(P) (OSMemoryBarrier (), (void *) *(P)) #if (MAC_OS_X_VERSION_MIN_REQUIRED > MAC_OS_X_VERSION_10_4 || __IPHONE_OS_VERSION_MIN_REQUIRED >= 20100) + +#if SIZEOF_VOID_P == 8 +#define fc_atomic_ptr_get(P) OSAtomicAdd64Barrier (0, (int64_t*)(P)) +#elif SIZEOF_VOID_P == 4 +#define fc_atomic_ptr_get(P) OSAtomicAdd32Barrier (0, (int32_t*)(P)) +#else +#error "SIZEOF_VOID_P not 4 or 8 (assumes CHAR_BIT is 8)" +#endif #define fc_atomic_ptr_cmpexch(P,O,N) OSAtomicCompareAndSwapPtrBarrier ((void *) (O), (void *) (N), (void **) (P)) + #else #error "Your macOS / iOS targets are too old" #endif + #elif !defined(FC_NO_MT) && defined(HAVE_INTEL_ATOMIC_PRIMITIVES) typedef int fc_atomic_int_t; #define FC_ATOMIC_INT_FORMAT "d" #define fc_atomic_int_add(AI, V) __sync_fetch_and_add (&(AI), (V)) -#define fc_atomic_ptr_get(P) (void *) (__sync_synchronize (), *(P)) +#define fc_atomic_ptr_get(P) (void *) (__sync_fetch_and_add ((P), 0)) #define fc_atomic_ptr_cmpexch(P,O,N) __sync_bool_compare_and_swap ((P), (O), (N)) diff --git a/src/fccache.c b/src/fccache.c index 1215154..b2392d3 100644 --- a/src/fccache.c +++ b/src/fccache.c @@ -509,7 +509,9 @@ retry: static void unlock_cache (void) { - FcMutexUnlock (cache_lock); + FcMutex *lock; + lock = fc_atomic_ptr_get (&cache_lock); + FcMutexUnlock (lock); } static void diff --git a/src/fccfg.c b/src/fccfg.c index 1c62cad..7b857bf 100644 --- a/src/fccfg.c +++ b/src/fccfg.c @@ -67,14 +67,15 @@ retry: static void unlock_config (void) { - FcMutexUnlock (_lock); + FcMutex *lock; + lock = fc_atomic_ptr_get (&_lock); + FcMutexUnlock (lock); } static void free_lock (void) { FcMutex *lock; - lock = fc_atomic_ptr_get (&_lock); if (lock && fc_atomic_ptr_cmpexch (&_lock, lock, NULL)) { _______________________________________________ Fontconfig mailing list Fontconfig@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/fontconfig