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

Jonathan Rose reviewboard at asterisk.org
Tue May 20 11:18:32 CDT 2014



> On May 20, 2014, 11 a.m., Joshua Colp wrote:
> > /branches/12/include/asterisk/bridge.h, lines 906-908
> > <https://reviewboard.asterisk.org/r/3485/diff/6/?file=58657#file58657line906>
> >
> >     This should be documented more since it's now a wrapper. Are there any expectations? Should it be allocated in any specific way? In the case of res_pjsip_refer it expects this as an AO2 object, but that's currently an implementation detail of the blind transfer/parking code that is not documented. Could your stack usage of this be passed into the announcement tracker and hilarity ensue?

Yeah, that's true. The data field for this wrapper is only valid during the transfer_channel_cb itself and not for anything that it spins off for instance. Trying to access it at that point could be pretty bad. I'll make a note of that in the documentation.

It will always be allocated as an AO2 object. That way transfer_channel_cb functions can bump the ref and track so that they can track the 'completed' variable in another function.

/*!
 * \brief AO2 object that wraps data for transfer_channel_cb
 */
struct transfer_channel_data {
	void *data;    /*! Data to be used by the transfer_channel_cb -- note that this
	                *  pointer is going to be pointing to something on the stack, so
	                *  it must not be used at any point after returning from the
	                *  transfer_channel_cb. */
	int completed; /*! Initially 0, This will be set to 1 by either the transfer
	                *  code or by transfer code hooks (e.g. parking) when the
	                *  transfer is completed and any remaining actions have taken
	                *  place (e.g. parking announcements). It will never be reset
	                *  to 0. This is used for deferring progress for channel
	                *  drivers that support deferred progress. */
};


- Jonathan


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


On May 19, 2014, 1:25 p.m., Jonathan Rose wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3485/
> -----------------------------------------------------------
> 
> (Updated May 19, 2014, 1:25 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 413303 
>   /branches/12/res/parking/parking_bridge_features.c 413303 
>   /branches/12/res/parking/parking_applications.c 413303 
>   /branches/12/main/parking.c 413303 
>   /branches/12/main/bridge_basic.c 413303 
>   /branches/12/main/bridge.c 413303 
>   /branches/12/include/asterisk/parking.h 413303 
>   /branches/12/include/asterisk/bridge.h 413303 
>   /branches/12/channels/sig_analog.c 413303 
>   /branches/12/channels/chan_sip.c 413303 
>   /branches/12/channels/chan_mgcp.c 413303 
>   /branches/12/channels/chan_dahdi.c 413303 
> 
> 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/20140520/a6e88a9a/attachment.html>


More information about the asterisk-dev mailing list