On Thu, Dec 28, 2017 at 12:54:27AM +0100, William Lallemand wrote: > I think that's better to open /dev/null and dup2 with 0, 1, 2 so we won't > have any > surprise with an fprintf(stderr, ".. anywhere in the code. >
Hi, I made a patch which does exactly that, however I think we will do a cleanup of how the quiet & verbose mode work for 1.9. It's a little bit messy, we should never edit the mode.global flags outside of the configuration and command line parsing. Two patches attached, including Pieter's one which was reformated. Regards, -- William Lallemand
>From 74af850251e890bb1656c233bb3fd503eaea13bf Mon Sep 17 00:00:00 2001 From: PiBa-NL <[email protected]> Date: Mon, 25 Dec 2017 21:03:31 +0100 Subject: [PATCH 1/2] BUG/MEDIUM: mworker: don't close stdio several time This patch makes sure that a frontend socket that gets created after initialization won't be closed when the master gets re-executed. When used in daemon mode, the master-worker is closing the FDs 0, 1, 2 after the fork of the children. When the master was reloading, those FDs were assigned again during the parsing of the configuration (probably for some listeners), and the workers were closing them thinking it was the stdio. This patch must be backported to 1.8. --- src/haproxy.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/haproxy.c b/src/haproxy.c index ffd7ea05e..f3970f7b6 100644 --- a/src/haproxy.c +++ b/src/haproxy.c @@ -2580,9 +2580,18 @@ int main(int argc, char **argv) /* MODE_QUIET can inhibit alerts and warnings below this line */ - if ((global.mode & MODE_QUIET) && !(global.mode & MODE_VERBOSE)) { - /* detach from the tty */ - fclose(stdin); fclose(stdout); fclose(stderr); + if (getenv("HAPROXY_MWORKER_REEXEC") != NULL) { + // either stdin/out/err are already closed or should stay as they are. + if ((global.mode & MODE_DAEMON)) { + // daemon mode re-executing, stdin/stdout/stderr are already closed so keep quiet + global.mode &= ~MODE_VERBOSE; + global.mode |= MODE_QUIET; /* ensure that we won't say anything from now */ + } + } else { + if ((global.mode & MODE_QUIET) && !(global.mode & MODE_VERBOSE)) { + /* detach from the tty */ + fclose(stdin); fclose(stdout); fclose(stderr); + } } /* open log & pid files before the chroot */ -- 2.13.6
>From 7c84c87eac8bf48e62bc8ee79cf6cec3ce33996d Mon Sep 17 00:00:00 2001 From: William Lallemand <[email protected]> Date: Thu, 28 Dec 2017 16:09:36 +0100 Subject: [PATCH 2/2] MINOR: don't close stdio anymore Closing the standard IO FDs (0,1,2) can be troublesome, especially in the case of the master-worker. Instead of closing those FDs, they are now pointing to /dev/null which prevents sending debugging messages to the wrong FDs. This patch could be backported in 1.8. --- src/haproxy.c | 43 ++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 40 insertions(+), 3 deletions(-) diff --git a/src/haproxy.c b/src/haproxy.c index f3970f7b6..2926af3ff 100644 --- a/src/haproxy.c +++ b/src/haproxy.c @@ -889,6 +889,32 @@ static void dump(struct sig_handler *sh) pool_gc(NULL); } +/* + * This function dup2 the stdio FDs (0,1,2) with <fd>, then closes <fd> + * If <fd> < 0, it opens /dev/null and use it to dup + * + * In the case of chrooting, you have to open /dev/null before the chroot, and + * pass the <fd> to this function + */ +static void stdio_quiet(int fd) +{ + + if (fd < 0) + fd = open("/dev/null", O_RDWR, 0); + + if (fd > -1) { + dup2(fd, 0); + dup2(fd, 1); + dup2(fd, 2); + close(fd); + return; + } + + ha_alert("Cannot open /dev/null\n"); + exit(EXIT_FAILURE); +} + + /* This function check if cfg_cfgfiles containes directories. * If it find one, it add all the files (and only files) it containes * in cfg_cfgfiles in place of the directory (and remove the directory). @@ -2590,7 +2616,7 @@ int main(int argc, char **argv) } else { if ((global.mode & MODE_QUIET) && !(global.mode & MODE_VERBOSE)) { /* detach from the tty */ - fclose(stdin); fclose(stdout); fclose(stderr); + stdio_quiet(-1); } } @@ -2683,6 +2709,7 @@ int main(int argc, char **argv) struct peers *curpeers; int ret = 0; int proc; + int devnullfd = -1; children = calloc(global.nbproc, sizeof(int)); /* @@ -2797,7 +2824,9 @@ int main(int argc, char **argv) if ((!(global.mode & MODE_QUIET) || (global.mode & MODE_VERBOSE)) && (global.mode & MODE_DAEMON)) { /* detach from the tty, this is required to properly daemonize. */ - fclose(stdin); fclose(stdout); fclose(stderr); + if ((getenv("HAPROXY_MWORKER_REEXEC") == NULL)) + stdio_quiet(-1); + global.mode &= ~MODE_VERBOSE; global.mode |= MODE_QUIET; /* ensure that we won't say anything from now */ setsid(); @@ -2816,6 +2845,14 @@ int main(int argc, char **argv) /* child must never use the atexit function */ atexit_flag = 0; + if (!(global.mode & MODE_QUIET) || (global.mode & MODE_VERBOSE)) { + devnullfd = open("/dev/null", O_RDWR, 0); + if (devnullfd < 0) { + ha_alert("Cannot open /dev/null\n"); + exit(EXIT_FAILURE); + } + } + /* Must chroot and setgid/setuid in the children */ /* chroot if needed */ if (global.chroot != NULL) { @@ -2908,7 +2945,7 @@ int main(int argc, char **argv) */ if (!(global.mode & MODE_QUIET) || (global.mode & MODE_VERBOSE)) { /* detach from the tty */ - fclose(stdin); fclose(stdout); fclose(stderr); + stdio_quiet(devnullfd); global.mode &= ~MODE_VERBOSE; global.mode |= MODE_QUIET; /* ensure that we won't say anything from now */ } -- 2.13.6
