[Asterisk-code-review] res musiconhold: Add 'kill escalation delay option to classes (asterisk[14])
George Joseph
asteriskteam at digium.com
Tue Jul 11 08:06:59 CDT 2017
George Joseph has uploaded this change for review. ( https://gerrit.asterisk.org/5985
Change subject: res_musiconhold: Add 'kill_escalation_delay option to classes
......................................................................
res_musiconhold: Add 'kill_escalation_delay option to classes
By default, when res_musiconhold reloads or unloads, it sends a HUP
signal to the custom application's process group, waits 100ms, then
sends a TERM signal, waits 100ms, then finally sends a KILL signal.
An application which is interacting with an external device and/or
spawns children of its own may not be able to exit cleanly in the
default times, expecially if sent a KILL signal.
* To allow extra time, the 'kill_escalation_delay' option was added
to the class definition which sets the number of milliseconds to
wait before escalating kill signals. The default remains 100ms.
* The killpg (kill process group) calls were replaced with simple
kills to allow the child process to decide how to terminate any
children rather than us sending signals to all descendants
directly.
Change-Id: Iff70a1a9405685a9021a68416830c0db5158603b
---
M CHANGES
M configs/samples/musiconhold.conf.sample
M res/res_musiconhold.c
3 files changed, 99 insertions(+), 46 deletions(-)
git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/85/5985/1
diff --git a/CHANGES b/CHANGES
index 9d6cea0..e078d5a 100644
--- a/CHANGES
+++ b/CHANGES
@@ -9,6 +9,21 @@
==============================================================================
------------------------------------------------------------------------------
+--- Functionality changes from Asterisk 14.6.0 to Asterisk 14.7.0 ------------
+------------------------------------------------------------------------------
+
+res_musiconhold
+------------------
+ * By default, when res_musiconhold reloads or unloads, it sends a HUP signal
+ to custom applications, waits 100ms, then sends a TERM signal, waits 100ms,
+ then finally sends a KILL signal. An application which is interacting with
+ an external device and/or spawns children of its own may not be able to
+ exit cleanly in the default times, expecially if sent a KILL signal. To
+ allow extra time, the 'kill_escalation_delay' option was added to the
+ class definition which sets the number of milliseconds to wait before
+ escalating kill signals.
+
+------------------------------------------------------------------------------
--- Functionality changes from Asterisk 14.5.0 to Asterisk 14.6.0 ------------
------------------------------------------------------------------------------
diff --git a/configs/samples/musiconhold.conf.sample b/configs/samples/musiconhold.conf.sample
index b2980fc..bd53afa 100644
--- a/configs/samples/musiconhold.conf.sample
+++ b/configs/samples/musiconhold.conf.sample
@@ -97,3 +97,17 @@
;mode=custom
;directory=/var/lib/asterisk/mohmp3
;application=/site/sw/bin/madplay -Q -o raw:- --mono -R 8000 -a -12
+
+; By default, when res_musiconhold reloads or unloads, it sends a HUP signal
+; to custom applications, waits 100ms, then sends a TERM signal, waits 100ms,
+; then finally sends a KILL signal. An application which is interacting with
+; an external device and/or spawns children of its own may not be able to
+; exit cleanly in the default times, expecially if sent a KILL signal. To
+; allow extra time, set 'kill_escalation_delay' to the number of milliseconds
+; to wait before escalating kill signals.
+;[sox_from_device]
+;mode=custom
+;directory=/var/lib/asterisk/mohmp3
+;application=/usr/bin/sox -q -t alsa -c 2 -r 48000 hw:1 -c 1 -r 8000 -t raw -s -
+; Wait 500ms before escalating kill signals
+;kill_escalation_delay=500
diff --git a/res/res_musiconhold.c b/res/res_musiconhold.c
index a070f31..e457c3a 100644
--- a/res/res_musiconhold.c
+++ b/res/res_musiconhold.c
@@ -181,6 +181,8 @@
int pid;
time_t start;
pthread_t thread;
+ /*! Millisecond delay between kill attempts */
+ size_t killdelay;
/*! Source of audio */
int srcfd;
/*! Generic timer */
@@ -682,6 +684,36 @@
return fds[0];
}
+static void killpid(int pid, size_t delay)
+{
+ if (kill(pid, SIGHUP) < 0) {
+ if (errno == ESRCH) {
+ return;
+ }
+ ast_log(LOG_WARNING, "Unable to send a SIGHUP to MOH process '%d'?!!: %s\n", pid, strerror(errno));
+ } else {
+ ast_debug(1, "Sent HUP to pid %d\n", pid);
+ }
+ usleep(delay);
+ if (kill(pid, SIGTERM) < 0) {
+ if (errno == ESRCH) {
+ return;
+ }
+ ast_log(LOG_WARNING, "Unable to terminate MOH process '%d'?!!: %s\n", pid, strerror(errno));
+ } else {
+ ast_debug(1, "Sent TERM to pid %d\n", pid);
+ }
+ usleep(delay);
+ if (kill(pid, SIGKILL) < 0) {
+ if (errno == ESRCH) {
+ return;
+ }
+ ast_log(LOG_WARNING, "Unable to kill MOH process '%d'?!!: %s\n", pid, strerror(errno));
+ } else {
+ ast_debug(1, "Sent KILL to pid %d\n", pid);
+ }
+}
+
static void *monmp3thread(void *data)
{
#define MOH_MS_INTERVAL 100
@@ -757,28 +789,7 @@
class->srcfd = -1;
pthread_testcancel();
if (class->pid > 1) {
- do {
- if (killpg(class->pid, SIGHUP) < 0) {
- if (errno == ESRCH) {
- break;
- }
- ast_log(LOG_WARNING, "Unable to send a SIGHUP to MOH process?!!: %s\n", strerror(errno));
- }
- usleep(100000);
- if (killpg(class->pid, SIGTERM) < 0) {
- if (errno == ESRCH) {
- break;
- }
- ast_log(LOG_WARNING, "Unable to terminate MOH process?!!: %s\n", strerror(errno));
- }
- usleep(100000);
- if (killpg(class->pid, SIGKILL) < 0) {
- if (errno == ESRCH) {
- break;
- }
- ast_log(LOG_WARNING, "Unable to kill MOH process?!!: %s\n", strerror(errno));
- }
- } while (0);
+ killpid(class->pid, class->killdelay);
class->pid = 0;
}
} else {
@@ -1359,6 +1370,7 @@
if (class) {
class->format = ao2_bump(ast_format_slin);
class->srcfd = -1;
+ class->killdelay = 100000;
}
return class;
@@ -1617,31 +1629,8 @@
ast_debug(1, "killing %d!\n", class->pid);
stime = time(NULL) + 2;
- pid = class->pid;
+ killpid(class->pid, class->killdelay);
class->pid = 0;
-
- /* Back when this was just mpg123, SIGKILL was fine. Now we need
- * to give the process a reason and time enough to kill off its
- * children. */
- do {
- if (killpg(pid, SIGHUP) < 0) {
- ast_log(LOG_WARNING, "Unable to send a SIGHUP to MOH process?!!: %s\n", strerror(errno));
- }
- usleep(100000);
- if (killpg(pid, SIGTERM) < 0) {
- if (errno == ESRCH) {
- break;
- }
- ast_log(LOG_WARNING, "Unable to terminate MOH process?!!: %s\n", strerror(errno));
- }
- usleep(100000);
- if (killpg(pid, SIGKILL) < 0) {
- if (errno == ESRCH) {
- break;
- }
- ast_log(LOG_WARNING, "Unable to kill MOH process?!!: %s\n", strerror(errno));
- }
- } while (0);
while ((ast_wait_for_input(class->srcfd, 100) > 0) &&
(bytes = read(class->srcfd, buff, 8192)) && time(NULL) < stime) {
@@ -1754,6 +1743,40 @@
/* For compatibility with the past, we overwrite any name=name
* with the context [name]. */
ast_copy_string(class->name, cat, sizeof(class->name));
+ for (var = ast_variable_browse(cfg, cat); var; var = var->next) {
+ if (!strcasecmp(var->name, "mode")) {
+ ast_copy_string(class->mode, var->value, sizeof(class->mode));
+ } else if (!strcasecmp(var->name, "directory")) {
+ ast_copy_string(class->dir, var->value, sizeof(class->dir));
+ } else if (!strcasecmp(var->name, "application")) {
+ ast_copy_string(class->args, var->value, sizeof(class->args));
+ } else if (!strcasecmp(var->name, "announcement")) {
+ ast_copy_string(class->announcement, var->value, sizeof(class->announcement));
+ ast_set_flag(class, MOH_ANNOUNCEMENT);
+ } else if (!strcasecmp(var->name, "digit") && (isdigit(*var->value) || strchr("*#", *var->value))) {
+ class->digit = *var->value;
+ } else if (!strcasecmp(var->name, "random")) {
+ ast_set2_flag(class, ast_true(var->value), MOH_RANDOMIZE);
+ } else if (!strcasecmp(var->name, "sort") && !strcasecmp(var->value, "random")) {
+ ast_set_flag(class, MOH_RANDOMIZE);
+ } else if (!strcasecmp(var->name, "sort") && !strcasecmp(var->value, "alpha")) {
+ ast_set_flag(class, MOH_SORTALPHA);
+ } else if (!strcasecmp(var->name, "format")) {
+ ao2_cleanup(class->format);
+ class->format = ast_format_cache_get(var->value);
+ if (!class->format) {
+ ast_log(LOG_WARNING, "Unknown format '%s' -- defaulting to SLIN\n", var->value);
+ class->format = ao2_bump(ast_format_slin);
+ }
+ } else if (!strcasecmp(var->name, "kill_escalation_delay")) {
+ if (sscanf(var->value, "%zu", &class->killdelay) == 1) {
+ class->killdelay *= 1000;
+ } else {
+ ast_log(LOG_WARNING, "kill_escalation_delay '%s' is invalid. Setting to 100ms\n", var->value);
+ class->killdelay = 100000;
+ }
+ }
+ }
if (ast_strlen_zero(class->dir)) {
if (!strcasecmp(class->mode, "custom")) {
@@ -1886,6 +1909,7 @@
ast_cli(a->fd, "\tDirectory: %s\n", S_OR(class->dir, "<none>"));
if (ast_test_flag(class, MOH_CUSTOM)) {
ast_cli(a->fd, "\tApplication: %s\n", S_OR(class->args, "<none>"));
+ ast_cli(a->fd, "\tKill Escalation Delay: %zu ms\n", class->killdelay / 1000);
}
if (strcasecmp(class->mode, "files")) {
ast_cli(a->fd, "\tFormat: %s\n", ast_format_get_name(class->format));
--
To view, visit https://gerrit.asterisk.org/5985
To unsubscribe, visit https://gerrit.asterisk.org/settings
Gerrit-Project: asterisk
Gerrit-Branch: 14
Gerrit-MessageType: newchange
Gerrit-Change-Id: Iff70a1a9405685a9021a68416830c0db5158603b
Gerrit-Change-Number: 5985
Gerrit-PatchSet: 1
Gerrit-Owner: George Joseph <gjoseph at digium.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20170711/0d0766f6/attachment-0001.html>
More information about the asterisk-code-review
mailing list