Subj : Re: Unexplained race condition with POSIX threads
To   : comp.programming.threads,comp.sources.d
From : Malte Starostik
Date : Tue Mar 29 2005 01:50 am

usenet@sta.samsung.com schrieb:
> Greetings,
> 
> Reproduced below is a ludicrously simple example of multi-threading
> that uses the Active-Object design pattern.  I have arrived at this
> example by adapting the real problem that I am working on and stripping
> all the unnecessary complexity from it.  Fortunately (or unfortunately)
> I am able to reproduce the problem even with this ridiculous example.

> So far so simple.  Now this is what happens.  Every once in a while,
> the sample program gets permanently blocked on the condition variable,
> and the sample program gets stuck.

Please see below.  Some nitpicks on the way to the actual problem, no
offence.  I snipped the error messages, so what comes after the snipped
part after the if(...) statements is not the body of them, but hey, you
know the code :-)
BTW, the endless loop has a termination condition in the real code I
assume? ;)

>     pthread_mutexattr_t attr;
>     pthread_mutexattr_init(&attr);
>     attr.__mutexkind = PTHREAD_MUTEX_RECURSIVE_NP;
> 
>     // Initialize the mutex
>     pthread_mutex_init(&m_mutex,&attr);
simpler alternative:
m_mutex = PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP;

>   ActiveObject::~ActiveObject()
>   {

>     if(pthread_mutex_lock(&m_mutex))

> 
>     // Destroy the mutex
>     if(pthread_mutex_destroy(&m_mutex))
error: must not destroy locked mutex

>   void *
>   ActiveObject::run (void *args)
>   {

>     return (void *) NULL;
unnecessary cast

>   }

>   void
>   ActiveObject::waitForMessages()
>   {

>       // Remove entries from the activation list
>       list<time_t>::iterator retItor;
>       retItor = m_activationList.erase(m_activationList.begin(),
> m_activationList.end());
why care so much about erase()'s result when you don't use it?
To clear the list w/o hassle:
        m_activationList.clear();

>       // Wait on the condition variable
>       if(pthread_cond_wait(&m_condition, &m_mutex))
This code implicitly "checks" whether the condition was really signalled
as it only does something when there are entries in the list.  If that
was not the case, you should add some other kind of flag to protect
against spurious wakeups.

>   void
>   ActiveObject::doDisplayTime(const time_t ts)
>   {
>     cout << ctime(&ts) << endl;
access to cout should be protected by a mutex
preferably a different one than the the AO uses internally
>   }
> 
> 
>   void
>   ActiveObject::displayActivationList()
>   {
>     char *errStr = NULL;
> 
>     cout << "\n\nDisplaying activation list\n"
>          << "**************************\n";
same here, so this method would lock both m_mutex and the
one for stdout

>     // Lock the mutex
>     if(pthread_mutex_lock(&m_mutex))

Here's the interesting part:
> void* testActiveObj(void * args)
> {
>   ActiveObject *ao = static_cast<ActiveObject *>(args);
> 
>   ao->displayTime(100000);
>   ao->displayTime(200000);
>   ao->displayTime(400000);
// if the intrinsic thread gets scheduled here, it processes
// the activation list before this thread arrives at the next line
// so the list will be empty then

>   ao->displayActivationList();

Cheers,
Malte

.