[asterisk-dev] [Code Review] 4381: res_pjsip_outbound_registration: Fix reload race condition.

[rmudgett]

> On Jan. 27, 2015, 9:01 p.m., Ashley Sanders wrote:
> > /branches/13/res/res_pjsip_outbound_registration.c, line 1543
> > <https://reviewboard.asterisk.org/r/4381/diff/1/?file=71121#file71121line1543>
> >
> >     Since there are no other attributes being initialized, does this line need a comma?

The comma is there so if another member is initialized the comma doesn't need to be added later.  It is common practice in Asterisk.


- rmudgett


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


On Jan. 27, 2015, 6:46 p.m., rmudgett wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4381/
> -----------------------------------------------------------
> 
> (Updated Jan. 27, 2015, 6:46 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-24729
>     https://issues.asterisk.org/jira/browse/ASTERISK-24729
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> Performing a CLI "module reload" command when there are new pjsip.conf registration objects defined frequently failed to load them correctly.  What happens is a race condtion between res_pjsip pushing its reload into an asynchronous task processor task and the thread that does the rest of the reloads when it gets to reloading the res_pjsip_outbound_registration module.
> 
> * Made res_pjsip.c reload_module() use ast_sip_push_task_synchronous() instead of ast_sip_push_task() to eliminate two threads processing config reloads at the same time.
> 
> * Made get_registrations() not replace the global current_states container.  You could never add/remove objects in the container without the possibility of the container being replaced out from under you by get_registrations().
> 
> * Added a registration loaded sorcery instance observer to purge any dead registration objects.  The sorcery instance observer (struct ast_sorcery_instance_observer) must be used because the callback happens immediately during the load process.  The external observers callbacks (struct ast_sorcery_observer) are pushed to a different thread.
> 
> * Added some global current_states NULL pointer checks in case the container dissapears because of unload_module().
> 
> * Made sorcery's instance loaded observer callback (struct ast_sorcery_instance_observer) guaranteed to be called before any external observers (struct ast_sorcery_observer) will be called.
> 
> * Moved the check for non-reloadable objects to before the sorcery instance loading callbacks happen to short circuit the attempt to reload non-reloadable types earlier and so the non-reloadable type message can only happens once for each non-reloadable type.  Previously the sorcery instance loading/loaded callbacks would always happen, the individual wizard loading would be prevented, and the non-reloadable type message would happen for each associated wizard.
> 
> 
> Diffs
> -----
> 
>   /branches/13/res/res_pjsip_outbound_registration.c 431239 
>   /branches/13/res/res_pjsip.c 431239 
>   /branches/13/main/sorcery.c 431239 
> 
> Diff: https://reviewboard.asterisk.org/r/4381/diff/
> 
> 
> Testing
> -------
> 
> Manually reloaded pjsip.conf with and without the registration type object define.  Without the patch, CLI "pjsip show registrations" frequently showed an empty list when there should have been an object displayed.  With the patch it no longer fails to load.
> 
> The test_config, test_sorcery, and test_sorcery_astdb unit tests still pass.
> 
> 
> Thanks,
> 
> rmudgett
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20150128/9e423a49/attachment-0001.html>