postgres.git / summary / log / commit / refs

commit    458b20f2da8482bbe8a53604365f38b7e8a606e9
Author:   Robert Haas <rhaas@postgresql.org>
Date:     Sat Feb 01 02:31:08 2014 +0000
First released in: 9.4.0, 8.4.20, 9.0.16, 9.1.12, 9.2.7, 9.3.3

    Clear MyProc and MyProcSignalState before they become invalid.
    
    Evidence from buildfarm member crake suggests that the new test_shm_mq
    module is routinely crashing the server due to the arrival of a SIGUSR1
    after the shared memory segment has been unmapped.  Although processes
    using the new dynamic background worker facilities are more likely to
    receive a SIGUSR1 around this time, the problem is also possible on older
    branches, so I'm back-patching the parts of this change that apply to
    older branches as far as they apply.
    
    It's already generally the case that code checks whether these pointers
    are NULL before deferencing them, so the important thing is mostly to
    make sure that they do get set to NULL before they become invalid.  But
    in master, there's one case in procsignal_sigusr1_handler that lacks a
    NULL guard, so add that.
    
    Patch by me; review by Tom Lane.


 src/backend/storage/lmgr/proc.c | 34 +++++++++++++++++++++++-----------
 1 file changed, 23 insertions(+), 11 deletions(-)

diff --git a/src/backend/storage/lmgr/proc.c b/src/backend/storage/lmgr/proc.c
index e62f76e03af..439bea29f1c 100644
--- a/src/backend/storage/lmgr/proc.c
+++ b/src/backend/storage/lmgr/proc.c
@@ -591,6 +591,7 @@ ProcKill(int code, Datum arg)
 {
 	/* use volatile pointer to prevent code rearrangement */
 	volatile PROC_HDR *procglobal = ProcGlobal;
+	PGPROC	   *proc;
 
 	Assert(MyProc != NULL);
 
@@ -601,23 +602,28 @@ ProcKill(int code, Datum arg)
 	 */
 	LWLockReleaseAll();
 
+	/*
+	 * Clear MyProc first before after putting it back on the global list,
+	 * so that signal handlers won't try to access it after it's no longer
+	 * ours.
+	 */
+	proc = MyProc;
+	MyProc = NULL;
+
 	SpinLockAcquire(ProcStructLock);
 
 	/* Return PGPROC structure (and semaphore) to freelist */
 	if (IsAutoVacuumWorkerProcess())
 	{
-		MyProc->links.next = (SHM_QUEUE *) procglobal->autovacFreeProcs;
-		procglobal->autovacFreeProcs = MyProc;
+		proc->links.next = (SHM_QUEUE *) procglobal->autovacFreeProcs;
+		procglobal->autovacFreeProcs = proc;
 	}
 	else
 	{
-		MyProc->links.next = (SHM_QUEUE *) procglobal->freeProcs;
-		procglobal->freeProcs = MyProc;
+		proc->links.next = (SHM_QUEUE *) procglobal->freeProcs;
+		procglobal->freeProcs = proc;
 	}
 
-	/* PGPROC struct isn't mine anymore */
-	MyProc = NULL;
-
 	/* Update shared estimate of spins_per_delay */
 	procglobal->spins_per_delay = update_spins_per_delay(procglobal->spins_per_delay);
 
@@ -645,6 +651,7 @@ AuxiliaryProcKill(int code, Datum arg)
 {
 	int			proctype = DatumGetInt32(arg);
 	PGPROC	   *auxproc;
+	PGPROC	   *proc;
 
 	Assert(proctype >= 0 && proctype < NUM_AUXILIARY_PROCS);
 
@@ -655,13 +662,18 @@ AuxiliaryProcKill(int code, Datum arg)
 	/* Release any LW locks I am holding (see notes above) */
 	LWLockReleaseAll();
 
+	/*
+	 * Clear MyProc first before after putting it back on the global list,
+	 * so that signal handlers won't try to access it after it's no longer
+	 * ours.
+	 */
+	proc = MyProc;
+	MyProc = NULL;
+
 	SpinLockAcquire(ProcStructLock);
 
 	/* Mark auxiliary proc no longer in use */
-	MyProc->pid = 0;
-
-	/* PGPROC struct isn't mine anymore */
-	MyProc = NULL;
+	proc->pid = 0;
 
 	/* Update shared estimate of spins_per_delay */
 	ProcGlobal->spins_per_delay = update_spins_per_delay(ProcGlobal->spins_per_delay);


[parent: 01b882fd8468]