[asterisk-commits] rmudgett: branch group/issue14068 r197695 - /team/group/issue14068/channels/

SVN commits to the Asterisk project asterisk-commits at lists.digium.com
Thu May 28 13:20:22 CDT 2009


Author: rmudgett
Date: Thu May 28 13:20:19 2009
New Revision: 197695

URL: http://svn.asterisk.org/svn-view/asterisk?view=rev&rev=197695
Log:
Fix potential deadlocking problems.

Modified:
    team/group/issue14068/channels/chan_dahdi.c

Modified: team/group/issue14068/channels/chan_dahdi.c
URL: http://svn.asterisk.org/svn-view/asterisk/team/group/issue14068/channels/chan_dahdi.c?view=diff&rev=197695&r1=197694&r2=197695
==============================================================================
--- team/group/issue14068/channels/chan_dahdi.c (original)
+++ team/group/issue14068/channels/chan_dahdi.c Thu May 28 13:20:19 2009
@@ -12831,15 +12831,49 @@
 #if defined(HAVE_PRI)
 /*!
  * \internal
- * \brief Handle the PRI subcommand events.
+ * \brief Obtain the DAHDI owner channel lock if the owner exists.
  * \since 1.6.3
  *
- * \param pri DAHDI PRI private structure.
+ * \param pri DAHDI PRI control structure.
+ * \param chanpos Channel position in the span.
+ *
+ * \note Assumes the pri->lock is already obtained.
+ * \note Assumes the pri->pvts[chanpos]->lock is already obtained.
+ *
+ * \return Nothing
+ */
+static void dahdi_pri_lock_owner(struct dahdi_pri *pri, int chanpos)
+{
+	for (;;) {
+		if (!pri->pvts[chanpos]->owner) {
+			/* There is no owner lock to get. */
+			break;
+		}
+		if (!ast_channel_trylock(pri->pvts[chanpos]->owner)) {
+			/* We got the lock */
+			break;
+		}
+		/* We must unlock the PRI to avoid the possibility of a deadlock */
+		ast_mutex_unlock(&pri->lock);
+		DEADLOCK_AVOIDANCE(&pri->pvts[chanpos]->lock);
+		ast_mutex_lock(&pri->lock);
+	}
+}
+#endif	/* defined(HAVE_PRI) */
+
+#if defined(HAVE_PRI)
+/*!
+ * \internal
+ * \brief Handle the call associated PRI subcommand events.
+ * \since 1.6.3
+ *
+ * \param pri DAHDI PRI control structure.
  * \param chanpos Channel position in the span.
  * \param event_id PRI event id
  * \param channel PRI encoded span/channel
  * \param subcmds Subcommands to process if any. (Could be NULL).
  *
+ * \note Assumes the pri->lock is already obtained.
  * \note Assumes the pri->pvts[chanpos]->lock is already obtained.
  *
  * \return Nothing
@@ -12847,6 +12881,7 @@
 static void dahdi_pri_handle_subcmds(struct dahdi_pri *pri, int chanpos, int event_id, int channel, const struct pri_subcommands *subcmds)
 {
 	int index;
+	struct ast_channel *owner;
 
 	if (!subcmds) {
 		return;
@@ -12854,10 +12889,10 @@
 	//ast_mutex_lock(&pri->pvts[chanpos]->lock);
 	for (index = 0; index < subcmds->counter_subcmd; ++index) {
 		const struct pri_subcommand *subcmd = &subcmds->subcmd[index];
-		struct ast_channel *owner;
 
 		switch (subcmd->cmd) {
 		case PRI_SUBCMD_CONNECTED_LINE:
+			dahdi_pri_lock_owner(pri, chanpos);
 			owner = pri->pvts[chanpos]->owner;
 			if (owner) {
 				struct ast_party_connected_line connected;
@@ -12882,13 +12917,18 @@
 
 				pri->pvts[chanpos]->subs[SUB_REAL].needcallerid = 1;
 				//dahdi_enable_ec(pri->pvts[chanpos]);
+
+				ast_channel_unlock(owner);
 			}
 			break;
 		case PRI_SUBCMD_REDIRECTING:
+			dahdi_pri_lock_owner(pri, chanpos);
 			owner = pri->pvts[chanpos]->owner;
 			if (owner) {
-				struct ast_party_redirecting redirecting = {{0,},};
+				struct ast_party_redirecting redirecting;
 				const struct pri_subcmd_redirecting *cmdr;
+
+				ast_party_redirecting_set_init(&redirecting, &owner->redirecting);
 
 				cmdr = &subcmd->redirecting;
 				redirecting.from.number = (char *) cmdr->party.from.number;
@@ -12903,12 +12943,16 @@
 					pri_to_ast_presentation(cmdr->party.to.number_presentation);
 				redirecting.count = cmdr->party.count;
 				redirecting.reason = pri_to_ast_reason(cmdr->party.reason);
+
+				ast_channel_set_redirecting(owner, &redirecting);
 				ast_channel_queue_redirecting_update(owner, &redirecting);
+
+				ast_channel_unlock(owner);
 			}
 			break;
 		default:
 			ast_log(LOG_WARNING,
-				"Unknown subcommand %d in %s event on channel %d/%d on span %d.\n",
+				"Unknown call subcommand(%d) in %s event on channel %d/%d on span %d.\n",
 				subcmd->cmd, pri_event2str(event_id), PRI_SPAN(channel),
 				PRI_CHANNEL(channel), pri->span);
 			break;
@@ -13311,6 +13355,7 @@
 							&& pri->pvts[chanpos]->call == e->ring.call
 							&& pri->pvts[chanpos]->owner) {
 							/* how to do that */
