An alternative approach to fixing the Event/EventGroup deadlock with less locking...
authorJim Carroll <thecarrolls@jiminger.com>
Sun, 3 Jul 2011 18:16:29 +0000 (14:16 -0400)
committerJim Carroll <thecarrolls@jiminger.com>
Tue, 5 Jul 2011 13:15:51 +0000 (09:15 -0400)
commitc0309c32ff1fe1dcb0c455c3bb4de0405238e5c7
tree64943ac211c75d2aba898a30c764e66ca6cbc5a7
parentfc4ea923b2f4c00236f208f70fd25b8bf2c27ac0
An alternative approach to fixing the Event/EventGroup deadlock with less locking and less code.

This change revisits the previous deadlock fix ( https://github.com/xbmc/xbmc/commit/1e1ff6503eac46d330e2263bcff2dadf9df4cd2a ) and accomplishes the same task with less code and less locking. It primarily does the following:

As a quick review, the deadlock was a result of accessing locks in different orders between the CEvent::Set call and the CEventGroup::wait call.

1) Separate the lock used by the condition variable in a CEvent from the lock used to prevent the list of groups the CEvent is part of from being updated.
2) There is no reason to hold a lock around a notify (Set/Signal) on a condition variable. Removing this lock means the helper guard class, whose job it was to lock all of the parents prior to locking the child's condition variable mutex to insure the proper ordering, is no longer needed.
3) The CEventGroup::wait must end up calling the condition variable's wait even when the wait time is zero in order to make sure that cross thread values are synchronized (previously there was an optimization which skipped the condVar.wait call when the timeout was zero).

Effects:
The 'Set' call has less locking. Now the notify/Set/Signal is called without holding any locks and then the group list lock is held to check and see if the CEvent is participating in any groups. If not no more locking is done. If so, each group is also 'Set' which holds the group's condition variable lock. This results in:

when the CEvent is part of a group: CEvent::Set obtains list mutex -> individual group mutex
when the CEvent is alone: CEvent::Set obtains the list mutex only.

This is better than the current which always holds two locks nested when CEvent::Set is called.
xbmc/threads/Condition.h
xbmc/threads/Event.cpp
xbmc/threads/Event.h
xbmc/threads/test/TestEvent.cpp