<p>George Joseph has uploaded this change for <strong>review</strong>.</p><p><a href="https://gerrit.asterisk.org/5984">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, 72 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/84/5984/1</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">diff --git a/CHANGES b/CHANGES<br>index c3a2d0a..e2a02f4 100644<br>--- a/CHANGES<br>+++ b/CHANGES<br>@@ -9,6 +9,21 @@<br> ==============================================================================<br> <br> ------------------------------------------------------------------------------<br>+--- Functionality changes from Asterisk 13.17.0 to Asterisk 13.18.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 13.16.0 to Asterisk 13.17.0 ----------<br> ------------------------------------------------------------------------------<br> <br>diff --git a/configs/samples/musiconhold.conf.sample b/configs/samples/musiconhold.conf.sample<br>index 8b2202d..8345a69 100644<br>--- a/configs/samples/musiconhold.conf.sample<br>+++ b/configs/samples/musiconhold.conf.sample<br>@@ -91,3 +91,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 c52c964..11489e4 100644<br>--- a/res/res_musiconhold.c<br>+++ b/res/res_musiconhold.c<br>@@ -178,6 +178,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>@@ -678,6 +680,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>@@ -753,28 +785,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>@@ -1328,6 +1339,7 @@<br> )) {<br> class->format = ao2_bump(ast_format_slin);<br> class->srcfd = -1;<br>+ class->killdelay = 100000;<br> }<br> <br> return class;<br>@@ -1605,31 +1617,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>@@ -1765,6 +1754,13 @@<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>@@ -1899,6 +1895,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/5984">change 5984</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/5984"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: 13 </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: 5984 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: George Joseph <gjoseph@digium.com> </div>