diff options
author | Walter Doekes <walter+asterisk@wjd.nu> | 2014-05-27 21:23:16 +0000 |
---|---|---|
committer | Walter Doekes <walter+asterisk@wjd.nu> | 2014-05-27 21:23:16 +0000 |
commit | d14983dbce5f5d51d24bd4f90a5e44d8b02535b3 (patch) | |
tree | ac3727a961c61b539d52d765f99192879e5c8933 /channels/chan_sip.c | |
parent | b14a4389e685e34ee1ebcd41316c1c4ddac03404 (diff) |
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
Diffstat (limited to 'channels/chan_sip.c')
-rw-r--r-- | channels/chan_sip.c | 36 |
1 files changed, 14 insertions, 22 deletions
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); } |