Fix status reporting for terminated bgworkers that were never started.

Previously, GetBackgroundWorkerPid() would return BGWH_NOT_YET_STARTED
if the slot used for the worker registration had not been reused by
unrelated activity, and BGWH_STOPPED if it had.  Either way, a process
that had requested notification when the state of one of its
background workers changed did not receive such notifications.  Fix
things so that GetBackgroundWorkerPid() always returns BGWH_STOPPED in
this situation, so that we do not erroneously give waiters the
impression that the worker will eventually be started; and send
notifications just as we would if the process terminated after having
been started, so that it's possible to wait for the postmaster to
process a worker termination request without polling.

Discovered by Amit Kapila during testing of parallel sequential scan.
Analysis and fix by me.  Back-patch to 9.4; there may not be anyone
relying on this interface yet, but if anyone is, the new behavior is a
clear improvement.

diff --git a/src/backend/postmaster/bgworker.c b/src/backend/postmaster/bgworker.c
index 267b91632712b9e7cfd00bd2bc94858fbcce7495..cf7524f4921ef372b0370ddb3d5d811f3e761635 100644 (file)
--- a/src/backend/postmaster/bgworker.c
+++ b/src/backend/postmaster/bgworker.c
@@ -244,14 +244,37 @@ BackgroundWorkerStateChange(void)
                                rw->rw_terminate = true;
                                if (rw->rw_pid != 0)
                                        kill(rw->rw_pid, SIGTERM);
+                               else
+                               {
+                                       /* Report never-started, now-terminated worker as dead. */
+                                       ReportBackgroundWorkerPID(rw);
+                               }
                        }
                        continue;
                }
 
-               /* If it's already flagged as do not restart, just release the slot. */
+               /*
+                * If the worker is marked for termination, we don't need to add it
+                * to the registered workers list; we can just free the slot.
+                * However, if bgw_notify_pid is set, the process that registered the
+                * worker may need to know that we've processed the terminate request,
+                * so be sure to signal it.
+                */
                if (slot->terminate)
                {
+                       int     notify_pid;
+
+                       /*
+                        * We need a memory barrier here to make sure that the load of
+                        * bgw_notify_pid completes before the store to in_use.
+                        */
+                       notify_pid = slot->worker.bgw_notify_pid;
+                       pg_memory_barrier();
+                       slot->pid = 0;
                        slot->in_use = false;
+                       if (notify_pid != 0)
+                               kill(notify_pid, SIGUSR1);
+
                        continue;
                }