+							struct ast_channel *owner;
 							int digitlen = strlen(e->ring.callednum);
 							int i;
 
@@ -13319,28 +13364,28 @@
 
 								dahdi_queue_frame(pri->pvts[chanpos], &f, pri);
 							}
-							if (pri->pvts[chanpos]->owner) {
+							dahdi_pri_lock_owner(pri, chanpos);
+							owner = pri->pvts[chanpos]->owner;
+							if (owner) {
 								char dnid[AST_MAX_EXTENSION];
 
 								/*
 								 * Append the received info digits to the end of
 								 * the exten and dnid strings
 								 */
-								strncat(pri->pvts[chanpos]->owner->exten,
-									e->ring.callednum,
-									sizeof(pri->pvts[chanpos]->owner->exten) - 1
-									- strlen(pri->pvts[chanpos]->owner->exten));
-								if (pri->pvts[chanpos]->owner->cid.cid_dnid) {
-									ast_copy_string(dnid,
-										pri->pvts[chanpos]->owner->cid.cid_dnid,
+								strncat(owner->exten, e->ring.callednum,
+									(sizeof(owner->exten) - 1) - strlen(owner->exten));
+								if (owner->cid.cid_dnid) {
+									ast_copy_string(dnid, owner->cid.cid_dnid,
 										sizeof(dnid));
-									ast_free(pri->pvts[chanpos]->owner->cid.cid_dnid);
+									ast_free(owner->cid.cid_dnid);
 								} else {
 									dnid[0] = 0;
 								}
 								strncat(dnid, e->ring.callednum,
-									sizeof(dnid) - 1 - strlen(dnid));
-								pri->pvts[chanpos]->owner->cid.cid_dnid = ast_strdup(dnid);
+									(sizeof(dnid) - 1) - strlen(dnid));
+								owner->cid.cid_dnid = ast_strdup(dnid);
+								ast_channel_unlock(owner);
 							}
 						}
 						ast_mutex_unlock(&pri->pvts[chanpos]->lock);
@@ -13438,6 +13483,7 @@
 					struct ast_party_redirecting redirecting = {{0,},};
 
 					ast_mutex_lock(&pri->pvts[chanpos]->lock);
+/* XXX BUGBUG handling subcommands may be done too soon here.! */
 					dahdi_pri_handle_subcmds(pri, chanpos, e->e, e->ring.channel,
 						e->ring.subcmds);
 					if (pri->switchtype == PRI_SWITCH_GR303_TMC) {
@@ -13693,7 +13739,28 @@
 						ast_log(LOG_WARNING, "Ringing requested on channel %d/%d not in use on span %d\n",
 							PRI_SPAN(e->ringing.channel), PRI_CHANNEL(e->ringing.channel), pri->span);
 					} else {
+						struct ast_channel *owner;
+
 						ast_mutex_lock(&pri->pvts[chanpos]->lock);
+
+						if (e->ringing.calledname[0] || e->ringing.callednum[0]) {
+							struct ast_party_connected_line connected;
+							
+							dahdi_pri_lock_owner(pri, chanpos);
+							owner = pri->pvts[chanpos]->owner;
+							if (owner) {
+								/* Update the connected line information on the other channel */
+								ast_party_connected_line_init(&connected);
+								connected.id.name = e->ringing.calledname;
+								connected.id.number = e->ringing.callednum;
+								connected.id.number_type = e->ringing.calledplan;
+								connected.id.number_presentation =
+									pri_to_ast_presentation(e->ringing.calledpres);
+								connected.source = AST_CONNECTED_LINE_UPDATE_SOURCE_ANSWER;
+								ast_channel_queue_connected_line_update(owner, &connected);
+								ast_channel_unlock(owner);
+							}
+						}
 						dahdi_pri_handle_subcmds(pri, chanpos, e->e, e->ringing.channel,
 							e->ringing.subcmds);
 						if (ast_strlen_zero(pri->pvts[chanpos]->dop.dialstr)) {
@@ -13720,25 +13787,15 @@
 
 #ifdef SUPPORT_USERUSER
 						if (!ast_strlen_zero(e->ringing.useruserinfo)) {
-							struct ast_channel *owner = pri->pvts[chanpos]->owner;
-							ast_mutex_unlock(&pri->pvts[chanpos]->lock);
-							pbx_builtin_setvar_helper(owner, "USERUSERINFO", e->ringing.useruserinfo);
-							ast_mutex_lock(&pri->pvts[chanpos]->lock);
+							dahdi_pri_lock_owner(pri, chanpos);
+							owner = pri->pvts[chanpos]->owner;
+							if (owner) {
+								pbx_builtin_setvar_helper(owner, "USERUSERINFO",
+									e->ringing.useruserinfo);
+								ast_channel_unlock(owner);
+							}
 						}
 #endif
-
-						if ((e->ringing.calledname[0] || e->ringing.callednum[0]) && pri->pvts[chanpos]->owner) {
-							struct ast_party_connected_line connected;
-
-							/* Update the connected line information on the other channel */
-							ast_party_connected_line_init(&connected);
-							connected.id.name = e->ringing.calledname;
-							connected.id.number = e->ringing.callednum;
-							connected.id.number_type = e->ringing.calledplan;
-							connected.id.number_presentation = pri_to_ast_presentation(e->ringing.calledpres);
-							connected.source = AST_CONNECTED_LINE_UPDATE_SOURCE_ANSWER;
-							ast_channel_queue_connected_line_update(pri->pvts[chanpos]->owner, &connected);
-						}
 
 						ast_mutex_unlock(&pri->pvts[chanpos]->lock);
 					}
@@ -13869,6 +13926,22 @@
 						struct ast_channel *owner;
 
 						ast_mutex_lock(&pri->pvts[chanpos]->lock);
+
+						dahdi_pri_lock_owner(pri, chanpos);
+						owner = pri->pvts[chanpos]->owner;
+						if (owner) {
+							/* Update the connected line information on the other channel */
+							ast_party_connected_line_init(&connected);
+							connected.id.name = e->answer.connectedname;
+							connected.id.number = e->answer.connectednum;
+							connected.id.number_type = e->answer.connectedplan;
+							connected.id.number_presentation =
+								pri_to_ast_presentation(e->answer.connectedpres);
+							connected.source =
+								pri_to_ast_connected_line_update_source(e->answer.source);
+							ast_channel_queue_connected_line_update(owner, &connected);
+							ast_channel_unlock(owner);
+						}
 						dahdi_pri_handle_subcmds(pri, chanpos, e->e, e->answer.channel,
 							e->answer.subcmds);
 						/* Now we can do call progress detection */
@@ -13908,25 +13981,16 @@
 							dahdi_enable_ec(pri->pvts[chanpos]);
 						}
 
-						owner = pri->pvts[chanpos]->owner;
 #ifdef SUPPORT_USERUSER
 						if (!ast_strlen_zero(e->answer.useruserinfo)) {
-							ast_mutex_unlock(&pri->pvts[chanpos]->lock);
-							pbx_builtin_setvar_helper(owner, "USERUSERINFO", e->answer.useruserinfo);
-							ast_mutex_lock(&pri->pvts[chanpos]->lock);
+							dahdi_pri_lock_owner(pri, chanpos);
+							owner = pri->pvts[chanpos]->owner;
+							if (owner) {
+								pbx_builtin_setvar_helper(owner, "USERUSERINFO",
+									e->answer.useruserinfo);
+							}
 						}
 #endif
-
-						if (owner) {
-							/* Update the connected line information on the other channel */
-							ast_party_connected_line_init(&connected);
-							connected.id.name = e->answer.connectedname;
-							connected.id.number = e->answer.connectednum;
-							connected.id.number_type = e->answer.connectedplan;
-							connected.id.number_presentation = pri_to_ast_presentation(e->answer.connectedpres);
-							connected.source = pri_to_ast_connected_line_update_source(e->answer.source);
-							ast_channel_queue_connected_line_update(owner, &connected);
-						}
 
 						ast_mutex_unlock(&pri->pvts[chanpos]->lock);
 					}
