INVITE w/Replaces deadlock fix

This patch cleans up the locking logic in chan_sip.c's
handle_invite_replaces() function as well as making use
of ast_do_masquerade() rather than forcing the masquerade
on an ast_read().  The code had several redundant unlocks
that would result in 'freed more times than we've locked!'
errors. I cleaned these up as well as moving all the unlock
logic to the end of the function.  This patch should also
resolve the issue people were having with the replacecall
channel never being unlocked with one legged calls.

(closes issue #15151)
Reported by: irroot
Patches:
      invite_w_replaces_1.4.diff uploaded by dvossel (license 671)
Tested by: irroot, dvossel

Review: https://reviewboard.asterisk.org/r/371/

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

diff --git a/channels/chan_sip.c b/channels/chan_sip.c
index c42559f9c1b72005acd6c344cab71765778827fb..50d4d2a3ea0cf10909585a662f6bef759b5afbf0 100644 (file)
--- a/channels/chan_sip.c
+++ b/channels/chan_sip.c
@@ -1574,7 +1574,7 @@ static int handle_request_message(struct sip_pvt *p, struct sip_request *req);
 static int handle_request_subscribe(struct sip_pvt *p, struct sip_request *req, struct sockaddr_in *sin, int seqno, char *e);
 static void handle_request_info(struct sip_pvt *p, struct sip_request *req);
 static int handle_request_options(struct sip_pvt *p, struct sip_request *req);
-static int handle_invite_replaces(struct sip_pvt *p, struct sip_request *req, int debug, int ignore, int seqno, struct sockaddr_in *sin);
+static int handle_invite_replaces(struct sip_pvt *p, struct sip_request *req, int debug, int ignore, int seqno, struct sockaddr_in *sin, int *nounlock);
 static int handle_request_notify(struct sip_pvt *p, struct sip_request *req, struct sockaddr_in *sin, int seqno, char *e);
 static int local_attended_transfer(struct sip_pvt *transferer, struct sip_dual *current, struct sip_request *req, int seqno);
 
@@ -14164,11 +14164,15 @@ static int handle_request_options(struct sip_pvt *p, struct sip_request *req)
        return res;
 }
 
