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.

3 files changed, 25 insertions, 18 deletions

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))
     {