Commit 814de0bcab99f82dc637ba7ae34df5a0e9e1138c

Russell Belfer 2013-07-11T11:00:41

Update git__swap thread helper This makes git__swap use the __sync_lock_test_and_set primitive with GCC and the InterlockedExchangePointer primitive with MSVC. Previously is used compare_and_swap in a way that was probably unintuitive for most thinking (i.e. it could fail to swap in the value if another thread raced in). Now it will always succeed and the last thread to run in a race will win instead of the first thread. This also fixes up a little confusion between volatile void ** and void * volatile * that came up with the Win32 compiler.

diff --git a/src/thread-utils.h b/src/thread-utils.h
index f8c4dc6..04e0295 100644
--- a/src/thread-utils.h
+++ b/src/thread-utils.h
@@ -38,16 +38,6 @@ typedef git_atomic git_atomic_ssize;
 
 #endif
 
-GIT_INLINE(void) git_atomic_set(git_atomic *a, int val)
-{
-	a->val = val;
-}
-
-GIT_INLINE(int) git_atomic_get(git_atomic *a)
-{
-	return (int)a->val;
-}
-
 #ifdef GIT_THREADS
 
 #define git_thread pthread_t
@@ -71,6 +61,17 @@ GIT_INLINE(int) git_atomic_get(git_atomic *a)
 #define git_cond_signal(c)	pthread_cond_signal(c)
 #define git_cond_broadcast(c)	pthread_cond_broadcast(c)
 
+GIT_INLINE(void) git_atomic_set(git_atomic *a, int val)
+{
+#if defined(GIT_WIN32)
+	InterlockedExchange(&a->val, (LONG)val);
+#elif defined(__GNUC__)
+	__sync_lock_test_and_set(&a->val, val);
+#else
+#	error "Unsupported architecture for atomic operations"
+#endif
+}
+
 GIT_INLINE(int) git_atomic_inc(git_atomic *a)
 {
 #if defined(GIT_WIN32)
@@ -105,7 +106,7 @@ GIT_INLINE(int) git_atomic_dec(git_atomic *a)
 }
 
 GIT_INLINE(void *) git___compare_and_swap(
-	volatile void **ptr, void *oldval, void *newval)
+	void * volatile *ptr, void *oldval, void *newval)
 {
 	volatile void *foundval;
 #if defined(GIT_WIN32)
@@ -118,6 +119,16 @@ GIT_INLINE(void *) git___compare_and_swap(
 	return (foundval == oldval) ? oldval : newval;
 }
 
+GIT_INLINE(volatile void *) git___swap(
+	void * volatile *ptr, void *newval)
+{
+#if defined(GIT_WIN32)
+	return InterlockedExchangePointer(ptr, newval);
+#else
+	return __sync_lock_test_and_set(ptr, newval);
+#endif
+}
+
 #ifdef GIT_ARCH_64
 
 GIT_INLINE(int64_t) git_atomic64_add(git_atomic64 *a, int64_t addend)
@@ -156,6 +167,11 @@ GIT_INLINE(int64_t) git_atomic64_add(git_atomic64 *a, int64_t addend)
 #define git_cond_signal(c) (void)0
 #define git_cond_broadcast(c) (void)0
 
+GIT_INLINE(void) git_atomic_set(git_atomic *a, int val)
+{
+	a->val = val;
+}
+
 GIT_INLINE(int) git_atomic_inc(git_atomic *a)
 {
 	return ++a->val;
@@ -173,7 +189,7 @@ GIT_INLINE(int) git_atomic_dec(git_atomic *a)
 }
 
 GIT_INLINE(void *) git___compare_and_swap(
-	volatile void **ptr, void *oldval, void *newval)
+	void * volatile *ptr, void *oldval, void *newval)
 {
 	if (*ptr == oldval)
 		*ptr = newval;
@@ -182,6 +198,14 @@ GIT_INLINE(void *) git___compare_and_swap(
 	return oldval;
 }
 
+GIT_INLINE(volatile void *) git___swap(
+	void * volatile *ptr, void *newval)
+{
+	volatile void *old = *ptr;
+	*ptr = newval;
+	return old;
+}
+
 #ifdef GIT_ARCH_64
 
 GIT_INLINE(int64_t) git_atomic64_add(git_atomic64 *a, int64_t addend)
@@ -194,13 +218,18 @@ GIT_INLINE(int64_t) git_atomic64_add(git_atomic64 *a, int64_t addend)
 
 #endif
 
+GIT_INLINE(int) git_atomic_get(git_atomic *a)
+{
+	return (int)a->val;
+}
+
 /* Atomically replace oldval with newval
  * @return oldval if it was replaced or newval if it was not
  */
 #define git__compare_and_swap(P,O,N) \
-	git___compare_and_swap((volatile void **)P, O, N)
+	git___compare_and_swap((void * volatile *)P, O, N)
 
-#define git__swap(ptr, val) git__compare_and_swap(&ptr, ptr, val)
+#define git__swap(ptr, val) (void *)git___swap((void * volatile *)&ptr, val)
 
 extern int git_online_cpus(void);