<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/4311/">https://reviewboard.asterisk.org/r/4311/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On January 6th, 2015, 2:34 p.m. CST, <b>George Joseph</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">So, I applied the patch but am having trouble seeing what this does in real life. res_pjsip can only load by itself if there's no configuration. Even a minimal one requires res_pjsip_session and res_pjsip_outbound_authenticator_digest. So in the end, you have to reload with no config, then start unloading modules. Why not just restart with the stack noloaded in modules.conf?
</pre>
</blockquote>
<p>On January 6th, 2015, 2:44 p.m. CST, <b>George Joseph</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Also you might want to update MODULEINFO to indicate that res_sorcery_config, res_sorcery_memory and res_sorcery_astdb are require to load res_pjsip in the first place.
</pre>
</blockquote>
<p>On January 6th, 2015, 2:57 p.m. CST, <b>George Joseph</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Unloading res_pjsip_outbound_registration segfaults if there's a registration defined. :)</pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">[My fault for not explaining the reasoning behind the patch better] This patch is the first step (should hopefully be followed by another/others at some point) in allowing res_pjsip and the modules that depend on it to be unloadable. At this time, as you noted, res_pjsip and some of the modules that depend on res_pjsip cannot be unloaded without causing problems of some sort.
The goal of this patch is to get res_pjsip and only res_pjsip to be able to unload successfully and/or shutdown without incident (crashes, leaks, etc...). Other dependent modules will still cause problems on unload (res_pjsip_session, res_pjsip_pubsub, etc...).
So yeah basically I just made sure, with the patch applied, that res_pjsip (with no other dependent modules loaded) could be succesfully unloaded and Asterisk could shutdown without any leaks or crashes that pertained directly to res_pjsip.</pre>
<br />
<p>- Kevin</p>
<br />
<p>On January 2nd, 2015, 1:46 p.m. CST, Kevin Harwell 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 Kevin Harwell.</div>
<p style="color: grey;"><i>Updated Jan. 2, 2015, 1:46 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-24485">ASTERISK-24485</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;">The res_pjsip module was previously unloadable. With this patch it can now be unloaded.
This patch is based off the original patch on the issue by Corey Farrell with a few modifications. Removed a few changes not required to make the module unloadable and also fixed a bug that would cause asterisk to crash on unloading.</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;">Made it so res_pjsip was the only pjsip module loaded and then issued an unload and noted it unloaded successfully (also loaded/unloaded it several times from the CLI). Also when loaded and with REF_DEBUG enabled issued a "core stop gracefully" and made sure there were no ref leaks for the module.
Also tested unloading with other dependent pjsip modules loaded and noted that the module would not unload (as it should since dependencies are currently loaded). And then shutdown asterisk and made sure it did not crash or anything.
Started asterisk with nominal and off nominal module and pjsip configurations to make sure things behaved appropriately (no crashes and such) and then attempted to, or successfully unload the res_pjsip module. Also made sure Asterisk continued to shutdown without incident.</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/pjsip_outbound_auth.c <span style="color: grey">(430178)</span></li>
<li>branches/13/res/res_pjsip/pjsip_options.c <span style="color: grey">(430178)</span></li>
<li>branches/13/res/res_pjsip/pjsip_global_headers.c <span style="color: grey">(430178)</span></li>
<li>branches/13/res/res_pjsip/pjsip_distributor.c <span style="color: grey">(430178)</span></li>
<li>branches/13/res/res_pjsip/pjsip_configuration.c <span style="color: grey">(430178)</span></li>
<li>branches/13/res/res_pjsip/location.c <span style="color: grey">(430178)</span></li>
<li>branches/13/res/res_pjsip/include/res_pjsip_private.h <span style="color: grey">(430178)</span></li>
<li>branches/13/res/res_pjsip/config_transport.c <span style="color: grey">(430178)</span></li>
<li>branches/13/res/res_pjsip/config_auth.c <span style="color: grey">(430178)</span></li>
<li>branches/13/res/res_pjsip.c <span style="color: grey">(430178)</span></li>
<li>branches/13/main/stasis_message_router.c <span style="color: grey">(430178)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/4311/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>