Dbus pthread mutex lock race condition.

[Simon McVittie]

On Wed, 14 Dec 2011 at 21:16:31 -0800, Jaikumar Ganesh wrote:
>     There is a race condition in the basic pthread mutex lock code for dbus.

Thanks! Is this the same thing as
<https://bugs.freedesktop.org/show_bug.cgi?id=43744>? If so, let's
discuss it there so it doesn't get lost; please add yourself to the cc.

> Another way to fix this is to swap the order of the following 2 statements
> in _dbus_pthread_condvar_wait_timeout and
> _dbus_pthread_condvar_wait pmutex->count = old_count; pmutex->holder =
> pthread_self(); The above 2 statements need to be swapped.

I'm wary of this sort of thing because I'm pretty sure those writes aren't
guaranteed to be atomic, and if they're not declared volatile, the compiler
is free  to optimize them back into the "wrong" order anyway.

The solution given in your patch is likely to be better, because it protects
holder with the underlying pthread mutex (but I don't think all the other
accesses are protected).

If it has the right semantics, then the solution given on the bug is probably
better still, because it delegates correct implementation of a recursive
mutex to the pthreads implementation, which is considerably more likely to
have got it right than we are! But I'm a bit concerned about doing that,
because there's a stated reason in the source why ordinary pthreads
recursive mutexes weren't used (although I haven't worked out why, or
indeed whether, it's still important).

    S