We are currently migrating Bugzilla to GitHub issues.
Any changes made to the bug tracker now will be lost, so please do not post new bugs or make changes to them.
When we're done, all bug URLs will redirect to their equivalent location on the new bug tracker.

[patch] Fix SDL pthread-based atomics support

SDL_pthread.patch (text/plain), 13.74 KB, created by binarycrusader on 2014-06-30 20:32:22 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: binarycrusader

Created: 2014-06-30 20:32:22 UTC

Size: 13.74 KB

patch

obsolete

># HG changeset patch
># User Shawn Walker <eviltypeguy@icculus.org>
># Date 1404160188 25200
>#      Mon Jun 30 13:29:48 2014 -0700
># Node ID 638b7c0fa2fec3835dd4a9e97a42634c659bdf71
># Parent  8ca9ce569f65d3c4c5ff3e7867ad71e88640f5d5
>Fix SDL pthread-based atomics support
>
>diff --git a/include/SDL_atomic.h b/include/SDL_atomic.h
>--- a/include/SDL_atomic.h
>+++ b/include/SDL_atomic.h
>@@ -31,8 +31,10 @@
>  * with full mutexes.
>  *
>  * The list of "safe" functions to use are:
>+ *  SDL_AtomicLockInit()
>  *  SDL_AtomicLock()
>  *  SDL_AtomicUnlock()
>+ *  SDL_AtomicLockDestroy()
>  *  SDL_AtomicIncRef()
>  *  SDL_AtomicDecRef()
>  *
>@@ -86,10 +88,35 @@
>  */
> /* @{ */
> 
>+#if (!(SDL_ATOMIC_DISABLED || \
>+    defined(_MSC_VER) || \
>+    (HAVE_GCC_ATOMICS || HAVE_GCC_SYNC_LOCK_TEST_AND_SET) || \
>+    (defined(__GNUC__) && defined(__arm__) && \
>+        (defined(__ARM_ARCH_4__) || defined(__ARM_ARCH_4T__) || \
>+         defined(__ARM_ARCH_5__) || defined(__ARM_ARCH_5TE__) || \
>+         defined(__ARM_ARCH_5TEJ__))) || \
>+    (defined(__GNUC__) && defined(__arm__)) || \
>+    (defined(__GNUC__) && (defined(__i386__) || defined(__x86_64__))) || \
>+    (defined(__MACOSX__) || defined(__IPHONEOS__))) && \
>+    HAVE_PTHREAD_SPINLOCK)
>+typedef pthread_spinlock_t SDL_SpinLock;
>+#else
> typedef int SDL_SpinLock;
>+#endif
> 
> /**
>- * \brief Try to lock a spin lock by setting it to a non-zero value.
>+ * \brief Initialize a spin lock; must be called before using any of the
>+ * lock functions on a lock.
>+ *
>+ * \param lock Points to the lock.
>+ *
>+ * \return SDL_TRUE if the init succeeded, SDL_FALSE if the init failed.
>+ */
>+extern DECLSPEC SDL_bool SDLCALL SDL_AtomicLockInit(SDL_SpinLock *lock);
>+
>+/**
>+ * \brief Try to lock an initialized spin lock by setting it to a non-zero
>+ * value.
>  *
>  * \param lock Points to the lock.
>  *
>@@ -98,19 +125,28 @@
> extern DECLSPEC SDL_bool SDLCALL SDL_AtomicTryLock(SDL_SpinLock *lock);
> 
> /**
>- * \brief Lock a spin lock by setting it to a non-zero value.
>+ * \brief Lock an initialized spin lock by setting it to a non-zero value.
>  *
>  * \param lock Points to the lock.
>  */
> extern DECLSPEC void SDLCALL SDL_AtomicLock(SDL_SpinLock *lock);
> 
> /**
>- * \brief Unlock a spin lock by setting it to 0. Always returns immediately
>+ * \brief Unlock an initialized spin lock by setting it to 0. Always returns
>+ * immediately.
>  *
>  * \param lock Points to the lock.
>  */
> extern DECLSPEC void SDLCALL SDL_AtomicUnlock(SDL_SpinLock *lock);
> 
>+/**
>+ * \brief Destroy a spin lock; should be called after a lock is no longer in
>+ * use.
>+ *
>+ * \param lock Points to the lock.
>+ */
>+extern DECLSPEC void SDLCALL SDL_AtomicLockDestroy(SDL_SpinLock *lock);
>+
> /* @} *//* SDL AtomicLock */
> 
> 
>@@ -126,7 +162,7 @@
> #define SDL_CompilerBarrier()   __asm__ __volatile__ ("" : : : "memory")
> #else
> #define SDL_CompilerBarrier()   \
>-{ SDL_SpinLock _tmp = 0; SDL_AtomicLock(&_tmp); SDL_AtomicUnlock(&_tmp); }
>+{ SDL_SpinLock _tmp; SDL_AtomicLockInit(&_tmp); SDL_AtomicLock(&_tmp); SDL_AtomicUnlock(&_tmp); SDL_AtomicLockDestroy(&_tmp); }
> #endif
> 
> /**
>diff --git a/src/SDL_assert.c b/src/SDL_assert.c
>--- a/src/SDL_assert.c
>+++ b/src/SDL_assert.c
>@@ -264,9 +264,14 @@
>                     int line)
> {
>     static int assertion_running = 0;
>-    static SDL_SpinLock spinlock = 0;
>+    static SDL_bool already_initialized = SDL_FALSE;
>+    static SDL_SpinLock spinlock;
>     SDL_assert_state state = SDL_ASSERTION_IGNORE;
> 
>+    if (!already_initialized) {
>+        already_initialized = SDL_TRUE;
>+        SDL_AtomicLockInit(&spinlock);
>+    }
>     SDL_AtomicLock(&spinlock);
>     if (assertion_mutex == NULL) { /* never called SDL_Init()? */
>         assertion_mutex = SDL_CreateMutex();
>diff --git a/src/atomic/SDL_atomic.c b/src/atomic/SDL_atomic.c
>--- a/src/atomic/SDL_atomic.c
>+++ b/src/atomic/SDL_atomic.c
>@@ -64,8 +64,16 @@
> static SDL_INLINE void
> enterLock(void *a)
> {
>+    static SDL_bool already_initialized = SDL_FALSE;
>     uintptr_t index = ((((uintptr_t)a) >> 3) & 0x1f);
> 
>+    if (!already_initialized) {
>+	int i;
>+        already_initialized = SDL_TRUE;
>+        for (i = 0; i < (sizeof(locks) / sizeof(locks[0])); i++) {
>+            SDL_AtomicLockInit(&locks[i]);
>+        }
>+    }
>     SDL_AtomicLock(&locks[index]);
> }
> 
>diff --git a/src/atomic/SDL_spinlock.c b/src/atomic/SDL_spinlock.c
>--- a/src/atomic/SDL_spinlock.c
>+++ b/src/atomic/SDL_spinlock.c
>@@ -24,10 +24,40 @@
> #include "../core/windows/SDL_windows.h"
> #endif
> 
>+#if HAVE_PTHREAD_SPINLOCK
>+#include <pthread.h>
>+#endif
>+
> #include "SDL_atomic.h"
> #include "SDL_mutex.h"
> #include "SDL_timer.h"
> 
>+SDL_bool
>+SDL_AtomicLockInit(SDL_SpinLock *lock)
>+{
>+#if (SDL_ATOMIC_DISABLED || \
>+    defined(_MSC_VER) || \
>+    (HAVE_GCC_ATOMICS || HAVE_GCC_SYNC_LOCK_TEST_AND_SET) || \
>+    (defined(__GNUC__) && defined(__arm__) && \
>+        (defined(__ARM_ARCH_4__) || defined(__ARM_ARCH_4T__) || \
>+         defined(__ARM_ARCH_5__) || defined(__ARM_ARCH_5TE__) || \
>+         defined(__ARM_ARCH_5TEJ__))) || \
>+    (defined(__GNUC__) && defined(__arm__)) || \
>+    (defined(__GNUC__) && (defined(__i386__) || defined(__x86_64__))) || \
>+    (defined(__MACOSX__) || defined(__IPHONEOS__)))
>+    /*
>+     * Currently, for all platforms not using pthreads, no lock initialization
>+     * is necessary.
>+     */
>+    return SDL_TRUE;
>+#elif HAVE_PTHREAD_SPINLOCK
>+    /* pthread implementations require resource allocation for spin locks */
>+    return (pthread_spin_init(lock, PTHREAD_PROCESS_SHARED) == 0);
>+#else
>+#error Please implement for your platform.
>+    return SDL_FALSE;
>+#endif
>+}
> 
> /* This function is where all the magic happens... */
> SDL_bool
>@@ -123,4 +153,31 @@
> #endif
> }
> 
>+void
>+SDL_AtomicLockDestroy(SDL_SpinLock *lock)
>+{
>+#if (SDL_ATOMIC_DISABLED || \
>+    defined(_MSC_VER) || \
>+    (HAVE_GCC_ATOMICS || HAVE_GCC_SYNC_LOCK_TEST_AND_SET) || \
>+    (defined(__GNUC__) && defined(__arm__) && \
>+        (defined(__ARM_ARCH_4__) || defined(__ARM_ARCH_4T__) || \
>+         defined(__ARM_ARCH_5__) || defined(__ARM_ARCH_5TE__) || \
>+         defined(__ARM_ARCH_5TEJ__))) || \
>+    (defined(__GNUC__) && defined(__arm__)) || \
>+    (defined(__GNUC__) && (defined(__i386__) || defined(__x86_64__))) || \
>+    (defined(__MACOSX__) || defined(__IPHONEOS__)))
>+    /*
>+     * Currently, for all platforms not using pthreads, no lock cleanup
>+     * is necessary.
>+     */
>+    return;
>+#elif HAVE_PTHREAD_SPINLOCK
>+    /* pthread implementations require resource deallocation for spin locks */
>+    (void) pthread_spin_destroy(lock);
>+#else
>+#error Please implement for your platform.
>+    return;
>+#endif
>+}
>+
> /* vi: set ts=4 sw=4 expandtab: */
>diff --git a/src/dynapi/SDL_dynapi.c b/src/dynapi/SDL_dynapi.c
>--- a/src/dynapi/SDL_dynapi.c
>+++ b/src/dynapi/SDL_dynapi.c
>@@ -282,7 +282,10 @@
>        SDL_ATOMIC_DISABLED, which we can't do here, so in such a
>        configuration, you're on your own. */
>     #if !SDL_ATOMIC_DISABLED
>-    static SDL_SpinLock lock = 0;
>+    static SDL_SpinLock lock;
>+    if (!already_initialized) {
>+        SDL_AtomicLockInit_REAL(&lock);
>+    }
>     SDL_AtomicLock_REAL(&lock);
>     #endif
> 
>diff --git a/src/dynapi/SDL_dynapi_overrides.h b/src/dynapi/SDL_dynapi_overrides.h
>--- a/src/dynapi/SDL_dynapi_overrides.h
>+++ b/src/dynapi/SDL_dynapi_overrides.h
>@@ -587,3 +587,5 @@
> #define SDL_CaptureMouse SDL_CaptureMouse_REAL
> #define SDL_SetWindowHitTest SDL_SetWindowHitTest_REAL
> #define SDL_GetGlobalMouseState SDL_GetGlobalMouseState_REAL
>+#define SDL_AtomicLockInit SDL_AtomicLockInit_REAL
>+#define SDL_AtomicLockDestroy SDL_AtomicLockDestroy_REAL
>diff --git a/src/dynapi/SDL_dynapi_procs.h b/src/dynapi/SDL_dynapi_procs.h
>--- a/src/dynapi/SDL_dynapi_procs.h
>+++ b/src/dynapi/SDL_dynapi_procs.h
>@@ -619,3 +619,5 @@
> SDL_DYNAPI_PROC(int,SDL_CaptureMouse,(SDL_bool a),(a),return)
> SDL_DYNAPI_PROC(int,SDL_SetWindowHitTest,(SDL_Window *a, SDL_HitTest b, void *c),(a,b,c),return)
> SDL_DYNAPI_PROC(Uint32,SDL_GetGlobalMouseState,(int *a, int *b),(a,b),return)
>+SDL_DYNAPI_PROC(SDL_bool,SDL_AtomicLockInit,(SDL_SpinLock *a),(a),return)
>+SDL_DYNAPI_PROC(void,SDL_AtomicLockDestroy,(SDL_SpinLock *a),(a),)
>diff --git a/src/thread/SDL_thread.c b/src/thread/SDL_thread.c
>--- a/src/thread/SDL_thread.c
>+++ b/src/thread/SDL_thread.c
>@@ -128,7 +128,12 @@
> 
> #if !SDL_THREADS_DISABLED
>     if (!SDL_generic_TLS_mutex) {
>+        static SDL_bool already_initialized = SDL_FALSE;
>         static SDL_SpinLock tls_lock;
>+        if (!already_initialized) {
>+            already_initialized = SDL_TRUE;
>+            SDL_AtomicLockInit(&tls_lock);
>+        }
>         SDL_AtomicLock(&tls_lock);
>         if (!SDL_generic_TLS_mutex) {
>             SDL_mutex *mutex = SDL_CreateMutex();
>@@ -204,6 +209,7 @@
> SDL_error *
> SDL_GetErrBuf(void)
> {
>+    static SDL_bool already_initialized = SDL_FALSE;
>     static SDL_SpinLock tls_lock;
>     static SDL_bool tls_being_created;
>     static SDL_TLSID tls_errbuf;
>@@ -211,6 +217,11 @@
>     const SDL_error *ALLOCATION_IN_PROGRESS = (SDL_error *)-1;
>     SDL_error *errbuf;
> 
>+    if (!already_initialized) {
>+        already_initialized = SDL_TRUE;
>+        SDL_AtomicLockInit(&tls_lock);
>+    }
>+
>     /* tls_being_created is there simply to prevent recursion if SDL_TLSCreate() fails.
>        It also means it's possible for another thread to also use SDL_global_errbuf,
>        but that's very unlikely and hopefully won't cause issues.
>diff --git a/src/thread/pthread/SDL_systls.c b/src/thread/pthread/SDL_systls.c
>--- a/src/thread/pthread/SDL_systls.c
>+++ b/src/thread/pthread/SDL_systls.c
>@@ -34,7 +34,12 @@
> SDL_SYS_GetTLSData()
> {
>     if (thread_local_storage == INVALID_PTHREAD_KEY && !generic_local_storage) {
>+        static SDL_bool already_initialized = SDL_FALSE;
>         static SDL_SpinLock lock;
>+        if (!already_initialized) {
>+            already_initialized = SDL_TRUE;
>+            SDL_AtomicLockInit(&lock);
>+        }
>         SDL_AtomicLock(&lock);
>         if (thread_local_storage == INVALID_PTHREAD_KEY && !generic_local_storage) {
>             pthread_key_t storage;
>diff --git a/src/thread/windows/SDL_systls.c b/src/thread/windows/SDL_systls.c
>--- a/src/thread/windows/SDL_systls.c
>+++ b/src/thread/windows/SDL_systls.c
>@@ -35,7 +35,12 @@
> SDL_SYS_GetTLSData()
> {
>     if (thread_local_storage == TLS_OUT_OF_INDEXES && !generic_local_storage) {
>+        static SDL_bool already_initialized = SDL_FALSE;
>         static SDL_SpinLock lock;
>+        if (!already_initialized) {
>+            already_initialized = SDL_TRUE;
>+            SDL_AtomicLockInit(&lock);
>+        }
>         SDL_AtomicLock(&lock);
>         if (thread_local_storage == TLS_OUT_OF_INDEXES && !generic_local_storage) {
>             DWORD storage = TlsAlloc();
>diff --git a/src/timer/SDL_timer.c b/src/timer/SDL_timer.c
>--- a/src/timer/SDL_timer.c
>+++ b/src/timer/SDL_timer.c
>@@ -209,6 +209,10 @@
> 
>     if (!data->active) {
>         const char *name = "SDLTimer";
>+
>+        /* Before any use of data, the lock must be initialized! */
>+        SDL_AtomicLockInit(&data->lock);
>+
>         data->timermap_lock = SDL_CreateMutex();
>         if (!data->timermap_lock) {
>             return -1;
>@@ -281,6 +285,9 @@
> 
>         SDL_DestroyMutex(data->timermap_lock);
>         data->timermap_lock = NULL;
>+
>+        /* After all other processing is complete, destroy lock. */
>+        SDL_AtomicLockDestroy(&data->lock);
>     }
> }
> 
>diff --git a/test/testatomic.c b/test/testatomic.c
>--- a/test/testatomic.c
>+++ b/test/testatomic.c
>@@ -40,17 +40,19 @@
> void RunBasicTest()
> {
>     int value;
>-    SDL_SpinLock lock = 0;
>+    SDL_SpinLock lock;
> 
>     SDL_atomic_t v;
>     SDL_bool tfret = SDL_FALSE;
> 
>     SDL_Log("\nspin lock---------------------------------------\n\n");
> 
>+    SDL_AtomicLockInit(&lock);
>     SDL_AtomicLock(&lock);
>     SDL_Log("AtomicLock                   lock=%d\n", lock);
>     SDL_AtomicUnlock(&lock);
>     SDL_Log("AtomicUnlock                 lock=%d\n", lock);
>+    SDL_AtomicLockDestroy(&lock);
> 
>     SDL_Log("\natomic -----------------------------------------\n\n");
> 
>@@ -298,13 +300,18 @@
> {
>     int i;
> 
>+#ifdef TEST_SPINLOCK_FIFO
>+    /* Before queue is used, lock must be initialized! */
>+    SDL_AtomicLockInit(&queue->lock);
>+#endif
>+
>     for (i = 0; i < MAX_ENTRIES; ++i) {
>         SDL_AtomicSet(&queue->entries[i].sequence, i);
>     }
>     SDL_AtomicSet(&queue->enqueue_pos, 0);
>     SDL_AtomicSet(&queue->dequeue_pos, 0);
> #ifdef TEST_SPINLOCK_FIFO
>-    queue->lock = 0;
>+    queue->lock;
>     SDL_AtomicSet(&queue->rwcount, 0);
>     SDL_AtomicSet(&queue->watcher, 0);
> #endif
>@@ -599,8 +606,8 @@
>     Uint32 start, end;
>     int i, j;
>     int grand_total;
>-	char textBuffer[1024];
>-	int len;
>+    char textBuffer[1024];
>+    int len;
> 
>     SDL_Log("\nFIFO test---------------------------------------\n\n");
>     SDL_Log("Mode: %s\n", lock_free ? "LockFree" : "Mutex");
>@@ -689,10 +696,10 @@
>         }
>         grand_total += total;
>         SDL_Log("Reader %d read %d events, had %d waits\n", i, total, readerData[i].waits);
>-		SDL_snprintf(textBuffer, sizeof(textBuffer), "  { ");
>+        SDL_snprintf(textBuffer, sizeof(textBuffer), "  { ");
>         for (j = 0; j < NUM_WRITERS; ++j) {
>             if (j > 0) {
>-				len = SDL_strlen(textBuffer);
>+                len = SDL_strlen(textBuffer);
>                 SDL_snprintf(textBuffer + len, sizeof(textBuffer) - len, ", ");
>             }
>             len = SDL_strlen(textBuffer);
>@@ -700,7 +707,7 @@
>         }
>         len = SDL_strlen(textBuffer);
>         SDL_snprintf(textBuffer + len, sizeof(textBuffer) - len, " }\n");
>-		SDL_Log(textBuffer);
>+        SDL_Log(textBuffer);
>     }
>     SDL_Log("Readers read %d total events\n", grand_total);
> }
>@@ -711,8 +718,8 @@
> int
> main(int argc, char *argv[])
> {
>-	/* Enable standard application logging */
>-	SDL_LogSetPriority(SDL_LOG_CATEGORY_APPLICATION, SDL_LOG_PRIORITY_INFO);
>+    /* Enable standard application logging */
>+    SDL_LogSetPriority(SDL_LOG_CATEGORY_APPLICATION, SDL_LOG_PRIORITY_INFO);
> 
>     RunBasicTest();
>     RunEpicTest();

Actions: View | Diff

Attachments on bug 2618: 1716 | 1718 | 1720 | 1723

We are currently migrating Bugzilla to GitHub issues.Any changes made to the bug tracker now will be lost, so please do not post new bugs or make changes to them.When we're done, all bug URLs will redirect to their equivalent location on the new bug tracker.

We are currently migrating Bugzilla to GitHub issues.
Any changes made to the bug tracker now will be lost, so please do not post new bugs or make changes to them.
When we're done, all bug URLs will redirect to their equivalent location on the new bug tracker.