On Don, 2008-03-06 at 07:39 -0400, Joshua Colp wrote:
> ---- Original Message -----
> From: Michael Neuhauser
> [mailto:mike@firmix.at]
> To: Asterisk Developers Mailing List
> [mailto:asterisk-dev@lists.digium.com]
> Sent: Thu, 06 Mar 2008 11:03:09
> -0400
> Subject: Re: [asterisk-dev] Asterisk 1.4.19-rc1 Now Available
> 
> 
> > On Mit, 2008-03-05 at 15:41 -0600, The Asterisk Development Team wrote:
> > > Greetings,
> > > 
> > > The Asterisk.org development team has released Asterisk 1.4.19-rc1.  This
> > is a
> > > test release for 1.4.19.  The official 1.4.19 release will be made after a
> > > 1.4.19 release candidate goes through a few days of testing without
> > finding any
> > > major regressions.
> > 
> > Not really a regression, but I just found out that the 1.6 bug 11914
> > (Redirect through AMI not working) is also present in 1.4.19-rc1. Would
> > be nice to have this fixed in the final 1.4.19.
> 
> Is this confirmed and tested to be the case or from a code review perspective?

Yes, it came from a real test. I'm developing an AMI server application
that dispatches calls with redirect while they wait in the dialplan,
something like this (in AEL syntax):
        UserEvent(DispatchMe);
        Wait(5);
        Log(ERROR|AMI did not dispatch call);
        // fallback to dialplan-only based call routing

When doing a redirect over AMI in this situation, the call was hung up
in 9 out of 10 cases. With the additional code from my previous email,
it never was hung up (several dozen test calls).

>   When I originally fixed this issue I did test 1.4 to make sure it
> was okay so I'm pretty confident it is fine.

I've added debug log statements to the pbx-loop and they showed that
_softhangup == AST_SOFTHANGUP_ASYNCGOTO and res == 0 and the following
code was executed:
	if (option_debug)
		ast_log(LOG_DEBUG, "Extension %s, priority %d returned normally even though call was hung up\n",
			c->exten, c->priority);
	error = 1;
	break;
The problem is that AST_SOFTHANGUP_ASYNCGOTO is only handled if
ast_spawn_extension() returns a non-zero value.

I think (i.e., this part is only based on code review) it is even worse
since it seems to be the case that no lock is protecting  c->_softhangup
in __ast_pbx_run(). So even with the added check, it could be possible
that a async-goto triggers a hangup  if another thread sets _softhangup
to AST_SOFTHANGUP_ASYNCGOTO while the pbx thread is right /**HERE**/:
	} else if (c->_softhangup == AST_SOFTHANGUP_ASYNCGOTO) {
		c->_softhangup = 0;
	} else /**HERE**/ if (c->_softhangup) {
Again, this is not verified in reality.

> The thing that might be throwing you off if this is a code review is
> that the PBX loops in 1.4 are quite different then trunk which renders
> the flow of execution different.

I've noticed the differences, but first came the error, then the review.
-- 
Dr. Michael Neuhauser                              mailto:mike@firmix.at
Firmix Software GmbH                                  sip:mike@firmix.at
Vienna/Austria/Europe                               tel:+43-1-7890849-30
Linux Development and Services                     http://www.firmix.at/