Fix Bug #69865

[m6w6]

php-fpm does not close stderr when using syslog

Someone with FPM knowledge should have another look.

[krakjoe]

This does merge cleanly in supported branch, but I'm not sure about it still ... actually I don't get the logic of it at all, and think the comment could do a better job of explaining the magic being used.

Can someone familiar with both fpm and stdio (in particular dup'ing) check this please.

I can't merge it without input from elsewhere ...

In the mean time, rebasing against supported branch would be a good idea @m6w6

[orlitzky]

When a program daemonizes itself (like php-fpm does), it's supposed to close the three std* handles that it has, because they become useless. In most cases, you overwrite stdout, stderr, and stdin with a handle to /dev/null to prevent unexpected surprises. However in php-fpm's case, it sometimes makes sense to overwrite stderr with a handle to the log file.

First, note from sapi/fpm/fpm/zlog.h that

#define ZLOG_SYSLOG -2

Then in sapi/fpm/fpm/fpm_stdio.c, we do immediately overwrite the stdin and stdout handles with a handle to /dev/null:

int fpm_stdio_init_main() /* {{{ */
{
        int fd = open("/dev/null", O_RDWR);
	...
	if (0 > dup2(fd, STDIN_FILENO) || 0 > dup2(fd, STDOUT_FILENO)) {

The only confusion is with what to do about stderr. When php-fpm is logging to an honest-to-god file, the usual stderr descriptor is overwritten with the descriptor to that log file. This is presumably so that any other part of the code that attempts to write to stderr will write to the log file instead:

int fpm_stdio_init_final() /* {{{ */
{
	if (fpm_use_error_log()) {
		/* prevent duping if logging to syslog */
		if (fpm_globals.error_log_fd > 0 && fpm_globals.error_log_fd != STDERR_FILENO) {

			/* there might be messages to stderr from other parts of the code, we need to log them all */
			if (0 > dup2(fpm_globals.error_log_fd, STDERR_FILENO)) {

The first "if" statement ensures that we're not running interactively, because in that case we should just print stuff to the screen. The second "if" statement ensures that we're not logging to syslog, because ZLOG_SYSLOG is negative. (It also checks that the upcoming dup2 is not redundant.) If we are logging to a file, and that file isn't syslog, the eventual call to dup2 overwrites stderr with a handle to that log file. Afterwards, anyone trying to write to stderr will write to the log file instead.

That omits a case, however. What do we do with stderr if we are logging to syslog? This pull request adds an "else if" to handle that case. If we're logging to syslog and some other part of the code tries to write to the stderr handle... well, too bad. It probably shouldn't be doing that anyway. But we don't want to leave stderr as it is, because when someone writes to it, the output goes nowhere. We might as well just close the handle and let those people write to /dev/null instead. That's what I think this commit does.

[krakjoe]

@orlitzky thanks for input, much appreciated ...

Merged 80a851b

Thanks :)

[m6w6]

Huh, totally missed that ping, sorry & thanks a lot!