Update .version and ChangeLog. Merge fixes.

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

.version

@@ -1 +1 @@
-1.8.9.0
+1.8.9.1

ChangeLog

@@ -1,3 +1,25 @@
+2012-02-06  Asterisk Development Team <asteriskteam@digium.com>
+
+	* Asterisk 1.8.9.1 Released.
+
+	* channels/chan_agent.c: Fixes deadlocks occuring in chan_agent due to
+	  r335976. Bad locking order was added to chan_agent to prevent
+	  segfaults from having no locking in a patch by irroot. This patch
+	  addresses the bad locking order by releasing locks before getting the
+	  right locking order to stop deadlocks from occuring when doing
+	  multiple interactions with agents. (closes issue ASTERISK-19285)
+	  Reported by: Alex Villacis Lasso
+	  Review: https://reviewboard.asterisk.org/r/1708/
+
+	* channels/chan_sip.c: Ensure entering T.38 passthrough does not cause
+	  an infinite loop. After R340970 Asterisk was still polling the RTCP
+	  file descriptor after RTCP is shut down and removed. If the
+	  descriptor happened to have data ready when the removal occured then
+	  Asterisk would go into an infinite loop trying to read data that it
+	  can never actually access. This change disables the audio RTCP file
+	  descriptor for the duration of the T.38 transaction. (closes issue
+	  ASTERISK-18951) Reported-by: Kristijan Vrban
+
 2012-01-27  Asterisk Development Team <asteriskteam@digium.com>
 
 	* Asterisk 1.8.9.0 Released.

channels/chan_agent.c

@@ -1,7 +1,7 @@
 /*
  * Asterisk -- An open source telephony toolkit.
  *
- * Copyright (C) 1999 - 2006, Digium, Inc.
+ * Copyright (C) 1999 - 2012, Digium, Inc.
  *
  * Mark Spencer <markster@digium.com>
  *
@@ -374,6 +374,43 @@ static const struct ast_channel_tech agent_tech = {
 	.set_base_channel = agent_set_base_channel,
 };
 
+/*!
+ * \brief Locks the owning channel for a LOCKED pvt while obeying locking order. The pvt
+ * must enter this function locked and will be returned locked, but this function will
+ * unlock the pvt for a short time, so it can't be used while expecting the pvt to remain
+ * static. If function returns a non NULL channel, it will need to be unlocked and
+ * unrefed once it is no longer needed.
+ *
+ * \param pvt Pointer to the LOCKED agent_pvt for which the owner is needed
+ * \ret locked channel which owns the pvt at the time of completion. NULL if not available.
+ */
+static struct ast_channel *agent_lock_owner(struct agent_pvt *pvt)
+{
+	struct ast_channel *owner;
+
+	for (;;) {
+		if (!pvt->owner) { /* No owner. Nothing to do. */
+			return NULL;
+		}
+
+		/* If we don't ref the owner, it could be killed when we unlock the pvt. */
+		owner = ast_channel_ref(pvt->owner);
+
+		/* Locking order requires us to lock channel, then pvt. */
+		ast_mutex_unlock(&pvt->lock);
+		ast_channel_lock(owner);
+		ast_mutex_lock(&pvt->lock);
+
+		/* Check if owner changed during pvt unlock period */
+		if (owner != pvt->owner) { /* Channel changed. Unref and do another pass. */
+			ast_channel_unlock(owner);
+			owner = ast_channel_unref(owner);
+		} else { /* Channel stayed the same. Return it. */
+			return owner;
+		}
+	}
+}
+
 /*!
  * Adds an agent to the global list of agents.
  *
@@ -554,7 +591,11 @@ static struct ast_frame *agent_read(struct ast_channel *ast)
 	struct ast_frame *f = NULL;
 	static struct ast_frame answer_frame = { AST_FRAME_CONTROL, { AST_CONTROL_ANSWER } };
 	int cur_time = time(NULL);
+	struct ast_channel *owner;
+
 	ast_mutex_lock(&p->lock);
+	owner = agent_lock_owner(p);
+
 	CHECK_FORMATS(ast, p);
 	if (!p->start) {
 		p->start = cur_time;
@@ -584,13 +625,11 @@ static struct ast_frame *agent_read(struct ast_channel *ast)
 			int howlong = cur_time - p->start;
 			if (p->autologoff && (howlong >= p->autologoff)) {
 				ast_log(LOG_NOTICE, "Agent '%s' didn't answer/confirm within %d seconds (waited %d)\n", p->name, p->autologoff, howlong);
-				if (p->owner || p->chan) {
-					while (p->owner && ast_channel_trylock(p->owner)) {
-						DEADLOCK_AVOIDANCE(&p->lock);
-					}
-					if (p->owner) {
-						ast_softhangup(p->owner, AST_SOFTHANGUP_EXPLICIT);
-						ast_channel_unlock(p->owner);
+				if (owner || p->chan) {
+					if (owner) {
+						ast_softhangup(owner, AST_SOFTHANGUP_EXPLICIT);
+						ast_channel_unlock(owner);
+						owner = ast_channel_unref(owner);
 					}
 
 					while (p->chan && ast_channel_trylock(p->chan)) {
@@ -652,6 +691,11 @@ static struct ast_frame *agent_read(struct ast_channel *ast)
 		}
 	}
 
+	if (owner) {
+		ast_channel_unlock(owner);
+		owner = ast_channel_unref(owner);
+	}
+
 	CLEANUP(ast,p);
 	if (p->chan && !p->chan->_bridge) {
 		if (strcasecmp(p->chan->tech->type, "Local")) {
@@ -887,6 +931,14 @@ int agent_set_base_channel(struct ast_channel *chan, struct ast_channel *base)
 static int agent_hangup(struct ast_channel *ast)
 {
 	struct agent_pvt *p = ast->tech_pvt;
+	struct ast_channel *indicate_chan = NULL;
+	char *tmp_moh; /* moh buffer for indicating after unlocking p */
+
+	if (p->pending) {
+		AST_LIST_LOCK(&agents);
+		AST_LIST_REMOVE(&agents, p, list);
+		AST_LIST_UNLOCK(&agents);
+	}
 
 	ast_mutex_lock(&p->lock);
 	p->owner = NULL;
