v2-0002-Refactor-CreateSharedMemoryAndSemaphores.patch

From 03f53ab9208240f45b7ec90bc435c81fe1140671 Mon Sep 17 00:00:00 2001
From: Heikki Linnakangas <heikki.linnakangas@iki.fi>
Date: Wed, 11 Oct 2023 13:36:16 +0300
Subject: [PATCH v2 2/6] Refactor CreateSharedMemoryAndSemaphores.

The order of process initialization steps is now more consistent
between !EXEC_BACKEND and EXEC_BACKEND modes. InitProcess() is called
at the same place in either mode. In EXEC_BACKEND mode, there's an
extra AttachSharedMemoryAndSemaphores() call, which is a no-op in
!EXEC_BACKEND mode. This reduces the number of "#ifdef EXEC_BACKEND"
blocks.

For clarity, have separate functions for *creating* the shared memory
and semaphores, at postmaster or single-user backend startup, and
for *attaching* to existing shared memory structures in EXEC_BACKEND
case.

I find it pretty confusing in all the *ShmemInit() functions too that
they are called in two different contexts: in postmaster when creating
the shmem structs, and in the child process in EXEC_BACKEND when
attaching to the already existing structs. But this commit doesn't
change that.

Reviewed-by: Tristan Partin, Andres Freund
Discussion: https://www.postgresql.org/message-id/7a59b073-5b5b-151e-7ed3-8b01ff7ce9ef@iki.fi
---
 src/backend/postmaster/autovacuum.c   | 22 ++++++------
 src/backend/postmaster/auxprocess.c   |  8 ++---
 src/backend/postmaster/bgworker.c     | 11 +++---
 src/backend/postmaster/postmaster.c   | 48 +++++----------------------
 src/backend/replication/walreceiver.c |  3 +-
 src/backend/storage/ipc/ipci.c        | 12 +++++++
 src/backend/storage/lmgr/proc.c       |  2 +-
 src/include/storage/ipc.h             |  1 +
 8 files changed, 44 insertions(+), 63 deletions(-)

diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index 327ea0d45ad..17f2b36b28a 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -476,14 +476,13 @@ AutoVacLauncherMain(int argc, char *argv[])
 	pqsignal(SIGCHLD, SIG_DFL);
 
 	/*
-	 * Create a per-backend PGPROC struct in shared memory, except in the
-	 * EXEC_BACKEND case where this was done in SubPostmasterMain. We must do
-	 * this before we can use LWLocks (and in the EXEC_BACKEND case we already
-	 * had to do some stuff with LWLocks).
+	 * Create a per-backend PGPROC struct in shared memory.  We must do this
+	 * before we can use LWLocks.
 	 */
-#ifndef EXEC_BACKEND
 	InitProcess();
-#endif
+
+	/* Attach process to shared data structures */
+	AttachSharedMemoryAndSemaphores();
 
 	/* Early initialization */
 	BaseInit();
@@ -1548,14 +1547,13 @@ AutoVacWorkerMain(int argc, char *argv[])
 	pqsignal(SIGCHLD, SIG_DFL);
 
 	/*
-	 * Create a per-backend PGPROC struct in shared memory, except in the
-	 * EXEC_BACKEND case where this was done in SubPostmasterMain. We must do
-	 * this before we can use LWLocks (and in the EXEC_BACKEND case we already
-	 * had to do some stuff with LWLocks).
+	 * Create a per-backend PGPROC struct in shared memory.  We must do this
+	 * before we can use LWLocks.
 	 */
-#ifndef EXEC_BACKEND
 	InitProcess();
-#endif
+
+	/* Attach process to shared data structures */
+	AttachSharedMemoryAndSemaphores();
 
 	/* Early initialization */
 	BaseInit();
diff --git a/src/backend/postmaster/auxprocess.c b/src/backend/postmaster/auxprocess.c
index cae6feb3562..536d9a2b3e4 100644
--- a/src/backend/postmaster/auxprocess.c
+++ b/src/backend/postmaster/auxprocess.c
@@ -97,12 +97,12 @@ AuxiliaryProcessMain(AuxProcType auxtype)
 	 */
 
 	/*
-	 * Create a PGPROC so we can use LWLocks.  In the EXEC_BACKEND case, this
-	 * was already done by SubPostmasterMain().
+	 * Create a PGPROC so we can use LWLocks.
 	 */
