Refactor EXEC_BACKEND code so that postmaster child processes reattach

to shared memory as soon as possible, ie, right after read_backend_variables.
The effective difference from the original code is that this happens
before instead of after read_nondefault_variables(), which loads GUC
information and is apparently capable of expanding the backend's memory
allocation more than you'd think it should.  This should fix the
failure-to-attach-to-shared-memory reports we've been seeing on Windows.
Also clean up a few bits of unnecessarily grotty EXEC_BACKEND code.

Author: tglsfdc

src/backend/port/sysv_shmem.c

@@ -10,7 +10,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/port/sysv_shmem.c,v 1.40 2004/11/09 21:30:13 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/port/sysv_shmem.c,v 1.41 2004/12/29 21:35:58 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -142,11 +142,7 @@ InternalIpcMemoryCreate(IpcMemoryKey memKey, uint32 size)
 	on_shmem_exit(IpcMemoryDelete, Int32GetDatum(shmid));
 
 	/* OK, should be able to attach to the segment */
-#ifdef EXEC_BACKEND
-	memAddress = shmat(shmid, UsedShmemSegAddr, PG_SHMAT_FLAGS);
-#else
 	memAddress = shmat(shmid, NULL, PG_SHMAT_FLAGS);
-#endif
 
 	if (memAddress == (void *) -1)
 		elog(FATAL, "shmat(id=%d) failed: %m", shmid);
@@ -279,7 +275,7 @@ PGSharedMemoryIsInUse(unsigned long id1, unsigned long id2)
  *
  * Create a shared memory segment of the given size and initialize its
  * standard header.  Also, register an on_shmem_exit callback to release
- * the storage.  For an exec'ed backend, it just attaches.
+ * the storage.
  *
  * Dead Postgres segments are recycled if found, but we do not fail upon
  * collision with non-Postgres shmem segments.	The idea here is to detect and
@@ -303,30 +299,6 @@ PGSharedMemoryCreate(uint32 size, bool makePrivate, int port)
 	struct stat statbuf;
 #endif
 
-#ifdef EXEC_BACKEND
-	/* If Exec case, just attach and return the pointer */
-	if (UsedShmemSegAddr != NULL && !makePrivate && IsUnderPostmaster)
-	{
-		void	   *origUsedShmemSegAddr = UsedShmemSegAddr;
-
-#ifdef __CYGWIN__
-		/* cygipc (currently) appears to not detach on exec. */
-		PGSharedMemoryDetach();
-		UsedShmemSegAddr = origUsedShmemSegAddr;
-#endif
-		elog(DEBUG3, "Attaching to %p", UsedShmemSegAddr);
-		hdr = PGSharedMemoryAttach((IpcMemoryKey) UsedShmemSegID, &shmid);
-		if (hdr == NULL)
-			elog(FATAL, "could not attach to proper memory at fixed address: shmget(key=%d, addr=%p) failed: %m",
-				 (int) UsedShmemSegID, UsedShmemSegAddr);
-		if (hdr != origUsedShmemSegAddr)
-			elog(FATAL, "attaching to shared mem returned unexpected address (got %p, expected %p)",
-				 hdr, UsedShmemSegAddr);
-		UsedShmemSegAddr = hdr;
-		return hdr;
-	}
-#endif
-
 	/* Room for a header? */
 	Assert(size > MAXALIGN(sizeof(PGShmemHeader)));
 
@@ -424,6 +396,49 @@ PGSharedMemoryCreate(uint32 size, bool makePrivate, int port)
 	return hdr;
 }
 
+#ifdef EXEC_BACKEND
+
+/*
+ * PGSharedMemoryReAttach
+ *
+ * Re-attach to an already existing shared memory segment.  In the non
+ * EXEC_BACKEND case this is not used, because postmaster children inherit
+ * the shared memory segment attachment via fork().
+ *
+ * UsedShmemSegID and UsedShmemSegAddr are implicit parameters to this
+ * routine.  The caller must have already restored them to the postmaster's
+ * values.
+ */
+void
+PGSharedMemoryReAttach(void)
+{
+	IpcMemoryId shmid;
+	void	   *hdr;
+	void	   *origUsedShmemSegAddr = UsedShmemSegAddr;
+
+	Assert(UsedShmemSegAddr != NULL);
+	Assert(IsUnderPostmaster);
+
+#ifdef __CYGWIN__
+	/* cygipc (currently) appears to not detach on exec. */
+	PGSharedMemoryDetach();
+	UsedShmemSegAddr = origUsedShmemSegAddr;
+#endif
+
+	elog(DEBUG3, "Attaching to %p", UsedShmemSegAddr);
+	hdr = (void *) PGSharedMemoryAttach((IpcMemoryKey) UsedShmemSegID, &shmid);
+	if (hdr == NULL)
+		elog(FATAL, "could not reattach to shared memory (key=%d, addr=%p): %m",
+			 (int) UsedShmemSegID, UsedShmemSegAddr);
+	if (hdr != origUsedShmemSegAddr)
+		elog(FATAL, "reattaching to shared memory returned unexpected address (got %p, expected %p)",
+			 hdr, origUsedShmemSegAddr);
+
+	UsedShmemSegAddr = hdr;		/* probably redundant */
+}
+
+#endif /* EXEC_BACKEND */
+
 /*
  * PGSharedMemoryDetach
  *

src/backend/postmaster/postmaster.c

@@ -37,7 +37,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/postmaster/postmaster.c,v 1.440 2004/11/17 08:30:09 neilc Exp $
+ *	  $PostgreSQL: pgsql/src/backend/postmaster/postmaster.c,v 1.441 2004/12/29 21:36:03 tgl Exp $
  *
  * NOTES
  *
@@ -1584,10 +1584,8 @@ processCancelRequest(Port *port, void *pkt)
 	int			backendPID;
 	long		cancelAuthCode;
 	Backend    *bp;
-
 #ifndef EXEC_BACKEND
 	Dlelem	   *curr;
-
 #else
 	int			i;
 #endif
@@ -3152,10 +3150,31 @@ SubPostmasterMain(int argc, char *argv[])
 
 	MyProcPid = getpid();		/* reset MyProcPid */
 
