From 9540b7fceae56052674309f67fd060646cb17db0 Mon Sep 17 00:00:00 2001 From: Russell Bryant Date: Mon, 29 Jan 2007 20:51:24 +0000 Subject: [PATCH] The changes for trunk are less extensive, but include - changing the actionlock to a rwlock - not locking the session before doing the action callback The crash issue in 8711 should not be an issue here. Merged revisions 52611 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.4 ........ r52611 | russell | 2007-01-29 14:39:20 -0600 (Mon, 29 Jan 2007) | 10 lines The session lock can not be held while calling action callbacks. If so, then when the WaitEvent callback gets called, then no event can happen because the session can't be locked by another thread. Also, the session needs to be locked in the HTTP callback when it reads out the output string. This fixes the deadlock reported in both 8711 and 8934. Regarding issue 8711, there still may be an issue. If there is a second action requested before the processing of the first action is finished, there could still be some corruption of the output string buffer used to build the result. (issue #8711, #8934) ........ git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@52613 65c4cc65-6c06-0410-ace0-fbb531ad65f3 --- main/manager.c | 37 ++++++++++++++----------------------- 1 file changed, 14 insertions(+), 23 deletions(-) diff --git a/main/manager.c b/main/manager.c index 4464d9dce2..18ba45a656 100644 --- a/main/manager.c +++ b/main/manager.c @@ -174,7 +174,7 @@ static AST_LIST_HEAD_STATIC(users, ast_manager_user); /*! \brief list of actions registered */ static struct manager_action *first_action; -AST_MUTEX_DEFINE_STATIC(actionlock); +AST_RWLOCK_DEFINE_STATIC(actionlock); static AST_RWLIST_HEAD_STATIC(manager_hooks, manager_custom_hook); @@ -332,7 +332,6 @@ static int ast_instring(const char *bigstr, const char *smallstr, const char del continue; } else return !strcmp(smallstr, val); - } while (*(val = (next + 1))); return 0; @@ -386,14 +385,14 @@ static char *complete_show_mancmd(const char *line, const char *word, int pos, i int l = strlen(word), which = 0; char *ret = NULL; - ast_mutex_lock(&actionlock); + ast_rwlock_rdlock(&actionlock); for (cur = first_action; cur; cur = cur->next) { /* Walk the list of actions */ if (!strncasecmp(word, cur->action, l) && ++which > state) { ret = ast_strdup(cur->action); break; /* make sure we exit even if ast_strdup() returns NULL */ } } - ast_mutex_unlock(&actionlock); + ast_rwlock_unlock(&actionlock); return ret; } @@ -412,7 +411,7 @@ static struct ast_manager_user *get_manager_by_name_locked(const char *name) return user; } - +/*! \note The actionlock is read-locked by the caller of this function */ static int handle_showmancmd(int fd, int argc, char *argv[]) { struct manager_action *cur; @@ -422,7 +421,6 @@ static int handle_showmancmd(int fd, int argc, char *argv[]) if (argc != 4) return RESULT_SHOWUSAGE; - ast_mutex_lock(&actionlock); for (cur = first_action; cur; cur = cur->next) { /* Walk the list of actions */ for (num = 3; num < argc; num++) { if (!strcasecmp(cur->action, argv[num])) { @@ -433,7 +431,6 @@ static int handle_showmancmd(int fd, int argc, char *argv[]) } } } - ast_mutex_unlock(&actionlock); return RESULT_SUCCESS; } @@ -534,10 +531,10 @@ static int handle_showmancmds(int fd, int argc, char *argv[]) ast_cli(fd, format, "Action", "Privilege", "Synopsis"); ast_cli(fd, format, "------", "---------", "--------"); - ast_mutex_lock(&actionlock); + ast_rwlock_rdlock(&actionlock); for (cur = first_action; cur; cur = cur->next) /* Walk the list of actions */ ast_cli(fd, format, cur->action, authority_to_str(cur->authority, &authority), cur->synopsis); - ast_mutex_unlock(&actionlock); + ast_rwlock_unlock(&actionlock); return RESULT_SUCCESS; } @@ -1259,19 +1256,18 @@ static char mandescr_listcommands[] = " action that is available to the user\n" "Variables: NONE\n"; +/*! \note The actionlock is read-locked by the caller of this function */ static int action_listcommands(struct mansession *s, const struct message *m) { struct manager_action *cur; struct ast_str *temp = ast_str_alloca(BUFSIZ); /* XXX very large ? */ astman_start_ack(s, m); - ast_mutex_lock(&actionlock); for (cur = first_action; cur; cur = cur->next) { /* Walk the list of actions */ if ((s->writeperm & cur->authority) == cur->authority) astman_append(s, "%s: %s (Priv: %s)\r\n", cur->action, cur->synopsis, authority_to_str(cur->authority, &temp)); } - ast_mutex_unlock(&actionlock); astman_append(s, "\r\n"); return 0; @@ -2068,9 +2064,7 @@ static int process_message(struct mansession *s, const struct message *m) ast_log(LOG_DEBUG, "Manager received command '%s'\n", action); if (ast_strlen_zero(action)) { - ast_mutex_lock(&s->__lock); astman_send_error(s, m, "Missing action in request"); - ast_mutex_unlock(&s->__lock); return 0; } @@ -2080,22 +2074,19 @@ static int process_message(struct mansession *s, const struct message *m) ast_mutex_unlock(&s->__lock); return 0; } - ast_mutex_lock(&actionlock); + ast_rwlock_rdlock(&actionlock); for (tmp = first_action ; tmp; tmp = tmp->next) { if (strcasecmp(action, tmp->action)) continue; - ast_mutex_lock(&s->__lock); if ((s->writeperm & tmp->authority) == tmp->authority) { if (tmp->func(s, m)) { /* error */ - ast_mutex_unlock(&s->__lock); return -1; } } else astman_send_error(s, m, "Permission denied"); - ast_mutex_unlock(&s->__lock); break; } - ast_mutex_unlock(&actionlock); + ast_rwlock_unlock(&actionlock); if (!tmp) { ast_mutex_lock(&s->__lock); astman_send_error(s, m, "Invalid/unknown command. Use Action: ListCommands to show available commands."); @@ -2402,7 +2393,7 @@ int ast_manager_unregister(char *action) { struct manager_action *cur, *prev; - ast_mutex_lock(&actionlock); + ast_rwlock_wrlock(&actionlock); for (cur = first_action, prev = NULL; cur; prev = cur, cur = cur->next) { if (!strcasecmp(action, cur->action)) { if (prev) @@ -2415,7 +2406,7 @@ int ast_manager_unregister(char *action) break; } } - ast_mutex_unlock(&actionlock); + ast_rwlock_unlock(&actionlock); return 0; } @@ -2431,12 +2422,12 @@ static int ast_manager_register_struct(struct manager_action *act) struct manager_action *cur, *prev = NULL; int ret; - ast_mutex_lock(&actionlock); + ast_rwlock_wrlock(&actionlock); for (cur = first_action; cur; prev = cur, cur = cur->next) { ret = strcasecmp(cur->action, act->action); if (ret == 0) { ast_log(LOG_WARNING, "Manager: Action '%s' already registered\n", act->action); - ast_mutex_unlock(&actionlock); + ast_rwlock_unlock(&actionlock); return -1; } if (ret > 0) /* Insert these alphabetically */ @@ -2450,7 +2441,7 @@ static int ast_manager_register_struct(struct manager_action *act) if (option_verbose > 1) ast_verbose(VERBOSE_PREFIX_2 "Manager registered action %s\n", act->action); - ast_mutex_unlock(&actionlock); + ast_rwlock_unlock(&actionlock); return 0; }