[asterisk-commits] rmudgett: branch 1.8 r368308 - /branches/1.8/apps/app_stack.c
SVN commits to the Asterisk project
asterisk-commits at lists.digium.com
Fri Jun 1 18:21:04 CDT 2012
Author: rmudgett
Date: Fri Jun 1 18:21:00 2012
New Revision: 368308
URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=368308
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
Modified:
branches/1.8/apps/app_stack.c
Modified: branches/1.8/apps/app_stack.c
URL: http://svnview.digium.com/svn/asterisk/branches/1.8/apps/app_stack.c?view=diff&rev=368308&r1=368307&r2=368308
==============================================================================
--- branches/1.8/apps/app_stack.c (original)
+++ branches/1.8/apps/app_stack.c Fri Jun 1 18:21:00 2012
@@ -370,9 +370,20 @@
{
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;
+ 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,83 @@
return -1;
}
- ast_channel_lock(chan);
+ /*
+ * 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(chan->context);
+ orig_exten = ast_strdupa(chan->exten);
+ orig_priority = chan->priority;
+ 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(chan->context);
+ dest_exten = ast_strdupa(chan->exten);
+ dest_priority = chan->priority;
+ if (ast_test_flag(chan, AST_FLAG_IN_AUTOLOOP)) {
+ ++dest_priority;
+ }
+ caller_id = S_COR(chan->caller.id.number.valid, chan->caller.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", chan->name);
+ ast_debug(1, "Channel %s has no datastore, so we're allocating one.\n",
+ chan->name);
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 +479,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(chan->context, chan->exten, chan->priority + 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, chan->context, chan->exten,
- ast_test_flag(chan, AST_FLAG_IN_AUTOLOOP) ? chan->priority + 1 : chan->priority,
- S_COR(chan->caller.id.number.valid, chan->caller.id.number.str, NULL))) {
- ast_log(LOG_ERROR, "Attempt to reach a non-existent destination for gosub: (Context:%s, Extension:%s, Priority:%d)\n",
- chan->context, chan->exten, ast_test_flag(chan, AST_FLAG_IN_AUTOLOOP) ? chan->priority + 1 : chan->priority);
- ast_copy_string(chan->context, newframe->context, sizeof(chan->context));
- ast_copy_string(chan->exten, newframe->extension, sizeof(chan->exten));
- chan->priority = 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 +500,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_copy_string(chan->context, orig_context, sizeof(chan->context));
+ ast_copy_string(chan->exten, orig_exten, sizeof(chan->exten));
+ chan->priority = 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