[asterisk-dev] [Code Review] app_queue: Makes call_queue objects ao2 objects so ref counting can be done

[Russell Bryant]

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



/branches/1.4/apps/app_queue.c
<https://reviewboard.asterisk.org/r/427/#comment2924>

    You can simplify this with ...
    
    if (AST_LIST_REMOVE(&queues, q, list)) {
        ao2_ref(q, -1);
    }



/branches/1.4/apps/app_queue.c
<https://reviewboard.asterisk.org/r/427/#comment2925>

    I appreciate the effort to make this patch non invasive.  However, I really think the lock should be removed from the queue object if it's going to be an astobj2 object.  I'm afraid of code being introduced later that uses the astobj2 lock instead of this one (that's what I would probably do).


- Russell


On 2009-11-12 15:56:40, David Vossel wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/427/
> -----------------------------------------------------------
> 
> (Updated 2009-11-12 15:56:40)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> In app_queue, it is possible for a call_queue to be destroyed while another object still holds a pointer to it.  This patch converts call_queue objects to ao2 objects allowing them to be ref counted.  This makes it safe for the queue_ent object in queue_exec() to reference it's parent call_queue even after it has left the queue.
> 
> 
> This addresses bug 15686.
>     https://issues.asterisk.org/view.php?id=15686
> 
> 
> Diffs
> -----
> 
>   /branches/1.4/apps/app_queue.c 229742 
> 
> Diff: https://reviewboard.asterisk.org/r/427/diff
> 
> 
> Testing
> -------
> 
> I developed a situation in which a call_queue would be destroyed after the queue_ent left the queue, but while the queue_ent would still attempt to access it's parent call_queue to do an update_queue() on hangup... This originally caused a horrible crash because the call_queue was already freed.  Now it exits correctly destroying the call_queue right before queue_exec() exits.
> 
> 
> Thanks,
> 
> David
> 
>