<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://reviewboard.asterisk.org/r/2275/">https://reviewboard.asterisk.org/r/2275/</a>
</td>
</tr>
</table>
<br />
<p>Ship it!</p>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.
</pre>
<br />
<p>- Matt</p>
<br />
<p>On January 14th, 2013, 4:57 p.m., dkerr wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://reviewboard.asterisk.org/media/rb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Asterisk Developers, mattjordan and dkerr.</div>
<div>By dkerr.</div>
<p style="color: grey;"><i>Updated Jan. 14, 2013, 4:57 p.m.</i></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.
</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.</pre>
</td>
</tr>
</table>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="https://issues.asterisk.org/jira/browse/ASTERISK-20440">ASTERISK-20440</a>,
<a href="https://issues.asterisk.org/jira/browse/ASTERISK-20462">ASTERISK-20462</a>
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>/trunk/apps/app_meetme.c <span style="color: grey">(379070)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/2275/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>