@@ -909,7 +961,7 @@ static int agent_hangup(struct ast_channel *ast)
 	if (p->start && (ast->_state != AST_STATE_UP)) {
 		p->start = 0;
 	} else
-		p->start = 0; 
+		p->start = 0;
 	if (p->chan) {
 		p->chan->_bridge = NULL;
 		/* If they're dead, go ahead and hang up on the agent now */
@@ -918,15 +970,21 @@ static int agent_hangup(struct ast_channel *ast)
 			ast_softhangup(p->chan, AST_SOFTHANGUP_EXPLICIT);
 			ast_channel_unlock(p->chan);
 		} else if (p->loginstart) {
-			ast_channel_lock(p->chan);
-			ast_indicate_data(p->chan, AST_CONTROL_HOLD, 
-				S_OR(p->moh, NULL),
-				!ast_strlen_zero(p->moh) ? strlen(p->moh) + 1 : 0);
-			ast_channel_unlock(p->chan);
+			indicate_chan = ast_channel_ref(p->chan);
+			tmp_moh = ast_strdupa(p->moh);
 		}
 	}
 	ast_mutex_unlock(&p->lock);
 
+	if (indicate_chan) {
+		ast_channel_lock(indicate_chan);
+		ast_indicate_data(indicate_chan, AST_CONTROL_HOLD,
+			S_OR(tmp_moh, NULL),
+			!ast_strlen_zero(tmp_moh) ? strlen(tmp_moh) + 1 : 0);
+		ast_channel_unlock(indicate_chan);
+		indicate_chan = ast_channel_unref(indicate_chan);
+	}
+
 	/* Only register a device state change if the agent is still logged in */
 	if (!p->loginstart) {
 		p->logincallerid[0] = '\0';
@@ -934,11 +992,6 @@ static int agent_hangup(struct ast_channel *ast)
 		ast_devstate_changed(AST_DEVICE_NOT_INUSE, "Agent/%s", p->agent);
 	}
 
