[asterisk-dev] [Code Review] Fix SLA bugs with SIP Channels (bugs 20440 and 20462)

[Matt Jordan]

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

Ship it!


I reviewed this again, as I had already taken a look at these patches on the Jira issues. My biggest concern was accessing args->station while in the loop; however, prior to dialing the trunk sla_station's ref count is increased (yes, the SLA code implements its own ref counting on the stations. Oh, for ao2 ref counted objects!) and is not decreased until after the condition has been signaled, which occurs after the loop returns.

So, this should be safe: the ref has been bumped for the duration of the dial.


- Matt


On Jan. 14, 2013, 4:57 p.m., dkerr wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2275/
> -----------------------------------------------------------
> 
> (Updated Jan. 14, 2013, 4:57 p.m.)
> 
> 
> Review request for Asterisk Developers, mattjordan and dkerr.
> 
> 
> Summary
> -------
> 
> SLA has two major problems when using the feature with SIP channels (and I suspect any channel type other than analog POTS).
> 1) No ringback is presented to the calling channel.
> 2) If the calling channel hangs up before called party answers, then the called channel is not hungup.
> See bugids ASTERISK-20440 and ASTERISK-20462 for more detail.  Both are included in a single patch as they hit the exact same area of code.
> 
> Note also the addition of a 1/10th second sleep. The SLA code has a tight loop during which it is polling for status change on the outgoing channel.  The loop ends when the channel answers.  This seems like an unnecessary CPU hog and so I added the 1/10th second sleep to moderate things -- seems unnecessary to be polling any more frequently than this.  However, the sleep is not integral to fixing either of these bugs.
> 
> 
> This addresses bugs ASTERISK-20440 and ASTERISK-20462.
>     https://issues.asterisk.org/jira/browse/ASTERISK-20440
>     https://issues.asterisk.org/jira/browse/ASTERISK-20462
> 
> 
> Diffs
> -----
> 
>   /trunk/apps/app_meetme.c 379070 
> 
> Diff: https://reviewboard.asterisk.org/r/2275/diff
> 
> 
> Testing
> -------
> 
> I have had this patch running on my system for several weeks and made many calls over two different SIP trunk providers and with a GTalk channel.  No problems encountered.
> 
> 
> Thanks,
> 
> dkerr
> 
>

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