[asterisk-dev] [Code Review] AMI module reload causes deadlock. (trunk)

Mark Michelson reviewboard at asterisk.org
Tue Mar 20 09:40:09 CDT 2012


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

Ship it!


This is a lot happier "ship it!" this time. Excellent job!

- Mark


On March 19, 2012, 4:07 p.m., rmudgett wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1820/
> -----------------------------------------------------------
> 
> (Updated March 19, 2012, 4:07 p.m.)
> 
> 
> Review request for Asterisk Developers and Mark Michelson.
> 
> 
> Summary
> -------
> 
> This is the trunk version of the https://reviewboard.asterisk.org/r/1818/ patch.  It fixes the concern Mark Michelson brought up about non-determinism.  Since it requires an API change to fix it, it can only be done on trunk.
> 
> Steps to invoke the deadlock:
> 1) Invoke an AMI Command action such as "dialplan reload" or "originate".
> 2) The command then causes an external script to request another AMI Command action.
> 3) Deadlock because the current code only allows each registered AMI action to have one active instance at a time.
> 
> This patch avoids the deadlock and tries to keep a module unload from potentially causing a crash by an active AMI action using that module.
> 
> 
> This addresses bug ASTERISK-19487.
>     https://issues.asterisk.org/jira/browse/ASTERISK-19487
> 
> 
> Diffs
> -----
> 
>   /trunk/include/asterisk/manager.h 359942 
>   /trunk/main/data.c 359942 
>   /trunk/main/db.c 359942 
>   /trunk/main/features.c 359942 
>   /trunk/main/manager.c 359942 
>   /trunk/main/pbx.c 359942 
> 
> Diff: https://reviewboard.asterisk.org/r/1820/diff
> 
> 
> Testing
> -------
> 
> Code inspection says it should work and the compiler agrees. :)
> 
> I also loaded Asterisk to make sure it can still load because ast_manager_register_xml() can only be used on modules that are loadable.  You must use ast_manager_register_xml_core() on non-loadable modules that are part of Asterisk's core.  If you use ast_manager_register_xml() when you should use ast_manager_register_core(), you will crash on load.  If you mix them up the other way, you run the risk of crashing on unload that the additional changes on trunk are to avoid.
> 
> 
> Thanks,
> 
> rmudgett
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20120320/08e4fa67/attachment-0001.htm>


More information about the asterisk-dev mailing list