database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2025-11-10 17:42:29 +03:00
Code Activity

Add more critical-section calls: all code sections that hold spinlocks

Browse Source 
are now critical sections, so as to ensure die() won't interrupt us while
we are munging shared-memory data structures.  Avoid insecure intermediate
states in some code that proc_exit will call, like palloc/pfree.  Rename
START/END_CRIT_CODE to START/END_CRIT_SECTION, since that seems to be
what people tend to call them anyway, and make them be called with () like
a function call, in hopes of not confusing pg_indent.
I doubt that this is sufficient to make SIGTERM safe anywhere; there's
just too much code that could get invoked during proc_exit().
This commit is contained in:
Tom Lane
2001-01-12 21:54:01 +00:00
parent be8477bc37
commit 6162432de9
17 changed files with 163 additions and 129 deletions
Download Patch File Download Diff File Expand all files Collapse all files

45
src/backend/storage/buffer/bufmgr.c
View File

@@ -8,7 +8,7 @@
*
*
* IDENTIFICATION
* $Header: /cvsroot/pgsql/src/backend/storage/buffer/bufmgr.c,v 1.102 2001/01/08 18:31:49 tgl Exp $
* $Header: /cvsroot/pgsql/src/backend/storage/buffer/bufmgr.c,v 1.103 2001/01/12 21:53:57 tgl Exp $
*
*-------------------------------------------------------------------------
*/
@@ -404,8 +404,7 @@ BufferAlloc(Relation reln,
*/
if ((buf->flags & BM_IO_ERROR) != 0)
{
PrivateRefCount[BufferDescriptorGetBuffer(buf) - 1] = 0;
buf->refcount--;
UnpinBuffer(buf);
buf = (BufferDesc *) NULL;
continue;
}
@@ -869,8 +868,10 @@ WaitIO(BufferDesc *buf, SPINLOCK spinlock)
while ((buf->flags & BM_IO_IN_PROGRESS) != 0)
{
SpinRelease(spinlock);
START_CRIT_SECTION(); /* don't want to die() holding the lock... */
S_LOCK(&(buf->io_in_progress_lock));
S_UNLOCK(&(buf->io_in_progress_lock));
END_CRIT_SECTION();
SpinAcquire(spinlock);
}
}
@@ -921,14 +922,11 @@ ResetBufferUsage()
* ResetBufferPool
*
* This routine is supposed to be called when a transaction aborts.
* it will release all the buffer pins held by the transaction.
* It will release all the buffer pins held by the transaction.
* Currently, we also call it during commit if BufferPoolCheckLeak
* detected a problem --- in that case, isCommit is TRUE, and we
* only clean up buffer pin counts.
*
* During abort, we also forget any pending fsync requests. Dirtied buffers
* will still get written, eventually, but there will be no fsync for them.
*
* ----------------------------------------------
*/
void
@@ -943,6 +941,7 @@ ResetBufferPool(bool isCommit)
BufferDesc *buf = &BufferDescriptors[i];
SpinAcquire(BufMgrLock);
PrivateRefCount[i] = 0;
Assert(buf->refcount > 0);
buf->refcount--;
if (buf->refcount == 0)
@@ -952,7 +951,6 @@ ResetBufferPool(bool isCommit)
}
SpinRelease(BufMgrLock);
}
PrivateRefCount[i] = 0;
}
ResetLocalBufferPool();
@@ -1900,7 +1898,7 @@ SetBufferCommitInfoNeedsSave(Buffer buffer)
}
void
UnlockBuffers()
UnlockBuffers(void)
{
BufferDesc *buf;
int i;
@@ -1913,6 +1911,8 @@ UnlockBuffers()
Assert(BufferIsValid(i + 1));
buf = &(BufferDescriptors[i]);
START_CRIT_SECTION(); /* don't want to die() holding the lock... */
S_LOCK(&(buf->cntx_lock));
if (BufferLocks[i] & BL_R_LOCK)
@@ -1940,6 +1940,8 @@ UnlockBuffers()
S_UNLOCK(&(buf->cntx_lock));
BufferLocks[i] = 0;
END_CRIT_SECTION();
}
}
@@ -1956,6 +1958,8 @@ LockBuffer(Buffer buffer, int mode)
buf = &(BufferDescriptors[buffer - 1]);
buflock = &(BufferLocks[buffer - 1]);
START_CRIT_SECTION(); /* don't want to die() holding the lock... */
S_LOCK(&(buf->cntx_lock));
if (mode == BUFFER_LOCK_UNLOCK)
@@ -1979,6 +1983,7 @@ LockBuffer(Buffer buffer, int mode)
else
{
S_UNLOCK(&(buf->cntx_lock));
END_CRIT_SECTION();
elog(ERROR, "UNLockBuffer: buffer %lu is not locked", buffer);
}
}
@@ -1990,7 +1995,9 @@ LockBuffer(Buffer buffer, int mode)
while (buf->ri_lock || buf->w_lock)
{
S_UNLOCK(&(buf->cntx_lock));
END_CRIT_SECTION();
S_LOCK_SLEEP(&(buf->cntx_lock), i++);
START_CRIT_SECTION();
S_LOCK(&(buf->cntx_lock));
}
(buf->r_locks)++;
@@ -2016,7 +2023,9 @@ LockBuffer(Buffer buffer, int mode)
buf->ri_lock = true;
}
S_UNLOCK(&(buf->cntx_lock));
END_CRIT_SECTION();
S_LOCK_SLEEP(&(buf->cntx_lock), i++);
START_CRIT_SECTION();
S_LOCK(&(buf->cntx_lock));
}
buf->w_lock = true;
@@ -2038,10 +2047,12 @@ LockBuffer(Buffer buffer, int mode)
else
{
S_UNLOCK(&(buf->cntx_lock));
END_CRIT_SECTION();
elog(ERROR, "LockBuffer: unknown lock mode %d", mode);
}
S_UNLOCK(&(buf->cntx_lock));
END_CRIT_SECTION();
}
/*
@@ -2062,7 +2073,9 @@ static bool IsForInput;
* BM_IO_IN_PROGRESS mask is not set for the buffer
* The buffer is Pinned
*
*/
* Because BufMgrLock is held, we are already in a CRIT_SECTION here,
* and do not need another.
*/
static void
StartBufferIO(BufferDesc *buf, bool forInput)
{
@@ -2094,7 +2107,9 @@ StartBufferIO(BufferDesc *buf, bool forInput)
* BufMgrLock is held
* The buffer is Pinned
*
*/
* Because BufMgrLock is held, we are already in a CRIT_SECTION here,
* and do not need another.
*/
static void
TerminateBufferIO(BufferDesc *buf)
{
@@ -2110,7 +2125,9 @@ TerminateBufferIO(BufferDesc *buf)
* BufMgrLock is held
* The buffer is Pinned
*
*/
* Because BufMgrLock is held, we are already in a CRIT_SECTION here,
* and do not need another.
*/
static void
ContinueBufferIO(BufferDesc *buf, bool forInput)
{
@@ -2132,7 +2149,7 @@ InitBufferIO(void)
* This function is called from ProcReleaseSpins().
* BufMgrLock isn't held when this function is called.
*
* If I/O was in progress, BM_IO_ERROR is always set.
* If I/O was in progress, we always set BM_IO_ERROR.
*/
void
AbortBufferIO(void)
@@ -2141,8 +2158,8 @@ AbortBufferIO(void)
if (buf)
{
Assert(buf->flags & BM_IO_IN_PROGRESS);
SpinAcquire(BufMgrLock);
Assert(buf->flags & BM_IO_IN_PROGRESS);
if (IsForInput)
Assert(!(buf->flags & BM_DIRTY) && !(buf->cntxDirty));
else

12
src/backend/storage/file/fd.c
View File

@@ -7,7 +7,7 @@
* Portions Copyright (c) 1994, Regents of the University of California
*
* IDENTIFICATION
* $Header: /cvsroot/pgsql/src/backend/storage/file/fd.c,v 1.69 2000/12/08 22:21:32 tgl Exp $
* $Header: /cvsroot/pgsql/src/backend/storage/file/fd.c,v 1.70 2001/01/12 21:53:58 tgl Exp $
*
* NOTES:
*
@@ -770,7 +770,11 @@ FileClose(File file)
* Delete the file if it was temporary
*/
if (VfdCache[file].fdstate & FD_TEMPORARY)
{
/* reset flag so that die() interrupt won't cause problems */
VfdCache[file].fdstate &= ~FD_TEMPORARY;
unlink(VfdCache[file].fileName);
}
/*
* Return the Vfd slot to the free list
@@ -1049,7 +1053,8 @@ AllocateFile(char *name, char *mode)
TryAgain:
if ((file = fopen(name, mode)) != NULL)
{
allocatedFiles[numAllocatedFiles++] = file;
allocatedFiles[numAllocatedFiles] = file;
numAllocatedFiles++;
return file;
}
@@ -1080,7 +1085,8 @@ FreeFile(FILE *file)
{
if (allocatedFiles[i] == file)
{
allocatedFiles[i] = allocatedFiles[--numAllocatedFiles];
numAllocatedFiles--;
allocatedFiles[i] = allocatedFiles[numAllocatedFiles];
break;
}
}

35
src/backend/storage/ipc/spin.c
View File

@@ -14,7 +14,7 @@
*
*
* IDENTIFICATION
* $Header: /cvsroot/pgsql/src/backend/storage/ipc/Attic/spin.c,v 1.27 2000/12/11 00:49:52 tgl Exp $
* $Header: /cvsroot/pgsql/src/backend/storage/ipc/Attic/spin.c,v 1.28 2001/01/12 21:53:59 tgl Exp $
*
*-------------------------------------------------------------------------
*/
@@ -144,8 +144,21 @@ SpinAcquire(SPINLOCK lockid)
SLock *slckP = &(SLockArray[lockid]);
PRINT_SLDEBUG("SpinAcquire", lockid, slckP);
/*
* Lock out die() until we exit the critical section protected by the
* spinlock. This ensures that die() will not interrupt manipulations
* of data structures in shared memory. We don't want die() to
* interrupt this routine between S_LOCK and PROC_INCR_SLOCK, either,
* so must do it before acquiring the lock, not after.
*/
START_CRIT_SECTION();
/*
* Acquire the lock, then record that we have done so (for recovery
* in case of elog(ERROR) during the critical section).
*/
S_LOCK(&(slckP->shlock));
PROC_INCR_SLOCK(lockid);
PRINT_SLDEBUG("SpinAcquire/done", lockid, slckP);
}
@@ -154,15 +167,22 @@ SpinRelease(SPINLOCK lockid)
{
SLock *slckP = &(SLockArray[lockid]);
PRINT_SLDEBUG("SpinRelease", lockid, slckP);
/*
* Check that we are actually holding the lock we are releasing. This
* can be done only after MyProc has been initialized.
*/
Assert(!MyProc || MyProc->sLocks[lockid] > 0);
/*
* Record that we no longer hold the spinlock, and release it.
*/
PROC_DECR_SLOCK(lockid);
PRINT_SLDEBUG("SpinRelease", lockid, slckP);
S_UNLOCK(&(slckP->shlock));
/*
* Exit the critical section entered in SpinAcquire().
*/
END_CRIT_SECTION();
PRINT_SLDEBUG("SpinRelease/done", lockid, slckP);
}
@@ -187,7 +207,7 @@ SpinRelease(SPINLOCK lockid)
*
* Note that the SpinLockIds array is not in shared memory; it is filled
* by the postmaster and then inherited through fork() by backends. This
* is OK because its contents do not change after system startup.
* is OK because its contents do not change after shmem initialization.
*/
#define SPINLOCKS_PER_SET PROC_NSEMS_PER_SET
@@ -285,6 +305,8 @@ SpinFreeAllSemaphores(void)
if (SpinLockIds[i] >= 0)
IpcSemaphoreKill(SpinLockIds[i]);
}
free(SpinLockIds);
SpinLockIds = NULL;
}
/*
@@ -295,6 +317,8 @@ SpinFreeAllSemaphores(void)
void
SpinAcquire(SPINLOCK lock)
{
/* See the TAS() version of this routine for commentary */
START_CRIT_SECTION();
IpcSemaphoreLock(SpinLockIds[0], lock);
PROC_INCR_SLOCK(lock);
}
@@ -307,15 +331,18 @@ SpinAcquire(SPINLOCK lock)
void
SpinRelease(SPINLOCK lock)
{
/* See the TAS() version of this routine for commentary */
#ifdef USE_ASSERT_CHECKING
/* Check it's locked */
int semval;
semval = IpcSemaphoreGetValue(SpinLockIds[0], lock);
Assert(semval < 1);
Assert(!MyProc || MyProc->sLocks[lockid] > 0);
#endif
PROC_DECR_SLOCK(lock);
IpcSemaphoreUnlock(SpinLockIds[0], lock);
END_CRIT_SECTION();
}
/*

29
src/backend/storage/lmgr/proc.c
View File

@@ -8,7 +8,7 @@
*
*
* IDENTIFICATION
* $Header: /cvsroot/pgsql/src/backend/storage/lmgr/proc.c,v 1.90 2001/01/09 09:38:57 inoue Exp $
* $Header: /cvsroot/pgsql/src/backend/storage/lmgr/proc.c,v 1.91 2001/01/12 21:53:59 tgl Exp $
*
*-------------------------------------------------------------------------
*/
@@ -48,7 +48,7 @@
* This is so that we can support more backends. (system-wide semaphore
* sets run out pretty fast.) -ay 4/95
*
* $Header: /cvsroot/pgsql/src/backend/storage/lmgr/proc.c,v 1.90 2001/01/09 09:38:57 inoue Exp $
* $Header: /cvsroot/pgsql/src/backend/storage/lmgr/proc.c,v 1.91 2001/01/12 21:53:59 tgl Exp $
*/
#include "postgres.h"
@@ -241,7 +241,6 @@ InitProcess(void)
MemSet(MyProc->sLocks, 0, sizeof(MyProc->sLocks));
MyProc->sLocks[ProcStructLock] = 1;
if (IsUnderPostmaster)
{
IpcSemaphoreId semId;
@@ -264,23 +263,16 @@ InitProcess(void)
else
MyProc->sem.semId = -1;
/* ----------------------
* Release the lock.
* ----------------------
*/
SpinRelease(ProcStructLock);
MyProc->pid = MyProcPid;
MyProc->databaseId = MyDatabaseId;
MyProc->xid = InvalidTransactionId;
MyProc->xmin = InvalidTransactionId;
/* ----------------
* Start keeping spin lock stats from here on. Any botch before
* this initialization is forever botched
* ----------------
/* ----------------------
* Release the lock.
* ----------------------
*/
MemSet(MyProc->sLocks, 0, MAX_SPINS * sizeof(*MyProc->sLocks));
SpinRelease(ProcStructLock);
/* -------------------------
* Install ourselves in the shmem index table. The name to
@@ -412,15 +404,6 @@ ProcKill(int exitStatus, Datum pid)
{
PROC *proc;
/* --------------------
* If this is a FATAL exit the postmaster will have to kill all the
* existing backends and reinitialize shared memory. So we don't
* need to do anything here.
* --------------------
*/
if (exitStatus != 0)
return;
if ((int) pid == MyProcPid)
{
proc = MyProc;