[asterisk-dev] Locking, coding guidelines addition

[Luigi Rizzo]

Probably my original reply on russel's email on locking coding guidelines

http://lists.digium.com/pipermail/asterisk-dev/2008-June/033662.html

did not make it so i am resending.

Documenting locking is certainly useful and the description you
give is well written.  It suffers from a problem though:

    while it is a faithful description of the way locking is implemented
    now, it does not clarify that even the trylock-based approach that
    is presented does not guarantee deadlock avoidance -- e.g. if both
    contenders have called lock() twice or more, they are still going
    to loop forever.

Basically there isn't a good solution with recursive locks(*).

All we could do, in terms of documentation, is suggest developers
to try and avoid exploiting recursive locks, and instead try as much
as possible to code as if locks were not recursive.
Eventually, this should let us move, in the long term, to non-recursive
locks that allows a better handling of deadlock, based on lock
ordering, or (if you really have to use the trylock/unlock trick)
makes it slightly easier to check that releasing and reacquiring a
lock does not lead to inconsistencies.

If you agree to this approach i might have some more suggestions on
how to reorganize the text on locking.

(*) One would think that the trylock loop can be made deadlock-free
using an unlock_all() call that invokes unlock() until the lock
is finally released, and also returns the count so later we can call
lock() N times. However there is a big danger here -- when you release
and reacquire the lock, the data structures they protect might change,
so the code should check that nothing bad happened.
But one uses recursive locks when he doesn't/cannot track whether
a function is called with the lock held or not, so you can imagine
how hard it is for the same code to check that releasing the lock is safe.

        cheers
        luigi