[asterisk-dev] [Code Review] 4498: res_pjsip: Enable unload of all modules at shutdown

Kevin Harwell reviewboard at asterisk.org
Tue Mar 24 16:18:31 CDT 2015



On March 24, 2015, 3:11 p.m., Corey Farrell wrote:
> > 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.
> 
> Corey Farrell wrote:
>     So 'module unload res_pjsip_outbound_registration.so' is outside the scope of these changes, since this does not change the conditions that allow 'dlclose' to be run on any module.  The modules that previously could not unload are now set to unload during shutdown only, meaning they will never have dlclose run on them.

I'll create on issue for it then.


- Kevin


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


On March 24, 2015, 4:06 p.m., Corey Farrell wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4498/
> -----------------------------------------------------------
> 
> (Updated March 24, 2015, 4:06 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-24731
>     https://issues.asterisk.org/jira/browse/ASTERISK-24731
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> 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.
> 
> 
> Diffs
> -----
> 
>   /branches/13/res/res_pjsip_session.c 433360 
>   /branches/13/res/res_pjsip_pubsub.c 433360 
>   /branches/13/res/res_pjsip_notify.c 433360 
>   /branches/13/res/res_pjsip_keepalive.c 433360 
>   /branches/13/res/res_pjsip.c 433360 
> 
> Diff: https://reviewboard.asterisk.org/r/4498/diff/
> 
> 
> Testing
> -------
> 
> 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.
> 
> 
> Thanks,
> 
> Corey Farrell
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20150324/8d242bde/attachment.html>


More information about the asterisk-dev mailing list