-	/* Read in file-based context */
+	/* In EXEC_BACKEND case we will not have inherited these settings */
+	IsPostmasterEnvironment = true;
+	whereToSendOutput = None;
+
+	/* Setup essential subsystems (to ensure elog() behaves sanely) */
+	MemoryContextInit();
+	InitializeGUCOptions();
+
+	/* Read in the variables file */
 	memset(&port, 0, sizeof(Port));
 	read_backend_variables(argv[2], &port);
 
+	/* Check we got appropriate args */
+	if (argc < 3)
+		elog(FATAL, "invalid subpostmaster invocation");
+
+	/*
+	 * If appropriate, physically re-attach to shared memory segment.
+	 * We want to do this before going any further to ensure that we
+	 * can attach at the same address the postmaster used.
+	 */
+	if (strcmp(argv[1], "-forkbackend") == 0 ||
+		strcmp(argv[1], "-forkboot") == 0)
+		PGSharedMemoryReAttach();
+
 	/*
 	 * Start our win32 signal implementation. This has to be done
 	 * after we read the backend variables, because we need to pick
@@ -3166,19 +3185,9 @@ SubPostmasterMain(int argc, char *argv[])
 #endif
 
 	/* In EXEC_BACKEND case we will not have inherited these settings */
-	IsPostmasterEnvironment = true;
-	whereToSendOutput = None;
 	pqinitmask();
 	PG_SETMASK(&BlockSig);
 
-	/* Setup essential subsystems */
-	MemoryContextInit();
-	InitializeGUCOptions();
-
-	/* Check we got appropriate args */
-	if (argc < 3)
-		elog(FATAL, "invalid subpostmaster invocation");
-
 	/* Read in remaining GUC variables */
 	read_nondefault_variables();
 
@@ -3187,7 +3196,7 @@ SubPostmasterMain(int argc, char *argv[])
 	{
 		/* BackendRun will close sockets */
 
-		/* Attach process to shared segments */
+		/* Attach process to shared data structures */
 		CreateSharedMemoryAndSemaphores(false, MaxBackends, 0);
 
 #ifdef USE_SSL
@@ -3208,7 +3217,7 @@ SubPostmasterMain(int argc, char *argv[])
 		/* Close the postmaster's sockets */
 		ClosePostmasterPorts(false);
 
-		/* Attach process to shared segments */
+		/* Attach process to shared data structures */
 		CreateSharedMemoryAndSemaphores(false, MaxBackends, 0);
 
 		BootstrapMain(argc - 2, argv + 2);
@@ -3259,6 +3268,7 @@ SubPostmasterMain(int argc, char *argv[])
 
 	return 1;					/* shouldn't get here */
 }
+
 #endif   /* EXEC_BACKEND */
 
 
@@ -3767,10 +3777,11 @@ read_inheritable_socket(SOCKET *dest, InheritableSocket *src)
 static void
 read_backend_variables(char *id, Port *port)
 {
+	BackendParameters param;
+
 #ifndef WIN32
 	/* Non-win32 implementation reads from file */
 	FILE *fp;
-	BackendParameters param;
 
 	/* Open file */
 	fp = AllocateFile(id, PG_BINARY_R);
@@ -3796,25 +3807,23 @@ read_backend_variables(char *id, Port *port)
 					 id, strerror(errno));
 		exit(1);
 	}
-
-	restore_backend_variables(&param, port);
 #else
 	/* Win32 version uses mapped file */
 	HANDLE paramHandle;
-	BackendParameters *param;
+	BackendParameters *paramp;
 
 	paramHandle = (HANDLE)atol(id);
-	param = MapViewOfFile(paramHandle, FILE_MAP_READ, 0, 0, 0);
-	if (!param)
+	paramp = MapViewOfFile(paramHandle, FILE_MAP_READ, 0, 0, 0);
+	if (!paramp)
 	{
 		write_stderr("could not map view of backend variables: error code %d\n",
 					 (int) GetLastError());
 		exit(1);
 	}
 
-	restore_backend_variables(param, port);
+	memcpy(&param, paramp, sizeof(BackendParameters));
 
-	if (!UnmapViewOfFile(param))
+	if (!UnmapViewOfFile(paramp))
 	{
 		write_stderr("could not unmap view of backend variables: error code %d\n",
 					 (int) GetLastError());
@@ -3828,6 +3837,8 @@ read_backend_variables(char *id, Port *port)
 		exit(1);
 	}
 #endif
+
+	restore_backend_variables(&param, port);
 }
 
 /* Restore critical backend variables from the BackendParameters struct */
@@ -3930,6 +3941,7 @@ ShmemBackendArrayRemove(pid_t pid)
 			(errmsg_internal("could not find backend entry with pid %d",
 							 (int) pid)));
 }
+
 #endif   /* EXEC_BACKEND */