[asterisk-commits] kmoore: trunk r411092 - in /trunk: ./ channels/chan_sip.c

[SVN commits to the Asterisk project]

Author: kmoore
Date: Tue Mar 25 11:06:57 2014
New Revision: 411092

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=411092
Log:
chan_sip: Fix incorrect use of timers

If update_provisional_keepalive() is called while
send_provisional_keepalive_full() is waiting on the PVT lock, then
pvt->provisional_keepalive_sched_id will be changed to a new sched_id
value by update_provisional_keepalive(), but that new sched_id then may
be overwritten with -1 by send_provisional_keepalive_full(), killing
the pvt's reference to a schedule and "leaking" the reference.

(closes issue ASTERISK-22079)
Review: https://reviewboard.asterisk.org/r/3368/
Reported by: Jamuel Starkey, Matteo, Leif Madsen, Steve Davies
Patches:
    provisional_keepalive_fix.diff uploaded by Steve Davies (license 5012)
........

Merged revisions 411088 from http://svn.asterisk.org/svn/asterisk/branches/1.8
........

Merged revisions 411089 from http://svn.asterisk.org/svn/asterisk/branches/11
........

Merged revisions 411091 from http://svn.asterisk.org/svn/asterisk/branches/12

Modified:
    trunk/   (props changed)
    trunk/channels/chan_sip.c

Propchange: trunk/
------------------------------------------------------------------------------
Binary property 'branch-12-merged' - no diff available.

Modified: trunk/channels/chan_sip.c
URL: http://svnview.digium.com/svn/asterisk/trunk/channels/chan_sip.c?view=diff&rev=411092&r1=411091&r2=411092
==============================================================================
--- trunk/channels/chan_sip.c (original)
+++ trunk/channels/chan_sip.c Tue Mar 25 11:06:57 2014
@@ -4712,8 +4712,20 @@
 	const char *msg = NULL;
 	struct ast_channel *chan;
 	int res = 0;
+	int old_sched_id = pvt->provisional_keepalive_sched_id;
 
 	chan = sip_pvt_lock_full(pvt);
+	/* Check that nothing has changed while we were waiting for the lock */
+	if (old_sched_id != pvt->provisional_keepalive_sched_id) {
+		/* Keepalive has been cancelled or rescheduled, clean up and leave */
+		if (chan) {
+			ast_channel_unlock(chan);
+			chan = ast_channel_unref(chan);
+		}
+		sip_pvt_unlock(pvt);
+		dialog_unref(pvt, "dialog ref for provisional keepalive");
+		return 0;
+	}
 
 	if (!pvt->last_provisional || !strncasecmp(pvt->last_provisional, "100", 3)) {
 		msg = "183 Session Progress";
@@ -4739,20 +4751,9 @@
 
 	sip_pvt_unlock(pvt);
 
-#if 0
-	/*
-	 * XXX BUG TODO
-	 *
-	 * Without this code, it appears as if this function is leaking its
-	 * reference to the sip_pvt.  However, adding it introduces a crash.
-	 * This points to some sort of reference count imbalance elsewhere,
-	 * but I'm not sure where ...
-	 */
 	if (!res) {
 		dialog_unref(pvt, "dialog ref for provisional keepalive");
 	}
-#endif
-
 	return res;
 }