Difference between revisions of "Locking Order"

Revision as of 15:56, 15 August 2011

If a class uses multiple locks, it must define a locking order. This is best explained with an example.

This is the locking order documented in tv_play.h 2011-08-15

// Locking order
//
// playerLock -> askAllowLock    -> osdLock
//            -> progListLock    -> osdLock
//            -> chanEditMapLock -> osdLock
//            -> lastProgramLock
//            -> is_tunable_cache_lock
//            -> recorderPlaybackInfoLock
//            -> timerIdLock
//            -> mainLoopCondLock
//            -> channelGroupLock

The locking order is there to prevent one thread from locking osdLock and then askAllowLock while another thread locks askAllowLock and then osdLock. What happens in that case is eventually one of those threads locks osdLock while the other locks askAllowLock and then neither can proceed.

The first line here tells you that you can lock playerLock, askAllowLock, and osdLock, but if you do they must be locked in that order. You can lock just osdLock, or just askAllowLock and osdLock, if you do it in this order. You may not lock askAllowLock and channelGroupLock at the same time, nor may you lock channelGroupLock and then osdLock.

You of course can change the code and change the locking order; but you must then audit the uses of the locks in the new locking order to ensure that they are always locked in that order and you must update the locking order comment.

Most locks in MythTV are non-recursive locks. These locks can be used with QWaitCondition so that another thread can be notified immediately when the lock is released and take action and deadlocks due to poor locking discipline can be detected more quickly. Try to structure your code so that you can use non-recursive locks; it can be very difficult to convert a class to non-recursive locks at a later date. Most locks in MythTV are a QMutex instance. These are the simplest locks and should be used whenever the same thread does the locking and unlocking and you don't need the advanced features of other lock types. Other lock types are QSemaphore and QReadWriteLock and tend to be used when multiple threads need to be able to read some state simultaneously, use these with care when necessary for acceptable performance.

There are times when multiple locks must be taken and there is no locking order. In these cases you need to write a loop uses tryLock and unlocks locks not acquired, waits and then tries again. This is inefficient, so try to impose some arbitrary locking order if at all possible. If not, document this unusual situation extensively and emphatically.

There are times when you can't return a copy of some object owned by an a particular class. There are two ways to deal with it: Return the object and transfer ownership to the calling class, or return the object and take hold of an internal lock. If you do the first, please document that the call transfers ownership and remove all internal references to the object. If you do the first call the method returning the object "Obj *GetXXXLock(...)" and call the method that returns the lock "ReturnXXXLock(... Obj*&)" Where the return method clears the Obj* pointer to NULL. This will cue the user of the class into the API. This is a often the source of locking order violations and anyone using a locked resource from another class should not call any method within that other class while holding the lock unless it has been explicitly documented as being safe.

Qt contains some classes that allow you to implement thread-safe code without locking, through the use of atomic increments and decrements. Please only implement these when there is a real performance gain to be had. The API's are by necessity complicated and it is easy to write code that is less efficient due to the creation of long running busy loops and of course it is easy to write code that is in fact not thread-safe.