[asterisk-dev] [Code Review] 3107: chan_sip: Prevent orphaned channel during a failed SIP transfer to Park

[Matt Jordan]

> On Jan. 13, 2014, 3:07 p.m., rmudgett wrote:
> > /branches/11/channels/chan_sip.c, lines 24188-24192
> > <https://reviewboard.asterisk.org/r/3107/diff/2/?file=51266#file51266line24188>
> >
> >     You might want to delay setting transferer_pvt to after the call to ast_park_call_exten().  That call can take awhile to return because it plays parking messages.

I'm not sure what the benefit of that would be - assigning the pvt to transferer_pvt should take no time at all, which doesn't delay the actual parking. Moving it after ast_park_call_exten won't impact the playing of messages there either.

I prefer to assign it earlier simply because the variable is used extensively throughout this routine, and that keeps the assignments of the various variables co-located together.


> On Jan. 13, 2014, 3:07 p.m., rmudgett wrote:
> > /branches/11/channels/chan_sip.c, line 24177
> > <https://reviewboard.asterisk.org/r/3107/diff/2/?file=51266#file51266line24177>
> >
> >     Should the sip_park() call hangup the channels as well if the masquerades or creating the sip_park_thread() fail?

sip_park already hangs up both channels if ast_channel_masquerade fails. ast_do_masquerade doesn't have an off nominal return.

Thread creation, however, does not hang up both channels. It probably should, as the channels have been masqueraded out of their threads and can't be saved if the park thread doesn't spawn. Fixed.


- Matt


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


On Jan. 8, 2014, 5:41 p.m., Matt Jordan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3107/
> -----------------------------------------------------------
> 
> (Updated Jan. 8, 2014, 5:41 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-22834 and ASTERISK-23047
>     https://issues.asterisk.org/jira/browse/ASTERISK-22834
>     https://issues.asterisk.org/jira/browse/ASTERISK-23047
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> When performing a SIP transfer to a Park extension, if the Park fails, chan_sip will currently not hang up either the transferer or the transfer target. This results in the channels being orphaned with no thread to service frames, resulting in stuck channels.
> 
> This patch immediately hangs up the two channels if a Park fails.
> 
> Note that this is also a bug in 1.8; the code is identical between 11 and 1.8 in this respect. This is not a bug in 12+.
> 
> 
> Diffs
> -----
> 
>   /branches/11/channels/chan_sip.c 404856 
> 
> Diff: https://reviewboard.asterisk.org/r/3107/diff/
> 
> 
> Testing
> -------
> 
> Prior to the patch, the channels were stuck.
> 
> With the patch, the two channels are properly hung up.
> 
> 
> Thanks,
> 
> Matt Jordan
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20140113/4e574881/attachment.html>