Mailing List archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[vdr] Re: vdr-1.3.17: thread issues with vdr-xine and vdr-osdteletext
Reinhard Nissl wrote:
[ ... fixing thread handling ... ]
Somehow this is getting a little too complicated for my taste.
There's way too much locking and condvar stuff being introduced.
I suggest the attached patch against thread.[hc] of VDR 1.3.17.
The main changes here are that the child thread no longer
accesses childTid at all (it calls pthread_self() directly),
and the running state is stored in a simple boolean variable,
which requires no extensive locking since it is read/written
atomically. parentTid has been removed, since it was never actually
used.
Please take a look at this and let me know if you can find a
possible loophole there.
Klaus
--- thread.h 2004/10/24 11:00:32 1.24
+++ thread.h 2004/12/19 10:43:10
@@ -4,7 +4,7 @@
* See the main source file 'vdr.c' for copyright information and
* how to reach the author.
*
- * $Id: thread.h 1.24 2004/10/24 11:00:32 kls Exp $
+ * $Id: thread.h 1.25 2004/11/26 13:33:26 kls Exp kls $
*/
#ifndef __THREAD_H
@@ -73,7 +73,8 @@
class cThread {
friend class cThreadLock;
private:
- pthread_t parentTid, childTid;
+ bool running;
+ pthread_t childTid;
cMutex mutex;
char *description;
static bool emergencyExitRequested;
--- thread.c 2004/11/20 16:21:14 1.37
+++ thread.c 2004/12/19 10:43:14
@@ -4,7 +4,7 @@
* See the main source file 'vdr.c' for copyright information and
* how to reach the author.
*
- * $Id: thread.c 1.37 2004/11/20 16:21:14 kls Exp $
+ * $Id: thread.c 1.39 2004/11/26 14:16:07 kls Exp kls $
*/
#include "thread.h"
@@ -197,7 +197,8 @@
cThread::cThread(const char *Description)
{
- parentTid = childTid = 0;
+ running = false;
+ childTid = 0;
description = NULL;
SetDescription(Description);
}
@@ -221,33 +222,35 @@
void *cThread::StartThread(cThread *Thread)
{
- Thread->childTid = pthread_self();
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->childTid = 0;
+ dsyslog("%s thread ended (pid=%d, tid=%ld)", Thread->description, getpid(), pthread_self());
+ Thread->running = false;
return NULL;
}
bool cThread::Start(void)
{
- Lock();
- 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;
+ }
}
- Unlock();
- return true; //XXX return value of pthread_create()???
+ return true;
}
bool cThread::Active(void)
{
- if (childTid) {
+ if (running) {
//
// Single UNIX Spec v2 says:
//
@@ -262,6 +265,7 @@
if (err != ESRCH)
LOG_ERROR;
childTid = 0;
+ running = false;
}
else
return true;
@@ -271,21 +275,18 @@
void cThread::Cancel(int WaitSeconds)
{
- if (childTid) {
+ if (running) {
if (WaitSeconds > 0) {
for (time_t t0 = time(NULL) + WaitSeconds; time(NULL) < t0; ) {
if (!Active())
return;
cCondWait::SleepMs(10);
}
- esyslog("ERROR: thread %ld won't end (waited %d seconds) - cancelling it...", childTid, WaitSeconds);
- }
- Lock();
- if (childTid) {
- pthread_cancel(childTid);
- childTid = 0;
+ esyslog("ERROR: thread %ld won't end (waited %d seconds) - canceling it...", childTid, WaitSeconds);
}
- Unlock();
+ pthread_cancel(childTid);
+ childTid = 0;
+ running = false;
}
}
Home |
Main Index |
Thread Index