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

[Andreas Schultz <Andreas.Schultz@aschultz.net>]

Hi,

Klaus Schmidinger wrote:

[...]

>>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


Or that "had" the lock before. lockingPid is not cleared when the lock 
is released. Therefor a thread could see it's own pid in lockingPid 
without holding the lock.
The solution is of course to clear lockingPid right before the lock is 
released.
If you are interrested, i have also a solution that works without the 
double check.

> 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).


Meanwhile, i believe that it is very unlikely that we would hit this 
race under normal circumstances.

Andreas

> 
> Klaus
>