[asterisk-dev] [Code Review] Fix broken redirect out of MeetMe() as well as the deeper issue at hand

[rmudgett]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/1082/#review3125
-----------------------------------------------------------

Ship it!


I don't see anything more to really comment about.


/branches/1.8/main/features.c
<https://reviewboard.asterisk.org/r/1082/#comment6389>

    I don't know why this line was even here to begin with.  I don't think that _softhangup will ever be nonzero here.


- rmudgett


On 2011-01-24 13:07:03, Russell Bryant wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1082/
> -----------------------------------------------------------
> 
> (Updated 2011-01-24 13:07:03)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> Mantis issue #18585 reports that a channel redirect out of MeetMe() stopped working properly.  This issue includes a patch that resolves the issue by removing a call to ast_check_hangup() from app_meetme.c.  I left that in my patch, as it doesn't need to be there.  However, the rest of the patch fixes this problem with or without the change to app_meetme.
> 
> The key difference between what happens before and after this patch is the effect of the END_OF_Q control frame.  After END_OF_Q is hit in ast_read(), ast_read() will return NULL.  With the ast_check_hangup() removed, app_meetme sees this which causes it to exit as intended.  Checking ast_check_hangup() caused app_meetme to exit earlier in the process, and the target of the redirect saw the condition where ast_read() returned NULL.
> 
> Removing ast_check_hangup() works around the issue in app_meetme, but doesn't solve the issue if another application did the same thing.  There are also other edge cases where if an application finishes at the same time that a redirect happens, the target of the redirect will think that the channel hung up.  So, I made some changes in pbx.c to resolve it at a deeper level.  There are already places that unset the SOFTHANGUP_ASYNCGOTO flag in an attempt to abort the hangup process.  My patch extends this to remove the END_OF_Q frame from the channel's read queue, making the "abort hangup" more complete. 
> 
> 
> This addresses bug 18585.
>     https://issues.asterisk.org/view.php?id=18585
> 
> 
> Diffs
> -----
> 
>   /branches/1.8/apps/app_meetme.c 303508 
>   /branches/1.8/include/asterisk/channel.h 303508 
>   /branches/1.8/main/channel.c 303508 
>   /branches/1.8/main/features.c 303508 
>   /branches/1.8/main/pbx.c 303508 
> 
> Diff: https://reviewboard.asterisk.org/r/1082/diff
> 
> 
> Testing
> -------
> 
> I replicated the bug easily.  I verified that removing ast_check_hangup() fixes the issue and determined why it fixes it.  I also verified that the issue is resolved with the pbx.c changes, both with and without ast_check_hangup() still used in app_meetme.
> 
> 
> Thanks,
> 
> Russell
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20110124/1861c1ea/attachment-0001.htm>