[asterisk-dev] [Code Review]: AMI module reload causes deadlock.

[rmudgett]

> On March 16, 2012, 4:47 p.m., Mark Michelson wrote:
> > I don't understand the point of counting the number of active function calls. Isn't this already taken care of inherently by the refcounting done on the actions? I fully understand not holding the action lock while the function call takes place, though.

The reference count is not quite the same thing.  The reference count is keeping track of how many references to the ao2 object.  The active_count is keeping track of how many of those references are actually calling the callback function.  The ast_manager_unregister() is usually called during a module unload so it is not a good thing if any of those references are still in the callback function when ast_manager_unregister() returns.

Locking the ao2 object guaranteed that there were no active callbacks that mattered when ast_manager_unregister() was called.  Unfortunately, this causes the deadlock situation.  The patch stops locking the ao2 object to allow multiple threads to invoke the callback re-entrantly.  There is no way to guarantee a module unload will not crash because of an active callback.  The code attempts to minimize the chance with the registered flag and the maximum 5 second delay before ast_manager_unregister() returns.


- rmudgett


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


On March 15, 2012, 6:13 p.m., rmudgett wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1818/
> -----------------------------------------------------------
> 
> (Updated March 15, 2012, 6:13 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> 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
> -----
> 
>   /branches/1.8/include/asterisk/manager.h 359708 
>   /branches/1.8/main/manager.c 359708 
> 
> Diff: https://reviewboard.asterisk.org/r/1818/diff
> 
> 
> Testing
> -------
> 
> Code inspection says it should work and the compiler agrees. :)
> 
> 
> Thanks,
> 
> rmudgett
> 
>

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