[asterisk-dev] [Code Review] 2696: chan_pjsip: Improve initial INVITE handling, session reference counting, and CANCEL handling

[Mark Michelson]

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



/trunk/res/res_sip/sip_distributor.c
<https://reviewboard.asterisk.org/r/2696/#comment18155>

    A couple of things:
    
    1) This code is reached for both requests and responses, so you should skip over this if the rdata is a response. Referring to rdata->msg_info.msg->line.req on a response can only lead to hardship.
    
    2) In addition to checking for CANCEL, you should check for BYE. The whole premise behind this section is that we are dealing with buggy clients that don't follow the rules. It's possible that an early BYE would lead to the same crash as an early CANCEL.



/trunk/res/res_sip_session.c
<https://reviewboard.asterisk.org/r/2696/#comment18158>

    Decrement the refcount of the session on these return paths. Otherwise, the reference held by the inv_session will be leaked.


- Mark Michelson


On July 24, 2013, 7:21 p.m., Joshua Colp wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2696/
> -----------------------------------------------------------
> 
> (Updated July 24, 2013, 7:21 p.m.)
> 
> 
> Review request for Asterisk Developers and Mark Michelson.
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> The attached change improves initial INVITE handling so it does not crash under certain circumstances.
> 
> 1. The session reference count is now incremented when a reference is taken by INVITE session, previously it stole it.
> 2. Most initial INVITE handling now occurs as a serialized task within the session.
> 3. For cases where the remote side is broken and sends a CANCEL before receiving a provisional response we now detect and reject it.
> 
> 
> Diffs
> -----
> 
>   /trunk/channels/chan_gulp.c 395284 
>   /trunk/res/res_sip/sip_distributor.c 395284 
>   /trunk/res/res_sip_session.c 395284 
> 
> Diff: https://reviewboard.asterisk.org/r/2696/diff/
> 
> 
> Testing
> -------
> 
> Tested normal scenarios with calls coming in, confirmed happy.
> Wrote a sipp scenario file to test CANCEL situation (before and after provisional response), confirmed happy.
> 
> 
> Thanks,
> 
> Joshua Colp
> 
>

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