<p>Jenkins2 <strong>merged</strong> this change.</p><p><a href="https://gerrit.asterisk.org/5987">View Change</a></p><div style="white-space:pre-wrap">Approvals:
Richard Mudgett: Looks good to me, but someone else must approve
Benjamin Keith Ford: Looks good to me, but someone else must approve
Joshua Colp: Looks good to me, approved; Verified
Jenkins2: Approved for Submit
</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">res_musiconhold: Add kill_escalation_delay, kill_method to class<br><br>By default, when res_musiconhold reloads or unloads, it sends a HUP<br>signal to custom applications (and all descendants), waits 100ms,<br>then sends a TERM signal, waits 100ms, then finally sends a KILL<br>signal. An application which is interacting with an external<br>device and/or spawns children of its own may not be able to exit<br>cleanly in the default times, expecially if sent a KILL signal, or<br>if it's children are getting signals directly from<br>res_musiconhoild.<br><br>* To allow extra time, the 'kill_escalation_delay'<br> class option can be used to set the number of milliseconds<br> res_musiconhold waits before escalating kill signals, with the<br> default being the current 100ms.<br><br>* To control to whom the signals are sent, the "kill_method" class<br> option can be set to "process_group" (the default, existing<br> behavior), which sends signals to the application and its<br> descendants directly, or "process" which sends signals only to the<br> application itself.<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, 123 insertions(+), 49 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">diff --git a/CHANGES b/CHANGES<br>index 910b3eb..50810e3 100644<br>--- a/CHANGES<br>+++ b/CHANGES<br>@@ -9,6 +9,26 @@<br> ==============================================================================<br> <br> ------------------------------------------------------------------------------<br>+--- Functionality changes from Asterisk 13.13-cert4 to Asterisk 13.13-cert5 --<br>+------------------------------------------------------------------------------<br>+<br>+res_musiconhold<br>+------------------<br>+ * By default, when res_musiconhold reloads or unloads, it sends a HUP signal<br>+ to custom applications (and all descendants), waits 100ms, then sends a<br>+ TERM signal, waits 100ms, then finally sends a KILL signal. An application<br>+ which is interacting with an external device and/or spawns children of its<br>+ own may not be able to exit cleanly in the default times, expecially if sent<br>+ a KILL signal, or if it's children are getting signals directly from<br>+ res_musiconhoild. To allow extra time, the 'kill_escalation_delay'<br>+ class option can be used to set the number of milliseconds res_musiconhold<br>+ waits before escalating kill signals, with the default being the current<br>+ 100ms. To control to whom the signals are sent, the "kill_method"<br>+ class option can be set to "process_group" (the default, existing behavior),<br>+ which sends signals to the application and its descendants directly, or<br>+ "process" which sends signals only to the application itself.<br>+<br>+------------------------------------------------------------------------------<br> --- Functionality changes from Asterisk 13.13.0 to Asterisk 13.13-cert1 ------<br> ------------------------------------------------------------------------------<br> <br>diff --git a/configs/samples/musiconhold.conf.sample b/configs/samples/musiconhold.conf.sample<br>index 1211c8a..f8f1206 100644<br>--- a/configs/samples/musiconhold.conf.sample<br>+++ b/configs/samples/musiconhold.conf.sample<br>@@ -87,3 +87,26 @@<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 (and all descendants), waits 100ms, then sends a<br>+; TERM signal, waits 100ms, then finally sends a KILL signal. An application<br>+; which is interacting with an external device and/or spawns children of its<br>+; own may not be able to exit cleanly in the default times, expecially if sent<br>+; a KILL signal, or if it's children are getting signals directly from<br>+; res_musiconhoild. To allow extra time, the 'kill_escalation_delay'<br>+; class option can be used to set the number of milliseconds res_musiconhold<br>+; waits before escalating kill signals, with the default being the current<br>+; 100ms. To control to whom the signals are sent, the "kill_method"<br>+; class option can be set to "process_group" (the default, existing behavior),<br>+; which sends signals to the application and its descendants directly, or<br>+; "process" which sends signals only to the application itself.<br>+<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>+; Send signals to just the child process instead of all descendants<br>+;kill_method=process<br>diff --git a/res/res_musiconhold.c b/res/res_musiconhold.c<br>index 6f51820..193d2da 100644<br>--- a/res/res_musiconhold.c<br>+++ b/res/res_musiconhold.c<br>@@ -157,6 +157,11 @@<br> <br> static struct ast_flags global_flags[1] = {{0}}; /*!< global MOH_ flags */<br> <br>+enum kill_methods {<br>+ KILL_METHOD_PROCESS_GROUP = 0,<br>+ KILL_METHOD_PROCESS<br>+};<br>+<br> struct mohclass {<br> char name[MAX_MUSICCLASS];<br> char dir[256];<br>@@ -177,6 +182,10 @@<br> int pid;<br> time_t start;<br> pthread_t thread;<br>+ /*! Millisecond delay between kill attempts */<br>+ size_t kill_delay;<br>+ /*! Kill method */<br>+ enum kill_methods kill_method;<br> /*! Source of audio */<br> int srcfd;<br> /*! Generic timer */<br>@@ -677,6 +686,51 @@<br> return fds[0];<br> }<br> <br>+static int killer(pid_t pid, int signum, enum kill_methods kill_method)<br>+{<br>+ switch (kill_method) {<br>+ case KILL_METHOD_PROCESS_GROUP:<br>+ return killpg(pid, signum);<br>+ case KILL_METHOD_PROCESS:<br>+ return kill(pid, signum);<br>+ }<br>+<br>+ return -1;<br>+}<br>+<br>+static void killpid(int pid, size_t delay, enum kill_methods kill_method)<br>+{<br>+ if (killer(pid, SIGHUP, kill_method) < 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%s\n", pid,<br>+ kill_method == KILL_METHOD_PROCESS_GROUP ? " and all children" : " only");<br>+ }<br>+ usleep(delay);<br>+ if (killer(pid, SIGTERM, kill_method) < 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%s\n", pid,<br>+ kill_method == KILL_METHOD_PROCESS_GROUP ? " and all children" : " only");<br>+ }<br>+ usleep(delay);<br>+ if (killer(pid, SIGKILL, kill_method) < 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%s\n", pid,<br>+ kill_method == KILL_METHOD_PROCESS_GROUP ? " and all children" : " only");<br>+ }<br>+}<br>+<br> static void *monmp3thread(void *data)<br> {<br> #define MOH_MS_INTERVAL 100<br>@@ -752,28 +806,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->kill_delay, class->kill_method);<br> class->pid = 0;<br> }<br> } else {<br>@@ -1344,6 +1377,7 @@<br> )) {<br> class->format = ao2_bump(ast_format_slin);<br> class->srcfd = -1;<br>+ class->kill_delay = 100000;<br> }<br> <br> return class;<br>@@ -1613,44 +1647,22 @@<br> <br> if (class->pid > 1) {<br> char buff[8192];<br>- int bytes, tbytes = 0, stime = 0, pid = 0;<br>+ int bytes, tbytes = 0, stime = 0;<br> <br> ast_debug(1, "killing %d!\n", class->pid);<br> <br> stime = time(NULL) + 2;<br>- pid = class->pid;<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>+ killpid(class->pid, class->kill_delay, class->kill_method);<br> <br> while ((ast_wait_for_input(class->srcfd, 100) > 0) && <br> (bytes = read(class->srcfd, buff, 8192)) && time(NULL) < stime) {<br> tbytes = tbytes + bytes;<br> }<br> <br>- ast_debug(1, "mpg123 pid %d and child died after %d bytes read\n", pid, tbytes);<br>+ ast_debug(1, "mpg123 pid %d and child died after %d bytes read\n",<br>+ class->pid, tbytes);<br> <br>+ class->pid = 0;<br> close(class->srcfd);<br> class->srcfd = -1;<br> }<br>@@ -1769,6 +1781,22 @@<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->kill_delay) == 1) {<br>+ class->kill_delay *= 1000;<br>+ } else {<br>+ ast_log(LOG_WARNING, "kill_escalation_delay '%s' is invalid. Setting to 100ms\n", var->value);<br>+ class->kill_delay = 100000;<br>+ }<br>+ } else if (!strcasecmp(var->name, "kill_method")) {<br>+ if (!strcasecmp(var->value, "process")) {<br>+ class->kill_method = KILL_METHOD_PROCESS;<br>+ } else if (!strcasecmp(var->value, "process_group")){<br>+ class->kill_method = KILL_METHOD_PROCESS_GROUP;<br>+ } else {<br>+ ast_log(LOG_WARNING, "kill_method '%s' is invalid. Setting to 'process_group'\n", var->value);<br>+ class->kill_method = KILL_METHOD_PROCESS_GROUP;<br> }<br> }<br> }<br>@@ -1904,6 +1932,9 @@<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->kill_delay / 1000);<br>+ ast_cli(a->fd, "\tKill Method: %s\n",<br>+ class->kill_method == KILL_METHOD_PROCESS ? "process" : "process_group");<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/5987">change 5987</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/5987"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: certified/13.13 </div>
<div style="display:none"> Gerrit-MessageType: merged </div>
<div style="display:none"> Gerrit-Change-Id: Iff70a1a9405685a9021a68416830c0db5158603b </div>
<div style="display:none"> Gerrit-Change-Number: 5987 </div>
<div style="display:none"> Gerrit-PatchSet: 3 </div>
<div style="display:none"> Gerrit-Owner: George Joseph <gjoseph@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Benjamin Keith Ford <bford@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins2 </div>
<div style="display:none"> Gerrit-Reviewer: Joshua Colp <jcolp@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Matthew Fredrickson <creslin@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Richard Mudgett <rmudgett@digium.com> </div>