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

(Updated July 21, 2014, 11:58 a.m.)


Review request for Asterisk Developers.


Changes
-------

Addressed Corey's finding.


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 (updated)
-----

  /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

-- 
_____________________________________________________________________
-- 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

Reply via email to