Dbus pthread mutex lock race condition.

[Jaikumar Ganesh]

Hello,

On Wed, Dec 14, 2011 at 9:16 PM, Jaikumar Ganesh <jaikumarg at gmail.com>wrote:

> Folks,
>     There is a race condition in the basic pthread mutex lock code for
> dbus.
>
> Thread 1:
>    Calls _dbus_connection_acquire_io_path -> grabs the mutex ->
>  io_path_acquired = true -> releases the mutex -> exits the function.
>
> Thread 2:
>    Calls _dbus_connection_acquire_io_path -> grabs the mutex
> -> io_path_acquired is already true -> _dbus_condvar_wait_timeout
>
> Thread 1:
>    Calls _dbus_connection_release_io_path -> grabs the mutex -> sets
> io_path_acquired = false -> signals thread 1 and releases the mutex.
>
>     Calls _dbus_connection_acquire_io_path again tries to grab the mutex.
>
>
> Now, in _dbus_pthread_condvar_wait_timeout:
>
>      result = pthread_cond_timedwait (&pcond->cond, &pmutex->lock,
> &end_time);
>     ......
>   _dbus_assert (pmutex->count == 0);
>   pmutex->count = old_count;
>   pmutex->holder = pthread_self();
>
>
> We set the holder back to Thread 2 only after some time.
> During this time Thread 1 can grab the mutex since pthread->holder is
> still set to Thread1.  In _dbus_pthread_mutex_lock function
> that check is enough to grab the lock.
>
> So currently, Thread 2 has woken up from cond_timewait -> gone back to the
> acquire_io_path_function and set io_path_acquired to true.
>
> Thread 1, has been able to grab the mutex -> checks io_path_acquired
> variable, sees that it already true -> calls condvar_wait_timeout.
>
> which asserts that the pthread->holder and pthread_self is the same. This
> assert will fail.
>
> Hope the above make sense. The attached patch fixes the problem for me.
>


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.


>
> Thanks
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/dbus/attachments/20111214/86a35578/attachment.html>