Mailing List archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[vdr] Re: Complete all-in-one safe threading patch for vanilla vdr-1.3.17(with poison)



Anssi Hannula wrote:
C.Y.M wrote:

With all the confusion and patches flying around these past few days, I have compiled an all-in-one patch that includes all the latest threading fixes for vanilla vdr-1.3.17. This include the fixes for the race conditions in cThread::Start by Reinhard Nissl and safe threading patches by Rainer Zocholl. Klaus, please consider adding this to vdr-1.3.18.

Thanks goes out to everyone that contributed their time and effort on this. Without them, this patch would not be possible.

Thanks for the patch.
Apparently the yesterday's lirc.c NPTL thread-safe patch (by Ludwig Nussel) didn't make it into the all-in-one patch.

Ooops, thanks for reminding me on that, Anssi. Its been hard trying to keep track of all the patches :). Here is a rev2 including Ludwig's lirc thread-safe patch. Also, here is a collection of "most" of the poison patches for the plugins. Im sure there are still some missing.

Regards,
diff -ruN vdr-1.3.17/channels.c vdr-1.3.17-ts/channels.c
--- vdr-1.3.17/channels.c	2004-11-02 10:07:05.000000000 -0800
+++ vdr-1.3.17-ts/channels.c	2004-12-06 07:59:37.000000000 -0800
@@ -705,7 +705,8 @@
            p = apidbuf;
            char *q;
            int NumApids = 0;
