aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authordvossel <dvossel@f38db490-d61c-443f-a65b-d21fe96a405b>2010-04-28 21:16:03 +0000
committerdvossel <dvossel@f38db490-d61c-443f-a65b-d21fe96a405b>2010-04-28 21:16:03 +0000
commit679faae3d7a781e4b8657b781927c653b07968a1 (patch)
tree67e0ecbf9e452f8dcaf09f1a70f672caeb3687ee
parenta691762a9e51fb7e6952551e7a901e344b77e0de (diff)
resolves deadlocks in chan_local
Issue_1. In the local_hangup() 3 locks must be held at the same time... pvt, pvt->chan, and pvt->owner. Proper deadlock avoidance is done when the channel to hangup is the outbound chan_local channel, but when it is not the outbound channel we have an issue... We attempt to do deadlock avoidance only on the tech pvt, when both the tech pvt and the pvt->owner are locked coming into that loop. By never giving up the pvt->owner channel deadlock avoidance is not entirely possible. This patch resolves that by doing deadlock avoidance on both the pvt->owner and the pvt when trying to get the pvt->chan lock. Issue_2. ast_prod() is used in ast_activate_generator() to queue a frame on the channel and make the channel's read function get called. This function is used in ast_activate_generator() while the channel is locked, which mean's the channel will have a lock both from the generator code and the frame_queue code by the time it gets to chan_local.c's local_queue_frame code... local_queue_frame contains some of the same crazy deadlock avoidance that local_hangup requires, and this recursive lock prevents that deadlock avoidance from happening correctly. This patch removes ast_prod() from the channel lock so only one lock is held during the local_queue_frame function. (closes issue #17185) Reported by: schmoozecom Patches: issue_17185_v1.diff uploaded by dvossel (license 671) issue_17185_v2.diff uploaded by dvossel (license 671) Tested by: schmoozecom, GameGamer43 Review: https://reviewboard.asterisk.org/r/631/ git-svn-id: http://svn.digium.com/svn/asterisk/branches/1.4@259858 f38db490-d61c-443f-a65b-d21fe96a405b
-rw-r--r--channels/chan_local.c19
-rw-r--r--main/channel.c7
2 files changed, 16 insertions, 10 deletions
diff --git a/channels/chan_local.c b/channels/chan_local.c
index d5441d047..e705ee582 100644
--- a/channels/chan_local.c
+++ b/channels/chan_local.c
@@ -543,12 +543,12 @@ static int local_hangup(struct ast_channel *ast)
/* Deadlock avoidance */
while (p->owner && ast_channel_trylock(p->owner)) {
ast_mutex_unlock(&p->lock);
- if (ast) {
- ast_channel_unlock(ast);
+ if (p->chan) {
+ ast_channel_unlock(p->chan);
}
usleep(1);
- if (ast) {
- ast_channel_lock(ast);
+ if (p->chan) {
+ ast_channel_lock(p->chan);
}
ast_mutex_lock(&p->lock);
}
@@ -563,8 +563,17 @@ static int local_hangup(struct ast_channel *ast)
} else {
ast_module_user_remove(p->u_owner);
while (p->chan && ast_channel_trylock(p->chan)) {
- DEADLOCK_AVOIDANCE(&p->lock);
+ ast_mutex_unlock(&p->lock);
+ if (p->owner) {
+ ast_channel_unlock(p->owner);
+ }
+ usleep(1);
+ if (p->owner) {
+ ast_channel_lock(p->owner);
+ }
+ ast_mutex_lock(&p->lock);
}
+
p->owner = NULL;
if (p->chan) {
ast_queue_hangup(p->chan);
diff --git a/main/channel.c b/main/channel.c
index 752911b59..c6ba4fd7b 100644
--- a/main/channel.c
+++ b/main/channel.c
@@ -1708,25 +1708,22 @@ int ast_activate_generator(struct ast_channel *chan, struct ast_generator *gen,
int res = 0;
ast_channel_lock(chan);
-
if (chan->generatordata) {
if (chan->generator && chan->generator->release)
chan->generator->release(chan, chan->generatordata);
chan->generatordata = NULL;
}
-
- ast_prod(chan);
if (gen->alloc && !(chan->generatordata = gen->alloc(chan, params))) {
res = -1;
}
-
if (!res) {
ast_settimeout(chan, 160, generator_force, chan);
chan->generator = gen;
}
-
ast_channel_unlock(chan);
+ ast_prod(chan);
+
return res;
}