[Asterisk-code-review] Restrict CLI/AMI commands on shutdown. (asterisk[13])
Mark Michelson
asteriskteam at digium.com
Thu Mar 10 17:06:17 CST 2016
Mark Michelson has uploaded a new change for review.
https://gerrit.asterisk.org/2372
Change subject: Restrict CLI/AMI commands on shutdown.
......................................................................
Restrict CLI/AMI commands on shutdown.
During stress testing, we have frequently seen crashes occur because a
CLI or AMI command attempts to access information that is in the process
of being destroyed.
When addressing how to fix this issue, we initially considered fixing
individual crashes we observed. However, the changes required to fix
those problems would introduce considerable overhead to the nominal
case. This is not reasonable in order to prevent a crash from occurring
while Asterisk is already shutting down.
Instead, this change makes it so AMI and CLI commands cannot be executed
if Asterisk is being shut down. For AMI, this is absolute. For CLI,
though, certain commands can be registered so that they may be run
during Asterisk shutdown.
Change-Id: I8887e215ac352fadf7f4c1e082da9089b1421990
---
M include/asterisk/cli.h
M main/asterisk.c
M main/cli.c
M main/manager.c
M main/utils.c
5 files changed, 56 insertions(+), 2 deletions(-)
git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/72/2372/3
diff --git a/include/asterisk/cli.h b/include/asterisk/cli.h
index 458ebc8..c51d89e 100644
--- a/include/asterisk/cli.h
+++ b/include/asterisk/cli.h
@@ -310,6 +310,18 @@
*/
char *ast_complete_channels(const char *line, const char *word, int pos, int state, int rpos);
+/*!
+ * \brief Allow a CLI command to be executed while Asterisk is shutting down.
+ *
+ * CLI commands by defeault are disabled when Asterisk is shutting down. This is
+ * to ensure the safety of the shutdown since CLI commands may attempt to access
+ * resources that have been freed as a result of the shutdown.
+ *
+ * If a CLI command should be allowed at shutdown, then the best way to enable this
+ * is to call ast_cli_allow_at_shutdown during the CLI_INIT state of the CLI handler.
+ */
+int ast_cli_allow_at_shutdown(struct ast_cli_entry *e);
+
#if defined(__cplusplus) || defined(c_plusplus)
}
#endif
diff --git a/main/asterisk.c b/main/asterisk.c
index 3985149..2d1dc5f 100644
--- a/main/asterisk.c
+++ b/main/asterisk.c
@@ -2441,6 +2441,7 @@
e->usage =
"Usage: core stop now\n"
" Shuts down a running Asterisk immediately, hanging up all active calls .\n";
+ ast_cli_allow_at_shutdown(e);
return NULL;
case CLI_GENERATE:
return NULL;
@@ -2501,6 +2502,7 @@
"Usage: core restart now\n"
" Causes Asterisk to hangup all calls and exec() itself performing a cold\n"
" restart.\n";
+ ast_cli_allow_at_shutdown(e);
return NULL;
case CLI_GENERATE:
return NULL;
@@ -2561,6 +2563,7 @@
"Usage: core abort shutdown\n"
" Causes Asterisk to abort an executing shutdown or restart, and resume normal\n"
" call operations.\n";
+ ast_cli_allow_at_shutdown(e);
return NULL;
case CLI_GENERATE:
return NULL;
diff --git a/main/cli.c b/main/cli.c
index 7f86eab..aab5c6f 100644
--- a/main/cli.c
+++ b/main/cli.c
@@ -63,6 +63,7 @@
#include "asterisk/bridge.h"
#include "asterisk/stasis_channels.h"
#include "asterisk/stasis_bridges.h"
+#include "asterisk/vector.h"
/*!
* \brief List of restrictions per user.
@@ -108,6 +109,8 @@
static struct module_level_list debug_modules = AST_RWLIST_HEAD_INIT_VALUE;
AST_THREADSTORAGE(ast_cli_buf);
+
+static AST_VECTOR(, struct ast_cli_entry *) shutdown_commands;
/*! \brief Initial buffer size for resulting strings in ast_cli() */
#define AST_CLI_INITLEN 256
@@ -2030,6 +2033,7 @@
/*! \brief initialize the _full_cmd string in * each of the builtins. */
void ast_builtins_init(void)
{
+ AST_VECTOR_INIT(&shutdown_commands, 0);
ast_cli_register_multiple(cli_cli, ARRAY_LEN(cli_cli));
ast_register_cleanup(cli_shutdown);
}
@@ -2679,10 +2683,23 @@
return __ast_cli_generator(text, word, state, 1);
}
+static int allowed_on_shutdown(struct ast_cli_entry *e)
+{
+ int i;
+
+ for (i = 0; i < AST_VECTOR_SIZE(&shutdown_commands); ++i) {
+ if (e == AST_VECTOR_GET(&shutdown_commands, i)) {
+ return 1;
+ }
+ }
+
+ return 0;
+}
+
int ast_cli_command_full(int uid, int gid, int fd, const char *s)
{
const char *args[AST_MAX_ARGS + 1];
- struct ast_cli_entry *e;
+ struct ast_cli_entry *e = NULL;
int x;
char *duplicate = parse_args(s, &x, args + 1, AST_MAX_ARGS, NULL);
char tmp[AST_MAX_ARGS + 1];
@@ -2703,6 +2720,11 @@
AST_RWLIST_UNLOCK(&helpers);
if (e == NULL) {
ast_cli(fd, "No such command '%s' (type 'core show help %s' for other possible commands)\n", s, find_best(args + 1));
+ goto done;
+ }
+
+ if (ast_shutting_down() && !allowed_on_shutdown(e)) {
+ ast_cli(fd, "Command '%s' cannot be run during shutdown\n", s);
goto done;
}
@@ -2728,8 +2750,11 @@
if (retval == CLI_FAILURE)
ast_cli(fd, "Command '%s' failed.\n", s);
}
- ast_atomic_fetchadd_int(&e->inuse, -1);
+
done:
+ if (e) {
+ ast_atomic_fetchadd_int(&e->inuse, -1);
+ }
ast_free(duplicate);
return 0;
}
@@ -2750,3 +2775,8 @@
}
return count;
}
+
+int ast_cli_allow_at_shutdown(struct ast_cli_entry *e)
+{
+ return AST_VECTOR_APPEND(&shutdown_commands, e);
+}
diff --git a/main/manager.c b/main/manager.c
index de00381..7c21550 100644
--- a/main/manager.c
+++ b/main/manager.c
@@ -6114,6 +6114,14 @@
return 0;
}
+ if (ast_shutting_down()) {
+ ast_log(LOG_ERROR, "Unable to process manager action '%s'. Asterisk is shutting down.\n", action);
+ mansession_lock(s);
+ astman_send_error(s, m, "Asterisk is shutting down");
+ mansession_unlock(s);
+ return 0;
+ }
+
if (!s->session->authenticated
&& strcasecmp(action, "Login")
&& strcasecmp(action, "Logoff")
diff --git a/main/utils.c b/main/utils.c
index 0b527f0..686af6b 100644
--- a/main/utils.c
+++ b/main/utils.c
@@ -1158,6 +1158,7 @@
"Usage: core show locks\n"
" This command is for lock debugging. It prints out which locks\n"
"are owned by each active thread.\n";
+ ast_cli_allow_on_shutdown(e);
return NULL;
case CLI_GENERATE:
--
To view, visit https://gerrit.asterisk.org/2372
To unsubscribe, visit https://gerrit.asterisk.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I8887e215ac352fadf7f4c1e082da9089b1421990
Gerrit-PatchSet: 3
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: Mark Michelson <mmichelson at digium.com>
More information about the asterisk-code-review
mailing list