-/*! \brief Handle the transfer part of INVITE with a replaces: header, 
-    meaning a target pickup or an attended transfer */
-static int handle_invite_replaces(struct sip_pvt *p, struct sip_request *req, int debug, int ignore, int seqno, struct sockaddr_in *sin)
+/*! \brief Handle the transfer part of INVITE with a replaces: header,
+    meaning a target pickup or an attended transfer
+
+       \note this function is called by handle_request_invite(). Four locks
+       held at the beginning of this function, p, p->owner, p->refer->refer_call->owner...
+       only p's lock should remain at the end of this function.  p's lock is held by sipsock_read()
+*/
+static int handle_invite_replaces(struct sip_pvt *p, struct sip_request *req, int debug, int ignore, int seqno, struct sockaddr_in *sin, int *nounlock)
 {
-       struct ast_frame *f;
        int earlyreplace = 0;
        int oneleggedreplace = 0;               /* Call with no bridge, propably IVR or voice message */
        struct ast_channel *c = p->owner;       /* Our incoming call */
@@ -14206,6 +14210,7 @@ static int handle_invite_replaces(struct sip_pvt *p, struct sip_request *req, in
                transmit_response_with_sdp(p, "200 OK", req, XMIT_RELIABLE);
                /* Do something more clever here */
                ast_channel_unlock(c);
+               ast_channel_unlock(replacecall);
                ast_mutex_unlock(&p->refer->refer_call->lock);
                return 1;
        } 
@@ -14215,6 +14220,7 @@ static int handle_invite_replaces(struct sip_pvt *p, struct sip_request *req, in
                transmit_response_reliable(p, "503 Service Unavailable", req);
                append_history(p, "Xfer", "INVITE/Replace Failed. No new channel.");
                sip_scheddestroy(p, DEFAULT_TRANS_TIMEOUT);
+               ast_channel_unlock(replacecall);
                ast_mutex_unlock(&p->refer->refer_call->lock);
                return 1;
        }
@@ -14246,12 +14252,6 @@ static int handle_invite_replaces(struct sip_pvt *p, struct sip_request *req, in
        ast_quiet_chan(targetcall);
        if (option_debug > 3)
                ast_log(LOG_DEBUG, "Invite/Replaces: preparing to masquerade %s into %s\n", c->name, replacecall->name);
-       /* Unlock clone, but not original (replacecall) */
-       if (!oneleggedreplace)
-               ast_channel_unlock(c);
-
-       /* Unlock PVT */
-       ast_mutex_unlock(&p->refer->refer_call->lock);
 
        /* Make sure that the masq does not free our PVT for the old call */
        if (! earlyreplace && ! oneleggedreplace )
@@ -14263,38 +14263,14 @@ static int handle_invite_replaces(struct sip_pvt *p, struct sip_request *req, in
        else if (option_debug > 3)
                ast_log(LOG_DEBUG, "Invite/Replaces: Going to masquerade %s into %s\n", c->name, replacecall->name);
 
-       /* The masquerade will happen as soon as someone reads a frame from the channel */
-
        /* C should now be in place of replacecall */
-       /* ast_read needs to lock channel */
-       ast_channel_unlock(c);
-       
+       if (ast_do_masquerade(replacecall)) {
+               ast_log(LOG_WARNING, "Failed to perform masquerade with INVITE replaces\n");
+       }
+
        if (earlyreplace || oneleggedreplace ) {
-               /* Force the masq to happen */
-               if ((f = ast_read(replacecall))) {      /* Force the masq to happen */
-                       ast_frfree(f);
-                       f = NULL;
-                       if (option_debug > 3)
-                               ast_log(LOG_DEBUG, "Invite/Replace:  Could successfully read frame from RING channel!\n");
-               } else {
-                       ast_log(LOG_WARNING, "Invite/Replace:  Could not read frame from RING channel \n");
-               }
                c->hangupcause = AST_CAUSE_SWITCH_CONGESTION;
-               if (!oneleggedreplace)
-                       ast_channel_unlock(replacecall);
-       } else {        /* Bridged call, UP channel */
-               if ((f = ast_read(replacecall))) {      /* Force the masq to happen */
-                       /* Masq ok */
-                       ast_frfree(f);
-                       f = NULL;
-                       if (option_debug > 2)
-                               ast_log(LOG_DEBUG, "Invite/Replace:  Could successfully read frame from channel! Masq done.\n");
-               } else {
-                       ast_log(LOG_WARNING, "Invite/Replace:  Could not read frame from channel. Transfer failed\n");
-               }
-               ast_channel_unlock(replacecall);
        }
-       ast_mutex_unlock(&p->refer->refer_call->lock);
 
        ast_setstate(c, AST_STATE_DOWN);
        if (option_debug > 3) {
@@ -14315,13 +14291,19 @@ static int handle_invite_replaces(struct sip_pvt *p, struct sip_request *req, in
                ast_log(LOG_DEBUG, "End After transfer:----------------------------\n");
        }
 
-       ast_channel_unlock(p->owner);   /* Unlock new owner */
-       if (!oneleggedreplace)
-               ast_mutex_unlock(&p->lock);     /* Unlock SIP structure */
+       /* unlock sip pvt and owner so hangup can do its thing */
+       ast_channel_unlock(replacecall);
+       ast_channel_unlock(c);
+       ast_mutex_unlock(&p->refer->refer_call->lock);
+       ast_mutex_unlock(&p->lock);
+       *nounlock = 1;
 
        /* The call should be down with no ast_channel, so hang it up */
        c->tech_pvt = NULL;
        ast_hangup(c);
+
+       ast_mutex_lock(&p->lock); /* lock PVT structure again after hangup */
+
        return 0;
 }
 
@@ -15018,7 +15000,7 @@ static int handle_request_invite(struct sip_pvt *p, struct sip_request *req, int
                /* Go and take over the target call */
                if (sipdebug && option_debug > 3)
                        ast_log(LOG_DEBUG, "Sending this call to the invite/replcaes handler %s\n", p->callid);
-               return handle_invite_replaces(p, req, debug, ast_test_flag(req, SIP_PKT_IGNORE), seqno, sin);
+               return handle_invite_replaces(p, req, debug, ast_test_flag(req, SIP_PKT_IGNORE), seqno, sin, nounlock);
        }