[asterisk-dev] [Code Review]: Make ast_unload_resource actually remove the module from the module list when it is unloaded

[Tilghman Lesher]

> On Feb. 22, 2012, 2:11 p.m., Tilghman Lesher wrote:
> > I like this solution much better.  There are some other modules for which this same treatment might be given, such as cdr_odbc, cdr_adaptive_odbc, and res_config_odbc's dependencies on res_odbc.

Another solution just occurred to me.  We presently define a module priority for each module, defining load order, such that the preload directive in modules.conf is no longer necessary.  For unload, it may be worthwhile to use the same value and unload in reverse order.

Or this solution is probably also fine; it just will need additional usecounts added to modules which need it.


- Tilghman


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


On Feb. 22, 2012, 12:12 p.m., Terry Wilson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1752/
> -----------------------------------------------------------
> 
> (Updated Feb. 22, 2012, 12:12 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> res_calendar calls ast_unload_resource for the related tech modules when it is unloaded. If this happens through a 'core stop gracefully', then it will be unloading the tech modules that are already in the list that is being traversed (supposedly safely) for unloading, eventually causing a double free. The problem seems to be that ast_unload_resource, while it calls the unload() callback function for the module, does not actually unlink the module from the list of modules. So the AST_LIST_TRAVERSE_SAFE_BEGIN {} still iterates over the unloaded module.
> 
> This patch causes ast_unload_resource to call AST_LIST_REMOVE on successfully unloaded modules.
> 
> 
> Diffs
> -----
> 
>   /trunk/include/asterisk/calendar.h 356213 
>   /trunk/main/loader.c 356213 
>   /trunk/res/res_calendar.c 356213 
> 
> Diff: https://reviewboard.asterisk.org/r/1752/diff
> 
> 
> Testing
> -------
> 
> Scenario: Start Asterisk with res_calendar and assorted calendar tech modules loaded. Run 'core stop gracefully'.
> Before patch: Crash.
> After patch: No crash.
> 
> 
> Thanks,
> 
> Terry
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20120222/260663ad/attachment.htm>