<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/4498/">https://reviewboard.asterisk.org/r/4498/</a>
</td>
</tr>
</table>
<br />
<div>
<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/4498/diff/2/?file=72709#file72709line424" style="color: black; font-weight: bold; text-decoration: underline;">/branches/13/res/res_pjsip_outbound_registration.c</a>
<span style="font-weight: normal;">
(Diff revision 2)
</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 void sip_outbound_registration_timer_cb(pj_timer_heap_t *timer_heap, struct pj_timer_entry *entry)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#ebb1ba" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">424</font></th>
<td bgcolor="#ffc5ce" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="n">ao2_ref</span><span class="p">(</span><span class="n">client_state</span><span class="p">,</span> <span class="o">-</span><span class="mi">1</span><span class="p">);</span></pre></td>
<th bgcolor="#ebb1ba" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#ffc5ce" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I ran the res_pjsip_outbound_registration testsuite test locally and never received any fracks/crashes with this line added or removed.
I *think* removing the unref causes a leak. The pjsip code holds onto a reference of the client state. Each time pjsip_regc_send gets called the ref on the client_state is incremented. The timer_cb removes this ref.</pre>
</div>
<br />
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">While testing res_pjsip_outbound_registration I wanted to see if a frack would occur while unloading during registration attempts, but instead Asterisk crashed after unloading. I believe this is due to the module unloading, yet pjsip is still attempting to register to a non existent remote server. After a timeout it then attempts to call back into the module, which has now been unloaded.
A similar thing occurred with res_pjsip_outbound_publish and some code was put into place that makes the module wait to fully unload until all registration attempts are complete. Something similar will need to be done here for outbound registrations.
That being said if you feel that fixing this is outside the scope of the changes found here then an issue can be made so it can be fixed at a later time.</pre>
<p>- Kevin Harwell</p>
<br />
<p>On March 20th, 2015, 11:17 p.m. CDT, Corey Farrell wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://reviewboard.asterisk.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Asterisk Developers.</div>
<div>By Corey Farrell.</div>
<p style="color: grey;"><i>Updated March 20, 2015, 11:17 p.m.</i></p>
<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-24731">ASTERISK-24731</a>
</div>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
Asterisk
</div>
<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 patch allows all PJSIP modules to shutdown cleanly, once approved we'll be able enable it in the Bamboo Reference Checks Job.
* Move most of res_pjsip:module_unload to unload_pjsip to resolve crashes caused by running PJSIP functions from non-PJSIP threads.
* Remove call to pjsip_endpt_destroy(ast_pjsip_endpoint), it was causing crashes in some cases. In theory pj_shutdown() should take care of this for us.
* Mark res_pjsip_keepalive and res_pjsip_session as allowed to unload at shutdown.
* Resolve leaked config global in res_pjsip_notify.
* Unregister pubsub pjsip service module.
* Implement cleanup for res_pjsip_session.
* Fix a pre-existing FRACK in res_pjsip_outbound_registration.
More about res_pjsip_outbound_registration:
tests/channels/pjsip/ami/show_registrations_outbound has an AO2 FRACK during shutdown. I get this error with or without my patch. I've removed what I believe is an extra unref, but this doesn't solve all the FRACK's.</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;">All of tests/channels/pjsip. Some tests still have reference leaks, but most tests do not.
Can someone retest tests/channels/pjsip/ami/show_registrations_outbound to confirm that I haven't made it worse? Seems to make no difference on my system, but Bamboo doesn't seem to have a problem with this test.</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>/branches/13/res/res_pjsip_session.c <span style="color: grey">(433244)</span></li>
<li>/branches/13/res/res_pjsip_pubsub.c <span style="color: grey">(433244)</span></li>
<li>/branches/13/res/res_pjsip_outbound_registration.c <span style="color: grey">(433244)</span></li>
<li>/branches/13/res/res_pjsip_notify.c <span style="color: grey">(433244)</span></li>
<li>/branches/13/res/res_pjsip_keepalive.c <span style="color: grey">(433244)</span></li>
<li>/branches/13/res/res_pjsip.c <span style="color: grey">(433244)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/4498/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>