[vdr] Re: poisoned vdr plugins

[Klaus Schmidinger <Klaus.Schmidinger@cadsoft.de>]

C.Y.M wrote:
> After poisoning vdr with the patch below, I went through all the plugins 
> and made them compile using the thread safe alternatives.
>
> --- vdr-1.3.17-mod-eepg-orig/config.h   2004-11-24 07:51:23.000000000 -0800
> +++ vdr-1.3.17-mod-eepg/config.h        2004-12-04 14:57:51.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"
>
>
> Some of the plugins, such as streamdev, only required a change in the 
> order of #includes (putting the vdr #includes after the system) to fix 
> the compilation.  But, unfortunately, streamdev still crashes with 
> buffer overflows if I include the recent changes to thread.[ch] found in 
> vdr-1.3.17 (vanilla thread.c in vdr-1.3.17 also does not work). 
> Streamdev still only appears to work if I revert vdr-1.3.17 back to the 
> previous threading model of 1.3.16.  It appears there is much more to 
> investigate.
>
> Regards,
> C.Y.M.
Maybe we should just lean back for a moment and take a close look
at the actual changes between the thread.[hc] files from VDR version
1.3.16 and the latest ones at ftp://ftp.cadsoft.de/vdr/Developer.

================================================
--- thread.h    2004/10/24 11:00:32     1.24
+++ thread.h    2004/11/26 13:33:26     1.25
@@ -74,6 +74,7 @@
   friend class cThreadLock;
 private:
   pthread_t parentTid, childTid;
+  cMutex childTidMutex;
   cMutex mutex;
   char *description;
   static bool emergencyExitRequested;
================================================

*** This just introduces a new member to control access to the 'childTid'.

================================================
--- thread.c    2004/10/31 09:54:02     1.36
+++ thread.c    2004/11/26 13:50:37     1.38
@@ -221,13 +221,17 @@

 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);
   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();
   return NULL;
 }
================================================

*** Here access to 'childTid' is controlled using the new 'childTidMutex'.
    I wouldn't expect any side effects from this.

================================================
@@ -235,9 +239,10 @@
 {
   if (!childTid) {
      parentTid = pthread_self();
-     pthread_create(&childTid, NULL, (void *(*) (void *))&StartThread, (void *)this);
-     pthread_detach(childTid); // auto-reap
-     pthread_setschedparam(childTid, SCHED_RR, 0);
+     pthread_t Tid;
+     pthread_create(&Tid, NULL, (void *(*) (void *))&StartThread, (void *)this);
+     pthread_detach(Tid); // auto-reap
+     pthread_setschedparam(Tid, SCHED_RR, 0);
      }
   return true; //XXX return value of pthread_create()???
 }
================================================

*** Previously this code (which runs in the foreground thread) has written
    to 'childTid', which was also done in StartThread() (see above), which
    runs in the actual background thread. To avoid a possible problem with
    both threads accessing 'childTid' at the same time, the forground thread
    now uses a separate variable 'Tid'.

================================================
@@ -254,6 +259,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)
================================================

*** The 'Active()' function now locks 'childTidMutex' before accessing
    'childTid'. It then just sends a signal to the thread to see if it
    is still alive and then returns, giving up the lock again.

================================================
@@ -277,10 +283,12 @@
             }
         esyslog("ERROR: thread %ld won't end (waited %d seconds) - cancelling it...", childTid, WaitSeconds);
         }
+     childTidMutex.Lock();
      if (childTid) {
         pthread_cancel(childTid);
         childTid = 0;
         }
+     childTidMutex.Unlock();
      }
 }
================================================

*** Again, access to 'childTid' is controlled by locking 'childTidMutex'


Now, does anybody have any idea why these changes would cause all
the described malfunctions in plugins?

Is everybody who has encountered problems with plugins *ABSOLUTELY*
*POSITIVELY* *ONEHUNDRED PERCENT* sure that they did

  make plugins-clean clean vdr plugins

before running VDR with the modified thread.[hc]?

Klaus


Klaus