Check for too many postmaster children before spawning a bgworker.

The postmaster's code path for spawning a bgworker neglected to check
whether we already have the max number of live child processes.  That's
a bit hard to hit, since it would necessarily be a transient condition;
but if we do, AssignPostmasterChildSlot() fails causing a postmaster
crash, as seen in a report from Bhargav Kamineni.

To fix, invoke canAcceptConnections() in the bgworker code path, as we
do in the other code paths that spawn children.  Since we don't want
the same pmState tests in this case, add a child-process-type parameter
to canAcceptConnections() so that it can know what to do.

Back-patch to 9.5.  In principle the same hazard exists in 9.4, but the
code is enough different that this patch wouldn't quite fix it there.
Given the tiny usage of bgworkers in that branch it doesn't seem worth
creating a variant patch for it.

Discussion: https://postgr.es/m/18733.1570382257@sss.pgh.pa.us

diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index fad807dc4f321d81e77a541f761882fa2fc50bf3..17feb6bef94d569f5da1bc87d6b4ab7920e12d72 100644 (file)
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -407,7 +407,7 @@ static void SendNegotiateProtocolVersion(List *unrecognized_protocol_options);
 static void processCancelRequest(Port *port, void *pkt);
 static int     initMasks(fd_set *rmask);
 static void report_fork_failure_to_client(Port *port, int errnum);
-static CAC_state canAcceptConnections(void);
+static CAC_state canAcceptConnections(int backend_type);
 static long PostmasterRandom(void);
 static void RandomSalt(char *md5Salt);
 static void signal_child(pid_t pid, int signal);
@@ -2336,16 +2336,21 @@ processCancelRequest(Port *port, void *pkt)
 }
 
 /*
- * canAcceptConnections --- check to see if database state allows connections.
+ * canAcceptConnections --- check to see if database state allows connections
+ * of the specified type.  backend_type can be BACKEND_TYPE_NORMAL,
+ * BACKEND_TYPE_AUTOVAC, or BACKEND_TYPE_BGWORKER.  (Note that we don't yet
+ * know whether a NORMAL connection might turn into a walsender.)
  */
 static CAC_state
-canAcceptConnections(void)
+canAcceptConnections(int backend_type)
 {
        CAC_state       result = CAC_OK;
 
        /*
         * Can't start backends when in startup/shutdown/inconsistent recovery
-        * state.
+        * state.  We treat autovac workers the same as user backends for this
+        * purpose.  However, bgworkers are excluded from this test; we expect
+        * bgworker_should_start_now() decided whether the DB state allows them.
         *
         * In state PM_WAIT_BACKUP only superusers can connect (this must be
         * allowed so that a superuser can end online backup mode); we return
@@ -2353,7 +2358,8 @@ canAcceptConnections(void)
         * that neither CAC_OK nor CAC_WAITBACKUP can safely be returned until we
         * have checked for too many children.
         */
-       if (pmState != PM_RUN)
+       if (pmState != PM_RUN &&
+               backend_type != BACKEND_TYPE_BGWORKER)
        {
                if (pmState == PM_WAIT_BACKUP)
                        result = CAC_WAITBACKUP;        /* allow superusers only */
@@ -2373,9 +2379,9 @@ canAcceptConnections(void)
        /*
         * Don't start too many children.
         *
-        * We allow more connections than we can have backends here because some
+        * We allow more connections here than we can have backends because some
         * might still be authenticating; they might fail auth, or some existing
-        * backend might exit before the auth cycle is completed. The exact
+        * backend might exit before the auth cycle is completed.  The exact
         * MaxBackends limit is enforced when a new backend tries to join the
         * shared-inval backend array.
         *
@@ -3977,7 +3983,7 @@ BackendStartup(Port *port)
        bn->cancel_key = MyCancelKey;
 
        /* Pass down canAcceptConnections state */
-       port->canAcceptConnections = canAcceptConnections();
+       port->canAcceptConnections = canAcceptConnections(BACKEND_TYPE_NORMAL);
        bn->dead_end = (port->canAcceptConnections != CAC_OK &&
                                        port->canAcceptConnections != CAC_WAITBACKUP);
 
@@ -5378,7 +5384,7 @@ StartAutovacuumWorker(void)
         * we have to check to avoid race-condition problems during DB state
         * changes.
         */
-       if (canAcceptConnections() == CAC_OK)
+       if (canAcceptConnections(BACKEND_TYPE_AUTOVAC) == CAC_OK)
        {
                bn = (Backend *) malloc(sizeof(Backend));
                if (bn)
@@ -5616,12 +5622,13 @@ do_start_bgworker(RegisteredBgWorker *rw)
 
        /*
         * Allocate and assign the Backend element.  Note we must do this before
-        * forking, so that we can handle out of memory properly.
+        * forking, so that we can handle failures (out of memory or child-process
+        * slots) cleanly.
         *
         * Treat failure as though the worker had crashed.  That way, the
-        * postmaster will wait a bit before attempting to start it again; if it
-        * tried again right away, most likely it'd find itself repeating the
-        * out-of-memory or fork failure condition.
+        * postmaster will wait a bit before attempting to start it again; if we
+        * tried again right away, most likely we'd find ourselves hitting the
+        * same resource-exhaustion condition.
         */
        if (!assign_backendlist_entry(rw))
        {
@@ -5745,8 +5752,22 @@ bgworker_should_start_now(BgWorkerStartTime start_time)
 static bool
 assign_backendlist_entry(RegisteredBgWorker *rw)
 {
-       Backend    *bn = malloc(sizeof(Backend));
+       Backend    *bn;
+
+       /*
+        * Check that database state allows another connection.  Currently the
+        * only possible failure is CAC_TOOMANY, so we just log an error message
+        * based on that rather than checking the error code precisely.
+        */
+       if (canAcceptConnections(BACKEND_TYPE_BGWORKER) != CAC_OK)
+       {
+               ereport(LOG,
+                               (errcode(ERRCODE_CONFIGURATION_LIMIT_EXCEEDED),
+                                errmsg("no slot available for new worker process")));
+               return false;
+       }
 
+       bn = malloc(sizeof(Backend));
        if (bn == NULL)
        {
                ereport(LOG,