[asterisk-dev] [svn-commits] tilghman: branch 1.4 r120001 - /branches/1.4/channels/chan_iax2.c

[Tilghman Lesher]

On Tuesday 03 June 2008 16:27:38 Russell Bryant wrote:
> SVN commits to the Digium repositories wrote:
> > Author: tilghman
> > Date: Tue Jun  3 11:10:53 2008
> > New Revision: 120001
> >
> > URL: http://svn.digium.com/view/asterisk?view=rev&rev=120001
> > Log:
> > Save the callno when we're poking, because our peer structure could
> > change during deadlock avoidance (and thus we unlock the wrong callno,
> > causing a cascade failure).
> > (closes issue #12717)
> >  Reported by: gewfie
> >  Patches:
> >        20080525__bug12717.diff.txt uploaded by Corydon76 (license 14)
> >  Tested by: gewfie
> >
> > Modified:
> >     branches/1.4/channels/chan_iax2.c
> >
> > Modified: branches/1.4/channels/chan_iax2.c
> > URL:
> > http://svn.digium.com/view/asterisk/branches/1.4/channels/chan_iax2.c?vie
> >w=diff&rev=120001&r1=120000&r2=120001
> > =========================================================================
> >===== --- branches/1.4/channels/chan_iax2.c (original)
> > +++ branches/1.4/channels/chan_iax2.c Tue Jun  3 11:10:53 2008
> > @@ -8842,6 +8842,7 @@
> >
> >  static int iax2_poke_peer(struct iax2_peer *peer, int heldcall)
> >  {
> > +	int callno;
> >  	if (!peer->maxms || (!peer->addr.sin_addr.s_addr && !peer->dnsmgr)) {
> >  		/* IF we have no IP without dnsmgr, or this isn't to be monitored,
> > return immediately after clearing things out */
> > @@ -8851,11 +8852,13 @@
> >  		peer->callno = 0;
> >  		return 0;
> >  	}
> > -	if (peer->callno > 0) {
> > +
> > +	/* The peer could change the callno inside iax2_destroy, since we do
> > deadlock avoidance */ +	if ((callno = peer->callno) > 0) {
> >  		ast_log(LOG_NOTICE, "Still have a callno...\n");
> > -		ast_mutex_lock(&iaxsl[peer->callno]);
> > -		iax2_destroy(peer->callno);
> > -		ast_mutex_unlock(&iaxsl[peer->callno]);
> > +		ast_mutex_lock(&iaxsl[callno]);
> > +		iax2_destroy(callno);
> > +		ast_mutex_unlock(&iaxsl[callno]);
> >  	}
> >  	if (heldcall)
> >  		ast_mutex_unlock(&iaxsl[heldcall]);
>
> What deadlock avoidance are you referring to here?  It looks to me like
> the real problem is that the peer->callno could change at any time,
> because the peer is not locked at this point.  However, with the change
> you have added, it's not needed anymore.  I just wanted to point out
> where the problem came from.

The deadlock avoidance occurs within iax2_destroy(callno)

-- 
Tilghman