[vdr] Is cMutex class thread safe? Possible bug?

["Colin Paton" <ac2crp@blueyonder.co.uk>]

Hi,

I've found some code that looks a bit 'funny' to me and I'm wondering if
anybody could advise.

The cMutex class is roughly as follows:

void cMutex::Lock(void)
{
  if (getpid() != lockingPid || !locked)
  {
    pthread_mutex_lock(&mutex);
    lockingPid = getpid();
  }
  locked++;
}

void cMutex::Unlock(void)
{
  if (!--locked)
  {
    lockingPid = 0;
    pthread_mutex_unlock(&mutex);
  }
}

It looks to me as though there could be a potential race condition here in
that the 'locked' count isn't protected by the mutex. This condition could
occur if one thread tried to unlock the mutex while the other thread tried
to lock it. In this case, 'locked' could be decremented by the thread
calling 'Unlock()' at the same time as a different thread tested 'locked' in
the Lock() method.

I think that recursive mutexes should be used instead. That way, the
pthreads library keeps track of lock counts, effectively having its own
internal 'locked' and 'lockingPid' variables which are accessed in a thread
safe way.

Therefore, the calls would just look like:

void cMutex::Lock(void)
{
  pthread_mutex_lock(&mutex);
}

void cMutex::Unlock(void)
{
  pthread_mutex_unlock(&mutex);
}

...and the mutex would be created with the type
PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP in pthread_mutex_init().

Of course, having written all this I have noticed that a similar discussion
appeared in 2001.

http://www.linuxtv.org/mailinglists/vdr/2001/09-2001/msg00270.html

This seems to state that using recursive mutexes leads to channel switching
hanging, which is exactly the problem that I am encountering, using the
default vdr 1.2.5 code (without using pthreads recursive mutexes). I
sometimes have to 'kill -9 vdr'

It seems that using the 'buggy' mutex above is potentially just hiding
another problem, and it would be best to use mutex code without a race
condition and then try to find out what is causing the hang/deadlock.

Before I do this however, I wonder if anybody could advise. The discussion
above occurred two years ago, and I'd just like to confirm that nothing has
changed before I try to track down the problem.

I am also wondering if the use of pthread_cancel is correct? It would seem
that if the thread has become locked for more than 'Cancel' time then a
fatal error should occur - something has become frozen? Again, I can
reproduce the freezing problem by halting xine (using Ctrl-Z in the shell)
and changing channel. I leave xine halted for more than 3 seconds and then
unfreeze it. VDR has died, as it has killed the transfer thread which still
(I believe) has the locked mutex.

As usual with threads problems I may been looking in the wrong area, but I
would appreciate comments.

Thanks,

Colin



-- 
Info:
To unsubscribe send a mail to ecartis@linuxtv.org with "unsubscribe vdr" as subject.