[asterisk-dev] [Code Review] Allow res_calendar to be unloaded

[Mark Michelson]

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



/trunk/res/res_calendar.c
<https://reviewboard.asterisk.org/r/1657/#comment9905>

    There's a potential problem here, and that is that the pthread_join() could potentially block for a long time under the right circumstances. This is because the setting of module_unloading and the ast_cond_signal() can happen while the do_refresh() thread is in the middle of its while loop. The result is that the signal can be "missed" and then you're at the mercy of the timedwait, however long that may be.
    
    The solution is to grab the refreshlock prior to setting module_unloading and calling ast_cond_sign(), then releasing it immediately afterwards. This way, you're guaranteed to wait until an appropriate time to send the signal and set the variable.


- Mark


On Jan. 18, 2012, 4:28 p.m., Terry Wilson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1657/
> -----------------------------------------------------------
> 
> (Updated Jan. 18, 2012, 4:28 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> I have seen res_calendar modules contribute to issues with core stop gracefully in part because res_calendar doesn't really unload or take care of the refresh thread that it spawns. There have been some non-calendar-related improvements that make the crash that was happening go away, but it is still very ugly for the module to not clean up after itself. This patch makes res_calendar unloadable and unloads the dependent calendar modules during its unload. 
> 
> The issue is only tangentially related to ASTERISK-16744, but it is what prompted me to go ahead and write the patch. Reloads are still handled through the main res_calendar module.
> 
> 
> This addresses bug ASTERISK-16744.
>     https://issues.asterisk.org/jira/browse/ASTERISK-16744
> 
> 
> Diffs
> -----
> 
>   /trunk/res/res_calendar.c 350018 
> 
> Diff: https://reviewboard.asterisk.org/r/1657/diff
> 
> 
> Testing
> -------
> 
> module unload res_calendar.so works, and the calendar tech modules are also unloaded. Loading the res_calendar module then a tech module creates the proper calendar entries from the config.
> 
> 
> Thanks,
> 
> Terry
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20120130/492b95a0/attachment.htm>