v4-0001-Adds-a-helper-for-places-that-shell-out-to-system.patch

From ec72e05e87c73dee3de73f9a6586e8e8db2d919e Mon Sep 17 00:00:00 2001
From: Jelte Fennema-Nio <github-tech@jeltef.nl>
Date: Sun, 23 Feb 2025 16:52:29 +0100
Subject: [PATCH v4 1/3] Adds a helper for places that shell out to system()

We need to call system() in a few places, and to do so safely we need
some pre/post-fork logic. This encapsulates that logic into a single
System helper function. The main reason to do this is that in a follow
up commit we want to add even more logic pre and post-fork.
---
 src/backend/access/transam/xlogarchive.c | 28 +-----------
 src/backend/archive/shell_archive.c      |  6 +--
 src/backend/postmaster/startup.c         | 54 +++++++++++++++++-------
 src/include/postmaster/startup.h         |  3 +-
 4 files changed, 43 insertions(+), 48 deletions(-)

diff --git a/src/backend/access/transam/xlogarchive.c b/src/backend/access/transam/xlogarchive.c
index 1ef1713c91a..59a0f191339 100644
--- a/src/backend/access/transam/xlogarchive.c
+++ b/src/backend/access/transam/xlogarchive.c
@@ -158,27 +158,7 @@ RestoreArchivedFile(char *path, const char *xlogfname,
 			(errmsg_internal("executing restore command \"%s\"",
 							 xlogRestoreCmd)));
 
-	fflush(NULL);
-	pgstat_report_wait_start(WAIT_EVENT_RESTORE_COMMAND);
-
-	/*
-	 * PreRestoreCommand() informs the SIGTERM handler for the startup process
-	 * that it should proc_exit() right away.  This is done for the duration
-	 * of the system() call because there isn't a good way to break out while
-	 * it is executing.  Since we might call proc_exit() in a signal handler,
-	 * it is best to put any additional logic before or after the
-	 * PreRestoreCommand()/PostRestoreCommand() section.
-	 */
-	PreRestoreCommand();
-
-	/*
-	 * Copy xlog from archival storage to XLOGDIR
-	 */
-	rc = system(xlogRestoreCmd);
-
-	PostRestoreCommand();
-
-	pgstat_report_wait_end();
+	rc = System(xlogRestoreCmd, WAIT_EVENT_RESTORE_COMMAND);
 	pfree(xlogRestoreCmd);
 
 	if (rc == 0)
@@ -325,11 +305,7 @@ ExecuteRecoveryCommand(const char *command, const char *commandName,
 	/*
 	 * execute the constructed command
 	 */
-	fflush(NULL);
-	pgstat_report_wait_start(wait_event_info);
-	rc = system(xlogRecoveryCmd);
-	pgstat_report_wait_end();
-
+	rc = System(xlogRecoveryCmd, wait_event_info);
 	pfree(xlogRecoveryCmd);
 
 	if (rc != 0)
diff --git a/src/backend/archive/shell_archive.c b/src/backend/archive/shell_archive.c
index 828723afe47..7977e5a5092 100644
--- a/src/backend/archive/shell_archive.c
+++ b/src/backend/archive/shell_archive.c
@@ -22,6 +22,7 @@
 #include "archive/shell_archive.h"
 #include "common/percentrepl.h"
 #include "pgstat.h"
+#include "postmaster/startup.h"
 
 static bool shell_archive_configured(ArchiveModuleState *state);
 static bool shell_archive_file(ArchiveModuleState *state,
@@ -75,10 +76,7 @@ shell_archive_file(ArchiveModuleState *state, const char *file,
 			(errmsg_internal("executing archive command \"%s\"",
 							 xlogarchcmd)));
 
-	fflush(NULL);
-	pgstat_report_wait_start(WAIT_EVENT_ARCHIVE_COMMAND);
-	rc = system(xlogarchcmd);
-	pgstat_report_wait_end();
+	rc = System(xlogarchcmd, WAIT_EVENT_ARCHIVE_COMMAND);
 
 	if (rc != 0)
 	{
diff --git a/src/backend/postmaster/startup.c b/src/backend/postmaster/startup.c
index 88eab3d0baf..b0cd40c081e 100644
--- a/src/backend/postmaster/startup.c
+++ b/src/backend/postmaster/startup.c
@@ -33,6 +33,7 @@
 #include "utils/guc.h"
 #include "utils/memutils.h"
 #include "utils/timeout.h"
+#include "utils/wait_event_types.h"
 
 
 #ifndef USE_POSTMASTER_DEATH_SIGNAL
@@ -264,24 +265,45 @@ StartupProcessMain(const void *startup_data, size_t startup_data_len)
 	proc_exit(0);
 }
 
-void
-PreRestoreCommand(void)
+/*
+ * A custom wrapper around the system() function that calls the necessary
+ * functions pre/post-fork.
+ */
+int
+System(const char *command, uint32 wait_event_info)
 {
-	/*
-	 * Set in_restore_command to tell the signal handler that we should exit
-	 * right away on SIGTERM. We know that we're at a safe point to do that.
-	 * Check if we had already received the signal, so that we don't miss a
-	 * shutdown request received just before this.
-	 */
-	in_restore_command = true;
-	if (shutdown_requested)
-		proc_exit(1);
-}
+	int			rc;
 
-void
-PostRestoreCommand(void)
-{
-	in_restore_command = false;
+	fflush(NULL);
+	pgstat_report_wait_start(wait_event_info);
+
+	if (wait_event_info == WAIT_EVENT_RESTORE_COMMAND)
+	{
+		/*
+		 * Set in_restore_command to tell the signal handler that we should
+		 * exit right away on SIGTERM. This is done for the duration of the
+		 * system() call because there isn't a good way to break out while it
+		 * is executing.  Since we might call proc_exit() in a signal handler,
+		 * it is best to put any additional logic outside of this section
+		 * where in_restore_command is set to true.
+		 */
+		in_restore_command = true;
+
+		/*
+		 * Also check if we had already received the signal, so that we don't
+		 * miss a shutdown request received just before this.
+		 */
+		if (shutdown_requested)
+			proc_exit(1);
+	}
+
+	rc = system(command);
+
+	if (wait_event_info == WAIT_EVENT_RESTORE_COMMAND)
+		in_restore_command = false;
+
+	pgstat_report_wait_end();
+	return rc;
 }
 
 bool
diff --git a/src/include/postmaster/startup.h b/src/include/postmaster/startup.h
index ae0f6347fc0..09f5dcd3d03 100644
--- a/src/include/postmaster/startup.h
+++ b/src/include/postmaster/startup.h
@@ -27,8 +27,7 @@ extern PGDLLIMPORT int log_startup_progress_interval;
 
 extern void HandleStartupProcInterrupts(void);
 extern void StartupProcessMain(const void *startup_data, size_t startup_data_len) pg_attribute_noreturn();
-extern void PreRestoreCommand(void);
-extern void PostRestoreCommand(void);
+extern int	System(const char *command, uint32 wait_event_info);
 extern bool IsPromoteSignaled(void);
 extern void ResetPromoteSignaled(void);
 

base-commit: 454c182f8542890d0e2eac85f70d9a254a34fce3
-- 
2.43.0