-#ifndef EXEC_BACKEND
 	InitAuxiliaryProcess();
-#endif
+
+	/* Attach process to shared data structures */
+	AttachSharedMemoryAndSemaphores();
 
 	BaseInit();
 
diff --git a/src/backend/postmaster/bgworker.c b/src/backend/postmaster/bgworker.c
index b3c2e65ba9f..15435965bf8 100644
--- a/src/backend/postmaster/bgworker.c
+++ b/src/backend/postmaster/bgworker.c
@@ -810,14 +810,13 @@ BackgroundWorkerMain(void)
 	PG_exception_stack = &local_sigjmp_buf;
 
 	/*
-	 * Create a per-backend PGPROC struct in shared memory, except in the
-	 * EXEC_BACKEND case where this was done in SubPostmasterMain. We must do
-	 * this before we can use LWLocks (and in the EXEC_BACKEND case we already
-	 * had to do some stuff with LWLocks).
+	 * Create a per-backend PGPROC struct in shared memory.  We must do this
+	 * before we can use LWLocks.
 	 */
-#ifndef EXEC_BACKEND
 	InitProcess();
-#endif
+
+	/* Attach process to shared data structures */
+	AttachSharedMemoryAndSemaphores();
 
 	/*
 	 * Early initialization.
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index e55f3a152aa..13ebf7ac4de 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -4097,15 +4097,6 @@ BackendStartup(Port *port)
 		/* Perform additional initialization and collect startup packet */
 		BackendInitialize(port);
 
-		/*
-		 * Create a per-backend PGPROC struct in shared memory. We must do
-		 * this before we can use LWLocks. In the !EXEC_BACKEND case (here)
-		 * this could be delayed a bit further, but EXEC_BACKEND needs to do
-		 * stuff with LWLocks before PostgresMain(), so we do it here as well
-		 * for symmetry.
-		 */
-		InitProcess();
-
 		/* And run the backend */
 		BackendRun(port);
 	}
@@ -4416,6 +4407,15 @@ BackendInitialize(Port *port)
 static void
 BackendRun(Port *port)
 {
+	/*
+	 * Create a per-backend PGPROC struct in shared memory.  We must do this
+	 * before we can use LWLocks (in AttachSharedMemoryAndSemaphores).
+	 */
+	InitProcess();
+
+	/* Attach process to shared data structures */
+	AttachSharedMemoryAndSemaphores();
+
 	/*
 	 * Make sure we aren't in PostmasterContext anymore.  (We can't delete it
 	 * just yet, though, because InitPostgres will need the HBA data.)
@@ -4911,12 +4911,6 @@ SubPostmasterMain(int argc, char *argv[])
 		/* Restore basic shared memory pointers */
 		InitShmemAccess(UsedShmemSegAddr);
 
-		/* Need a PGPROC to run CreateSharedMemoryAndSemaphores */
-		InitProcess();
-
-		/* Attach process to shared data structures */
-		CreateSharedMemoryAndSemaphores();
-
 		/* And run the backend */
 		BackendRun(&port);		/* does not return */
 	}
@@ -4929,12 +4923,6 @@ SubPostmasterMain(int argc, char *argv[])
 		/* Restore basic shared memory pointers */
 		InitShmemAccess(UsedShmemSegAddr);
 
-		/* Need a PGPROC to run CreateSharedMemoryAndSemaphores */
-		InitAuxiliaryProcess();
-
-		/* Attach process to shared data structures */
-		CreateSharedMemoryAndSemaphores();
-
 		auxtype = atoi(argv[3]);
 		AuxiliaryProcessMain(auxtype);	/* does not return */
 	}
@@ -4943,12 +4931,6 @@ SubPostmasterMain(int argc, char *argv[])
 		/* Restore basic shared memory pointers */
 		InitShmemAccess(UsedShmemSegAddr);
 
-		/* Need a PGPROC to run CreateSharedMemoryAndSemaphores */
-		InitProcess();
-
-		/* Attach process to shared data structures */
-		CreateSharedMemoryAndSemaphores();
-
 		AutoVacLauncherMain(argc - 2, argv + 2);	/* does not return */
 	}
 	if (strcmp(argv[1], "--forkavworker") == 0)
