Use an shmem_exit callback to remove backend from PMChildFlags on exit

This seems nicer than having to duplicate the logic between
InitProcess() and ProcKill() for which child processes have a
PMChildFlags slot.

Move the MarkPostmasterChildActive() call earlier in InitProcess(),
out of the section protected by the spinlock.

Reviewed-by: Andres Freund <[email protected]>
Discussion: https://www.postgresql.org/message-id/a102f15f-eac4-4ff2-af02-f9ff209ec66f@iki.fi

diff --git a/src/backend/storage/ipc/pmsignal.c b/src/backend/storage/ipc/pmsignal.c
index 27844b46a2b1ec8bf56436684b147d635693edc3..c801e9bec51eb624cb279cf0e278cc78af4d7797 100644 (file)
--- a/src/backend/storage/ipc/pmsignal.c
+++ b/src/backend/storage/ipc/pmsignal.c
@@ -24,6 +24,7 @@
 #include "miscadmin.h"
 #include "postmaster/postmaster.h"
 #include "replication/walsender.h"
+#include "storage/ipc.h"
 #include "storage/pmsignal.h"
 #include "storage/shmem.h"
 #include "utils/memutils.h"
@@ -121,6 +122,8 @@ postmaster_death_handler(SIGNAL_ARGS)
 
 #endif                         /* USE_POSTMASTER_DEATH_SIGNAL */
 
+static void MarkPostmasterChildInactive(int code, Datum arg);
+
 /*
  * PMSignalShmemSize
  *     Compute space needed for pmsignal.c's shared memory
@@ -316,11 +319,14 @@ IsPostmasterChildWalSender(int slot)
 }
 
 /*
- * MarkPostmasterChildActive - mark a postmaster child as about to begin
+ * RegisterPostmasterChildActive - mark a postmaster child as about to begin
  * actively using shared memory.  This is called in the child process.
+ *
+ * This register an shmem exit hook to mark us as inactive again when the
+ * process exits normally.
  */
 void
-MarkPostmasterChildActive(void)
+RegisterPostmasterChildActive(void)
 {
    int         slot = MyPMChildSlot;
 
@@ -328,6 +334,9 @@ MarkPostmasterChildActive(void)
    slot--;
    Assert(PMSignalState->PMChildFlags[slot] == PM_CHILD_ASSIGNED);
    PMSignalState->PMChildFlags[slot] = PM_CHILD_ACTIVE;
+
+   /* Arrange to clean up at exit. */
+   on_shmem_exit(MarkPostmasterChildInactive, 0);
 }
 
 /*
@@ -352,8 +361,8 @@ MarkPostmasterChildWalSender(void)
  * MarkPostmasterChildInactive - mark a postmaster child as done using
  * shared memory.  This is called in the child process.
  */
-void
-MarkPostmasterChildInactive(void)
+static void
+MarkPostmasterChildInactive(int code, Datum arg)
 {
    int         slot = MyPMChildSlot;
 

diff --git a/src/backend/storage/lmgr/proc.c b/src/backend/storage/lmgr/proc.c
index 0d8162a2cca3b45b8f800121830075c484627236..eaf3916f282305f7dde80c19e7e2fafc0bdfd1a8 100644 (file)
--- a/src/backend/storage/lmgr/proc.c
+++ b/src/backend/storage/lmgr/proc.c
@@ -354,6 +354,19 @@ InitProcess(void)
    if (MyProc != NULL)
        elog(ERROR, "you already exist");
 
+   /*
+    * Before we start accessing the shared memory in a serious way, mark
+    * ourselves as an active postmaster child; this is so that the postmaster
+    * can detect it if we exit without cleaning up.  (XXX autovac launcher
+    * currently doesn't participate in this; it probably should.)
+    *
+    * Slot sync worker also does not participate in it, see comments atop
+    * 'struct bkend' in postmaster.c.
+    */
+   if (IsUnderPostmaster && !AmAutoVacuumLauncherProcess() &&
+       !AmLogicalSlotSyncWorkerProcess())
+       RegisterPostmasterChildActive();
+
    /* Decide which list should supply our PGPROC. */
    if (AmAutoVacuumLauncherProcess() || AmAutoVacuumWorkerProcess())
        procgloballist = &ProcGlobal->autovacFreeProcs;
@@ -406,19 +419,6 @@ InitProcess(void)
     */
    Assert(MyProc->procgloballist == procgloballist);
 
-   /*
-    * Now that we have a PGPROC, mark ourselves as an active postmaster
-    * child; this is so that the postmaster can detect it if we exit without
-    * cleaning up.  (XXX autovac launcher currently doesn't participate in
-    * this; it probably should.)
-    *
-    * Slot sync worker also does not participate in it, see comments atop
-    * 'struct bkend' in postmaster.c.
-    */
-   if (IsUnderPostmaster && !AmAutoVacuumLauncherProcess() &&
-       !AmLogicalSlotSyncWorkerProcess())
-       MarkPostmasterChildActive();
-
    /*
     * Initialize all fields of MyProc, except for those previously
     * initialized by InitProcGlobal.
@@ -993,18 +993,6 @@ ProcKill(int code, Datum arg)
 
    SpinLockRelease(ProcStructLock);
 
-   /*
-    * This process is no longer present in shared memory in any meaningful
-    * way, so tell the postmaster we've cleaned up acceptably well. (XXX
-    * autovac launcher should be included here someday)
-    *
-    * Slot sync worker is also not a postmaster child, so skip this shared
-    * memory related processing here.
-    */
-   if (IsUnderPostmaster && !AmAutoVacuumLauncherProcess() &&
-       !AmLogicalSlotSyncWorkerProcess())
-       MarkPostmasterChildInactive();
-
    /* wake autovac launcher if needed -- see comments in FreeWorkerInfo */
    if (AutovacuumLauncherPid != 0)
        kill(AutovacuumLauncherPid, SIGUSR2);

diff --git a/src/include/storage/pmsignal.h b/src/include/storage/pmsignal.h
index e5be0f121c242d793a00e0c358f91c8c2a709202..ce4620af1f31930fdc8d9c59ae803a77dfbe555e 100644 (file)
--- a/src/include/storage/pmsignal.h
+++ b/src/include/storage/pmsignal.h
@@ -73,8 +73,7 @@ extern QuitSignalReason GetQuitSignalReason(void);
 extern int AssignPostmasterChildSlot(void);
 extern bool ReleasePostmasterChildSlot(int slot);
 extern bool IsPostmasterChildWalSender(int slot);
-extern void MarkPostmasterChildActive(void);
-extern void MarkPostmasterChildInactive(void);
+extern void RegisterPostmasterChildActive(void);
 extern void MarkPostmasterChildWalSender(void);
 extern bool PostmasterIsAliveInternal(void);
 extern void PostmasterDeathSignalInit(void);