From d14983dbce5f5d51d24bd4f90a5e44d8b02535b3 Mon Sep 17 00:00:00 2001 From: Walter Doekes Date: Tue, 27 May 2014 21:23:16 +0000 Subject: chan_sip: Start session timer at 200, not at INVITE. Asterisk started counting the session timer at INVITE while the other end correctly started at 200. This meant that for short session-expiries (90 seconds) combined with long ringing times (e.g. 30 seconds), asterisk would wrongly assume that the timer was hit before the other end thought it was time to send a session refresh. This resulted in prematurely ended calls. This changes the session timer to start counting first at 200 like RFC says it should. (Also removed a few excess NULL checks that would never hit, because if they did, asterisk would have crashed already.) ASTERISK-22551 #close Reported by: i2045 Review: https://reviewboard.asterisk.org/r/3562/ ........ Merged revisions 414620 from http://svn.asterisk.org/svn/asterisk/branches/1.8 ........ Merged revisions 414628 from http://svn.asterisk.org/svn/asterisk/branches/11 ........ Merged revisions 414636 from http://svn.asterisk.org/svn/asterisk/branches/12 git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@414643 65c4cc65-6c06-0410-ace0-fbb531ad65f3 --- channels/chan_sip.c | 36 ++++++++++++++---------------------- 1 file changed, 14 insertions(+), 22 deletions(-) (limited to 'channels') diff --git a/channels/chan_sip.c b/channels/chan_sip.c index 6c49236ce..2d3786256 100644 --- a/channels/chan_sip.c +++ b/channels/chan_sip.c @@ -7403,6 +7403,11 @@ static int sip_answer(struct ast_channel *ast) ast_rtp_instance_update_source(p->rtp); res = transmit_response_with_sdp(p, "200 OK", &p->initreq, XMIT_CRITICAL, oldsdp, TRUE); ast_set_flag(&p->flags[1], SIP_PAGE2_DIALOG_ESTABLISHED); + /* RFC says the session timer starts counting on 200, + * not on INVITE. */ + if (p->stimer->st_active == TRUE) { + start_session_timer(p); + } } sip_pvt_unlock(p); return res; @@ -25721,12 +25726,8 @@ static int handle_request_invite(struct sip_pvt *p, struct sip_request *req, str /* Check if OLI/ANI-II is present in From: */ parse_oli(req, p->owner); - if (p->stimer->st_active == TRUE) { - if (reinvite == 0) { - start_session_timer(p); - } else { - restart_session_timer(p); - } + if (reinvite && p->stimer->st_active == TRUE) { + restart_session_timer(p); } if (!req->ignore && p) @@ -26574,7 +26575,9 @@ static int handle_request_bye(struct sip_pvt *p, struct sip_request *req) } stop_media_flows(p); /* Immediately stop RTP, VRTP and UDPTL as applicable */ - stop_session_timer(p); /* Stop Session-Timer */ + if (p->stimer) { + stop_session_timer(p); /* Stop Session-Timer */ + } if (!ast_strlen_zero(sip_get_header(req, "Also"))) { ast_log(LOG_NOTICE, "Client '%s' using deprecated BYE/Also transfer method. Ask vendor to support REFER instead\n", @@ -28935,11 +28938,6 @@ static void acl_change_stasis_cb(void *data, struct stasis_subscription *sub, /*! \brief Session-Timers: Restart session timer */ static void restart_session_timer(struct sip_pvt *p) { - if (!p->stimer) { - ast_log(LOG_WARNING, "Null stimer in restart_session_timer - %s\n", p->callid); - return; - } - if (p->stimer->st_active == TRUE) { ast_debug(2, "Session timer stopped: %d - %s\n", p->stimer->st_schedid, p->callid); AST_SCHED_DEL_UNREF(sched, p->stimer->st_schedid, @@ -28952,11 +28950,6 @@ static void restart_session_timer(struct sip_pvt *p) /*! \brief Session-Timers: Stop session timer */ static void stop_session_timer(struct sip_pvt *p) { - if (!p->stimer) { - ast_log(LOG_WARNING, "Null stimer in stop_session_timer - %s\n", p->callid); - return; - } - if (p->stimer->st_active == TRUE) { p->stimer->st_active = FALSE; ast_debug(2, "Session timer stopped: %d - %s\n", p->stimer->st_schedid, p->callid); @@ -28971,11 +28964,6 @@ static void start_session_timer(struct sip_pvt *p) { unsigned int timeout_ms; - if (!p->stimer) { - ast_log(LOG_WARNING, "Null stimer in start_session_timer - %s\n", p->callid); - return; - } - 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); @@ -29066,7 +29054,11 @@ return_unref: /* An error occurred. Stop session timer processing */ if (p->stimer) { ast_debug(2, "Session timer stopped: %d - %s\n", p->stimer->st_schedid, p->callid); + /* Don't pass go, don't collect $200.. we are the scheduled + * callback. We can rip ourself out here. */ p->stimer->st_schedid = -1; + /* Calling stop_session_timer is nice for consistent debug + * logs. */ stop_session_timer(p); } -- cgit v1.2.3