From: Robert Haas <rhaas@postgresql.org>
Date: Thu, 19 Mar 2015 14:56:34 +0000 (-0400)
Subject: Fix status reporting for terminated bgworkers that were never started.
X-Git-Tag: REL9_5_ALPHA1~599
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=bf740ce9e5d82612889d131f34c079215973ca00;p=postgresql

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 267b916327..cf7524f492 100644
--- 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;
 		}