-	if (p->pending) {
-		AST_LIST_LOCK(&agents);
-		AST_LIST_REMOVE(&agents, p, list);
-		AST_LIST_UNLOCK(&agents);
-	}
 	if (p->abouttograb) {
 		/* Let the "about to grab" thread know this isn't valid anymore, and let it
 		   kill it later */
@@ -1491,6 +1544,8 @@ static force_inline int powerof(unsigned int d)
 /*!
  * Lists agents and their status to the Manager API.
  * It is registered on load_module() and it gets called by the manager backend.
+ * This function locks both the pvt and the channel that owns it for a while, but
+ * does not keep these locks.
  * \param s
  * \param m
  * \returns 
@@ -1513,7 +1568,9 @@ static int action_agents(struct mansession *s, const struct message *m)
 	astman_send_ack(s, m, "Agents will follow");
 	AST_LIST_LOCK(&agents);
 	AST_LIST_TRAVERSE(&agents, p, list) {
-        	ast_mutex_lock(&p->lock);
+		struct ast_channel *owner;
+		ast_mutex_lock(&p->lock);
+		owner = agent_lock_owner(p);
 
 		/* Status Values:
 		   AGENT_LOGGEDOFF - Agent isn't logged in
@@ -1528,16 +1585,14 @@ static int action_agents(struct mansession *s, const struct message *m)
 
 		if (p->chan) {
 			loginChan = ast_strdupa(p->chan->name);
-			if (p->owner && p->owner->_bridge) {
+			if (owner && owner->_bridge) {
 				talkingto = S_COR(p->chan->caller.id.number.valid,
 					p->chan->caller.id.number.str, "n/a");
-				ast_channel_lock(p->owner);
-				if ((bridge = ast_bridged_channel(p->owner))) {
+				if ((bridge = ast_bridged_channel(owner))) {
 					talkingtoChan = ast_strdupa(bridge->name);
 				} else {
 					talkingtoChan = "n/a";
 				}
-				ast_channel_unlock(p->owner);
 				status = "AGENT_ONCALL";
 			} else {
 				talkingto = "n/a";
@@ -1551,6 +1606,11 @@ static int action_agents(struct mansession *s, const struct message *m)
 			status = "AGENT_LOGGEDOFF";
 		}
 
+		if (owner) {
+			ast_channel_unlock(owner);
+			owner = ast_channel_unref(owner);
+		}
+
 		astman_append(s, "Event: Agents\r\n"
 			"Agent: %s\r\n"
 			"Name: %s\r\n"
@@ -1582,14 +1642,14 @@ static int agent_logoff(const char *agent, int soft)
 			ret = 0;
 			if (p->owner || p->chan) {
 				if (!soft) {
+					struct ast_channel *owner;
 					ast_mutex_lock(&p->lock);
+					owner = agent_lock_owner(p);
 
-					while (p->owner && ast_channel_trylock(p->owner)) {
-						DEADLOCK_AVOIDANCE(&p->lock);
-					}
-					if (p->owner) {
-						ast_softhangup(p->owner, AST_SOFTHANGUP_EXPLICIT);
-						ast_channel_unlock(p->owner);
+					if (owner) {
+						ast_softhangup(owner, AST_SOFTHANGUP_EXPLICIT);
+						ast_channel_unlock(owner);
+						owner = ast_channel_unref(owner);
 					}
 
 					while (p->chan && ast_channel_trylock(p->chan)) {
@@ -1726,7 +1786,9 @@ static char *agents_show(struct ast_cli_entry *e, int cmd, struct ast_cli_args *
 
 	AST_LIST_LOCK(&agents);
 	AST_LIST_TRAVERSE(&agents, p, list) {
+		struct ast_channel *owner;
 		ast_mutex_lock(&p->lock);
+		owner = agent_lock_owner(p);
 		if (p->pending) {
 			if (p->group)
 				ast_cli(a->fd, "-- Pending call to group %d\n", powerof(p->group));
@@ -1739,10 +1801,11 @@ static char *agents_show(struct ast_cli_entry *e, int cmd, struct ast_cli_args *
 				username[0] = '\0';
 			if (p->chan) {
 				snprintf(location, sizeof(location), "logged in on %s", p->chan->name);
-				if (p->owner && ast_bridged_channel(p->owner))
+				if (owner && ast_bridged_channel(owner)) {
 					snprintf(talkingto, sizeof(talkingto), " talking to %s", ast_bridged_channel(p->owner)->name);
-				 else 
+				} else {
 					strcpy(talkingto, " is idle");
+				}
 				online_agents++;
 			} else {
 				strcpy(location, "not logged in");
@@ -1755,6 +1818,11 @@ static char *agents_show(struct ast_cli_entry *e, int cmd, struct ast_cli_args *
 				username, location, talkingto, music);
 			count_agents++;
 		}
+
+		if (owner) {
+			ast_channel_unlock(owner);
+			owner = ast_channel_unref(owner);
+		}
 		ast_mutex_unlock(&p->lock);
 	}
 	AST_LIST_UNLOCK(&agents);
@@ -1795,21 +1863,32 @@ static char *agents_show_online(struct ast_cli_entry *e, int cmd, struct ast_cli
 
 	AST_LIST_LOCK(&agents);
 	AST_LIST_TRAVERSE(&agents, p, list) {
+		struct ast_channel *owner;
+
 		agent_status = 0;       /* reset it to offline */
 		ast_mutex_lock(&p->lock);
+		owner = agent_lock_owner(p);
+
 		if (!ast_strlen_zero(p->name))
 			snprintf(username, sizeof(username), "(%s) ", p->name);
 		else
 			username[0] = '\0';
 		if (p->chan) {
 			snprintf(location, sizeof(location), "logged in on %s", p->chan->name);
-			if (p->owner && ast_bridged_channel(p->owner)) 
-				snprintf(talkingto, sizeof(talkingto), " talking to %s", ast_bridged_channel(p->owner)->name);
-			else 
+			if (owner && ast_bridged_channel(owner)) {
+				snprintf(talkingto, sizeof(talkingto), " talking to %s", ast_bridged_channel(owner)->name);
+			} else {
 				strcpy(talkingto, " is idle");
+			}
 			agent_status = 1;
 			online_agents++;
 		}
+
+		if (owner) {
+			ast_channel_unlock(owner);
+			owner = ast_channel_unref(owner);
+		}
+
 		if (!ast_strlen_zero(p->moh))
 			snprintf(music, sizeof(music), " (musiconhold is '%s')", p->moh);
 		if (agent_status)
@@ -2381,12 +2460,16 @@ static int agents_data_provider_get(const struct ast_data_search *search,
 
 	AST_LIST_LOCK(&agents);
 	AST_LIST_TRAVERSE(&agents, p, list) {
+		struct ast_channel *owner;
+
 		data_agent = ast_data_add_node(data_root, "agent");
 		if (!data_agent) {
 			continue;
 		}
 
 		ast_mutex_lock(&p->lock);
+		owner = agent_lock_owner(p);
+
 		if (!(p->pending)) {
 			ast_data_add_str(data_agent, "id", p->agent);
 			ast_data_add_structure(agent_pvt, data_agent, p);
@@ -2397,17 +2480,25 @@ static int agents_data_provider_get(const struct ast_data_search *search,
 				if (!data_channel) {
 					ast_mutex_unlock(&p->lock);
 					ast_data_remove_node(data_root, data_agent);
+					if (owner) {
+						ast_channel_unlock(owner);
+						owner = ast_channel_unref(owner);
+					}
 					continue;
 				}
 				ast_channel_data_add_structure(data_channel, p->chan, 0);
-				if (p->owner && ast_bridged_channel(p->owner)) {
+				if (owner && ast_bridged_channel(owner)) {
 					data_talkingto = ast_data_add_node(data_agent, "talkingto");
 					if (!data_talkingto) {
 						ast_mutex_unlock(&p->lock);
 						ast_data_remove_node(data_root, data_agent);
+						if (owner) {
+							ast_channel_unlock(owner);
+							owner = ast_channel_unref(owner);
+						}
 						continue;
 					}
-					ast_channel_data_add_structure(data_talkingto, ast_bridged_channel(p->owner), 0);
+					ast_channel_data_add_structure(data_talkingto, ast_bridged_channel(owner), 0);
 				}
 			} else {
 				ast_data_add_node(data_agent, "talkingto");
@@ -2415,6 +2506,12 @@ static int agents_data_provider_get(const struct ast_data_search *search,
 			}
 			ast_data_add_str(data_agent, "musiconhold", p->moh);
 		}
+
+		if (owner) {
+			ast_channel_unlock(owner);
+			owner = ast_channel_unref(owner);
+		}
+
 		ast_mutex_unlock(&p->lock);
 
 		/* if this agent doesn't match remove the added agent. */

channels/chan_sip.c

@@ -9122,6 +9122,10 @@ static int process_sdp(struct sip_pvt *p, struct sip_request *req, int t38action
 			/* Ensure RTCP is enabled since it may be inactive
 			   if we're coming back from a T.38 session */
 			ast_rtp_instance_set_prop(p->rtp, AST_RTP_PROPERTY_RTCP, 1);
+			/* Ensure audio RTCP reads are enabled */
+			if (p->owner) {
+				ast_channel_set_fd(p->owner, 1, ast_rtp_instance_fd(p->rtp, 1));
+			}
 
 			if (ast_test_flag(&p->flags[0], SIP_DTMF) == SIP_DTMF_AUTO) {
 				ast_clear_flag(&p->flags[0], SIP_DTMF);
@@ -9138,6 +9142,10 @@ static int process_sdp(struct sip_pvt *p, struct sip_request *req, int t38action
 		} else if (udptlportno > 0) {
 			if (debug)
 				ast_verbose("Got T.38 Re-invite without audio. Keeping RTP active during T.38 session.\n");
+			/* Prevent audio RTCP reads */
+			if (p->owner) {
+				ast_channel_set_fd(p->owner, 1, -1);
+			}
 			/* Silence RTCP while audio RTP is inactive */
 			ast_rtp_instance_set_prop(p->rtp, AST_RTP_PROPERTY_RTCP, 0);
 		} else {