[Asterisk-code-review] res musiconhold: Add 'kill escalation delay option to classes (asterisk[certified/13.13])
George Joseph
asteriskteam at digium.com
Tue Jul 11 08:18:00 CDT 2017
George Joseph has uploaded this change for review. ( https://gerrit.asterisk.org/5987
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, 72 insertions(+), 46 deletions(-)
git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/87/5987/1
diff --git a/CHANGES b/CHANGES
index 910b3eb..2ec5784 100644
--- a/CHANGES
+++ b/CHANGES
@@ -9,6 +9,21 @@
==============================================================================
------------------------------------------------------------------------------
+--- Functionality changes from Asterisk 13.13-cert4 to Asterisk 13.13-cert5 --
+------------------------------------------------------------------------------
+
+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 13.13.0 to Asterisk 13.13-cert1 ------
------------------------------------------------------------------------------
diff --git a/configs/samples/musiconhold.conf.sample b/configs/samples/musiconhold.conf.sample
index 1211c8a..1ea76de 100644
--- a/configs/samples/musiconhold.conf.sample
+++ b/configs/samples/musiconhold.conf.sample
@@ -87,3 +87,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 6f51820..39b8b20 100644
--- a/res/res_musiconhold.c
+++ b/res/res_musiconhold.c
@@ -177,6 +177,8 @@
int pid;
time_t start;
pthread_t thread;
+ /*! Millisecond delay between kill attempts */
+ size_t killdelay;
/*! Source of audio */
int srcfd;
/*! Generic timer */
@@ -677,6 +679,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
@@ -752,28 +784,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 {
@@ -1344,6 +1355,7 @@
)) {
class->format = ao2_bump(ast_format_slin);
class->srcfd = -1;
+ class->killdelay = 100000;
}
return class;
@@ -1618,31 +1630,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) {
@@ -1769,6 +1758,13 @@
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;
}
}
}
@@ -1904,6 +1900,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/5987
To unsubscribe, visit https://gerrit.asterisk.org/settings
Gerrit-Project: asterisk
Gerrit-Branch: certified/13.13
Gerrit-MessageType: newchange
Gerrit-Change-Id: Iff70a1a9405685a9021a68416830c0db5158603b
Gerrit-Change-Number: 5987
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/a1a524aa/attachment-0001.html>
More information about the asterisk-code-review
mailing list