[asterisk-dev] [Code Review] Keep bridge up if parking attempt fails

[Russell Bryant]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.digium.com/r/133/#review353
-----------------------------------------------------------



/branches/1.4/res/res_features.c
<http://reviewboard.digium.com/r/133/#comment864>

    You have a new race condition related to this code.
    
    The parking thread makes some assumptions about the parkeduser structs that are in the list.  For example, it expects that a non-NULL channel is attached to this struct.  However, in between calling this function and when you finish initializing the parkeduser that was returned, you have the possibility for the parking thread to operate on an uninitialized parkeduser.


- Russell


On 2009-01-27 16:33:21, Jeff Peeler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.digium.com/r/133/
> -----------------------------------------------------------
> 
> (Updated 2009-01-27 16:33:21)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> This corrects the unexpected behavior of having a bridged call drop just because of a failed parking attempt. Now, the bridge stays up if the parking attempt fails giving the user an opportunity to try again later or try another extension.
> 
> I pulled out the slot assignment functionality from park_call_full and put into park_space_reserve, enabling one to check and see if an attempt to enter the parking lot will succeed without doing so. The idea is to call park_space_reserve before doing any masquerades and bail out if the parking lot entry will fail.
> 
> The only thing I'm concerned about is if anywhere in park_call_full needs to be protected with the parkinglock.
> 
> 
> This addresses bug 13494.
>     http://bugs.digium.com/view.php?id=13494
> 
> 
> Diffs
> -----
> 
>   /branches/1.4/res/res_features.c 171694 
> 
> Diff: http://reviewboard.digium.com/r/133/diff
> 
> 
> Testing
> -------
> 
> Tested cases with both attended and blind transfer to the park extension as well as with one touch parking.
> 
> 
> Thanks,
> 
> Jeff
> 
>