@@ -13973,27 +14037,32 @@
 								}
 							}
 							ast_verb(3, "Channel %d/%d, span %d got hangup, cause %d\n",
-									pri->pvts[chanpos]->logicalspan, pri->pvts[chanpos]->prioffset, pri->span, e->hangup.cause);
+								pri->pvts[chanpos]->logicalspan, pri->pvts[chanpos]->prioffset, pri->span, e->hangup.cause);
 						} else {
 							pri_hangup(pri->pri, pri->pvts[chanpos]->call, e->hangup.cause);
 							pri->pvts[chanpos]->call = NULL;
 						}
 						if (e->hangup.cause == PRI_CAUSE_REQUESTED_CHAN_UNAVAIL) {
 							ast_verb(3, "Forcing restart of channel %d/%d on span %d since channel reported in use\n",
-									PRI_SPAN(e->hangup.channel), PRI_CHANNEL(e->hangup.channel), pri->span);
+								PRI_SPAN(e->hangup.channel), PRI_CHANNEL(e->hangup.channel), pri->span);
 							pri_reset(pri->pri, PVT_TO_CHANNEL(pri->pvts[chanpos]));
 							pri->pvts[chanpos]->resetting = 1;
 						}
 						if (e->hangup.aoc_units > -1)
 							ast_verb(3, "Channel %d/%d, span %d received AOC-E charging %d unit%s\n",
-									pri->pvts[chanpos]->logicalspan, pri->pvts[chanpos]->prioffset, pri->span, (int)e->hangup.aoc_units, (e->hangup.aoc_units == 1) ? "" : "s");
+								pri->pvts[chanpos]->logicalspan, pri->pvts[chanpos]->prioffset, pri->span, (int)e->hangup.aoc_units, (e->hangup.aoc_units == 1) ? "" : "s");
 
 #ifdef SUPPORT_USERUSER