@@ -4956,12 +4938,6 @@ SubPostmasterMain(int argc, char *argv[])
 		/* Restore basic shared memory pointers */
 		InitShmemAccess(UsedShmemSegAddr);
 
-		/* Need a PGPROC to run CreateSharedMemoryAndSemaphores */
-		InitProcess();
-
-		/* Attach process to shared data structures */
-		CreateSharedMemoryAndSemaphores();
-
 		AutoVacWorkerMain(argc - 2, argv + 2);	/* does not return */
 	}
 	if (strncmp(argv[1], "--forkbgworker", 14) == 0)
@@ -4972,12 +4948,6 @@ SubPostmasterMain(int argc, char *argv[])
 		/* Restore basic shared memory pointers */
 		InitShmemAccess(UsedShmemSegAddr);
 
-		/* Need a PGPROC to run CreateSharedMemoryAndSemaphores */
-		InitProcess();
-
-		/* Attach process to shared data structures */
-		CreateSharedMemoryAndSemaphores();
-
 		StartBackgroundWorker();
 	}
 	if (strcmp(argv[1], "--forklog") == 0)
diff --git a/src/backend/replication/walreceiver.c b/src/backend/replication/walreceiver.c
index feff7094351..073668d4f2e 100644
--- a/src/backend/replication/walreceiver.c
+++ b/src/backend/replication/walreceiver.c
@@ -194,7 +194,7 @@ WalReceiverMain(void)
 	TimeLineID	startpointTLI;
 	TimeLineID	primaryTLI;
 	bool		first_stream;
-	WalRcvData *walrcv = WalRcv;
+	WalRcvData *walrcv;
 	TimestampTz now;
 	char	   *err;
 	char	   *sender_host = NULL;
@@ -204,6 +204,7 @@ WalReceiverMain(void)
 	 * WalRcv should be set up already (if we are a backend, we inherit this
 	 * by fork() or EXEC_BACKEND mechanism from the postmaster).
 	 */
+	walrcv = WalRcv;
 	Assert(walrcv != NULL);
 
 	/*
diff --git a/src/backend/storage/ipc/ipci.c b/src/backend/storage/ipc/ipci.c
index a3d8eacb8dc..4f31ceca073 100644
--- a/src/backend/storage/ipc/ipci.c
+++ b/src/backend/storage/ipc/ipci.c
@@ -171,6 +171,18 @@ CalculateShmemSize(int *num_semaphores)
  * check IsUnderPostmaster, rather than EXEC_BACKEND, to detect this case.
  * This is a bit code-wasteful and could be cleaned up.)
  */
+void
+AttachSharedMemoryAndSemaphores(void)
+{
+	/* InitProcess must've been called already */
+	Assert(MyProc != NULL);
+
+	/* Init !EXEC_BACKEND mode, we inherited everything through the fork */
+#ifdef EXEC_BACKEND
+	CreateSharedMemoryAndSemaphores();
+#endif
+}
+
 void
 CreateSharedMemoryAndSemaphores(void)
 {
diff --git a/src/backend/storage/lmgr/proc.c b/src/backend/storage/lmgr/proc.c
index 5b663a2997c..c1d259b1405 100644
--- a/src/backend/storage/lmgr/proc.c
+++ b/src/backend/storage/lmgr/proc.c
@@ -468,7 +468,7 @@ InitProcess(void)
  *
  * This is separate from InitProcess because we can't acquire LWLocks until
  * we've created a PGPROC, but in the EXEC_BACKEND case ProcArrayAdd won't
- * work until after we've done CreateSharedMemoryAndSemaphores.
+ * work until after we've done AttachSharedMemoryAndSemaphores.
  */
 void
 InitProcessPhase2(void)
diff --git a/src/include/storage/ipc.h b/src/include/storage/ipc.h
index 888c08b3067..e75656f5242 100644
--- a/src/include/storage/ipc.h
+++ b/src/include/storage/ipc.h
@@ -79,6 +79,7 @@ extern PGDLLIMPORT shmem_startup_hook_type shmem_startup_hook;
 
 extern Size CalculateShmemSize(int *num_semaphores);
 extern void CreateSharedMemoryAndSemaphores(void);
+extern void AttachSharedMemoryAndSemaphores(void);
 extern void InitializeShmemGUCs(void);
 
 #endif							/* IPC_H */
-- 
2.39.2