-           while ((q = strtok(p, ",")) != NULL) {
+           char * lasts=NULL;
+           while ((q = strtok_r(p, ",", &lasts)) != NULL) {
                  if (NumApids < MAXAPIDS) {
                     char *l = strchr(q, '=');
                     if (l) {
@@ -725,7 +726,8 @@
               char *p = dpidbuf;
               char *q;
               int NumDpids = 0;
-              while ((q = strtok(p, ",")) != NULL) {
+              char * lasts=NULL;
+              while ((q = strtok_r(p, ",", &lasts)) != NULL) {
                     if (NumDpids < MAXAPIDS) {
                        char *l = strchr(q, '=');
                        if (l) {
@@ -747,7 +749,8 @@
               char *p = caidbuf;
               char *q;
               int NumCaIds = 0;
-              while ((q = strtok(p, ",")) != NULL) {
+              char * lasts=NULL;
+              while ((q = strtok_r(p, ",", &lasts)) != NULL) {
                     if (NumCaIds < MAXCAIDS) {
                        caids[NumCaIds++] = strtol(q, NULL, 16) & 0xFFFF;
                        if (NumCaIds == 1 && caids[0] <= 0x00FF)
diff -ruN vdr-1.3.17/config.h vdr-1.3.17-ts/config.h
--- vdr-1.3.17/config.h	2004-11-16 08:57:43.000000000 -0800
+++ vdr-1.3.17-ts/config.h	2004-12-06 07:59:37.000000000 -0800
@@ -16,6 +16,9 @@
 #include <string.h>
 #include <time.h>
 #include <unistd.h>
+#pragma GCC poison alctime ctime gmtime localtime
+#pragma GCC poison getgrgid getgrnam getpwnam getpwuid
+#pragma GCC poison rand readdir strtok
 #include "device.h"
 #include "i18n.h"
 #include "tools.h"
diff -ruN vdr-1.3.17/eit.c vdr-1.3.17-ts/eit.c
--- vdr-1.3.17/eit.c	2004-10-31 04:56:24.000000000 -0800
+++ vdr-1.3.17-ts/eit.c	2004-12-06 07:59:37.000000000 -0800
@@ -244,9 +244,10 @@
   time_t loctim = time(NULL);
 
   if (abs(sattim - loctim) > 2) {
+     char timebuf[26]; /* This magic number is based on the ctime(3c) man page */
      mutex.Lock();
-     isyslog("System Time = %s (%ld)\n", ctime(&loctim), loctim);
-     isyslog("Local Time  = %s (%ld)\n", ctime(&sattim), sattim);
+     isyslog("System Time = %s (%ld)\n", ctime_r(&loctim, timebuf), loctim);
+     isyslog("Local Time  = %s (%ld)\n", ctime_r(&sattim, timebuf), sattim);
      if (stime(&sattim) < 0)
         esyslog("ERROR while setting system time: %m");
      mutex.Unlock();
diff -ruN vdr-1.3.17/keys.c vdr-1.3.17-ts/keys.c
--- vdr-1.3.17/keys.c	2003-09-14 03:07:47.000000000 -0700
+++ vdr-1.3.17-ts/keys.c	2004-12-06 07:59:37.000000000 -0800
@@ -195,7 +195,8 @@
 {
   int n = 0;
   char *p;
-  while ((p = strtok(s, " \t")) != NULL) {
+  char * lasts=NULL;
+  while ((p = strtok_r(s, " \t", &lasts)) != NULL) {
         if (n < MAXKEYSINMACRO) {
            if (*p == '@') {
               if (plugin) {
diff -ruN vdr-1.3.17/lirc.c vdr-1.3.17-ts/lirc.c
--- vdr-1.3.17/lirc.c	2003-10-18 04:34:02.000000000 -0700
+++ vdr-1.3.17-ts/lirc.c	2004-12-06 08:03:26.000000000 -0800
@@ -41,6 +41,7 @@
 cLircRemote::~cLircRemote()
 {
   Cancel();
+  close(f);
 }
 
 bool cLircRemote::Ready(void)
@@ -59,8 +60,6 @@
 
   for (; f >= 0;) {
 
-      LOCK_THREAD;
-
       bool ready = cFile::FileReady(f, timeout);
       int ret = ready ? safe_read(f, buf, sizeof(buf)) : -1;
 
diff -ruN vdr-1.3.17/plugin.c vdr-1.3.17-ts/plugin.c
--- vdr-1.3.17/plugin.c	2004-05-22 04:25:22.000000000 -0700
+++ vdr-1.3.17-ts/plugin.c	2004-12-06 07:59:37.000000000 -0800
@@ -7,12 +7,12 @@
  * $Id: plugin.c 1.11 2004/05/22 11:25:22 kls Exp $
  */
 
-#include "plugin.h"
 #include <ctype.h>
 #include <dirent.h>
 #include <dlfcn.h>
 #include <stdlib.h>
 #include <time.h>
+#include "plugin.h"
 #include "config.h"
 
 #define LIBVDR_PREFIX  "libvdr-"
@@ -258,8 +258,8 @@
   if (strcmp(Args, "*") == 0) {
      DIR *d = opendir(directory);
      if (d) {
-        struct dirent *e;
-        while ((e = readdir(d)) != NULL) {
+        struct dirent *e, p;
+        while (!readdir_r(d, &p, &e) && e != NULL) {
               if (strstr(e->d_name, LIBVDR_PREFIX) == e->d_name) {
                  char *p = strstr(e->d_name, SO_INDICATOR);
                  if (p) {
diff -ruN vdr-1.3.17/recording.c vdr-1.3.17-ts/recording.c
--- vdr-1.3.17/recording.c	2004-11-01 06:04:47.000000000 -0800
+++ vdr-1.3.17-ts/recording.c	2004-12-06 07:59:37.000000000 -0800
@@ -7,7 +7,6 @@
  * $Id: recording.c 1.92 2004/11/01 14:04:47 kls Exp $
  */
 
-#include "recording.h"
 #include <dirent.h>
 #include <errno.h>
 #include <fcntl.h>
@@ -15,6 +14,7 @@
 #include <string.h>
 #include <sys/stat.h>
 #include <unistd.h>
+#include "recording.h"
 #include "channels.h"
 #include "i18n.h"
 #include "interface.h"
@@ -630,8 +630,8 @@
 {
   DIR *d = opendir(DirName);
   if (d) {
-     struct dirent *e;
-     while ((e = readdir(d)) != NULL) {
+     struct dirent *e, p;
+     while (!readdir_r(d, &p, &e) && e != NULL) {
            if (strcmp(e->d_name, ".") && strcmp(e->d_name, "..")) {
               char *buffer;
               asprintf(&buffer, "%s/%s", DirName, e->d_name);
diff -ruN vdr-1.3.17/svdrp.c vdr-1.3.17-ts/svdrp.c
--- vdr-1.3.17/svdrp.c	2004-10-31 02:09:53.000000000 -0800
+++ vdr-1.3.17-ts/svdrp.c	2004-12-06 07:59:37.000000000 -0800
@@ -555,8 +555,9 @@
      char buf[strlen(Option) + 1];
      char *p = strcpy(buf, Option);
      const char *delim = " \t";
-     FileName = strtok(p, delim);
-     if ((p = strtok(NULL, delim)) != NULL) {
+     char * lasts=NULL;
+     FileName = strtok_r(p, delim, &lasts);
+     if ((p = strtok_r(NULL, delim, &lasts)) != NULL) {
         if (strcasecmp(p, "JPEG") == 0)
            Jpeg = true;
         else if (strcasecmp(p, "PNM") == 0)
@@ -566,7 +567,7 @@
            return;
            }
         }
-     if ((p = strtok(NULL, delim)) != NULL) {
+     if ((p = strtok_r(NULL, delim, &lasts)) != NULL) {
         if (isnumber(p))
            Quality = atoi(p);
         else {
@@ -574,14 +575,14 @@
            return;
            }
         }
-     if ((p = strtok(NULL, delim)) != NULL) {
+     if ((p = strtok_r(NULL, delim, &lasts)) != NULL) {
         if (isnumber(p))
            SizeX = atoi(p);
         else {
            Reply(501, "Illegal sizex \"%s\"", p);
            return;
            }
-        if ((p = strtok(NULL, delim)) != NULL) {
+        if ((p = strtok_r(NULL, delim, &lasts)) != NULL) {
            if (isnumber(p))
               SizeY = atoi(p);
            else {
@@ -594,7 +595,7 @@
            return;
            }
         }
-     if ((p = strtok(NULL, delim)) != NULL) {
+     if ((p = strtok_r(NULL, delim, &lasts)) != NULL) {
         Reply(501, "Unexpected parameter \"%s\"", p);
         return;
         }
@@ -727,7 +728,8 @@
         char buf[strlen(Option) + 1];
         strcpy(buf, Option);
         const char *delim = " \t";
-        char *p = strtok(buf, delim);
+        char * lasts=NULL;
+        char *p = strtok_r(buf, delim, &lasts);
         while (p && DumpMode == dmAll) {
               if (strcasecmp(p, "NOW") == 0)
                  DumpMode = dmPresent;
@@ -735,7 +737,7 @@
                  DumpMode = dmFollowing;
               else if (strcasecmp(p, "AT") == 0) {
                  DumpMode = dmAtTime;
-                 if ((p = strtok(NULL, delim)) != NULL) {
+                 if ((p = strtok_r(NULL, delim, &lasts)) != NULL) {
                     if (isnumber(p))
                        AtTime = strtol(p, NULL, 10);
                     else {
@@ -770,7 +772,7 @@
                  Reply(501, "Unknown option: \"%s\"", p);
                  return;
                  }
-              p = strtok(NULL, delim);
+              p = strtok_r(NULL, delim, &lasts);
               }
         }
      FILE *f = fdopen(file, "w");
@@ -996,7 +998,8 @@
      time_t Start = t->StartTime();
      int Number = t->Index() + 1;
      if (!*Option) {
-        char *s = ctime(&Start);
+        char timebuf[26]; /* This magic number is based on the ctime(3c) man page */
+        char *s = ctime_r(&Start, timebuf);
         s[strlen(s) - 1] = 0; // strip trailing newline
         Reply(250, "%d %s", Number, s);
         }
@@ -1152,7 +1155,8 @@
         char buffer[BUFSIZ];
         gethostname(buffer, sizeof(buffer));
         time_t now = time(NULL);
-        Reply(220, "%s SVDRP VideoDiskRecorder %s; %s", buffer, VDRVERSION, ctime(&now));
+        char timebuf[26]; /* This magic number is based on the ctime(3c) man page */
+        Reply(220, "%s SVDRP VideoDiskRecorder %s; %s", buffer, VDRVERSION, ctime_r(&now, timebuf));
         }
      if (NewConnection)
         lastActivity = time(NULL);
diff -ruN vdr-1.3.17/themes.c vdr-1.3.17-ts/themes.c
--- vdr-1.3.17/themes.c	2004-06-18 08:05:07.000000000 -0700
+++ vdr-1.3.17-ts/themes.c	2004-12-06 07:59:37.000000000 -0800
@@ -244,8 +244,8 @@
   if (themesDirectory) {
      DIR *d = opendir(themesDirectory);
      if (d) {
-        struct dirent *e;
-        while ((e = readdir(d)) != NULL) {
+        struct dirent *e, p;
+        while (!readdir_r(d, &p, &e) && e != NULL) {
               if (strcmp(e->d_name, ".") && strcmp(e->d_name, "..")) {
                  if (strstr(e->d_name, SkinName) == e->d_name && e->d_name[strlen(SkinName)] == '-') {
                     const char *FileName = AddDirectory(themesDirectory, e->d_name);
diff -ruN vdr-1.3.17/thread.c vdr-1.3.17-ts/thread.c
--- vdr-1.3.17/thread.c	2004-11-20 08:21:14.000000000 -0800
+++ vdr-1.3.17-ts/thread.c	2004-12-06 07:59:37.000000000 -0800
@@ -221,27 +221,35 @@
 
 void *cThread::StartThread(cThread *Thread)
 {
+  Thread->childTidMutex.Lock();
   Thread->childTid = pthread_self();
+  Thread->childTidChanged.Broadcast();
+  Thread->childTidMutex.Unlock();
   if (Thread->description)
      dsyslog("%s thread started (pid=%d, tid=%ld)", Thread->description, getpid(), Thread->childTid);
   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->childTidChanged.Broadcast();
+  Thread->childTidMutex.Unlock();
   return NULL;
 }
 
 bool cThread::Start(void)
 {
-  Lock();
+  cMutexLock startLock(&startMutex);
   if (!childTid) {
      parentTid = pthread_self();
      pthread_t Tid;
-     pthread_create(&Tid, NULL, (void *(*) (void *))&StartThread, (void *)this);
+     cMutexLock childTidLock(&childTidMutex);
+     int err = pthread_create(&Tid, NULL, (void *(*) (void *))&StartThread, (void *)this);
      pthread_detach(Tid); // auto-reap
      pthread_setschedparam(Tid, SCHED_RR, 0);
+     if (!err)
+       childTidChanged.Wait(childTidMutex);
      }
-  Unlock();
   return true; //XXX return value of pthread_create()???
 }
 
@@ -257,6 +265,7 @@
      // 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)
@@ -280,12 +289,12 @@
             }
         esyslog("ERROR: thread %ld won't end (waited %d seconds) - cancelling it...", childTid, WaitSeconds);
         }
-     Lock();
+     childTidMutex.Lock();
      if (childTid) {
         pthread_cancel(childTid);
         childTid = 0;
         }
-     Unlock();
+     childTidMutex.Unlock();
      }
 }
 
diff -ruN vdr-1.3.17/thread.h vdr-1.3.17-ts/thread.h
--- vdr-1.3.17/thread.h	2004-10-24 04:00:32.000000000 -0700
+++ vdr-1.3.17-ts/thread.h	2004-12-06 07:59:37.000000000 -0800
@@ -74,7 +74,9 @@
   friend class cThreadLock;
 private:
   pthread_t parentTid, childTid;
-  cMutex mutex;
+  cMutex childTidMutex;
+  cCondVar childTidChanged;
+  cMutex mutex, startMutex;
   char *description;
   static bool emergencyExitRequested;
   static void *StartThread(cThread *Thread);
diff -ruN vdr-1.3.17/vdr.c vdr-1.3.17-ts/vdr.c
--- vdr-1.3.17/vdr.c	2004-11-06 02:30:30.000000000 -0800
+++ vdr-1.3.17-ts/vdr.c	2004-12-06 07:59:37.000000000 -0800
@@ -865,9 +865,10 @@
                     free(buf);
                     }
                  if (!Next || Delta > Setup.MinEventTimeout * 60 || ForceShutdown) {
+                    char timebuf[26]; /* This magic number is based on the ctime(3c) man page */
                     ForceShutdown = false;
                     if (timer)
-                       dsyslog("next timer event at %s", ctime(&Next));
+                       dsyslog("next timer event at %s", ctime_r(&Next, timebuf));
                     if (WatchdogTimeout > 0)
                        signal(SIGALRM, SIG_IGN);
                     if (Interface->Confirm(tr("Press any key to cancel shutdown"), UserShutdown ? 5 : SHUTDOWNWAIT, true)) {

Attachment: vdr-1.3.17-poison-patches.tar.bz2
Description: Binary data


Home | Main Index | Thread Index