From eeeb782e60082327963d2f1742240d4a893eb9db Mon Sep 17 00:00:00 2001
From: Heikki Linnakangas <heikki.linnakangas@iki.fi>
Date: Tue, 31 Mar 2009 05:18:33 +0000
Subject: [PATCH] Fix a rare race condition when commit_siblings > 0 and a
 transaction commits at the same instant as a new backend is spawned. Since
 CountActiveBackends() doesn't hold ProcArrayLock, it needs to be prepared for
 the case that a pointer at the end of the proc array is still NULL even
 though numProcs says it should be valid, since it doesn't hold ProcArrayLock.
 Backpatch to 8.1. 8.0 and earlier had this right, but it was broken in the
 split of PGPROC and sinval shared memory arrays.

Per report and proposal by Marko Kreen.
---
 src/backend/storage/ipc/procarray.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/src/backend/storage/ipc/procarray.c b/src/backend/storage/ipc/procarray.c
index 0c0d448bddb..8acd2014a0d 100644
--- a/src/backend/storage/ipc/procarray.c
+++ b/src/backend/storage/ipc/procarray.c
@@ -23,7 +23,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/storage/ipc/procarray.c,v 1.47 2009/01/01 17:23:47 momjian Exp $
+ *	  $PostgreSQL: pgsql/src/backend/storage/ipc/procarray.c,v 1.48 2009/03/31 05:18:33 heikki Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -201,6 +201,7 @@ ProcArrayRemove(PGPROC *proc, TransactionId latestXid)
 		if (arrayP->procs[index] == proc)
 		{
 			arrayP->procs[index] = arrayP->procs[arrayP->numProcs - 1];
+			arrayP->procs[arrayP->numProcs - 1] = NULL; /* for debugging */
 			arrayP->numProcs--;
 			LWLockRelease(ProcArrayLock);
 			return;
@@ -1108,6 +1109,20 @@ CountActiveBackends(void)
 	{
 		volatile PGPROC *proc = arrayP->procs[index];
 
+		/*
+		 * Since we're not holding a lock, need to check that the pointer is
+		 * valid. Someone holding the lock could have incremented numProcs
+		 * already, but not yet inserted a valid pointer to the array.
+		 *
+		 * If someone just decremented numProcs, 'proc' could also point to a
+		 * PGPROC entry that's no longer in the array. It still points to a
+		 * PGPROC struct, though, because freed PGPPROC entries just go to
+		 * the free list and are recycled. Its contents are nonsense in that
+		 * case, but that's acceptable for this function.
+		 */
+		if (proc != NULL)
+			continue;
+
 		if (proc == MyProc)
 			continue;			/* do not count myself */
 		if (proc->pid == 0)
-- 
GitLab