[asterisk-dev] [Code Review] 3595: app_queue: delayed state event can trigger leavewhenempty ringing too early

[Mark Michelson]

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



/branches/1.8/apps/app_queue.c
<https://reviewboard.asterisk.org/r/3595/#comment22144>

    I'm not a fan of the name "recheck" since it doesn't really describe what is going on. Something that indicates that it is calling into the device state system directly instead of checking local state would be a better fit here. Perhaps bypass_local_state?



/branches/1.8/apps/app_queue.c
<https://reviewboard.asterisk.org/r/3595/#comment22143>

    I had a bad feeling about this, so I ran a quick test.
    
    I set up a queue with a single member and joinempty=ringing. I then placed a call to the queue and let the member's phone ring. I then placed a second call to the queue.
    
    This caused a nearly immediate crash in Asterisk. The crash was due to a stack overflow. When Asterisk crashed, the stack was 2334 frames high, and 2313 of those were in the get_member_status() function. Since the member was ringing, the get_member_status() function repeatedly called itself until the stack was exhausted.
    
    I think that altering this if to be
    
    if (!recheck && (conditions & QUEUE_EMPTY_RINGING))
    
    would solve the recursion problem and also solve the bug you're seeing. If member->status and ast_device_state(member->state_interface) both report the member as ringing, it's a safe bet that the member actually is ringing and we should move on.


- Mark Michelson


On June 10, 2014, 2:43 p.m., Scott Griepentrog wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3595/
> -----------------------------------------------------------
> 
> (Updated June 10, 2014, 2:43 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: AST-1248
>     https://issues.asterisk.org/jira/browse/AST-1248
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> If a single agent is available and doesn't answer the call, the state change from ringing to available can arrive after app_queue finds no agents available and kicks the caller out of the queue.
> 
> This change rechecks the agent's actual channel status to avoid that issue.
> 
> 
> Diffs
> -----
> 
>   /branches/1.8/apps/app_queue.c 415341 
> 
> Diff: https://reviewboard.asterisk.org/r/3595/diff/
> 
> 
> Testing
> -------
> 
> Tested by reporter.
> 
> 
> Thanks,
> 
> Scott Griepentrog
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20140610/111ff172/attachment.html>