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:
Hi,

Klaus Schmidinger 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.

But I don't think that you'll succeed without it.

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.

But it's not a single atomic read modify write instruction but two independent atomic instructions.

Please take a look at this and let me know if you can find a
possible loophole there.

 bool cThread::Start(void)
 {
+  if (!running) {

If a context switch happens here then you'll get two threads started. It's even more likely to happen with HT processors or on real SMP systems.

+ 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;
}

At least the above location needs to be protected by a Mutex. The other locations where this variable is used seem to be ok but I'm not sure about it. There might be race conditions if different threads start / cancel the same worker thread.

What happens if e. g. the transfer thread ends and detaches a receiver but at the same time osdteletext attaches a receiver to the same device?

A) device thread ends, teletext receiver never get's any data.
B) device thread stays up and get's canceled as it didn't terminate on request of the transfer receiver.
C) device thread ends for transfer receiver and get's started again for teletext receiver.
This will be taken care of by extending the locked area in cDevice::AttachReceiver(),
which actually has never been threadsafe with respect to receivers being attached from
within different threads. The same applies to cDevice::Detach(cReceiver *Receiver).

Klaus




Home | Main Index | Thread Index