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

git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@411092 65c4cc65-6c06-0410-ace0-fbb531ad65f3

diff --git a/channels/chan_sip.c b/channels/chan_sip.c
index 23c502c8240c514cf970e335198f373aa046134f..dbd3903a34cae7faea929590dc4337fd5ba12101 100644 (file)
--- a/channels/chan_sip.c
+++ b/channels/chan_sip.c
@@ -4712,8 +4712,20 @@ static int send_provisional_keepalive_full(struct sip_pvt *pvt, int with_sdp)
        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 @@ static int send_provisional_keepalive_full(struct sip_pvt *pvt, int with_sdp)
 
        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;
 }