[asterisk-dev] [Code Review] Don't keep channel up while we are waiting for possible retransmissions of non-200 responses on an INVITE

[Joshua Colp]

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

Ship it!


I like it, much cleaner and easier than I was expecting.

- Joshua


On Jan. 16, 2012, 1:41 p.m., Terry Wilson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1672/
> -----------------------------------------------------------
> 
> (Updated Jan. 16, 2012, 1:41 p.m.)
> 
> 
> Review request for Asterisk Developers, Joshua Colp, Mark Michelson, and Olle E Johansson.
> 
> 
> Summary
> -------
> 
> When handle a non-2xx final response on an INVITE transaction, we have to keep the transaction around after we send an ACK in case we receive a retransmission of the response so we can re-transmit the ACK, but tear down the ast_channel as soon as we transmit the ACK. Before this patch, we could fail at both of these things. Calling sip_alreadygone/needdestroy prevented us from keeping the transaction up and retransmitting the ACK, and queueing CONGESTION was not sufficient to cause the channel to be torn down when originating calls via the CLI, for example.
> 
> This patch queues a hangup with CONGESTION instead of just queueing CONGESTION for these responses and removes the sip_alreadygone and sip_needdestroy calls from handle_response_invite on non-2xx responses. It currently relies on the hangup calling sip_scheddestroy. I couldn't think of a reason why we wouldn't have a channel and be able to hang up in these cases. I'm leaning toward adding a sip_scheddestroy_final() to each of the applicable cases just to be sure, but I'm not 100% sure it is necessary...
> 
> For more information, see section 17.1.1.1 of RFC 3261.
> 
> 
> This addresses bug ASTERISK-17717.
>     https://issues.asterisk.org/jira/browse/ASTERISK-17717
> 
> 
> Diffs
> -----
> 
>   /branches/1.8/channels/chan_sip.c 350414 
> 
> Diff: https://reviewboard.asterisk.org/r/1672/diff
> 
> 
> Testing
> -------
> 
> Tested various 4xx responses with sipp and verified that channels went away, but the sip transaction stayed up and we re-send ACK on retransmitted non-2xx INVITE responses.
> 
> 
> Thanks,
> 
> Terry
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20120116/6996794f/attachment-0001.htm>