From 0e6d3f5ee5a617853d03621726c8207bde63a82d Mon Sep 17 00:00:00 2001
From: Walter Doekes <walter+asterisk@wjd.nu>
Date: Thu, 2 Jul 2015 13:25:36 +0200
Subject: [PATCH] chan_sip: Fix early call pickup caused deadlock.
If non-magic pickup (no "pickup-" in callid) is used, chan_sip locks the
channel it wants to pick up, and a bit further down, it locks the
channel list when allocating a new channel.
That causes a deadlock when another part of the code traverses over the
channel list, locking all the channels one by one.
This changeset fixes it by releasing the locks before calling sip_new
and reacquiring them afterwards. Unfortunately this involves doing the
checks we already did again (because the channel may have changed).
While trying to avoid duplicate code, I did some refactoring for
readability:
- if refer_locked == 1, we guarantee there is a locked channel
- magic_callid holds a cached version of !ast_strlen_zero(pickup.exten)
This is for branch 11 only. It appears that the changed code in 13 does
not lock the components like it does in 11 and below. Reproducing the
deadlock on 13 has thusfar failed.
ASTERISK-25213 #close
Change-Id: Ie1d15bec7d634035f48892e1ed6227411d7de2c1
---
channels/chan_sip.c | 130 ++++++++++++++++++++++++++++++++------------
1 file changed, 96 insertions(+), 34 deletions(-)
diff --git a/channels/chan_sip.c b/channels/chan_sip.c
index de23063f96..f4867aeefe 100644
--- a/channels/chan_sip.c
+++ b/channels/chan_sip.c
@@ -25121,6 +25121,39 @@ static int sip_t38_abort(const void *data)
return 0;
}
+/*! \brief Checks state of the p->refer->refer_call state: can it be picked up?
+ *
+ * It will terminate the call if it cannot be picked up; e.g. because it
+ * was gone, or because it wasn't in a ringing state.
+ *
+ * \return 1 if terminated, 0 if ok
+ */
+static int terminate_on_invalid_replaces_state(struct sip_pvt *p, struct sip_request *req, const char *replace_id)
+{
+ if (p->refer->refer_call == p) {
+ ast_log(LOG_NOTICE, "INVITE with replaces into its own call id (%s == %s)!\n", replace_id, p->callid);
+ transmit_response_reliable(p, "400 Bad request", req); /* The best way to not not accept the transfer */
+
+ } else if (!p->refer->refer_call->owner) {
+ /* Oops, someting wrong anyway, no owner, no call */
+ ast_log(LOG_NOTICE, "Supervised transfer attempted to replace non-existing call id (%s)!\n", replace_id);
+ /* Check for better return code */
+ transmit_response_reliable(p, "481 Call Leg Does Not Exist (Replace)", req);
+
+ } else if (ast_channel_state(p->refer->refer_call->owner) != AST_STATE_RINGING &&
+ ast_channel_state(p->refer->refer_call->owner) != AST_STATE_RING &&
+ ast_channel_state(p->refer->refer_call->owner) != AST_STATE_UP) {
+ ast_log(LOG_NOTICE, "Supervised transfer attempted to replace non-ringing or active call id (%s)!\n", replace_id);
+ transmit_response_reliable(p, "603 Declined (Replaces)", req);
+
+ } else {
+ /* Ok */
+ return 0;
+ }
+ /* Terminated */
+ return 1;
+}
+
/*!
* \brief bare-bones support for SIP UPDATE
*
@@ -25322,6 +25355,7 @@ static int handle_request_invite(struct sip_pvt *p, struct sip_request *req, str
int gotdest;
const char *p_replaces;
char *replace_id = NULL;
+ int magic_call_id = 0;
int refer_locked = 0;
const char *required;
unsigned int required_profile = 0;
@@ -25553,40 +25587,27 @@ static int handle_request_invite(struct sip_pvt *p, struct sip_request *req, str
ast_channel_unlock(subscription->owner);
}
subscription = dialog_unref(subscription, "unref dialog subscription");
+ magic_call_id = !ast_strlen_zero(pickup.exten);
}
}
- /* This locks both refer_call pvt and refer_call pvt's owner!!!*/
- if (!error && ast_strlen_zero(pickup.exten) && (p->refer->refer_call = get_sip_pvt_byid_locked(replace_id, totag, fromtag)) == NULL) {
- ast_log(LOG_NOTICE, "Supervised transfer attempted to replace non-existent call id (%s)!\n", replace_id);
- transmit_response_reliable(p, "481 Call Leg Does Not Exist (Replaces)", req);
- error = 1;
- } else {
- refer_locked = 1;
- }
-
- /* The matched call is the call from the transferer to Asterisk .
- We want to bridge the bridged part of the call to the
- incoming invite, thus taking over the refered call */
-
- if (p->refer->refer_call == p) {
- ast_log(LOG_NOTICE, "INVITE with replaces into it's own call id (%s == %s)!\n", replace_id, p->callid);
- transmit_response_reliable(p, "400 Bad request", req); /* The best way to not not accept the transfer */
- error = 1;
- }
-
- if (!error && ast_strlen_zero(pickup.exten) && !p->refer->refer_call->owner) {
- /* Oops, someting wrong anyway, no owner, no call */
- ast_log(LOG_NOTICE, "Supervised transfer attempted to replace non-existing call id (%s)!\n", replace_id);
- /* Check for better return code */
- transmit_response_reliable(p, "481 Call Leg Does Not Exist (Replace)", req);
- error = 1;
- }
-
- if (!error && ast_strlen_zero(pickup.exten) && ast_channel_state(p->refer->refer_call->owner) != AST_STATE_RINGING && ast_channel_state(p->refer->refer_call->owner) != AST_STATE_RING && ast_channel_state(p->refer->refer_call->owner) != AST_STATE_UP) {
- ast_log(LOG_NOTICE, "Supervised transfer attempted to replace non-ringing or active call id (%s)!\n", replace_id);
- transmit_response_reliable(p, "603 Declined (Replaces)", req);
- error = 1;
+ if (!error) {
+ if (magic_call_id) {
+ ;
+ /* This locks both refer_call pvt and refer_call pvt's owner!!!*/
+ } else if ((p->refer->refer_call = get_sip_pvt_byid_locked(replace_id, totag, fromtag))) {
+ /* The matched call is the call from the transferer to Asterisk .
+ We want to bridge the bridged part of the call to the
+ incoming invite, thus taking over the refered call */
+ refer_locked = 1;
+ if (terminate_on_invalid_replaces_state(p, req, replace_id)) {
+ error = 1;
+ }
+ } else {
+ ast_log(LOG_NOTICE, "Supervised transfer attempted to replace non-existent call id (%s)!\n", replace_id);
+ transmit_response_reliable(p, "481 Call Leg Does Not Exist (Replaces)", req);
+ error = 1;
+ }
}
if (error) { /* Give up this dialog */
@@ -25828,11 +25849,52 @@ static int handle_request_invite(struct sip_pvt *p, struct sip_request *req, str
goto request_invite_cleanup;
}
+ /* We cannot call sip_new with any channel locks
+ * held. Unlock the referred channel if locked. */
+ if (refer_locked) {
+ sip_pvt_unlock(p->refer->refer_call);
+ if (p->refer->refer_call->owner) {
+ ast_channel_unlock(p->refer->refer_call->owner);
+ }
+ }
+
/* First invitation - create the channel. Allocation
* failures are handled below. */
-
c = sip_new(p, AST_STATE_DOWN, S_OR(p->peername, NULL), NULL, p->logger_callid);
+ /* Reacquire the lock on the referred call. */
+ if (refer_locked) {
+ /* Reuse deadlock avoid pattern found in
+ * get_sip_pvt_byid_locked. */
+ sip_pvt_lock(p->refer->refer_call);
+ while (p->refer->refer_call->owner && ast_channel_trylock(p->refer->refer_call->owner)) {
+ sip_pvt_unlock(p->refer->refer_call);
+ usleep(1);
+ sip_pvt_lock(p->refer->refer_call);
+ }
+
+ /* And now, check once more that the call is still
+ * still available. Yuck. Bail out if it isn't. */
+ if (terminate_on_invalid_replaces_state(p, req, replace_id)) {
+ /* Free the referred call: it's not ours anymore. */
+ sip_pvt_unlock(p->refer->refer_call);
+ if (p->refer->refer_call->owner) {
+ ast_channel_unlock(p->refer->refer_call->owner);
+ }
+ p->refer->refer_call = dialog_unref(p->refer->refer_call, "unref dialog p->refer->refer_call");
+ refer_locked = 0;
+ /* Kill the channel we just created. */
+ sip_pvt_unlock(p);
+ ast_hangup(c);
+ sip_pvt_lock(p); /* pvt is expected to remain locked on return, so re-lock it */
+ /* Mark the call as failed. */
+ sip_scheddestroy(p, DEFAULT_TRANS_TIMEOUT);
+ p->invitestate = INV_COMPLETED;
+ res = INV_REQ_ERROR;
+ goto request_invite_cleanup;
+ }
+ }
+
if (cc_recall_core_id != -1) {
ast_setup_cc_recall_datastore(c, cc_recall_core_id);
ast_cc_agent_set_interfaces_chanvar(c);
@@ -25891,7 +25953,7 @@ static int handle_request_invite(struct sip_pvt *p, struct sip_request *req, str
p->lastinvite = seqno;
if (c && replace_id) { /* Attended transfer or call pickup - we're the target */
- if (!ast_strlen_zero(pickup.exten)) {
+ if (magic_call_id) {
append_history(p, "Xfer", "INVITE/Replace received");
/* Let the caller know we're giving it a shot */
@@ -26062,7 +26124,7 @@ static int handle_request_invite(struct sip_pvt *p, struct sip_request *req, str
request_invite_cleanup:
- if (refer_locked && p->refer && p->refer->refer_call) {
+ if (refer_locked) {
sip_pvt_unlock(p->refer->refer_call);
if (p->refer->refer_call->owner) {
ast_channel_unlock(p->refer->refer_call->owner);
--
GitLab