Fixed a race condition in starting a thread

4 files changed, 30 insertions, 29 deletions

diff --git a/CONTRIBUTORS b/CONTRIBUTORS
index 6418984a..d4c6385d 100644
--- a/CONTRIBUTORS
+++ b/CONTRIBUTORS
@@ -897,6 +897,7 @@ Reinhard Nissl <rnissl@gmx.de>
  to clear the transfer buffer to avoid overflows
  for reporting a high CPU load in still picture mode after removing the usleep()
  call from cDvbPlayer::Action()
+ for reporting a race condition in starting a thread
 
 Richard Robson <richard_robson@beeb.net>
  for reporting freezing replay if a timer starts while in Transfer Mode from the
diff --git a/HISTORY b/HISTORY
index b7d04702..8d8dca2c 100644
--- a/HISTORY
+++ b/HISTORY
@@ -3207,3 +3207,5 @@ Video Disk Recorder Revision History
 - Switched the character set to iso8859-15 for English, German and Finnish (thanks
   to Andreas Brugger for reporting the missing Euro sign in iso8859-1).
 - Added 'channels.conf.terr' entries for Lübeck (thanks to Stefan Hußfeldt).
+- Fixed a race condition in starting a thread (thanks to Reinhard Nissl for
+  reporting this one).
diff --git a/thread.c b/thread.c
index ebb0943d..fe3359bf 100644
--- a/thread.c
+++ b/thread.c
@@ -4,7 +4,7 @@
  * See the main source file 'vdr.c' for copyright information and
  * how to reach the author.
  *
- * $Id: thread.c 1.39 2004/11/26 14:16:07 kls Exp $
+ * $Id: thread.c 1.40 2004/12/19 10:43:14 kls Exp $
  */
 
 #include "thread.h"
@@ -197,7 +197,8 @@ bool cThread::emergencyExitRequested = false;
 
 cThread::cThread(const char *Description)
 {
-  parentTid = childTid = 0;
+  running = false;
+  childTid = 0;
   description = NULL;
   SetDescription(Description);
 }
@@ -221,35 +222,35 @@ void cThread::SetDescription(const char *Description, ...)
 
 void *cThread::StartThread(cThread *Thread)
 {
-  Thread->childTidMutex.Lock();
-  Thread->childTid = pthread_self();
-  Thread->childTidMutex.Unlock();
   if (Thread->description)
-     dsyslog("%s thread started (pid=%d, tid=%ld)", Thread->description, getpid(), Thread->childTid);
+     dsyslog("%s thread started (pid=%d, tid=%ld)", Thread->description, getpid(), pthread_self());
   Thread->Action();
   if (Thread->description)
-     dsyslog("%s thread ended (pid=%d, tid=%ld)", Thread->description, getpid(), Thread->childTid);
-  Thread->childTidMutex.Lock();
-  Thread->childTid = 0;
-  Thread->childTidMutex.Unlock();
+     dsyslog("%s thread ended (pid=%d, tid=%ld)", Thread->description, getpid(), pthread_self());
+  Thread->running = false;
   return NULL;
 }
 
 bool cThread::Start(void)
 {
-  if (!childTid) {
-     parentTid = pthread_self();
-     pthread_t Tid;
-     pthread_create(&Tid, NULL, (void *(*) (void *))&StartThread, (void *)this);
-     pthread_detach(Tid); // auto-reap
-     pthread_setschedparam(Tid, SCHED_RR, 0);
+  if (!running) {
+     running = true;
+     if (pthread_create(&childTid, NULL, (void *(*) (void *))&StartThread, (void *)this) == 0) {
+        pthread_detach(childTid); // auto-reap
+        pthread_setschedparam(childTid, SCHED_RR, 0);
+        }
+     else {
+        LOG_ERROR;
+        running = false;
+        return false;
+        }
      }
-  return true; //XXX return value of pthread_create()???
+  return true;
 }
 
 bool cThread::Active(void)
 {
-  if (childTid) {
+  if (running) {
      //
      // Single UNIX Spec v2 says:
      //
@@ -259,12 +260,12 @@ bool cThread::Active(void)
      // As in kill(), if sig is zero, error checking is
      // performed but no signal is actually sent.
      //
-     cMutexLock MutexLock(&childTidMutex);
      int err;
      if ((err = pthread_kill(childTid, 0)) != 0) {
         if (err != ESRCH)
            LOG_ERROR;
         childTid = 0;
+        running = false;
         }
      else
         return true;
@@ -274,7 +275,7 @@ bool cThread::Active(void)
 
 void cThread::Cancel(int WaitSeconds)
 {
-  if (childTid) {
+  if (running) {
      if (WaitSeconds > 0) {
         for (time_t t0 = time(NULL) + WaitSeconds; time(NULL) < t0; ) {
             if (!Active())
@@ -283,12 +284,9 @@ void cThread::Cancel(int WaitSeconds)
             }
         esyslog("ERROR: thread %ld won't end (waited %d seconds) - canceling it...", childTid, WaitSeconds);
         }
-     childTidMutex.Lock();
-     if (childTid) {
-        pthread_cancel(childTid);
-        childTid = 0;
-        }
-     childTidMutex.Unlock();
+     pthread_cancel(childTid);
+     childTid = 0;
+     running = false;
      }
 }
 
diff --git a/thread.h b/thread.h
index c3d8d06a..6e096a64 100644
--- a/thread.h
+++ b/thread.h
@@ -4,7 +4,7 @@
  * See the main source file 'vdr.c' for copyright information and
  * how to reach the author.
  *
- * $Id: thread.h 1.25 2004/11/26 13:33:26 kls Exp $
+ * $Id: thread.h 1.26 2004/12/19 10:43:10 kls Exp $
  */
 
 #ifndef __THREAD_H
@@ -73,8 +73,8 @@ public:
 class cThread {
   friend class cThreadLock;
 private:
-  pthread_t parentTid, childTid;
-  cMutex childTidMutex;
+  bool running;
+  pthread_t childTid;
   cMutex mutex;
   char *description;
   static bool emergencyExitRequested;