<p>Jenkins2 <strong>merged</strong> this change.</p><p><a href="https://gerrit.asterisk.org/5985">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, 150 insertions(+), 49 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">diff --git a/CHANGES b/CHANGES<br>index 9d6cea0..5907e13 100644<br>--- a/CHANGES<br>+++ b/CHANGES<br>@@ -9,6 +9,26 @@<br> ==============================================================================<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 (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 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..741bde6 100644<br>--- a/configs/samples/musiconhold.conf.sample<br>+++ b/configs/samples/musiconhold.conf.sample<br>@@ -97,3 +97,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 a070f31..9c995bf 100644<br>--- a/res/res_musiconhold.c<br>+++ b/res/res_musiconhold.c<br>@@ -161,6 +161,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>@@ -181,6 +186,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>@@ -682,6 +691,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>@@ -757,28 +811,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>@@ -1359,6 +1392,7 @@<br> if (class) {<br> class->format = ao2_bump(ast_format_slin);<br> class->srcfd = -1;<br>+ class->kill_delay = 100000;<br> }<br> <br> return class;<br>@@ -1612,44 +1646,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>@@ -1754,6 +1766,49 @@<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->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> <br> if (ast_strlen_zero(class->dir)) {<br> if (!strcasecmp(class->mode, "custom")) {<br>@@ -1886,6 +1941,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/5985">change 5985</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/5985"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: 14 </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: 5985 </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>