-						if (pri->pvts[chanpos]->owner && !ast_strlen_zero(e->hangup.useruserinfo)) {
-							struct ast_channel *owner = pri->pvts[chanpos]->owner;
-							ast_mutex_unlock(&pri->pvts[chanpos]->lock);
-							pbx_builtin_setvar_helper(owner, "USERUSERINFO", e->hangup.useruserinfo);
-							ast_mutex_lock(&pri->pvts[chanpos]->lock);
+						if (!ast_strlen_zero(e->hangup.useruserinfo)) {
+							struct ast_channel *owner;
+
+							dahdi_pri_lock_owner(pri, chanpos);
+							owner = pri->pvts[chanpos]->owner;
+							if (owner) {
+								pbx_builtin_setvar_helper(owner, "USERUSERINFO",
+									e->hangup.useruserinfo);
+								ast_channel_unlock(owner);
+							}
 						}
 #endif
 
@@ -14045,24 +14114,29 @@
 							ast_verb(3, "Channel %d/%d, span %d got hangup request, cause %d\n", PRI_SPAN(e->hangup.channel), PRI_CHANNEL(e->hangup.channel), pri->span, e->hangup.cause);
 							if (e->hangup.aoc_units > -1)
 								ast_verb(3, "Channel %d/%d, span %d received AOC-E charging %d unit%s\n",
-										pri->pvts[chanpos]->logicalspan, pri->pvts[chanpos]->prioffset, pri->span, (int)e->hangup.aoc_units, (e->hangup.aoc_units == 1) ? "" : "s");
+									pri->pvts[chanpos]->logicalspan, pri->pvts[chanpos]->prioffset, pri->span, (int)e->hangup.aoc_units, (e->hangup.aoc_units == 1) ? "" : "s");
 						} else {
 							pri_hangup(pri->pri, pri->pvts[chanpos]->call, e->hangup.cause);
 							pri->pvts[chanpos]->call = NULL;
 						}
 						if (e->hangup.cause == PRI_CAUSE_REQUESTED_CHAN_UNAVAIL) {
 							ast_verb(3, "Forcing restart of channel %d/%d span %d since channel reported in use\n",
-									PRI_SPAN(e->hangup.channel), PRI_CHANNEL(e->hangup.channel), pri->span);
+								PRI_SPAN(e->hangup.channel), PRI_CHANNEL(e->hangup.channel), pri->span);
 							pri_reset(pri->pri, PVT_TO_CHANNEL(pri->pvts[chanpos]));
 							pri->pvts[chanpos]->resetting = 1;
 						}
 
 #ifdef SUPPORT_USERUSER
 						if (!ast_strlen_zero(e->hangup.useruserinfo)) {
-							struct ast_channel *owner = pri->pvts[chanpos]->owner;
-							ast_mutex_unlock(&pri->pvts[chanpos]->lock);
-							pbx_builtin_setvar_helper(owner, "USERUSERINFO", e->hangup.useruserinfo);
-							ast_mutex_lock(&pri->pvts[chanpos]->lock);
+							struct ast_channel *owner;
+
+							dahdi_pri_lock_owner(pri, chanpos);
+							owner = pri->pvts[chanpos]->owner;
+							if (owner) {
+								pbx_builtin_setvar_helper(owner, "USERUSERINFO",
+									e->hangup.useruserinfo);
+								ast_channel_unlock(owner);
+							}
 						}
 #endif
 
@@ -14089,10 +14163,15 @@
 
 #ifdef SUPPORT_USERUSER
 						if (!ast_strlen_zero(e->hangup.useruserinfo)) {
-							struct ast_channel *owner = pri->pvts[chanpos]->owner;
-							ast_mutex_unlock(&pri->pvts[chanpos]->lock);
-							pbx_builtin_setvar_helper(owner, "USERUSERINFO", e->hangup.useruserinfo);
-							ast_mutex_lock(&pri->pvts[chanpos]->lock);
+							struct ast_channel *owner;
+
+							dahdi_pri_lock_owner(pri, chanpos);
+							owner = pri->pvts[chanpos]->owner;
+							if (owner) {
+								pbx_builtin_setvar_helper(owner, "USERUSERINFO",
+									e->hangup.useruserinfo);
+								ast_channel_unlock(owner);
+							}
 						}
 #endif
 




More information about the asterisk-commits mailing list