[asterisk-dev] [Code Review] chan_sip: fixes provisional keepalive scheduled item crash

[Russell Bryant]

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

Ship it!


Really nice work, David.  I wish I could think of a cleaner way to handle this, but this certainly looks like it will do the trick.


/branches/1.4/channels/chan_sip.c
<https://reviewboard.asterisk.org/r/849/#comment5565>

    use sched_yield()



/branches/1.4/channels/chan_sip.c
<https://reviewboard.asterisk.org/r/849/#comment5566>

    Will you move these curly braces while you're in the area?  They are burning my eyes.  :-)



/branches/1.4/channels/chan_sip.c
<https://reviewboard.asterisk.org/r/849/#comment5564>

    reduce indentation here


- Russell


On 2010-08-06 15:44:54, David Vossel wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/849/
> -----------------------------------------------------------
> 
> (Updated 2010-08-06 15:44:54)
> 
> 
> Review request for Asterisk Developers and Russell Bryant.
> 
> 
> Summary
> -------
> 
> There is a scheduler item in chan_sip that keeps sending the last provisional message in response to an INVITE Request for a period of time until a final response to that INVITE is sent.  Because of the way this scheduler item works, it requires a reference to a sip_pvt pointer to work properly.  The problem with this is that it is currently possible (but rare) for the sip_pvt to get destroyed and that scheduler item to still exist.  When this occurs, the scheduler event fires and attempts to access a freed sip_pvt which causes a crash.
> 
> In 1.6.x through trunk this is not an issue because we have ref counted dialogs, but in 1.4 this is a pain.  Instead of backporting ref counted dialogs into 1.4 I have created a separate ref counted object just for this scheduler item that both the scheduler and the dialog own.  This makes it possible for the dialog to get destroyed and the scheduler callback detect that that dialog no longer exists it it was not able to be removed.
> 
> For all other scheduler items this does not appear to be an issue.  It is because this scheduled item can be scheduled/deleted from multiple threads through the sip_indicate() function that this is a problem.
> 
> 
> Diffs
> -----
> 
>   /branches/1.4/channels/chan_sip.c 281083 
> 
> Diff: https://reviewboard.asterisk.org/r/849/diff
> 
> 
> Testing
> -------
> 
> I made calls and verified that provisional keepalive stops once the final response is sent to an incoming INVITE.
> 
> 
> Thanks,
> 
> David
> 
>