On Sun, 1 May 2011, Enlightenment SVN wrote:
> Log: > oh dear. this new eina_lock thing is a bit of a mess isn't it now? > some fundamental errors there. don't go replacing pthread locks with > wrappers unless you know full well what u are doing. havnig threads > only work while "threads are initted" and then init/shtudown the thread > thing every time u spawn a thread.. is pretty silly. what if a thread > ends in the background WHILE u have a lock.. u try unlock.. u know > what ? your unlock DOES nothing. so you retain a lock. next time u > want to lock once a thread is around.. u have a deadlock issue. > > even better - the checking if threads are initted and up is not > locked, so it can come up while it is being checked. more race > conditions. u need to clokc the init/shutdown AND lock the checking of > the value... and even then u STILl have problem #1 above. so that code > is now gone. > > also trylock trturn inverse logic to the original pthread func and the > macros in evas that used it were not changed accordingly! aaagh! > > i've also added backtrace debug ability to eina threads if compiled in > - u can get a bt of who last locked something. i had to do this just to > begin to grasp what on earth was going on. it's off by default. > also... the locks are error check locks to trylock can detect > deadlocks. speacil "2" return for now. better than a poke in the eye > with a sharp stick until we decide what to do. for now i hopew i have > killed this thread lock bug. can i try to mimic that to debug threads on Windows too ? Vincent > > > > > Author: raster > Date: 2011-05-01 06:24:08 -0700 (Sun, 01 May 2011) > New Revision: 59085 > Trac: http://trac.enlightenment.org/e/changeset/59085 > > Modified: > trunk/eina/src/include/eina_inline_lock_posix.x > > Modified: trunk/eina/src/include/eina_inline_lock_posix.x > =================================================================== > --- trunk/eina/src/include/eina_inline_lock_posix.x 2011-05-01 12:58:35 UTC > (rev 59084) > +++ trunk/eina/src/include/eina_inline_lock_posix.x 2011-05-01 13:24:08 UTC > (rev 59085) > @@ -19,6 +19,7 @@ > #ifndef EINA_INLINE_LOCK_POSIX_X_ > #define EINA_INLINE_LOCK_POSIX_X_ > > +#include <errno.h> > #ifndef __USE_UNIX98 > # define __USE_UNIX98 > # include <pthread.h> > @@ -27,8 +28,29 @@ > # include <pthread.h> > #endif > > -typedef pthread_mutex_t Eina_Lock; > +/* > +#define EINA_LOCK_DEBUG 1 > +*/ > > +#ifdef EINA_LOCK_DEBUG > +#include <execinfo.h> > +#define EINA_LOCK_DEBUG_BT_NUM 64 > +typedef void (*Eina_Lock_Bt_Func) (); > +#endif > + > +typedef struct _Eina_Lock Eina_Lock; > + > +struct _Eina_Lock > +{ > + pthread_mutex_t mutex; > +#ifdef EINA_LOCK_DEBUG > + pthread_t lock_thread_id; > + Eina_Lock_Bt_Func lock_bt[EINA_LOCK_DEBUG_BT_NUM]; > + int lock_bt_num; > + Eina_Bool locked : 1; > +#endif > +}; > + > EAPI extern Eina_Bool _eina_threads_activated; > > #ifdef EINA_HAVE_DEBUG_THREADS > @@ -44,12 +66,19 @@ > > if (pthread_mutexattr_init(&attr) != 0) > return EINA_FALSE; > +/* use errorcheck locks. detect deadlocks. > #ifdef PTHREAD_MUTEX_RECURSIVE > if (pthread_mutexattr_settype(&attr, PTHREAD_MUTEX_RECURSIVE) != 0) > return EINA_FALSE; > #endif > - if (pthread_mutex_init(mutex, &attr) != 0) > + */ > + if (pthread_mutexattr_settype(&attr, PTHREAD_MUTEX_ERRORCHECK) != 0) > return EINA_FALSE; > +#ifdef EINA_LOCK_DEBUG > + memset(mutex, 0, sizeof(Eina_Lock)); > +#endif > + if (pthread_mutex_init(&(mutex->mutex), &attr) != 0) > + return EINA_FALSE; > > pthread_mutexattr_destroy(&attr); > > @@ -59,53 +88,95 @@ > static inline void > eina_lock_free(Eina_Lock *mutex) > { > - pthread_mutex_destroy(mutex); > + pthread_mutex_destroy(&(mutex->mutex)); > +#ifdef EINA_LOCK_DEBUG > + memset(mutex, 0, sizeof(Eina_Lock)); > +#endif > } > > static inline Eina_Bool > eina_lock_take(Eina_Lock *mutex) > { > - if (_eina_threads_activated) > - { > + Eina_Bool ret; > #ifdef EINA_HAVE_DEBUG_THREADS > - if (_eina_threads_debug) > - { > - struct timeval t0, t1; > - int dt; > - > - gettimeofday(&t0, NULL); > - pthread_mutex_lock(&(x)); > - gettimeofday(&t1, NULL); > - > - dt = (t1.tv_sec - t0.tv_sec) * 1000000; > - if (t1.tv_usec > t0.tv_usec) > - dt += (t1.tv_usec - t0.tv_usec); > - else > - dt -= t0.tv_usec - t1.tv_usec; > - dt /= 1000; > - > - if (dt > _eina_threads_debug) abort(); > - } > + if (_eina_threads_debug) > + { > + struct timeval t0, t1; > + int dt; > + > + gettimeofday(&t0, NULL); > + pthread_mutex_lock(&(x)); > + gettimeofday(&t1, NULL); > + > + dt = (t1.tv_sec - t0.tv_sec) * 1000000; > + if (t1.tv_usec > t0.tv_usec) > + dt += (t1.tv_usec - t0.tv_usec); > + else > + dt -= t0.tv_usec - t1.tv_usec; > + dt /= 1000; > + > + if (dt > _eina_threads_debug) abort(); > + } > #endif > - return (pthread_mutex_lock(mutex) == 0) ? EINA_TRUE : EINA_FALSE; > - } > - return EINA_FALSE; > + ret = (pthread_mutex_lock(&(mutex->mutex)) == 0) ? > + EINA_TRUE : EINA_FALSE; > +#ifdef EINA_LOCK_DEBUG > + mutex->locked = 1; > + mutex->lock_thread_id = pthread_self(); > + mutex->lock_bt_num = backtrace((void **)(mutex->lock_bt), > EINA_LOCK_DEBUG_BT_NUM); > +#endif > + return ret; > } > > static inline Eina_Bool > eina_lock_take_try(Eina_Lock *mutex) > { > - if (_eina_threads_activated) > - return (pthread_mutex_trylock(mutex) == 0) ? EINA_TRUE : EINA_FALSE; > - return EINA_FALSE; > + Eina_Bool ret = EINA_FALSE; > + int ok; > + > + ok = pthread_mutex_trylock(&(mutex->mutex)); > + if (ok == 0) ret = EINA_TRUE; > + else if (ok == EDEADLK) > + { > + printf("ERROR ERROR: DEADLOCK on lock %p\n", mutex); > + ret = 2; // magic > + } > +#ifdef EINA_LOCK_DEBUG > + if (ret == EINA_TRUE) > + { > + mutex->locked = 1; > + mutex->lock_thread_id = pthread_self(); > + mutex->lock_bt_num = backtrace((void **)(mutex->lock_bt), > EINA_LOCK_DEBUG_BT_NUM); > + } > +#endif > + return ret; > } > > static inline Eina_Bool > eina_lock_release(Eina_Lock *mutex) > { > - if (_eina_threads_activated) > - return (pthread_mutex_unlock(mutex) == 0) ? EINA_TRUE : EINA_FALSE; > - return EINA_FALSE; > + Eina_Bool ret; > +#ifdef EINA_LOCK_DEBUG > + mutex->locked = 0; > + mutex->lock_thread_id = 0; > + memset(mutex->lock_bt, 0, EINA_LOCK_DEBUG_BT_NUM * > sizeof(Eina_Lock_Bt_Func)); > + mutex->lock_bt_num = 0; > +#endif > + ret = (pthread_mutex_unlock(&(mutex->mutex)) == 0) ? > + EINA_TRUE : EINA_FALSE; > + return ret; > } > > +static inline void > +eina_lock_debug(Eina_Lock *mutex) > +{ > +#ifdef EINA_LOCK_DEBUG > + printf("lock %p, locked: %i, by %i\n", > + mutex, (int)mutex->locked, (int)mutex->lock_thread_id); > + backtrace_symbols_fd((void **)mutex->lock_bt, mutex->lock_bt_num, 1); > +#else > + mutex = 0; > #endif > +} > + > +#endif > > > ------------------------------------------------------------------------------ > WhatsUp Gold - Download Free Network Management Software > The most intuitive, comprehensive, and cost-effective network > management toolset available today. Delivers lowest initial > acquisition cost and overall TCO of any competing solution. > http://p.sf.net/sfu/whatsupgold-sd > _______________________________________________ > enlightenment-svn mailing list > [email protected] > https://lists.sourceforge.net/lists/listinfo/enlightenment-svn > > ------------------------------------------------------------------------------ WhatsUp Gold - Download Free Network Management Software The most intuitive, comprehensive, and cost-effective network management toolset available today. Delivers lowest initial acquisition cost and overall TCO of any competing solution. http://p.sf.net/sfu/whatsupgold-sd _______________________________________________ enlightenment-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
