<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/2305/">https://reviewboard.asterisk.org/r/2305/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On February 5th, 2013, 10:42 a.m., <b>jcolp</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://reviewboard.asterisk.org/r/2305/diff/1/?file=33189#file33189line512" style="color: black; font-weight: bold; text-decoration: underline;">/team/group/pimp_my_sip/channels/chan_gulp.c</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">static int gulp_digit_end(struct ast_channel *ast, char digit, unsigned int duration)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">419</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">if</span> <span class="p">(</span><span class="n">pjsip_inv_invite</span><span class="p">(</span><span class="n">session</span><span class="o">-></span><span class="n">inv_session</span><span class="p">,</span> <span class="o">&</span><span class="n">packet</span><span class="p">)</span> <span class="o">!=</span> <span class="n">PJ_SUCCESS</span><span class="p">)</span> <span class="p">{</span></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">491</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">if</span> <span class="p">(</span><span class="n">pjsip_inv_invite</span><span class="p">(</span><span class="n">session</span><span class="o">-></span><span class="n">inv_session</span><span class="p">,</span> <span class="o">&</span><span class="n">packet</span><span class="p">)</span> <span class="o">!=</span> <span class="n">PJ_SUCCESS</span><span class="p">)</span> <span class="p">{</span></pre></td>
</tr>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">420</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">return</span> <span class="o">-</span><span class="mi">1</span><span class="p">;</span></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">492</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">return</span> <span class="o">-</span><span class="mi">1</span><span class="p">;</span></pre></td>
</tr>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">421</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="p">}</span></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">493</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="p">}</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Failure case here causes different behavior than before. Previously whatever is executing the call would have received an error and now it will not. I think we'll have to queue up a hangup with a suitable cause code instead.</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Hm, what if I pushed a synchronous task instead? Since no SIP traffic has occurred on this dialog, this *should* be top of the queue to execute.</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On February 5th, 2013, 10:42 a.m., <b>jcolp</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://reviewboard.asterisk.org/r/2305/diff/1/?file=33199#file33199line1236" style="color: black; font-weight: bold; text-decoration: underline;">/team/group/pimp_my_sip/res/res_sip_session.c</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">static void session_inv_on_rx_offer(pjsip_inv_session *inv, const pjmedia_sdp_session *offer)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">1227</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">ast_log</span><span class="p">(</span><span class="n">LOG_WARNING</span><span class="p">,</span> <span class="s">"Synchronous threadpool task to create SDP answer failed. A crash may occur</span><span class="se">\n</span><span class="s">"</span><span class="p">);</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Fairly certain this log message is actually incorrect. It'll just use the old local SDP for the negotiation.</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">My thought was for the case where we didn't have a local SDP. Thinking more deeply though, I can't think of a case where we wouldn't already have a local SDP though.</pre>
<br />
<p>- Mark</p>
<br />
<p>On January 31st, 2013, 1:01 p.m., Mark Michelson 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, Matt Jordan and jcolp.</div>
<div>By Mark Michelson.</div>
<p style="color: grey;"><i>Updated Jan. 31, 2013, 1:01 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;">This changeset introduces threadpool usage into the new SIP work. Function calls that originate from the Asterisk core (such as channel callbacks) and from PJSIP's event handling thread (such as incoming request/response callbacks) now push their work into the SIP threadpool.
The benefits to this changeset are:
1) Frees up PJSIP event handling threads so that they can process more incoming SIP messages at a time
2) Places similar SIP tasks into their own task queue so that they can be completed in sequence, thus limiting resource contention. So far, session tasks are the only ones that use this mechanism
I added a new API call called ast_sip_push_task_synchronous(), that allows for you to push a task to a servant and block until the task completes. This is used in several cases where Asterisk threads need to make use of a PJSIP feature but need to have the result of the pushed task before returning. A good example of this is gulp_request() in chan_gulp. We need to call some PJSIP routines to set up a UAC and a dialog and such. But we also need to return a new ast_channel before gulp_request() returns.
Please have a look over this to see if I've made any obvious mistakes (e.g. memory leaks, tasks being not being executed in the threadpool when they should be)</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've run the OPTIONS test in the testsuite and can confirm that Asterisk shutdown does not crash Asterisk any more. I also ran incoming and outgoing calls and ensured that they completed correctly with no issue.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>/team/group/pimp_my_sip/channels/chan_gulp.c <span style="color: grey">(380670)</span></li>
<li>/team/group/pimp_my_sip/include/asterisk/res_sip.h <span style="color: grey">(380670)</span></li>
<li>/team/group/pimp_my_sip/include/asterisk/res_sip_session.h <span style="color: grey">(380670)</span></li>
<li>/team/group/pimp_my_sip/include/asterisk/threadpool.h <span style="color: grey">(380670)</span></li>
<li>/team/group/pimp_my_sip/main/taskprocessor.c <span style="color: grey">(380670)</span></li>
<li>/team/group/pimp_my_sip/main/threadpool.c <span style="color: grey">(380670)</span></li>
<li>/team/group/pimp_my_sip/res/res_sip.c <span style="color: grey">(380670)</span></li>
<li>/team/group/pimp_my_sip/res/res_sip.exports.in <span style="color: grey">(380670)</span></li>
<li>/team/group/pimp_my_sip/res/res_sip/config_transport.c <span style="color: grey">(380670)</span></li>
<li>/team/group/pimp_my_sip/res/res_sip/sip_options.c <span style="color: grey">(380670)</span></li>
<li>/team/group/pimp_my_sip/res/res_sip_session.c <span style="color: grey">(380670)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/2305/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>