[asterisk-commits] dvossel: branch 1.4 r219303 - /branches/1.4/channels/chan_sip.c
SVN commits to the Asterisk project
asterisk-commits at lists.digium.com
Thu Sep 17 16:29:45 CDT 2009
Author: dvossel
Date: Thu Sep 17 16:29:37 2009
New Revision: 219303
URL: http://svn.asterisk.org/svn-view/asterisk?view=rev&rev=219303
Log:
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/
Modified:
branches/1.4/channels/chan_sip.c
Modified: branches/1.4/channels/chan_sip.c
URL: http://svn.asterisk.org/svn-view/asterisk/branches/1.4/channels/chan_sip.c?view=diff&rev=219303&r1=219302&r2=219303
==============================================================================
--- branches/1.4/channels/chan_sip.c (original)
+++ branches/1.4/channels/chan_sip.c Thu Sep 17 16:29:37 2009
@@ -1574,7 +1574,7 @@
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 @@
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)
-{
- struct ast_frame *f;
+/*! \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)
+{
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
/* 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);
}
More information about the asterisk-commits
mailing list