From 8077bf3d1067a52ad3f5dec2f86b8ef99bc811fe Mon Sep 17 00:00:00 2001 From: Sam Lantinga Date: Mon, 7 Jul 2014 21:27:42 -0700 Subject: [PATCH] Fixed bug 2618 - incomplete pthread-based lock support should be removed binarycrusader Since changeset 358696c354a8, SDL 2.0 has been broken on Solaris when compiling with the Solaris Studio compiler (which uses the pthread implementation of SDL_AtomicLock). Notably, it gets stuck at the MemoryBarrierRelease in SDL_GetErrBuf: 6585 # 218 6586 if (!tls_errbuf && !tls_being_created) { 6587 SDL_AtomicLock_REAL ( & tls_lock ); 6588 if (!tls_errbuf) { 6589 SDL_TLSID slot; 6590 tls_being_created = SDL_TRUE; 6591 slot = SDL_TLSCreate_REAL ( ); 6592 tls_being_created = SDL_FALSE; 6593 { SDL_SpinLock _tmp = 0 ; SDL_AtomicLock_REAL ( & _tmp ) ; SDL_AtomicUnlock_REAL ( & _tmp ) ; }; ^^^ loops forever above 6594 tls_errbuf = slot; 6595 } 6596 SDL_AtomicUnlock_REAL ( & tls_lock ); 6597 } Running: testthread (process id 28926) ^Cdbx: warning: Interrupt ignored but forwarded to child. signal INT (Interrupt) in __nanosleep at 0xfe52a875 0xfe52a875: __nanosleep+0x0015: jae __nanosleep+0x23 [ 0xfe52a883, .+0xe ] Current function is SDL_Delay_REAL 204 was_error = nanosleep(&tv, &elapsed); (dbx) where [1] __nanosleep(0xfeffe848, 0xfeffe850, 0xfe75a5ac, 0xfe5169d8), at 0xfe52a875 [2] nanosleep(0xfeffe848, 0xfeffe850), at 0xfe516a3b =>[3] SDL_Delay_REAL(ms = 0), line 204 in "SDL_systimer.c" [4] SDL_AtomicLock_REAL(lock = 0xfeffe88c), line 104 in "SDL_spinlock.c" [5] SDL_GetErrBuf(), line 225 in "SDL_thread.c" [6] SDL_ClearError_REAL(), line 216 in "SDL_error.c" [7] SDL_InitSubSystem_REAL(flags = 0), line 116 in "SDL.c" [8] SDL_Init_REAL(flags = 0), line 244 in "SDL.c" [9] SDL_Init(a = 0), line 89 in "SDL_dynapi_procs.h" [10] main(argc = 1, argv = 0xfeffe948), line 65 in "testthread.c" As far as I can tell, this is because pthread_spin_trylock() always returns EBUSY for this particular lock; since it works in other places, I'm suspicious. Different Solaris Studio compiler versions seem to make no difference. I've verified this is broken on Linux as well if SDL_spinlock.c is modified to use the pthread implementation. This appears to be because pthread_spin_init() and pthread_spin_destroy() are not used with the locks as required. --- cmake/sdlchecks.cmake | 7 ------- configure.in | 9 --------- include/SDL_config.h.cmake | 1 - include/SDL_config.h.in | 1 - premake/Linux/SDL_config_premake.h | 1 - premake/config/SDL_config_cygwin.template.h | 1 - premake/config/SDL_config_linux.template.h | 1 - src/atomic/SDL_spinlock.c | 7 ------- 8 files changed, 28 deletions(-) diff --git a/cmake/sdlchecks.cmake b/cmake/sdlchecks.cmake index 2cd09e6ffff5b..08347ece2fc85 100644 --- a/cmake/sdlchecks.cmake +++ b/cmake/sdlchecks.cmake @@ -753,13 +753,6 @@ macro(CheckPTHREAD) endif(HAVE_PTHREADS_SEM) endif(PTHREADS_SEM) - check_c_source_compiles(" - #include - int main(int argc, char** argv) { - pthread_spin_trylock(NULL); - return 0; - }" HAVE_PTHREAD_SPINLOCK) - check_c_source_compiles(" #include #include diff --git a/configure.in b/configure.in index d4c537eea5c3f..3c87ea754c53f 100644 --- a/configure.in +++ b/configure.in @@ -2384,15 +2384,6 @@ AC_HELP_STRING([--enable-pthread-sem], [use pthread semaphores [[default=yes]]]) AC_MSG_RESULT($have_sem_timedwait) fi - AC_MSG_CHECKING(for pthread_spin_trylock) - AC_TRY_LINK_FUNC(pthread_spin_trylock, [ - has_pthread_spin_trylock=yes - AC_DEFINE(HAVE_PTHREAD_SPINLOCK, 1, [ ]) - ],[ - has_pthread_spin_trylock=no - ]) - AC_MSG_RESULT($has_pthread_spin_trylock) - AC_CHECK_HEADER(pthread_np.h, have_pthread_np_h=yes, have_pthread_np_h=no, [ #include ]) if test x$have_pthread_np_h = xyes; then AC_DEFINE(HAVE_PTHREAD_NP_H, 1, [ ]) diff --git a/include/SDL_config.h.cmake b/include/SDL_config.h.cmake index 2c9ce6c6cad60..9bcf6e909c298 100644 --- a/include/SDL_config.h.cmake +++ b/include/SDL_config.h.cmake @@ -46,7 +46,6 @@ #cmakedefine HAVE_GCC_ATOMICS @HAVE_GCC_ATOMICS@ #cmakedefine HAVE_GCC_SYNC_LOCK_TEST_AND_SET @HAVE_GCC_SYNC_LOCK_TEST_AND_SET@ -#cmakedefine HAVE_PTHREAD_SPINLOCK @HAVE_PTHREAD_SPINLOCK@ #cmakedefine HAVE_DXGI_H @HAVE_DXGI_H@ diff --git a/include/SDL_config.h.in b/include/SDL_config.h.in index ea25249ac5e86..145a7b7728c13 100644 --- a/include/SDL_config.h.in +++ b/include/SDL_config.h.in @@ -49,7 +49,6 @@ #endif #undef HAVE_GCC_ATOMICS #undef HAVE_GCC_SYNC_LOCK_TEST_AND_SET -#undef HAVE_PTHREAD_SPINLOCK #undef HAVE_DDRAW_H #undef HAVE_DINPUT_H diff --git a/premake/Linux/SDL_config_premake.h b/premake/Linux/SDL_config_premake.h index f76594a98ef4c..01a135800a7f9 100644 --- a/premake/Linux/SDL_config_premake.h +++ b/premake/Linux/SDL_config_premake.h @@ -50,7 +50,6 @@ #endif #define HAVE_GCC_ATOMICS 1 /* #undef HAVE_GCC_SYNC_LOCK_TEST_AND_SET */ -#define HAVE_PTHREAD_SPINLOCK 1 /* Comment this if you want to build without any C library requirements */ #define HAVE_LIBC 1 diff --git a/premake/config/SDL_config_cygwin.template.h b/premake/config/SDL_config_cygwin.template.h index 46d6f36b6152b..651989eb7157a 100644 --- a/premake/config/SDL_config_cygwin.template.h +++ b/premake/config/SDL_config_cygwin.template.h @@ -46,7 +46,6 @@ #define SIZEOF_VOIDP 4 #define HAVE_GCC_ATOMICS 1 /* #undef HAVE_GCC_SYNC_LOCK_TEST_AND_SET */ -#define HAVE_PTHREAD_SPINLOCK 1 /* Comment this if you want to build without any C library requirements */ #define HAVE_LIBC 0 diff --git a/premake/config/SDL_config_linux.template.h b/premake/config/SDL_config_linux.template.h index 7560a880ba6a5..8a62fdb99a22d 100644 --- a/premake/config/SDL_config_linux.template.h +++ b/premake/config/SDL_config_linux.template.h @@ -50,7 +50,6 @@ #endif #define HAVE_GCC_ATOMICS 1 /* #undef HAVE_GCC_SYNC_LOCK_TEST_AND_SET */ -#define HAVE_PTHREAD_SPINLOCK 1 /* Comment this if you want to build without any C library requirements */ #define HAVE_LIBC 1 diff --git a/src/atomic/SDL_spinlock.c b/src/atomic/SDL_spinlock.c index 11aff019bf602..29b5190e55d1b 100644 --- a/src/atomic/SDL_spinlock.c +++ b/src/atomic/SDL_spinlock.c @@ -89,10 +89,6 @@ SDL_AtomicTryLock(SDL_SpinLock *lock) /* Maybe used for PowerPC, but the Intel asm or gcc atomics are favored. */ return OSAtomicCompareAndSwap32Barrier(0, 1, lock); -#elif HAVE_PTHREAD_SPINLOCK - /* pthread instructions */ - return (pthread_spin_trylock(lock) == 0); - #elif defined(__SOLARIS__) && defined(_LP64) /* Used for Solaris with non-gcc compilers. */ return (SDL_bool) ((int) atomic_cas_64((volatile uint64_t*)lock, 0, 1) == 0); @@ -126,9 +122,6 @@ SDL_AtomicUnlock(SDL_SpinLock *lock) #elif HAVE_GCC_ATOMICS || HAVE_GCC_SYNC_LOCK_TEST_AND_SET __sync_lock_release(lock); -#elif HAVE_PTHREAD_SPINLOCK - pthread_spin_unlock(lock); - #elif defined(__SOLARIS__) /* Used for Solaris when not using gcc. */ *lock = 0;