aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorRobert Haas <rhaas@postgresql.org>2014-01-31 21:31:08 -0500
committerRobert Haas <rhaas@postgresql.org>2014-01-31 21:36:23 -0500
commit8e6bfc9ebfd0e2cbc9a03b6bcbac0e878e94e880 (patch)
treec8928052df113817a60b74cde493552c6a4cdfa0
parentaf259c691f13bafe4977f61adf10d81c7ece6c69 (diff)
downloadpostgresql-8e6bfc9ebfd0e2cbc9a03b6bcbac0e878e94e880.tar.gz
postgresql-8e6bfc9ebfd0e2cbc9a03b6bcbac0e878e94e880.zip
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.
-rw-r--r--src/backend/storage/ipc/procsignal.c7
-rw-r--r--src/backend/storage/lmgr/proc.c38
2 files changed, 30 insertions, 15 deletions
diff --git a/src/backend/storage/ipc/procsignal.c b/src/backend/storage/ipc/procsignal.c
index 28bcaa7e1ea..f4a3133e12c 100644
--- a/src/backend/storage/ipc/procsignal.c
+++ b/src/backend/storage/ipc/procsignal.c
@@ -143,6 +143,13 @@ CleanupProcSignalState(int status, Datum arg)
slot = &ProcSignalSlots[pss_idx - 1];
Assert(slot == MyProcSignalSlot);
+ /*
+ * Clear MyProcSignalSlot, so that a SIGUSR1 received after this point
+ * won't try to access it after it's no longer ours (and perhaps even
+ * after we've unmapped the shared memory segment).
+ */
+ MyProcSignalSlot = NULL;
+
/* sanity check */
if (slot->pss_pid != MyProcPid)
{
diff --git a/src/backend/storage/lmgr/proc.c b/src/backend/storage/lmgr/proc.c
index 1cabefcea2c..507ab09194d 100644
--- a/src/backend/storage/lmgr/proc.c
+++ b/src/backend/storage/lmgr/proc.c
@@ -701,6 +701,7 @@ ProcKill(int code, Datum arg)
{
/* use volatile pointer to prevent code rearrangement */
volatile PROC_HDR *procglobal = ProcGlobal;
+ PGPROC *proc;
Assert(MyProc != NULL);
@@ -714,26 +715,29 @@ ProcKill(int code, Datum arg)
*/
LWLockReleaseAll();
- /* Release ownership of the process's latch, too */
- DisownLatch(&MyProc->procLatch);
+ /*
+ * Clear MyProc first; then disown the process latch. This is so that
+ * signal handlers won't try to clear the process latch after it's no
+ * longer ours.
+ */
+ proc = MyProc;
+ MyProc = NULL;
+ DisownLatch(&proc->procLatch);
SpinLockAcquire(ProcStructLock);
/* Return PGPROC structure (and semaphore) to appropriate freelist */
if (IsAnyAutoVacuumProcess())
{
- 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);
@@ -762,6 +766,7 @@ AuxiliaryProcKill(int code, Datum arg)
{
int proctype = DatumGetInt32(arg);
PGPROC *auxproc;
+ PGPROC *proc;
Assert(proctype >= 0 && proctype < NUM_AUXILIARY_PROCS);
@@ -772,16 +777,19 @@ AuxiliaryProcKill(int code, Datum arg)
/* Release any LW locks I am holding (see notes above) */
LWLockReleaseAll();
- /* Release ownership of the process's latch, too */
- DisownLatch(&MyProc->procLatch);
+ /*
+ * Clear MyProc first; then disown the process latch. This is so that
+ * signal handlers won't try to clear the process latch after it's no
+ * longer ours.
+ */
+ proc = MyProc;
+ MyProc = NULL;
+ DisownLatch(&proc->procLatch);
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);