[asterisk-dev] tilghman: branch 1.4 r125740 - /branches/1.4/channels/chan_local.c

[Tilghman Lesher]

On Friday 27 June 2008 09:27:15 Russell Bryant wrote:
> On Jun 27, 2008, at 8:19 AM, SVN commits to the Digium repositories
>
> wrote:
> > Author: tilghman
> > Date: Fri Jun 27 07:19:39 2008
> > New Revision: 125740
> >
> > URL: http://svn.digium.com/view/asterisk?view=rev&rev=125740
> > Log:
> > Add proper deadlock avoidance.
> > (closes issue #12914)
> > Reported by: ozan
> > Patches:
> >       20080625__bug12914.diff.txt uploaded by Corydon76 (license 14)
> > Tested by: ozan
> >
> > Modified:
> >    branches/1.4/channels/chan_local.c
> >
> > Modified: branches/1.4/channels/chan_local.c
> > URL:
> > http://svn.digium.com/view/asterisk/branches/1.4/channels/chan_local.c?vi
> >ew=diff&rev=125740&r1=125739&r2=125740 =
> > =
> > =
> > =
> > =
> > =
> > =
> > =
> > ======================================================================
> > --- branches/1.4/channels/chan_local.c (original)
> > +++ branches/1.4/channels/chan_local.c Fri Jun 27 07:19:39 2008
> > @@ -186,13 +186,15 @@
> > 	while (other && ast_channel_trylock(other)) {
> > 		ast_mutex_unlock(&p->lock);
> > 		if (us && us_locked) {
> > -			ast_channel_unlock(us);
> > -		}
> > -		usleep(1);
> > -		if (us && us_locked) {
> > -			ast_channel_lock(us);
> > -		}
> > -		ast_mutex_lock(&p->lock);
> > +			do {
> > +				ast_channel_unlock(us);
> > +				usleep(1);
> > +				ast_channel_lock(us);
> > +			} while (ast_mutex_trylock(&p->lock));
> > +		} else {
> > +			usleep(1);
> > +			ast_mutex_lock(&p->lock);
> > +		}
> > 		other = isoutbound ? p->owner : p->chan;
> > 	}
> >
> > @@ -510,7 +512,12 @@
> > 	if (!p)
> > 		return -1;
> >
> > -	ast_mutex_lock(&p->lock);
> > +	while (ast_mutex_trylock(&p->lock)) {
> > +		ast_channel_unlock(ast);
> > +		usleep(1);
> > +		ast_channel_lock(ast);
> > +	}
> > +
> > 	isoutbound = IS_OUTBOUND(ast, p);
> > 	if (isoutbound) {
> > 		const char *status = pbx_builtin_getvar_helper(p->chan,
> > "DIALSTATUS");
>
> As I indicated in another response, this code construct is not correct
> since the channel must always be locked before the pvt.  When going
> that direction, this avoidance code should not be used.  This deadlock
> avoidance is expensive, and should be avoided when unnecessary.

This second case may have been overkill.  I'll revert it.

-- 
Tilghman