On Mon, 2 May 2011, Carsten Haitzler (The Rasterman) wrote:

> On Sun, 1 May 2011 16:16:43 +0200 (CEST) Vincent Torri <[email protected]>
> said:
>
>>
>>
>> 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 ?
>
> do you have a working "backtrace". yes - gnu extensions. :) or you mean the
> deadlock detection thats in pthread for error check locks?

For backtrace, I have that with vc++:

http://msdn.microsoft.com/en-us/library/ff552119%28VS.85%29.aspx

This function has already been used to capture the stack in Boost.

Otherwise, with mingw, I think that I have backtrace().

about deadlock, I don't know. I'm not very good with threads...

Vincent

>
>> 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
>>
>
>
> -- 
> ------------- Codito, ergo sum - "I code, therefore I am" --------------
> The Rasterman (Carsten Haitzler)    [email protected]
>
>

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

Reply via email to