[vdr] Re: vdr[3330]: ERROR: can't record MPEG1! - possible cause

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

Andreas Schultz wrote:
> 
> Hi all,
> 
> The MPEG1 problem caught my intressted when it became apparent that
> there could be a race condition involved that is only been triggered on
> fast systems.
> 
> And indeed there is a race that could be related to the problem.
> thread.c, arround line 70:
> 
> void cMutex::Lock(void)
> {
>   if (getpid() != lockingPid || !locked)
>      pthread_mutex_lock(&mutex);
>   lockingPid = getpid();
>   locked++;
> }
> 
> The race is in the if condition. Consider this: execution switches from
> thread A to thread B, after thread A has executed the first part of the
> condition (getpid() != lockingPid), but not the second part, at this
> point thread B could execute the full statement above, setting locked to
> 1, if at this point execution switches back to thread A, the second part
> of the condition will return true, making thread A belive that he is
> holding the lock, and therefore doesn't need to aquire it, where in
> truth thread B is holding the lock.

Meanwhile I am beginning to doubt that there can be a race condition here
at all. 'lockingPid' is 0 at startup, so any thread trying to lock the mutex
will see the first part of the 'if' as 'true' and will directly go to calling
'pthread_mutex_lock()'. Of course, only one thread will return from this call
and actually "have" the lock. This thread will set 'lockingPid' to its very own
PID and increment 'locked'.

At this point, again, any other thread trying to to set the lock will see the
first part of the 'if' as true and directly go to calling 'pthread_mutex_lock()'.
The only thread that will ever actually see the first part of the 'if' as 'false'
is the thread that currently "has" the lock. And for that thread it is absolutely
safe to check the second part of the 'if' - it will see that 'locked' is not 0
and will _not_ call 'pthread_mutex_lock()'.

Since the only thread that can see the first part of the 'if' as false is the
one actually holding the lock, no two threads can see that part of the condition
as true at the same time.

What _could_ be changed is the setting of 'lockingPid = getpid()', which would make
sense to be done only inside the 'if' branch. But then agian, I don't think that
that would make any difference.

Please correct me if I'm wrong, but as far as I can see there is no race condition
here, so I guess using recursive mutexes won't get us any further (except that
channel switching will hang).

Klaus
-- 
_______________________________________________________________

Klaus Schmidinger                       Phone: +49-8635-6989-10
CadSoft Computer GmbH                   Fax:   +49-8635-6989-40
Hofmark 2                               Email:   kls@cadsoft.de
D-84568 Pleiskirchen, Germany           URL:     www.cadsoft.de
_______________________________________________________________