[asterisk-dev] A general locking strategy in Asterisk - completely missing

[Hans Petter Selasky]

Hi,

I'm debugging a deadlock in Asterisk 1.2.13 for someone and I am a little 
frustrated that there are no clear locking guidelines in Asterisk. In my 
channel driver I have tried to make a list of the locking order, at least:

 *
 * 1. cc_mutex_lock(&do_periodic_lock); (private lock)
 *
 * 2. cc_mutex_lock(&modlock); (See Asterisk source code)
 *
 * 3. cc_mutex_lock(&chlock); (See Asterisk source code)
 *
 * 4. cc_mutex_lock(&cd->pbx_chan->lock); (struct ast_channel *pbx_chan)
 *
 * 5. cc_mutex_lock(&p_app->lock); (my private channel driver lock)
 *
 * 6. cc_mutex_lock(&capi_global_lock); (my global lock)
 *
 * 7. cc_mutex_lock(&capi_verbose_lock); (my verbose lock)

My conclution is that Asterisk is a single locked system by design.

For example when I am at level 5) I cannot call:

ast_update_use_count();
ast_channel_free();
ast_hangup();

And many more, simply because these functions lock &modlock or &chlock, which 
are mutexes at a higher level. Of course this is no problem if these locks 
are already locked, but if the CPU jumps in at level 5, and I want to hangup 
a channel atomically, then I cannot do that, because I have to unlock the 
mutex at level 5, and lock the mutex at level 3 first. This breaks atomicity. 
So in other words, to allow me to do what I want to do, I need to lock mutex 
2 & 3 & 4 first, before I lock mutex 5. Asterisk is a single locked system in 
other words, hence I always have to lock mutex 2 no matter what I do, to 
ensure 100% atomicity.

Here is one root of the problem, see "channel_find_locked()" in the Asterisk 
sources:

lock(&chlock);
lock(&pbx_chan->lock);
unlock(&chlock);
  do something();
unlock(&pbx_chan->lock);

It locks "chlock" when it traverses the channel-list. Then it locks the 
channel's lock. Then it unlocks the "chlock". This is headless programming.

The solution is this (so that &chlock gets as far down as possible in the 
locking hiherarchy):

lock(&chlock);
unlock(&chlock);

lock(&pbx_chan->lock);
  do something();
unlock(&pbx_chan->lock);

Of course this has a problem: The channel might be freed in the unlocked gap. 
But hey. There is something called refcounts. You need to keep refcounts:

lock(&chlock);
pbx_chan->refcount ++;
unlock(&chlock);

lock(&pbx_chan->lock);
  do something();
unlock(&pbx_chan->lock);

lock(&chlock);
pbx_chan->refcount --;
if (pbx_chan->refcount == 0) {
	free(pbx_chan);
}
unlock(&chlock);

--HPS

RULE #1: The Gianter mutex should be locked last.