diff --git a/channels/chan_agent.c b/channels/chan_agent.c
index 733341fb9feb49958ade871a8b13a2d7947a8ef3..ca9a9ed21360454728ec4c7c999f51bde2e29f3a 100644
--- a/channels/chan_agent.c
+++ b/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>
*
@@ -373,6 +373,43 @@ static 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.
*
@@ -553,7 +590,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;
@@ -583,13 +624,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)) {
@@ -651,6 +690,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")) {
@@ -888,6 +932,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;
@@ -910,7 +962,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 */
@@ -919,15 +971,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';
@@ -935,11 +993,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 */
@@ -1492,6 +1545,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
@@ -1514,7 +1569,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
@@ -1529,16 +1586,14 @@ static int action_agents(struct mansession *s, const struct message *m)
if (p->chan) {
loginChan = ast_strdupa(ast_channel_name(p->chan));
- 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(ast_channel_name(bridge));
} else {
talkingtoChan = "n/a";
}
- ast_channel_unlock(p->owner);
status = "AGENT_ONCALL";
} else {
talkingto = "n/a";
@@ -1552,6 +1607,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"
@@ -1583,14 +1643,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)) {
@@ -1727,7 +1787,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));
@@ -1740,10 +1802,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", ast_channel_name(p->chan));
- if (p->owner && ast_bridged_channel(p->owner))
+ if (owner && ast_bridged_channel(owner)) {
snprintf(talkingto, sizeof(talkingto), " talking to %s", ast_channel_name(ast_bridged_channel(p->owner)));
- else
+ } else {
strcpy(talkingto, " is idle");
+ }
online_agents++;
} else {
strcpy(location, "not logged in");
@@ -1756,6 +1819,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);
@@ -1796,21 +1864,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", ast_channel_name(p->chan));
- if (p->owner && ast_bridged_channel(p->owner))
+ if (p->owner && ast_bridged_channel(p->owner)) {
snprintf(talkingto, sizeof(talkingto), " talking to %s", ast_channel_name(ast_bridged_channel(p->owner)));
- else
+ } 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)
@@ -2386,12 +2465,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);
@@ -2402,17 +2485,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");
@@ -2420,6 +2511,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. */