[asterisk-dev] [Code Review] 3777: module loader: Unload modules in reverse order of their start order

[Joshua Colp]

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

Ship it!


Ship It!

- Joshua Colp


On July 21, 2014, 4:58 p.m., Matt Jordan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3777/
> -----------------------------------------------------------
> 
> (Updated July 21, 2014, 4:58 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> When Asterisk starts a module (calling its load_module function), it re-orders the module list, sorting it alphabetically. Ostensibly, this was done so that the output of 'module show' listed modules in alphabetic order. This had the unfortunate side effect of making modules with complex usage patterns practically unloadable. A module that has a large number of modules that depend on it is typically abandoned during the unloading process. This results in its memory not being reclaimed during exit.
> 
> Generally, this isn't harmful - when the process is destroyed, the operating system will reclaim all memory allocated by the process. However, it makes tracking memory leaks or ref debug leaks a real pain.
> 
> While this patch is not a complete overhaul of the module loader - such an effort would be beyond the scope of what could be done for Asterisk 13 - this does make some marginal improvements to the loader such that modules like res_pjsip or res_stasis *may* be made properly un-loadable in the future.
> 1. The linked list of modules has been replaced with a doubly linked list. This allows traversal of the module list to occur backwards. The module shutdown routine now walks the global list backwards when it attempts to unload modules.
> 2. The alphabetic reorganization of the module list on startup has been removed. Instead, a started module is placed at the end of the module list.
> 3. The ast_update_module_list function - which is used by the CLI to display the modules - now does the sorting alphabetically itself. It creates its own linked list and inserts the modules into it in alphabetic order. This allows for the intent of the previous code to be maintained.
> 
> This patch also contains a fix for res_calendar. Without calendar.conf, the calendar modules were improperly bumping the use count of res_calendar, then failing to load themselves. This patch makes it so that we detect whether or not calendaring is enabled before altering the use count.
> 
> 
> Diffs
> -----
> 
>   /trunk/res/res_calendar.c 419109 
>   /trunk/main/loader.c 419109 
> 
> Diff: https://reviewboard.asterisk.org/r/3777/diff/
> 
> 
> Testing
> -------
> 
> Verified that the CLI command worked appropriately.
> 
> Verified that module loading/unloading of res_calendar (whose calendar modules modify the res_calendar use count) loaded/unloaded properly:
> 
>  Asterisk Dynamic Loader Starting:
> [Jul 14 15:14:52] NOTICE[11877]: loader.c:1317 load_modules: 6 modules will be loaded.
>  Loading res_calendar.so.
>  Loading res_calendar_icalendar.so.
>  Loading res_calendar_exchange.so.
>  Loading res_calendar_caldav.so.
>  Loading res_calendar_ews.so.
> Asterisk Ready.
> *CLI> core stop gracefully
> Waiting for inactivity to perform halt...
>  Unloading res_calendar_ews.so
>  Unloading res_calendar_caldav.so
>  Unloading res_calendar_exchange.so
>  Unloading res_calendar_icalendar.so
>  Unloading res_calendar.so
> Asterisk cleanly ending (0).
> Executing last minute cleanups
> 
> Verified that using autoload with all modules, module are started as they were previously, and now are stopped/unloaded in the reverse order.
> 
> 
> Thanks,
> 
> Matt Jordan
> 
>

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