From 8b76f89c37973082b3d64f5a27937efcca9d65f6 Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@paquier.xyz>
Date: Fri, 8 Oct 2021 11:08:35 +0900
Subject: [PATCH] Refactor fallback to stderr for csvlog to handle better WIN32
 service case

send_message_to_server_log() would force a redirection of a log entry to
stderr in some cases for csvlog, like the syslogger not being available
yet.  If this happens, csvlog would fall back to stderr to log
some information rather than nothing.  The code was organized so as
stderr is done before csvlog, with csvlog checking that stderr did not
happen yet with a reversed condition.  With this code organization, it
could be possible to lose some messages if running Postgres as a service
on WIN32, as there is no usable stderr, and the handling of the
StringInfoData holding the message for stderr was rather confusing
because of that.

This commit moves the csvlog handling to be before stderr, as as we are
able to track down if it is necessary to log something to stderr.  The
reduces the handling of stderr to be in a single code path, adding a
fallback to event logs for a WIN32 service.  This also simplifies the
way we handle the StringInfoData for stderr, making easier the
integration of new file-based log destinations.  I got to play with
services and event logs on Windows while checking this change.

Reviewed-by: Chris Bandy
Discussion: https://postgr.es/m/YV0vwBovEKf1WXkl@paquier.xyz
---
 src/backend/utils/error/elog.c | 54 +++++++++++++++-------------------
 1 file changed, 24 insertions(+), 30 deletions(-)

diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index 2af87ee3bd1..f33729513a0 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -3008,6 +3008,7 @@ static void
 send_message_to_server_log(ErrorData *edata)
 {
 	StringInfoData buf;
+	bool		fallback_to_stderr = false;
 
 	initStringInfo(&buf);
 
@@ -3159,8 +3160,27 @@ send_message_to_server_log(ErrorData *edata)
 	}
 #endif							/* WIN32 */
 
-	/* Write to stderr, if enabled */
-	if ((Log_destination & LOG_DESTINATION_STDERR) || whereToSendOutput == DestDebug)
+	/* Write to csvlog, if enabled */
+	if (Log_destination & LOG_DESTINATION_CSVLOG)
+	{
+		/*
+		 * Send CSV data if it's safe to do so (syslogger doesn't need the
+		 * pipe).  If this is not possible, fallback to an entry written to
+		 * stderr.
+		 */
+		if (redirection_done || MyBackendType == B_LOGGER)
+			write_csvlog(edata);
+		else
+			fallback_to_stderr = true;
+	}
+
+	/*
+	 * Write to stderr, if enabled or if required because of a previous
+	 * limitation.
+	 */
+	if ((Log_destination & LOG_DESTINATION_STDERR) ||
+		whereToSendOutput == DestDebug ||
+		fallback_to_stderr)
 	{
 		/*
 		 * Use the chunking protocol if we know the syslogger should be
@@ -3189,34 +3209,8 @@ send_message_to_server_log(ErrorData *edata)
 	if (MyBackendType == B_LOGGER)
 		write_syslogger_file(buf.data, buf.len, LOG_DESTINATION_STDERR);
 
-	/* Write to CSV log if enabled */
-	if (Log_destination & LOG_DESTINATION_CSVLOG)
-	{
-		if (redirection_done || MyBackendType == B_LOGGER)
-		{
-			/*
-			 * send CSV data if it's safe to do so (syslogger doesn't need the
-			 * pipe). First get back the space in the message buffer.
-			 */
-			pfree(buf.data);
-			write_csvlog(edata);
-		}
-		else
-		{
-			/*
-			 * syslogger not up (yet), so just dump the message to stderr,
-			 * unless we already did so above.
-			 */
-			if (!(Log_destination & LOG_DESTINATION_STDERR) &&
-				whereToSendOutput != DestDebug)
-				write_console(buf.data, buf.len);
-			pfree(buf.data);
-		}
-	}
-	else
-	{
-		pfree(buf.data);
-	}
+	/* No more need of the message formatted for stderr */
+	pfree(buf.data);
 }
 
 /*