Merged revisions 166380 via svnmerge from
authorMark Michelson <mmichelson@digium.com>
Mon, 22 Dec 2008 21:08:03 +0000 (21:08 +0000)
committerMark Michelson <mmichelson@digium.com>
Mon, 22 Dec 2008 21:08:03 +0000 (21:08 +0000)
https://origsvn.digium.com/svn/asterisk/branches/1.4

........
r166380 | mmichelson | 2008-12-22 14:56:29 -0600 (Mon, 22 Dec 2008) | 36 lines

Fix a deadlock relating to channel locks and autoservice

It has been discovered that if a channel is locked prior
to a call to ast_autoservice_stop, then it is likely that
a deadlock will occur. The reason is that the call to
ast_autoservice_stop has a check built into it to be sure
that the thread running autoservice is not currently trying
to manipulate the channel we are about to pull out of
autoservice.

The autoservice thread, however, cannot advance beyond where
it currently is, though, because it is trying to acquire
the lock of the channel for which autoservice is attempting
to be stopped.

The gist of all this is that a channel MUST NOT be locked
when attempting to stop autoservice on the channel.

In this particular case, the channel was locked by a call
to ast_read. A call to ast_exists_extension led to autoservice
being started and stopped due to the existence of dialplan
switches.

It may be that there are future commits which handle the same
symptoms but in a different location, but based on my looks through
the code, it is very rare to see a construct such as this one.

(closes issue #14057)
Reported by: rtrauntvein
Patches:
      14057v3.patch uploaded by putnopvut (license 60)
Tested by: rtrauntvein

Review: http://reviewboard.digium.com/r/107/

........

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

channels/chan_dahdi.c

index 221aab9..7da671c 100644 (file)
@@ -4425,18 +4425,31 @@ static void dahdi_handle_dtmfup(struct ast_channel *ast, int idx, struct ast_fra
                        if (strcmp(ast->exten, "fax")) {
                                const char *target_context = S_OR(ast->macrocontext, ast->context);
 
                        if (strcmp(ast->exten, "fax")) {
                                const char *target_context = S_OR(ast->macrocontext, ast->context);
 
+                               /* We need to unlock 'ast' here because ast_exists_extension has the
+                                * potential to start autoservice on the channel. Such action is prone
+                                * to deadlock.
+                                */
+                               ast_mutex_unlock(&p->lock);
+                               ast_channel_unlock(ast);
                                if (ast_exists_extension(ast, target_context, "fax", 1, ast->cid.cid_num)) {
                                if (ast_exists_extension(ast, target_context, "fax", 1, ast->cid.cid_num)) {
+                                       ast_channel_lock(ast);
+                                       ast_mutex_lock(&p->lock);
                                        ast_verb(3, "Redirecting %s to fax extension\n", ast->name);
                                        /* Save the DID/DNIS when we transfer the fax call to a "fax" extension */
                                        pbx_builtin_setvar_helper(ast, "FAXEXTEN", ast->exten);
                                        if (ast_async_goto(ast, target_context, "fax", 1))
                                                ast_log(LOG_WARNING, "Failed to async goto '%s' into fax of '%s'\n", ast->name, target_context);
                                        ast_verb(3, "Redirecting %s to fax extension\n", ast->name);
                                        /* Save the DID/DNIS when we transfer the fax call to a "fax" extension */
                                        pbx_builtin_setvar_helper(ast, "FAXEXTEN", ast->exten);
                                        if (ast_async_goto(ast, target_context, "fax", 1))
                                                ast_log(LOG_WARNING, "Failed to async goto '%s' into fax of '%s'\n", ast->name, target_context);
-                               } else
+                               } else {
+                                       ast_channel_lock(ast);
+                                       ast_mutex_lock(&p->lock);
                                        ast_log(LOG_NOTICE, "Fax detected, but no fax extension\n");
                                        ast_log(LOG_NOTICE, "Fax detected, but no fax extension\n");
-                       } else
+                               }
+                       } else {
                                ast_debug(1, "Already in a fax extension, not redirecting\n");
                                ast_debug(1, "Already in a fax extension, not redirecting\n");
-               } else
+                       }
+               } else {
                        ast_debug(1, "Fax already handled\n");
                        ast_debug(1, "Fax already handled\n");
+               }
                dahdi_confmute(p, 0);
                p->subs[idx].f.frametype = AST_FRAME_NULL;
                p->subs[idx].f.subclass = 0;
                dahdi_confmute(p, 0);
                p->subs[idx].f.frametype = AST_FRAME_NULL;
                p->subs[idx].f.subclass = 0;