Merged revisions 326144 via svnmerge from

https://origsvn.digium.com/svn/asterisk/branches/1.8

........
  r326144 | rmudgett | 2011-07-01 16:07:22 -0500 (Fri, 01 Jul 2011) | 16 lines

  Better way to get chan and pvt lock for issue ASTERISK-17431.

  Redoes -r308945 for issue ASTERISK-17431 deadlock fix for
  sip_set_udptl_peer() and sip_set_rtp_peer().

  * Lock the channels in the defined order and avoid the need for a deadlock
  avoidance loop.

  * Lock the channel before getting the pointer to the private structure to
  be sure that the pointer will not change due to a masquerade or channel
  hangup.

  * To preserve sanity, check that chan and p->owner are the same.  (Pointer
  rearangements should not happen without the protection of locks because
  bad things tend to happen otherwise.)
........

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

diff --git a/channels/chan_sip.c b/channels/chan_sip.c
index 77e17f6..378f253 100644 (file)
--- a/channels/chan_sip.c
+++ b/channels/chan_sip.c
@@ -28775,24 +28775,22 @@ static int sip_set_udptl_peer(struct ast_channel *chan, struct ast_udptl *udptl)
 {
        struct sip_pvt *p;
 
+       /* Lock the channel and the private safely. */
+       ast_channel_lock(chan);
        p = chan->tech_pvt;
        if (!p) {
+               ast_channel_unlock(chan);
                return -1;
        }
-       /*
-        * Lock both the pvt and it's owner safely.
-        */
        sip_pvt_lock(p);
-       while (p->owner && ast_channel_trylock(p->owner)) {
-               sip_pvt_unlock(p);
-               usleep(1);
-               sip_pvt_lock(p);
-       }
-
-       if (!p->owner) {
+       if (p->owner != chan) {
+               /* I suppose it could be argued that if this happens it is a bug. */
+               ast_debug(1, "The private is not owned by channel %s anymore.\n", chan->name);
                sip_pvt_unlock(p);
+               ast_channel_unlock(chan);
                return 0;
        }
+
        if (udptl) {
                ast_udptl_get_peer(udptl, &p->udptlredirip);
        } else {
@@ -28811,8 +28809,8 @@ static int sip_set_udptl_peer(struct ast_channel *chan, struct ast_udptl *udptl)
        }
        /* Reset lastrtprx timer */
        p->lastrtprx = p->lastrtptx = time(NULL);
-       ast_channel_unlock(p->owner);
        sip_pvt_unlock(p);
+       ast_channel_unlock(chan);
        return 0;
 }
 
@@ -28919,10 +28917,21 @@ static int sip_set_rtp_peer(struct ast_channel *chan, struct ast_rtp_instance *i
        struct sip_pvt *p;
        int changed = 0;
 
+       /* Lock the channel and the private safely. */
+       ast_channel_lock(chan);
        p = chan->tech_pvt;
        if (!p) {
+               ast_channel_unlock(chan);
                return -1;
        }
+       sip_pvt_lock(p);
+       if (p->owner != chan) {
+               /* I suppose it could be argued that if this happens it is a bug. */
+               ast_debug(1, "The private is not owned by channel %s anymore.\n", chan->name);
+               sip_pvt_unlock(p);
+               ast_channel_unlock(chan);
+               return 0;
+       }
 
        /* Disable early RTP bridge  */
        if ((instance || vinstance || tinstance) &&
@@ -28931,25 +28940,10 @@ static int sip_set_rtp_peer(struct ast_channel *chan, struct ast_rtp_instance *i
                        return 0;
        }
 
-       /*
-        * Lock both the pvt and it's owner safely.
-        */
-       sip_pvt_lock(p);
-       while (p->owner && ast_channel_trylock(p->owner)) {
-               sip_pvt_unlock(p);
-               usleep(1);
-               sip_pvt_lock(p);
-       }
-
-       if (!p->owner) {
-               sip_pvt_unlock(p);
-               return 0;
-       }
-
        if (p->alreadygone) {
                /* If we're destroyed, don't bother */
-               ast_channel_unlock(p->owner);
                sip_pvt_unlock(p);
+               ast_channel_unlock(chan);
                return 0;
        }
 
@@ -28957,8 +28951,8 @@ static int sip_set_rtp_peer(struct ast_channel *chan, struct ast_rtp_instance *i
           that are known to be behind a NAT, then stop the process now
        */
        if (nat_active && !ast_test_flag(&p->flags[0], SIP_DIRECT_MEDIA_NAT)) {
-               ast_channel_unlock(p->owner);
                sip_pvt_unlock(p);
+               ast_channel_unlock(chan);
                return 0;
        }
 
@@ -29000,8 +28994,8 @@ static int sip_set_rtp_peer(struct ast_channel *chan, struct ast_rtp_instance *i
        }
        /* Reset lastrtprx timer */
        p->lastrtprx = p->lastrtptx = time(NULL);
-       ast_channel_unlock(p->owner);
        sip_pvt_unlock(p);
+       ast_channel_unlock(chan);
        return 0;
 }