[asterisk-dev] Should internal timer implementing session timers be stopped and restarted on response to re-invite?

[Mark Michelson]

On 11/11/2014 12:46 PM, Dave WOOLLEY wrote:
>
> In trying to do a back port of some of the fixes to session timers, we 
> encountered a situation where multiple refreshes are sent in quick 
> succession (with incrementing CSEQ values).  Asterisk survives for a 
> little while, but then gets very confused.
>
> Looking at the code, it seems to me that every time Asterisk receives 
> a 200 OK response, it will start a fresh internal timer for session 
> timers, without cancelling the earlier one.  I was hoping to find a 
> fix for this in a later version, but there doesn’t seem to be any up 
> to version 12.
>
> Is there some valid reason why start_session_timer is called for all 
> 200 OK responses where session timers are in use, rather than using 
> restart, for re-invites, as done when handling an incoming re-invite, 
> or doing a stop followed by a start?  Alternatively, have a missed 
> somewhere where the internal time is stopped when Asterisk initiates a 
> re-invite, or starts to handle the response to one?  (The re-invite 
> suspected of triggering the problem is a direct media one, rather than 
> a session timer one, but all 200 Oks are supposed to reset session 
> timer states.)
>
> What is worrying me a little about the analysis is that I am surprised 
> that this problem would have survived for so long in the wild.
>
> I’ve not gone into great detail about the failure symptoms, as we are 
> debugging on a version that is long past end of life, and I’m not 
> asking for people to debug that, or the back port, only to comment on 
> the validity of the current code.
>
>
> BTS Holdings PLC - Registered office: BTS House, Manor Road, 
> Wallington, SM6 0DD - Registered in England: 1517630
>
>

Looking in my Asterisk 11 version of chan_sip.c, in 
start_session_timer(), the first if block looks like this:

     if (p->stimer->st_schedid > -1) {
         /* in the event a timer is already going, stop it */
         ast_debug(2, "Session timer stopped: %d - %s\n", 
p->stimer->st_schedid, p->callid);
         AST_SCHED_DEL_UNREF(sched, p->stimer->st_schedid,
             dialog_unref(p, "unref stimer->st_schedid from dialog"));
     }

So it appears that calling start_session_timer() should be stopping any 
existing timers if they are active. Basically, restart_session_timer() 
is redundant and could be removed.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20141111/b6ccdaba/attachment.html>