[asterisk-dev] [Code Review] 3485: pjsip: Fix a bug where transferring to a parking extension causes calls to hang

[Matt Jordan]

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



/branches/12/include/asterisk/bridge.h
<https://reviewboard.asterisk.org/r/3485/#comment21571>

    I'm not sure I like this change.
    
    Why is a blind transfer to parking not just a blind transfer success? The caller of ast_bridge_transfer_blind doesn't really care whether or not the channel ended up in Park or Limbo or Purgatory - so long as something accepted the channel, it's happy.
    
    I'd lose this enum value, which reduces the scope of this change substantially.



/branches/12/main/bridge.c
<https://reviewboard.asterisk.org/r/3485/#comment21572>

    Just return AST_BRIDGE_TRANSFER_SUCCESS here.



/branches/12/res/res_pjsip_refer.c
<https://reviewboard.asterisk.org/r/3485/#comment21573>

    Why would we defer termination on a bridge transfer success, but not for parking?



/branches/12/res/res_pjsip_refer.c
<https://reviewboard.asterisk.org/r/3485/#comment21574>

    This feels like the wrong way to solve this problem. Having a 'parked' variable bleed up into this handling feels like we've got a design flaw in how we're handling parking.
    
    refer_progress_bridge *should* be detecting that the channel entered into a holding bridge. That should be sending the 200 notification. If that isn't happening, then there is some other bug here that this solution is masking.
    
    


- Matt Jordan


On April 25, 2014, 5:48 p.m., Jonathan Rose wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3485/
> -----------------------------------------------------------
> 
> (Updated April 25, 2014, 5:48 p.m.)
> 
> 
> Review request for Asterisk Developers, Matt Jordan and Mark Michelson.
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> If a PJSIP endpoint attempts to blind transfer to a parking extension, there is an override to the normal transfer logic that can make things act a little weird. We noticed that this would leave various phones hanging on transfer screens without progressing. When the transfer was considered successful, PJSIP deferred the actual action of sending the 200 notify and the actual trigger for that happening never occurs when the transfer is to a parking extension.
> 
> In order to handle this, the bridge function that handles blind transfers now returns a different value if a call was parked and if the channel driver needs to react differently in this case, it can.  In the case of PJSIP, we respond to transfers to park by immediately sending the notify with 200 OK sip frag instead of deferring the action.
> 
> 
> Diffs
> -----
> 
>   /branches/12/res/res_pjsip_refer.c 412824 
>   /branches/12/main/manager.c 412824 
>   /branches/12/main/bridge_basic.c 412824 
>   /branches/12/main/bridge.c 412824 
>   /branches/12/include/asterisk/bridge.h 412824 
>   /branches/12/channels/chan_unistim.c 412824 
>   /branches/12/channels/chan_skinny.c 412824 
>   /branches/12/channels/chan_sip.c 412824 
>   /branches/12/channels/chan_oss.c 412824 
>   /branches/12/channels/chan_iax2.c 412824 
> 
> Diff: https://reviewboard.asterisk.org/r/3485/diff/
> 
> 
> Testing
> -------
> 
> Before patch:
> * Blind transfer on Polycom SPIP: Phone is on the blind transfer screen until it either manually hangs up or 60 seconds pass and Asterisk terminates the session.
> 
> After the patch:
> * Blind transfer on Polycom SPIP: Phone immediately leaves the blind transfer screen and goes back to idle mode.
> 
> 
> Thanks,
> 
> Jonathan Rose
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20140428/2e90d3de/attachment-0001.html>