[asterisk-dev] Proper use of locking.

[Michael Cargile]

On Wed, 2007-07-18 at 16:43 -0500, Russell Bryant wrote:
> Kevin P. Fleming wrote:
> > Asterisk has always required and used recursive (reentrant is not the
> > right word) mutex locks (and now rwlocks, in trunk). All the code in
> > Asterisk that uses locks depends on this behavior and would break badly
> > if the mutexes were not recursive. This is documented in a comment very
> > early in include/asterisk/lock.h, so I'm not sure why you came to the
> > conclusion that non-recursive mutexes were in use.
> 
> Just as a related comment, the rwlocks aren't recursive, because they
> don't support it.  But *all* mutexes in Asterisk are defined as
> recursive, as you stated.
> 

I stand corrected. As far as naming is concerned (reentrant vs
recursive) it appears to have more to do with whom you learned locking
from and the terms that they used. As far as how I got that idea, the
last time we did deadlock debugging in the Asterisk source code
(somewhere in the head of 1.4) there were issues with some of the locks
being non-recursive. Since then, I have not looked too much into the
Asterisk code till recently when I got sick of my call quality going out
the window due to performance problems. On top of that my boss within
the last few weeks was having a discussion via IRC with other developers
on how to fix some non-recursive locks (most likely the rwlocks). You
are right though I should have taken the time to research the recursive
locks more thoroughly in Asterisk before I stated that.

This does not change the fact that locking is being poorly used in
Asterisk and is having a negative impact on performance as well as
making it extremely difficult to debug problems. Any good primer on
locking will tell you to acquire a lock and release it as fast as
possible. Hell even the developer of the Marco-Exclusive application
within Asterisk says to do so. Waiting on a slow, shared resource such
as a hard drive is just plain dumb. Plus there seems to be plenty of
places where rather than using several locks that only block threads
that could perform conflicting actions there is one monolithic lock that
blocks all threads. 

With the increasing rise in popularity of Asterisk and the increase
number of application, codecs, channels, function, etc that are being
added to it, it becomes more and more important that the Asterisk code
is properly locked, thread safe, and performing at it's optimum. Doing
so will insure that it not only performs better on monster multi-cored
systems but on small embedded devices as well. 

The only way I see to facilitate this is for those people who are
committing patches to reject ones that do silly things like those that I
wrote about in my previous email, have a janitorial project to scour the
code looking for these issues and fix them, and to write a good locking
practices section into the coding guidelines. I am more than willing to
help with all of these (except the checking patches for commit as I do
not have commit writes). We will probably be hiring someone else to do
so as well here in the not too distant future. We are planning to use
Asterisk for very a large call volume application shortly and having
Asterisk stable and performing optimally will save us far more than the
cost of a full time developer.

Michael Cargile
Explido Software USA Inc.
www.explido.us