-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3777/#review12702
-----------------------------------------------------------
This file is terrible when it comes to {} around code blocks. I'm not flagging
issues for this, but it would be great if you could add brackets to places you
are making changes.
/trunk/main/loader.c
<https://reviewboard.asterisk.org/r/3777/#comment22981>
Not sure we want this, but if we do I think it should be ast_debug.
- Corey Farrell
On July 14, 2014, 4:16 p.m., Matt Jordan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3777/
> -----------------------------------------------------------
>
> (Updated July 14, 2014, 4:16 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 418436
> /trunk/main/loader.c 418436
>
> 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
>
>
--
_____________________________________________________________________
-- Bandwidth and Colocation Provided by http://www.api-digital.com --
asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
http://lists.digium.com/mailman/listinfo/asterisk-dev
