Commit eeeb782e authored by Heikki Linnakangas's avatar Heikki Linnakangas

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.
parent db20729e
...@@ -23,7 +23,7 @@ ...@@ -23,7 +23,7 @@
* *
* *
* IDENTIFICATION * 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) ...@@ -201,6 +201,7 @@ ProcArrayRemove(PGPROC *proc, TransactionId latestXid)
if (arrayP->procs[index] == proc) if (arrayP->procs[index] == proc)
{ {
arrayP->procs[index] = arrayP->procs[arrayP->numProcs - 1]; arrayP->procs[index] = arrayP->procs[arrayP->numProcs - 1];
arrayP->procs[arrayP->numProcs - 1] = NULL; /* for debugging */
arrayP->numProcs--; arrayP->numProcs--;
LWLockRelease(ProcArrayLock); LWLockRelease(ProcArrayLock);
return; return;
...@@ -1108,6 +1109,20 @@ CountActiveBackends(void) ...@@ -1108,6 +1109,20 @@ CountActiveBackends(void)
{ {
volatile PGPROC *proc = arrayP->procs[index]; 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) if (proc == MyProc)
continue; /* do not count myself */ continue; /* do not count myself */
if (proc->pid == 0) if (proc->pid == 0)
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment