I wrote:
> Andres Freund <and...@anarazel.de> writes:
>> Only PM_CHILD_ACTIVE and PM_CHILD_WALSENDER though. We could afford another
>> MaxLivePostmasterChildren() sized array...
> Oh, I see what you mean --- one private and one public array.
> Maybe that makes more sense than what I did, not sure.
Yeah, that's definitely a better way. I'll push this after the
release freeze lifts.
regards, tom lane
diff --git a/src/backend/storage/ipc/pmsignal.c b/src/backend/storage/ipc/pmsignal.c
index 3f0ec5e6b8..c85521d364 100644
--- a/src/backend/storage/ipc/pmsignal.c
+++ b/src/backend/storage/ipc/pmsignal.c
@@ -26,6 +26,7 @@
#include "replication/walsender.h"
#include "storage/pmsignal.h"
#include "storage/shmem.h"
+#include "utils/memutils.h"
/*
@@ -75,12 +76,21 @@ struct PMSignalData
QuitSignalReason sigquit_reason; /* why SIGQUIT was sent */
/* per-child-process flags */
int num_child_flags; /* # of entries in PMChildFlags[] */
- int next_child_flag; /* next slot to try to assign */
sig_atomic_t PMChildFlags[FLEXIBLE_ARRAY_MEMBER];
};
+/* PMSignalState pointer is valid in both postmaster and child processes */
NON_EXEC_STATIC volatile PMSignalData *PMSignalState = NULL;
+/*
+ * These static variables are valid only in the postmaster. We keep a
+ * duplicative private array so that we can trust its state even if some
+ * failing child has clobbered the PMSignalData struct in shared memory.
+ */
+static int num_child_inuse; /* # of entries in PMChildInUse[] */
+static int next_child_inuse; /* next slot to try to assign */
+static bool *PMChildInUse; /* true if i'th flag slot is assigned */
+
/*
* Signal handler to be notified if postmaster dies.
*/
@@ -142,7 +152,25 @@ PMSignalShmemInit(void)
{
/* initialize all flags to zeroes */
MemSet(unvolatize(PMSignalData *, PMSignalState), 0, PMSignalShmemSize());
- PMSignalState->num_child_flags = MaxLivePostmasterChildren();
+ num_child_inuse = MaxLivePostmasterChildren();
+ PMSignalState->num_child_flags = num_child_inuse;
+
+ /*
+ * Also allocate postmaster's private PMChildInUse[] array. We
+ * might've already done that in a previous shared-memory creation
+ * cycle, in which case free the old array to avoid a leak. (Do it
+ * like this to support the possibility that MaxLivePostmasterChildren
+ * changed.) In a standalone backend, we do not need this.
+ */
+ if (PostmasterContext != NULL)
+ {
+ if (PMChildInUse)
+ pfree(PMChildInUse);
+ PMChildInUse = (bool *)
+ MemoryContextAllocZero(PostmasterContext,
+ num_child_inuse * sizeof(bool));
+ }
+ next_child_inuse = 0;
}
}
@@ -218,21 +246,24 @@ GetQuitSignalReason(void)
int
AssignPostmasterChildSlot(void)
{
- int slot = PMSignalState->next_child_flag;
+ int slot = next_child_inuse;
int n;
/*
- * Scan for a free slot. We track the last slot assigned so as not to
- * waste time repeatedly rescanning low-numbered slots.
+ * Scan for a free slot. Notice that we trust nothing about the contents
+ * of PMSignalState, but use only postmaster-local data for this decision.
+ * We track the last slot assigned so as not to waste time repeatedly
+ * rescanning low-numbered slots.
*/
- for (n = PMSignalState->num_child_flags; n > 0; n--)
+ for (n = num_child_inuse; n > 0; n--)
{
if (--slot < 0)
- slot = PMSignalState->num_child_flags - 1;
- if (PMSignalState->PMChildFlags[slot] == PM_CHILD_UNUSED)
+ slot = num_child_inuse - 1;
+ if (!PMChildInUse[slot])
{
+ PMChildInUse[slot] = true;
PMSignalState->PMChildFlags[slot] = PM_CHILD_ASSIGNED;
- PMSignalState->next_child_flag = slot;
+ next_child_inuse = slot;
return slot + 1;
}
}
@@ -254,7 +285,7 @@ ReleasePostmasterChildSlot(int slot)
{
bool result;
- Assert(slot > 0 && slot <= PMSignalState->num_child_flags);
+ Assert(slot > 0 && slot <= num_child_inuse);
slot--;
/*
@@ -264,17 +295,18 @@ ReleasePostmasterChildSlot(int slot)
*/
result = (PMSignalState->PMChildFlags[slot] == PM_CHILD_ASSIGNED);
PMSignalState->PMChildFlags[slot] = PM_CHILD_UNUSED;
+ PMChildInUse[slot] = false;
return result;
}
/*
* IsPostmasterChildWalSender - check if given slot is in use by a
- * walsender process.
+ * walsender process. This is called only by the postmaster.
*/
bool
IsPostmasterChildWalSender(int slot)
{
- Assert(slot > 0 && slot <= PMSignalState->num_child_flags);
+ Assert(slot > 0 && slot <= num_child_inuse);
slot--;
if (PMSignalState->PMChildFlags[slot] == PM_CHILD_WALSENDER)
