[vdr] Re: vdr-1.3.17: thread issues with vdr-xine and vdr-osdteletext

[Reinhard Nissl <rnissl@gmx.de>]

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.

I'm not sure whether the above scenario can actually happen or whether 
another instance of some class mananges such a situation properly.

Anyway, care should be taken that starting and canceling a thread are 
mutually exclusive actions.

I didn't think about that in my previous patch either and it seems that 
such a situation didn't occur so far or it was already properly 
protected by the recently introduced calls to Lock() respectively Unlock().

Bye.
--
Dipl.-Inform. (FH) Reinhard Nissl
mailto:rnissl@gmx.de