[vdr] Re: Threading issues on RH9

[s.huelswitt@gmx.de (Stefan Huelswitt)]

On 31 May 2003 Jon Burgess <mplayer@jburgess.uklinux.net> wrote:

> The main problem is that getpid() returns the same PID for all threads. 
> There are a number of places in the VDR code which assume getpid() 
> returns a unique number for each thread.

I stumbled over the same problem when trying to debug VDR with
valgrind (a memory leak checker).

I think the mutex/condvar think can be solved much cleaner using
error checking type mutex instead of the default fast type ones.

For the thread/kill() issue I have found a work around, which
should work for every semi-normal thread termination, but I guess
that there are some races in there.

The biggest problem is, that I don't know how to check if a
thread is still running using only the thread id and pthread
functions. Any suggestions?

I attached my patch (it's still against 1.1.33, but will probably
work with 1.2.0pre1 too).

Let me know, if this allows you to run VDR on RH9.
The patch should have no negative effects on a normal VDR system,
so it could probably be included in some 1.3.x version.

-- 
Stefan Huelswitt
huels@iname.com  | http://home.pages.de/~nathan
diff -uN vdr-1.1.33-orig/thread.c vdr-1.1.33-valgrind/thread.c
--- vdr-1.1.33-orig/thread.c	2003-05-18 14:45:13.000000000 +0200
+++ vdr-1.1.33-valgrind/thread.c	2003-05-29 15:39:25.000000000 +0200
@@ -30,7 +30,7 @@
 
 void cCondVar::Wait(cMutex &Mutex)
 {
-  if (Mutex.locked && Mutex.lockingPid == getpid()) {
+  if (Mutex.locked) {
      int locked = Mutex.locked;
      Mutex.locked = 0; // have to clear the locked count here, as pthread_cond_wait
                        // does an implizit unlock of the mutex
@@ -43,7 +43,7 @@
 {
   bool r = true; // true = condition signaled false = timeout
 
-  if (Mutex.locked && Mutex.lockingPid == getpid()) {
+  if (Mutex.locked) {
      struct timeval now;                   // unfortunately timedwait needs the absolute time, not the delta :-(
      if (gettimeofday(&now, NULL) == 0) {  // get current time
         now.tv_usec += TimeoutMs * 1000;   // add the timeout
@@ -61,7 +61,6 @@
         if (pthread_cond_timedwait(&cond, &Mutex.mutex, &abstime) == ETIMEDOUT)
            r = false;
         Mutex.locked = locked;
-        Mutex.lockingPid = getpid();
         }
      }
   return r;
@@ -83,9 +82,9 @@
 
 cMutex::cMutex(void)
 {
-  lockingPid = 0;
   locked = 0;
-  pthread_mutex_init(&mutex, NULL);
+  pthread_mutexattr_t attr = { PTHREAD_MUTEX_ERRORCHECK_NP };
+  pthread_mutex_init(&mutex, &attr);
 }
 
 cMutex::~cMutex()
@@ -95,17 +94,13 @@
 
 void cMutex::Lock(void)
 {
-  if (getpid() != lockingPid || !locked) {
-     pthread_mutex_lock(&mutex);
-     lockingPid = getpid();
-     }
+  pthread_mutex_lock(&mutex);
   locked++;
 }
 
 void cMutex::Unlock(void)
 {
  if (!--locked) {
-    lockingPid = 0;
     pthread_mutex_unlock(&mutex);
     }
 }
@@ -141,6 +136,7 @@
 {
   Thread->threadPid = getpid();
   Thread->Action();
+  Thread->threadPid = 0;
   return NULL;
 }
 
diff -uN vdr-1.1.33-orig/thread.h vdr-1.1.33-valgrind/thread.h
--- vdr-1.1.33-orig/thread.h	2003-05-03 16:03:36.000000000 +0200
+++ vdr-1.1.33-valgrind/thread.h	2003-05-29 15:39:15.000000000 +0200
@@ -32,7 +32,6 @@
   friend class cCondVar;
 private:
   pthread_mutex_t mutex;
-  pid_t lockingPid;
   int locked;
 public:
   cMutex(void);