[asterisk-commits] rmudgett: trunk r368311 - in /trunk: ./ apps/app_stack.c
SVN commits to the Asterisk project
asterisk-commits at lists.digium.com
Fri Jun 1 18:54:02 CDT 2012
Author: rmudgett
Date: Fri Jun 1 18:53:59 2012
New Revision: 368311
URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=368311
Log:
Fix deadlock when Gosub used with alternate dialplan switches.
Attempting to remove a channel from autoservice with the channel lock held
will result in deadlock.
* Restructured gosub_exec() to not call ast_parseable_goto() and
ast_exists_extension() with the channel lock held.
(closes issue ASTERISK-19764)
Reported by: rmudgett
Tested by: rmudgett
........
Merged revisions 368308 from http://svn.asterisk.org/svn/asterisk/branches/1.8
........
Merged revisions 368310 from http://svn.asterisk.org/svn/asterisk/branches/10
Modified:
trunk/ (props changed)
trunk/apps/app_stack.c
Propchange: trunk/
------------------------------------------------------------------------------
Binary property 'branch-10-merged' - no diff available.
Modified: trunk/apps/app_stack.c
URL: http://svnview.digium.com/svn/asterisk/trunk/apps/app_stack.c?view=diff&rev=368311&r1=368310&r2=368311
==============================================================================
--- trunk/apps/app_stack.c (original)
+++ trunk/apps/app_stack.c Fri Jun 1 18:53:59 2012
@@ -369,10 +369,21 @@
static int gosub_exec(struct ast_channel *chan, const char *data)
{
struct ast_datastore *stack_store;
- AST_LIST_HEAD(,gosub_stack_frame) *oldlist;
- struct gosub_stack_frame *newframe, *lastframe;
- char argname[15], *tmp = ast_strdupa(data), *label, *endparen;
- int i, max_argc = 0;
+ AST_LIST_HEAD(, gosub_stack_frame) *oldlist;
+ struct gosub_stack_frame *newframe;
+ struct gosub_stack_frame *lastframe;
+ char argname[15];
+ char *parse;
+ char *label;
+ char *caller_id;
+ char *orig_context;
+ char *orig_exten;
+ char *dest_context;
+ char *dest_exten;
+ int orig_priority;
+ int dest_priority;
+ int i;
+ int max_argc = 0;
AST_DECLARE_APP_ARGS(args2,
AST_APP_ARG(argval)[100];
);
@@ -382,26 +393,84 @@
return -1;
}
+ /*
+ * Separate the arguments from the label
+ *
+ * NOTE: You cannot use ast_app_separate_args for this, because
+ * '(' cannot be used as a delimiter.
+ */
+ parse = ast_strdupa(data);
+ label = strsep(&parse, "(");
+ if (parse) {
+ char *endparen;
+
+ endparen = strrchr(parse, ')');
+ if (endparen) {
+ *endparen = '\0';
+ } else {
+ ast_log(LOG_WARNING, "Ouch. No closing paren: '%s'?\n", data);
+ }
+ AST_STANDARD_RAW_ARGS(args2, parse);
+ } else {
+ args2.argc = 0;
+ }
+
ast_channel_lock(chan);
+ orig_context = ast_strdupa(ast_channel_context(chan));
+ orig_exten = ast_strdupa(ast_channel_exten(chan));
+ orig_priority = ast_channel_priority(chan);
+ ast_channel_unlock(chan);
+
+ if (ast_parseable_goto(chan, label)) {
+ ast_log(LOG_ERROR, "%s address is invalid: '%s'\n", app_gosub, data);
+ goto error_exit;
+ }
+
+ ast_channel_lock(chan);
+ dest_context = ast_strdupa(ast_channel_context(chan));
+ dest_exten = ast_strdupa(ast_channel_exten(chan));
+ dest_priority = ast_channel_priority(chan);
+ if (ast_test_flag(ast_channel_flags(chan), AST_FLAG_IN_AUTOLOOP)) {
+ ++dest_priority;
+ }
+ caller_id = S_COR(ast_channel_caller(chan)->id.number.valid,
+ ast_channel_caller(chan)->id.number.str, NULL);
+ if (caller_id) {
+ caller_id = ast_strdupa(caller_id);
+ }
+ ast_channel_unlock(chan);
+
+ if (!ast_exists_extension(chan, dest_context, dest_exten, dest_priority, caller_id)) {
+ ast_log(LOG_ERROR, "Attempt to reach a non-existent destination for %s: (Context:%s, Extension:%s, Priority:%d)\n",
+ app_gosub, dest_context, dest_exten, dest_priority);
+ goto error_exit;
+ }
+
+ /* Now we know that we're going to a new location */
+
+ ast_channel_lock(chan);
+
+ /* Find stack datastore return list. */
if (!(stack_store = ast_channel_datastore_find(chan, &stack_info, NULL))) {
- ast_debug(1, "Channel %s has no datastore, so we're allocating one.\n", ast_channel_name(chan));
+ ast_debug(1, "Channel %s has no datastore, so we're allocating one.\n",
+ ast_channel_name(chan));
stack_store = ast_datastore_alloc(&stack_info, NULL);
if (!stack_store) {
- ast_log(LOG_ERROR, "Unable to allocate new datastore. Gosub will fail.\n");
- ast_channel_unlock(chan);
- return -1;
+ ast_log(LOG_ERROR, "Unable to allocate new datastore. %s failed.\n",
+ app_gosub);
+ goto error_exit_locked;
}
oldlist = ast_calloc(1, sizeof(*oldlist));
if (!oldlist) {
- ast_log(LOG_ERROR, "Unable to allocate datastore list head. Gosub will fail.\n");
+ ast_log(LOG_ERROR, "Unable to allocate datastore list head. %s failed.\n",
+ app_gosub);
ast_datastore_free(stack_store);
- ast_channel_unlock(chan);
- return -1;
- }
+ goto error_exit_locked;
+ }
+ AST_LIST_HEAD_INIT(oldlist);
stack_store->data = oldlist;
- AST_LIST_HEAD_INIT(oldlist);
ast_channel_datastore_add(chan, stack_store);
} else {
oldlist = stack_store->data;
@@ -411,53 +480,18 @@
max_argc = lastframe->arguments;
}
- /* Separate the arguments from the label */
- /* NOTE: you cannot use ast_app_separate_args for this, because '(' cannot be used as a delimiter. */
- label = strsep(&tmp, "(");
- if (tmp) {
- endparen = strrchr(tmp, ')');
- if (endparen)
- *endparen = '\0';
- else
- ast_log(LOG_WARNING, "Ouch. No closing paren: '%s'?\n", (char *)data);
- AST_STANDARD_RAW_ARGS(args2, tmp);
- } else
- args2.argc = 0;
-
- /* Mask out previous arguments in this invocation */
+ /* Mask out previous Gosub arguments in this invocation */
if (args2.argc > max_argc) {
max_argc = args2.argc;
}
- /* Create the return address, but don't save it until we know that the Gosub destination exists */
- newframe = gosub_allocate_frame(ast_channel_context(chan), ast_channel_exten(chan), ast_channel_priority(chan) + 1, max_argc);
-
+ /* Create the return address */
+ newframe = gosub_allocate_frame(orig_context, orig_exten, orig_priority + 1, max_argc);
if (!newframe) {
- ast_channel_unlock(chan);
- return -1;
- }
-
- if (ast_parseable_goto(chan, label)) {
- ast_log(LOG_ERROR, "Gosub address is invalid: '%s'\n", (char *)data);
- ast_free(newframe);
- ast_channel_unlock(chan);
- return -1;
- }
-
- if (!ast_exists_extension(chan, ast_channel_context(chan), ast_channel_exten(chan),
- ast_test_flag(ast_channel_flags(chan), AST_FLAG_IN_AUTOLOOP) ? ast_channel_priority(chan) + 1 : ast_channel_priority(chan),
- S_COR(ast_channel_caller(chan)->id.number.valid, ast_channel_caller(chan)->id.number.str, NULL))) {
- ast_log(LOG_ERROR, "Attempt to reach a non-existent destination for gosub: (Context:%s, Extension:%s, Priority:%d)\n",
- ast_channel_context(chan), ast_channel_exten(chan), ast_test_flag(ast_channel_flags(chan), AST_FLAG_IN_AUTOLOOP) ? ast_channel_priority(chan) + 1 : ast_channel_priority(chan));
- ast_channel_context_set(chan, newframe->context);
- ast_channel_exten_set(chan, newframe->extension);
- ast_channel_priority_set(chan, newframe->priority - 1);
- ast_free(newframe);
- ast_channel_unlock(chan);
- return -1;
- }
-
- /* Now that we know for certain that we're going to a new location, set our arguments */
+ goto error_exit_locked;
+ }
+
+ /* Set our arguments */
for (i = 0; i < max_argc; i++) {
snprintf(argname, sizeof(argname), "ARG%d", i + 1);
frame_set_var(chan, newframe, argname, i < args2.argc ? args2.argval[i] : "");
@@ -467,13 +501,23 @@
frame_set_var(chan, newframe, "ARGC", argname);
/* And finally, save our return address */
- oldlist = stack_store->data;
AST_LIST_LOCK(oldlist);
AST_LIST_INSERT_HEAD(oldlist, newframe, entries);
AST_LIST_UNLOCK(oldlist);
ast_channel_unlock(chan);
return 0;
+
+error_exit:
+ ast_channel_lock(chan);
+
+error_exit_locked:
+ /* Restore the original dialplan location. */
+ ast_channel_context_set(chan, orig_context);
+ ast_channel_exten_set(chan, orig_exten);
+ ast_channel_priority_set(chan, orig_priority);
+ ast_channel_unlock(chan);
+ return -1;
}
static int gosubif_exec(struct ast_channel *chan, const char *data)
More information about the asterisk-commits
mailing list