<p>George Joseph has uploaded this change for <strong>review</strong>.</p><p><a href="https://gerrit.asterisk.org/5986">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">res_musiconhold: Add 'kill_escalation_delay option to classes<br><br>By default, when res_musiconhold reloads or unloads, it sends a HUP<br>signal to the custom application's process group, waits 100ms, then<br>sends a TERM signal, waits 100ms, then finally sends a KILL signal.<br>An application which is interacting with an external device and/or<br>spawns children of its own may not be able to exit cleanly in the<br>default times, expecially if sent a KILL signal.<br><br>* To allow extra time, the 'kill_escalation_delay' option was added<br> to the class definition which sets the number of milliseconds to<br> wait before escalating kill signals. The default remains 100ms.<br><br>* The killpg (kill process group) calls were replaced with simple<br> kills to allow the child process to decide how to terminate any<br> children rather than us sending signals to all descendants<br> directly.<br><br>Change-Id: Iff70a1a9405685a9021a68416830c0db5158603b<br>---<br>M CHANGES<br>M configs/samples/musiconhold.conf.sample<br>M res/res_musiconhold.c<br>3 files changed, 99 insertions(+), 46 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/86/5986/1</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">diff --git a/CHANGES b/CHANGES<br>index f2760c3..ff36e0a 100644<br>--- a/CHANGES<br>+++ b/CHANGES<br>@@ -18,6 +18,21 @@<br> been defined.<br> <br> ------------------------------------------------------------------------------<br>+--- Functionality changes from Asterisk 14.6.0 to Asterisk 14.7.0 ------------<br>+------------------------------------------------------------------------------<br>+<br>+res_musiconhold<br>+------------------<br>+ * By default, when res_musiconhold reloads or unloads, it sends a HUP signal<br>+ to custom applications, waits 100ms, then sends a TERM signal, waits 100ms,<br>+ then finally sends a KILL signal. An application which is interacting with<br>+ an external device and/or spawns children of its own may not be able to<br>+ exit cleanly in the default times, expecially if sent a KILL signal. To<br>+ allow extra time, the 'kill_escalation_delay' option was added to the<br>+ class definition which sets the number of milliseconds to wait before<br>+ escalating kill signals.<br>+<br>+------------------------------------------------------------------------------<br> --- Functionality changes from Asterisk 14.5.0 to Asterisk 14.6.0 ------------<br> ------------------------------------------------------------------------------<br> <br>diff --git a/configs/samples/musiconhold.conf.sample b/configs/samples/musiconhold.conf.sample<br>index b2980fc..bd53afa 100644<br>--- a/configs/samples/musiconhold.conf.sample<br>+++ b/configs/samples/musiconhold.conf.sample<br>@@ -97,3 +97,17 @@<br> ;mode=custom<br> ;directory=/var/lib/asterisk/mohmp3<br> ;application=/site/sw/bin/madplay -Q -o raw:- --mono -R 8000 -a -12<br>+<br>+; By default, when res_musiconhold reloads or unloads, it sends a HUP signal<br>+; to custom applications, waits 100ms, then sends a TERM signal, waits 100ms,<br>+; then finally sends a KILL signal. An application which is interacting with<br>+; an external device and/or spawns children of its own may not be able to<br>+; exit cleanly in the default times, expecially if sent a KILL signal. To<br>+; allow extra time, set 'kill_escalation_delay' to the number of milliseconds<br>+; to wait before escalating kill signals.<br>+;[sox_from_device]<br>+;mode=custom<br>+;directory=/var/lib/asterisk/mohmp3<br>+;application=/usr/bin/sox -q -t alsa -c 2 -r 48000 hw:1 -c 1 -r 8000 -t raw -s -<br>+; Wait 500ms before escalating kill signals<br>+;kill_escalation_delay=500<br>diff --git a/res/res_musiconhold.c b/res/res_musiconhold.c<br>index be50e9c..1d0cef3 100644<br>--- a/res/res_musiconhold.c<br>+++ b/res/res_musiconhold.c<br>@@ -179,6 +179,8 @@<br> int pid;<br> time_t start;<br> pthread_t thread;<br>+ /*! Millisecond delay between kill attempts */<br>+ size_t killdelay;<br> /*! Source of audio */<br> int srcfd;<br> /*! Generic timer */<br>@@ -680,6 +682,36 @@<br> return fds[0];<br> }<br> <br>+static void killpid(int pid, size_t delay)<br>+{<br>+ if (kill(pid, SIGHUP) < 0) {<br>+ if (errno == ESRCH) {<br>+ return;<br>+ }<br>+ ast_log(LOG_WARNING, "Unable to send a SIGHUP to MOH process '%d'?!!: %s\n", pid, strerror(errno));<br>+ } else {<br>+ ast_debug(1, "Sent HUP to pid %d\n", pid);<br>+ }<br>+ usleep(delay);<br>+ if (kill(pid, SIGTERM) < 0) {<br>+ if (errno == ESRCH) {<br>+ return;<br>+ }<br>+ ast_log(LOG_WARNING, "Unable to terminate MOH process '%d'?!!: %s\n", pid, strerror(errno));<br>+ } else {<br>+ ast_debug(1, "Sent TERM to pid %d\n", pid);<br>+ }<br>+ usleep(delay);<br>+ if (kill(pid, SIGKILL) < 0) {<br>+ if (errno == ESRCH) {<br>+ return;<br>+ }<br>+ ast_log(LOG_WARNING, "Unable to kill MOH process '%d'?!!: %s\n", pid, strerror(errno));<br>+ } else {<br>+ ast_debug(1, "Sent KILL to pid %d\n", pid);<br>+ }<br>+}<br>+<br> static void *monmp3thread(void *data)<br> {<br> #define MOH_MS_INTERVAL 100<br>@@ -755,28 +787,7 @@<br> class->srcfd = -1;<br> pthread_testcancel();<br> if (class->pid > 1) {<br>- do {<br>- if (killpg(class->pid, SIGHUP) < 0) {<br>- if (errno == ESRCH) {<br>- break;<br>- }<br>- ast_log(LOG_WARNING, "Unable to send a SIGHUP to MOH process?!!: %s\n", strerror(errno));<br>- }<br>- usleep(100000);<br>- if (killpg(class->pid, SIGTERM) < 0) {<br>- if (errno == ESRCH) {<br>- break;<br>- }<br>- ast_log(LOG_WARNING, "Unable to terminate MOH process?!!: %s\n", strerror(errno));<br>- }<br>- usleep(100000);<br>- if (killpg(class->pid, SIGKILL) < 0) {<br>- if (errno == ESRCH) {<br>- break;<br>- }<br>- ast_log(LOG_WARNING, "Unable to kill MOH process?!!: %s\n", strerror(errno));<br>- }<br>- } while (0);<br>+ killpid(class->pid, class->killdelay);<br> class->pid = 0;<br> }<br> } else {<br>@@ -1357,6 +1368,7 @@<br> if (class) {<br> class->format = ao2_bump(ast_format_slin);<br> class->srcfd = -1;<br>+ class->killdelay = 100000;<br> }<br> <br> return class;<br>@@ -1615,31 +1627,8 @@<br> ast_debug(1, "killing %d!\n", class->pid);<br> <br> stime = time(NULL) + 2;<br>- pid = class->pid;<br>+ killpid(class->pid, class->killdelay);<br> class->pid = 0;<br>-<br>- /* Back when this was just mpg123, SIGKILL was fine. Now we need<br>- * to give the process a reason and time enough to kill off its<br>- * children. */<br>- do {<br>- if (killpg(pid, SIGHUP) < 0) {<br>- ast_log(LOG_WARNING, "Unable to send a SIGHUP to MOH process?!!: %s\n", strerror(errno));<br>- }<br>- usleep(100000);<br>- if (killpg(pid, SIGTERM) < 0) {<br>- if (errno == ESRCH) {<br>- break;<br>- }<br>- ast_log(LOG_WARNING, "Unable to terminate MOH process?!!: %s\n", strerror(errno));<br>- }<br>- usleep(100000);<br>- if (killpg(pid, SIGKILL) < 0) {<br>- if (errno == ESRCH) {<br>- break;<br>- }<br>- ast_log(LOG_WARNING, "Unable to kill MOH process?!!: %s\n", strerror(errno));<br>- }<br>- } while (0);<br> <br> while ((ast_wait_for_input(class->srcfd, 100) > 0) && <br> (bytes = read(class->srcfd, buff, 8192)) && time(NULL) < stime) {<br>@@ -1752,6 +1741,40 @@<br> /* For compatibility with the past, we overwrite any name=name<br> * with the context [name]. */<br> ast_copy_string(class->name, cat, sizeof(class->name));<br>+ for (var = ast_variable_browse(cfg, cat); var; var = var->next) {<br>+ if (!strcasecmp(var->name, "mode")) {<br>+ ast_copy_string(class->mode, var->value, sizeof(class->mode));<br>+ } else if (!strcasecmp(var->name, "directory")) {<br>+ ast_copy_string(class->dir, var->value, sizeof(class->dir));<br>+ } else if (!strcasecmp(var->name, "application")) {<br>+ ast_copy_string(class->args, var->value, sizeof(class->args));<br>+ } else if (!strcasecmp(var->name, "announcement")) {<br>+ ast_copy_string(class->announcement, var->value, sizeof(class->announcement));<br>+ ast_set_flag(class, MOH_ANNOUNCEMENT);<br>+ } else if (!strcasecmp(var->name, "digit") && (isdigit(*var->value) || strchr("*#", *var->value))) {<br>+ class->digit = *var->value;<br>+ } else if (!strcasecmp(var->name, "random")) {<br>+ ast_set2_flag(class, ast_true(var->value), MOH_RANDOMIZE);<br>+ } else if (!strcasecmp(var->name, "sort") && !strcasecmp(var->value, "random")) {<br>+ ast_set_flag(class, MOH_RANDOMIZE);<br>+ } else if (!strcasecmp(var->name, "sort") && !strcasecmp(var->value, "alpha")) {<br>+ ast_set_flag(class, MOH_SORTALPHA);<br>+ } else if (!strcasecmp(var->name, "format")) {<br>+ ao2_cleanup(class->format);<br>+ class->format = ast_format_cache_get(var->value);<br>+ if (!class->format) {<br>+ ast_log(LOG_WARNING, "Unknown format '%s' -- defaulting to SLIN\n", var->value);<br>+ class->format = ao2_bump(ast_format_slin);<br>+ }<br>+ } else if (!strcasecmp(var->name, "kill_escalation_delay")) {<br>+ if (sscanf(var->value, "%zu", &class->killdelay) == 1) {<br>+ class->killdelay *= 1000;<br>+ } else {<br>+ ast_log(LOG_WARNING, "kill_escalation_delay '%s' is invalid. Setting to 100ms\n", var->value);<br>+ class->killdelay = 100000;<br>+ }<br>+ }<br>+ }<br> <br> if (ast_strlen_zero(class->dir)) {<br> if (!strcasecmp(class->mode, "custom")) {<br>@@ -1884,6 +1907,7 @@<br> ast_cli(a->fd, "\tDirectory: %s\n", S_OR(class->dir, "<none>"));<br> if (ast_test_flag(class, MOH_CUSTOM)) {<br> ast_cli(a->fd, "\tApplication: %s\n", S_OR(class->args, "<none>"));<br>+ ast_cli(a->fd, "\tKill Escalation Delay: %zu ms\n", class->killdelay / 1000);<br> }<br> if (strcasecmp(class->mode, "files")) {<br> ast_cli(a->fd, "\tFormat: %s\n", ast_format_get_name(class->format));<br></pre><p>To view, visit <a href="https://gerrit.asterisk.org/5986">change 5986</a>. To unsubscribe, visit <a href="https://gerrit.asterisk.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.asterisk.org/5986"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-MessageType: newchange </div>
<div style="display:none"> Gerrit-Change-Id: Iff70a1a9405685a9021a68416830c0db5158603b </div>
<div style="display:none"> Gerrit-Change-Number: 5986 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: George Joseph <gjoseph@digium.com> </div>