Update of /usr/cvsroot/asterisk/res
In directory mongoose.digium.com:/tmp/cvs-serv11456/res
Modified Files:
res_musiconhold.c
Log Message:
fix threading portability problem with FreeBSD (bug #4532)
ensure that all mpg123 child processes get killed when the parent is killed
(bug #4532)
Index: res_musiconhold.c
===================================================================
RCS file: /usr/cvsroot/asterisk/res/res_musiconhold.c,v
retrieving revision 1.62
retrieving revision 1.63
diff -u -d -r1.62 -r1.63
--- res_musiconhold.c 30 Jun 2005 18:08:27 -0000 1.62
+++ res_musiconhold.c 11 Jul 2005 21:42:25 -0000 1.63
@@ -402,19 +402,44 @@
ast_log(LOG_WARNING, "Fork failed: %s\n", strerror(errno));
return -1;
}
- if (!class->pid) {
- int x;
- close(fds[0]);
- /* Stdout goes to pipe */
- dup2(fds[1], STDOUT_FILENO);
- /* Close unused file descriptors */
- for (x=3;x<8192;x++) {
- if (-1 != fcntl(x, F_GETFL)) {
- close(x);
+ if (class->pid != 0) { /* parent */
+ close(fds[1]);
+ return fds[0];
+ } else {
+ /* Child */
+ int i;
+ /*
+ * On BSD systems with userland pthreads libraries, we need
+ * to call fcntl() _before_ close() to avoid resetting
+ * O_NONBLOCK on the internal descriptors.
+ * It should not harm in other systems.
+ *
+ * After that, close the descriptors not needed in the child.
+ * It is also important that we do not share descriptors
+ * with the child process or it could change their blocking
+ * state and give trouble in the thread scheduling.
+ * Here, parent and child are connected only through the
+ * endpoints of a pipe, so they share no descriptors.
+ */
+ for (i=0; i < getdtablesize(); i++) {
+ long fl = fcntl(i, F_GETFL);
+ if (fl != -1 && i != fds[1]) {
+ /* open and must be closed in the child */
+ fcntl(i, F_SETFL, O_NONBLOCK | fl);
+ close(i);
}
}
- /* Child */
+ /* Stdout in the child goes to pipe */
+ dup2(fds[1], STDOUT_FILENO);
chdir(class->dir);
+ /*
+ * mpg123 may fork children to be more responsive, and we
+ * want to kill them all when we exit. To do so we set
+ * the process group id of mpg123 and children to a different
+ * value than the asterisk process so we can kill all at once.
+ * So remember, class->pid is really class->pgid!
+ */
+ setpgid(0, getpid());
if (ast_test_flag(class, MOH_CUSTOM)) {
execv(argv[0], argv);
} else {
@@ -425,14 +450,9 @@
/* Check PATH as a last-ditch effort */
execvp("mpg123", argv);
}
- ast_log(LOG_WARNING, "Exec failed: %s\n", strerror(errno));
- close(fds[1]);
exit(1);
- } else {
- /* Parent */
- close(fds[1]);
+ return 0; /* unreached */
}
- return fds[0];
}
static void *monmp3thread(void *data)
@@ -511,7 +531,7 @@
close(class->srcfd);
class->srcfd = -1;
if (class->pid) {
- kill(class->pid, SIGKILL);
+ killpg(class->pid, SIGKILL); /* pgid! */
class->pid = 0;
}
} else
@@ -957,7 +977,7 @@
stime = time(NULL) + 2;
pid = moh->pid;
moh->pid = 0;
- kill(pid, SIGKILL);
+ killpg(pid, SIGKILL); /* pgid! */
while ((ast_wait_for_input(moh->srcfd, 100) > -1) &&
(bytes = read(moh->srcfd, buff, 8192)) && time(NULL) < stime) {
tbytes = tbytes + bytes;
}
_______________________________________________
Asterisk-Cvs mailing list
[email protected]
http://lists.digium.com/mailman/listinfo/asterisk-cvs
