Fix a deadlock involving channel autoservice and chan_local that was debugged

and fixed by mmichelson and me.

We observed a system that had a bunch of threads stuck in ast_autoservice_stop().
The reason these threads were waiting around is because this function waits to
ensure that the channel list in the autoservice thread gets rebuilt before the
stop() function returns.  However, the autoservice thread was also locked, so
the autoservice channel list was never getting rebuilt.

The autoservice thread was stuck waiting for the channel lock on a local channel.
However, the local channel was locked by a thread that was stuck in the autoservice
stop function.

It turned out that the issue came down to the local_queue_frame() function in
chan_local.  This function assumed that one of the channels passed in as an
argument was locked when called.  However, that was not always the case.  There
were multiple cases in which this channel was not locked when the function was
called.  We fixed up chan_local to indicate to this function whether this channel
was locked or not.  The previous assumption had caused local_queue_frame() to
improperly return with the channel locked, where it would then never get unlocked.

(closes issue #12584)
(related to issue #12603)


git-svn-id: https://origsvn.digium.com/svn/asterisk/branches/1.4@116038 65c4cc65-6c06-0410-ace0-fbb531ad65f3
This commit is contained in:
Russell Bryant
2008-05-13 21:17:23 +00:00
parent 92ff649dd6
commit 01de8fa419

View File

@@ -160,7 +160,8 @@ static struct local_pvt *local_pvt_destroy(struct local_pvt *pvt)
return NULL; return NULL;
} }
static int local_queue_frame(struct local_pvt *p, int isoutbound, struct ast_frame *f, struct ast_channel *us) static int local_queue_frame(struct local_pvt *p, int isoutbound, struct ast_frame *f,
struct ast_channel *us, int us_locked)
{ {
struct ast_channel *other = NULL; struct ast_channel *other = NULL;
@@ -184,11 +185,13 @@ static int local_queue_frame(struct local_pvt *p, int isoutbound, struct ast_fra
/* Ensure that we have both channels locked */ /* Ensure that we have both channels locked */
while (other && ast_channel_trylock(other)) { while (other && ast_channel_trylock(other)) {
ast_mutex_unlock(&p->lock); ast_mutex_unlock(&p->lock);
if (us) if (us && us_locked) {
ast_channel_unlock(us); ast_channel_unlock(us);
}
usleep(1); usleep(1);
if (us) if (us && us_locked) {
ast_channel_lock(us); ast_channel_lock(us);
}
ast_mutex_lock(&p->lock); ast_mutex_lock(&p->lock);
other = isoutbound ? p->owner : p->chan; other = isoutbound ? p->owner : p->chan;
} }
@@ -217,7 +220,7 @@ static int local_answer(struct ast_channel *ast)
if (isoutbound) { if (isoutbound) {
/* Pass along answer since somebody answered us */ /* Pass along answer since somebody answered us */
struct ast_frame answer = { AST_FRAME_CONTROL, AST_CONTROL_ANSWER }; struct ast_frame answer = { AST_FRAME_CONTROL, AST_CONTROL_ANSWER };
res = local_queue_frame(p, isoutbound, &answer, ast); res = local_queue_frame(p, isoutbound, &answer, ast, 1);
} else } else
ast_log(LOG_WARNING, "Huh? Local is being asked to answer?\n"); ast_log(LOG_WARNING, "Huh? Local is being asked to answer?\n");
if (!res) if (!res)
@@ -306,7 +309,7 @@ static int local_write(struct ast_channel *ast, struct ast_frame *f)
if (f && (f->frametype == AST_FRAME_VOICE || f->frametype == AST_FRAME_VIDEO)) if (f && (f->frametype == AST_FRAME_VOICE || f->frametype == AST_FRAME_VIDEO))
check_bridge(p, isoutbound); check_bridge(p, isoutbound);
if (!ast_test_flag(p, LOCAL_ALREADY_MASQED)) if (!ast_test_flag(p, LOCAL_ALREADY_MASQED))
res = local_queue_frame(p, isoutbound, f, ast); res = local_queue_frame(p, isoutbound, f, ast, 1);
else { else {
if (option_debug) if (option_debug)
ast_log(LOG_DEBUG, "Not posting to queue since already masked on '%s'\n", ast->name); ast_log(LOG_DEBUG, "Not posting to queue since already masked on '%s'\n", ast->name);
@@ -361,7 +364,7 @@ static int local_indicate(struct ast_channel *ast, int condition, const void *da
f.subclass = condition; f.subclass = condition;
f.data = (void*)data; f.data = (void*)data;
f.datalen = datalen; f.datalen = datalen;
if (!(res = local_queue_frame(p, isoutbound, &f, ast))) if (!(res = local_queue_frame(p, isoutbound, &f, ast, 1)))
ast_mutex_unlock(&p->lock); ast_mutex_unlock(&p->lock);
} }
@@ -381,7 +384,7 @@ static int local_digit_begin(struct ast_channel *ast, char digit)
ast_mutex_lock(&p->lock); ast_mutex_lock(&p->lock);
isoutbound = IS_OUTBOUND(ast, p); isoutbound = IS_OUTBOUND(ast, p);
f.subclass = digit; f.subclass = digit;
if (!(res = local_queue_frame(p, isoutbound, &f, ast))) if (!(res = local_queue_frame(p, isoutbound, &f, ast, 0)))
ast_mutex_unlock(&p->lock); ast_mutex_unlock(&p->lock);
return res; return res;
@@ -401,7 +404,7 @@ static int local_digit_end(struct ast_channel *ast, char digit, unsigned int dur
isoutbound = IS_OUTBOUND(ast, p); isoutbound = IS_OUTBOUND(ast, p);
f.subclass = digit; f.subclass = digit;
f.len = duration; f.len = duration;
if (!(res = local_queue_frame(p, isoutbound, &f, ast))) if (!(res = local_queue_frame(p, isoutbound, &f, ast, 0)))
ast_mutex_unlock(&p->lock); ast_mutex_unlock(&p->lock);
return res; return res;
@@ -421,7 +424,7 @@ static int local_sendtext(struct ast_channel *ast, const char *text)
isoutbound = IS_OUTBOUND(ast, p); isoutbound = IS_OUTBOUND(ast, p);
f.data = (char *) text; f.data = (char *) text;
f.datalen = strlen(text) + 1; f.datalen = strlen(text) + 1;
if (!(res = local_queue_frame(p, isoutbound, &f, ast))) if (!(res = local_queue_frame(p, isoutbound, &f, ast, 0)))
ast_mutex_unlock(&p->lock); ast_mutex_unlock(&p->lock);
return res; return res;
} }
@@ -441,7 +444,7 @@ static int local_sendhtml(struct ast_channel *ast, int subclass, const char *dat
f.subclass = subclass; f.subclass = subclass;
f.data = (char *)data; f.data = (char *)data;
f.datalen = datalen; f.datalen = datalen;
if (!(res = local_queue_frame(p, isoutbound, &f, ast))) if (!(res = local_queue_frame(p, isoutbound, &f, ast, 0)))
ast_mutex_unlock(&p->lock); ast_mutex_unlock(&p->lock);
return res; return res;
} }
@@ -564,7 +567,7 @@ static int local_hangup(struct ast_channel *ast)
/* Need to actually hangup since there is no PBX */ /* Need to actually hangup since there is no PBX */
ochan = p->chan; ochan = p->chan;
else else
res = local_queue_frame(p, isoutbound, &f, NULL); res = local_queue_frame(p, isoutbound, &f, NULL, 1);
if (!res) if (!res)
ast_mutex_unlock(&p->lock); ast_mutex_unlock(&p->lock);
if (ochan) if (ochan)