From: Tom Lane Date: Fri, 12 Jan 2001 21:54:01 +0000 (+0000) Subject: Add more critical-section calls: all code sections that hold spinlocks X-Git-Tag: REL7_1~847 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=6162432de9fb023b710c171f196e27b910e45fa7;p=postgresql Add more critical-section calls: all code sections that hold spinlocks 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(). --- diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c index 612e4c6861..db443218b4 100644 --- a/src/backend/access/heap/heapam.c +++ b/src/backend/access/heap/heapam.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/access/heap/heapam.c,v 1.106 2001/01/07 22:14:31 tgl Exp $ + * $Header: /cvsroot/pgsql/src/backend/access/heap/heapam.c,v 1.107 2001/01/12 21:53:54 tgl Exp $ * * * INTERFACE ROUTINES @@ -1359,7 +1359,7 @@ heap_insert(Relation relation, HeapTuple tup) buffer = RelationGetBufferForTuple(relation, tup->t_len); /* NO ELOG(ERROR) from here till changes are logged */ - START_CRIT_CODE; + START_CRIT_SECTION(); RelationPutHeapTuple(relation, buffer, tup); /* XLOG stuff */ @@ -1405,7 +1405,7 @@ heap_insert(Relation relation, HeapTuple tup) PageSetLSN(page, recptr); PageSetSUI(page, ThisStartUpID); } - END_CRIT_CODE; + END_CRIT_SECTION(); LockBuffer(buffer, BUFFER_LOCK_UNLOCK); WriteBuffer(buffer); @@ -1503,7 +1503,7 @@ l1: return result; } - START_CRIT_CODE; + START_CRIT_SECTION(); /* store transaction information of xact deleting the tuple */ TransactionIdStore(GetCurrentTransactionId(), &(tp.t_data->t_xmax)); tp.t_data->t_cmax = GetCurrentCommandId(); @@ -1532,7 +1532,7 @@ l1: PageSetLSN(dp, recptr); PageSetSUI(dp, ThisStartUpID); } - END_CRIT_CODE; + END_CRIT_SECTION(); #ifdef TUPLE_TOASTER_ACTIVE /* ---------- @@ -1702,7 +1702,7 @@ l2: } /* NO ELOG(ERROR) from here till changes are logged */ - START_CRIT_CODE; + START_CRIT_SECTION(); RelationPutHeapTuple(relation, newbuf, newtup); /* insert new tuple */ if (buffer == newbuf) @@ -1734,7 +1734,7 @@ l2: PageSetLSN(BufferGetPage(buffer), recptr); PageSetSUI(BufferGetPage(buffer), ThisStartUpID); } - END_CRIT_CODE; + END_CRIT_SECTION(); if (newbuf != buffer) LockBuffer(newbuf, BUFFER_LOCK_UNLOCK); diff --git a/src/backend/access/nbtree/nbtinsert.c b/src/backend/access/nbtree/nbtinsert.c index cdd5c6d620..1aae86c002 100644 --- a/src/backend/access/nbtree/nbtinsert.c +++ b/src/backend/access/nbtree/nbtinsert.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/access/nbtree/nbtinsert.c,v 1.72 2000/12/29 20:47:16 vadim Exp $ + * $Header: /cvsroot/pgsql/src/backend/access/nbtree/nbtinsert.c,v 1.73 2001/01/12 21:53:55 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -518,7 +518,7 @@ _bt_insertonpg(Relation rel, } else { - START_CRIT_CODE; + START_CRIT_SECTION(); _bt_pgaddtup(rel, page, itemsz, btitem, newitemoff, "page"); itup_off = newitemoff; itup_blkno = BufferGetBlockNumber(buf); @@ -563,7 +563,7 @@ _bt_insertonpg(Relation rel, PageSetSUI(page, ThisStartUpID); } - END_CRIT_CODE; + END_CRIT_SECTION(); /* Write out the updated page and release pin/lock */ _bt_wrtbuf(rel, buf); } @@ -774,7 +774,7 @@ _bt_split(Relation rel, Buffer buf, OffsetNumber firstright, * NO ELOG(ERROR) till right sibling is updated. * */ - START_CRIT_CODE; + START_CRIT_SECTION(); { xl_btree_split xlrec; int flag = (newitemonleft) ? @@ -863,7 +863,7 @@ _bt_split(Relation rel, Buffer buf, OffsetNumber firstright, /* write and release the old right sibling */ if (!P_RIGHTMOST(ropaque)) _bt_wrtbuf(rel, sbuf); - END_CRIT_CODE; + END_CRIT_SECTION(); /* split's done */ return rbuf; @@ -1160,7 +1160,7 @@ _bt_newroot(Relation rel, Buffer lbuf, Buffer rbuf) metad = BTPageGetMeta(metapg); /* NO ELOG(ERROR) from here till newroot op is logged */ - START_CRIT_CODE; + START_CRIT_SECTION(); /* set btree special data */ rootopaque = (BTPageOpaque) PageGetSpecialPointer(rootpage); @@ -1253,7 +1253,7 @@ _bt_newroot(Relation rel, Buffer lbuf, Buffer rbuf) PageSetSUI(metapg, ThisStartUpID); } - END_CRIT_CODE; + END_CRIT_SECTION(); /* write and let go of the new root buffer */ _bt_wrtbuf(rel, rootbuf); diff --git a/src/backend/access/nbtree/nbtpage.c b/src/backend/access/nbtree/nbtpage.c index e2e403a2af..81faf71c9d 100644 --- a/src/backend/access/nbtree/nbtpage.c +++ b/src/backend/access/nbtree/nbtpage.c @@ -9,7 +9,7 @@ * * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/access/nbtree/nbtpage.c,v 1.45 2000/12/29 20:47:17 vadim Exp $ + * $Header: /cvsroot/pgsql/src/backend/access/nbtree/nbtpage.c,v 1.46 2001/01/12 21:53:55 tgl Exp $ * * NOTES * Postgres btree pages look like ordinary relation pages. The opaque @@ -165,7 +165,7 @@ _bt_getroot(Relation rel, int access) rootpage = BufferGetPage(rootbuf); /* NO ELOG(ERROR) till meta is updated */ - START_CRIT_CODE; + START_CRIT_SECTION(); metad->btm_root = rootblkno; metad->btm_level = 1; @@ -197,7 +197,7 @@ _bt_getroot(Relation rel, int access) PageSetSUI(metapg, ThisStartUpID); } - END_CRIT_CODE; + END_CRIT_SECTION(); _bt_wrtnorelbuf(rel, rootbuf); @@ -410,7 +410,7 @@ _bt_pagedel(Relation rel, ItemPointer tid) buf = _bt_getbuf(rel, blkno, BT_WRITE); page = BufferGetPage(buf); - START_CRIT_CODE; + START_CRIT_SECTION(); PageIndexTupleDelete(page, offno); /* XLOG stuff */ { @@ -435,7 +435,7 @@ _bt_pagedel(Relation rel, ItemPointer tid) PageSetLSN(page, recptr); PageSetSUI(page, ThisStartUpID); } - END_CRIT_CODE; + END_CRIT_SECTION(); /* write the buffer and release the lock */ _bt_wrtbuf(rel, buf); diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c index 50f4f1a100..e3f4a5618f 100644 --- a/src/backend/access/transam/xact.c +++ b/src/backend/access/transam/xact.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/access/transam/xact.c,v 1.91 2000/12/28 13:00:08 vadim Exp $ + * $Header: /cvsroot/pgsql/src/backend/access/transam/xact.c,v 1.92 2001/01/12 21:53:56 tgl Exp $ * * NOTES * Transaction aborts can now occur two ways: @@ -678,7 +678,7 @@ RecordTransactionCommit() rdata.len = SizeOfXactCommit; rdata.next = NULL; - START_CRIT_CODE; + START_CRIT_SECTION(); /* * SHOULD SAVE ARRAY OF RELFILENODE-s TO DROP */ @@ -697,7 +697,7 @@ RecordTransactionCommit() TransactionIdCommit(xid); MyProc->logRec.xrecoff = 0; - END_CRIT_CODE; + END_CRIT_SECTION(); } if (leak) @@ -800,12 +800,12 @@ RecordTransactionAbort(void) rdata.len = SizeOfXactAbort; rdata.next = NULL; - START_CRIT_CODE; + START_CRIT_SECTION(); recptr = XLogInsert(RM_XACT_ID, XLOG_XACT_ABORT, &rdata); TransactionIdAbort(xid); MyProc->logRec.xrecoff = 0; - END_CRIT_CODE; + END_CRIT_SECTION(); } /* diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index c0b6104db5..e995b06f91 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -6,7 +6,7 @@ * Portions Copyright (c) 1996-2000, PostgreSQL, Inc * Portions Copyright (c) 1994, Regents of the University of California * - * $Header: /cvsroot/pgsql/src/backend/access/transam/xlog.c,v 1.48 2001/01/09 06:24:32 vadim Exp $ + * $Header: /cvsroot/pgsql/src/backend/access/transam/xlog.c,v 1.49 2001/01/12 21:53:56 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -42,7 +42,7 @@ int XLOGbuffers = 8; int XLOGfiles = 0; /* how many files to pre-allocate */ XLogRecPtr MyLastRecPtr = {0, 0}; -uint32 CritSectionCount = 0; +volatile uint32 CritSectionCount = 0; bool InRecovery = false; StartUpID ThisStartUpID = 0; XLogRecPtr RedoRecPtr; @@ -382,7 +382,7 @@ begin:; if (len == 0 || len > MAXLOGRECSZ) elog(STOP, "XLogInsert: invalid record len %u", len); - START_CRIT_CODE; + START_CRIT_SECTION(); /* obtain xlog insert lock */ if (TAS(&(XLogCtl->insert_lck))) /* busy */ @@ -447,7 +447,7 @@ begin:; if (repeat) { S_UNLOCK(&(XLogCtl->insert_lck)); - END_CRIT_CODE; + END_CRIT_SECTION(); goto begin; } @@ -618,7 +618,7 @@ begin:; S_UNLOCK(&(XLogCtl->info_lck)); } - END_CRIT_CODE; + END_CRIT_SECTION(); return (RecPtr); } @@ -647,7 +647,7 @@ XLogFlush(XLogRecPtr record) if (XLByteLE(record, LgwrResult.Flush)) return; - START_CRIT_CODE; + START_CRIT_SECTION(); WriteRqst = LgwrRqst.Write; for (;;) @@ -659,7 +659,7 @@ XLogFlush(XLogRecPtr record) if (XLByteLE(record, LgwrResult.Flush)) { S_UNLOCK(&(XLogCtl->info_lck)); - END_CRIT_CODE; + END_CRIT_SECTION(); return; } if (XLByteLT(XLogCtl->LgwrRqst.Flush, record)) @@ -705,7 +705,7 @@ XLogFlush(XLogRecPtr record) if (XLByteLE(record, LgwrResult.Flush)) { S_UNLOCK(&(XLogCtl->lgwr_lck)); - END_CRIT_CODE; + END_CRIT_SECTION(); return; } if (XLByteLT(LgwrResult.Write, WriteRqst)) @@ -715,7 +715,7 @@ XLogFlush(XLogRecPtr record) S_UNLOCK(&(XLogCtl->lgwr_lck)); if (XLByteLT(LgwrResult.Flush, record)) elog(STOP, "XLogFlush: request is not satisfied"); - END_CRIT_CODE; + END_CRIT_SECTION(); return; } break; @@ -756,7 +756,7 @@ XLogFlush(XLogRecPtr record) S_UNLOCK(&(XLogCtl->lgwr_lck)); - END_CRIT_CODE; + END_CRIT_SECTION(); return; } @@ -2081,7 +2081,7 @@ CreateCheckPoint(bool shutdown) if (MyLastRecPtr.xrecoff != 0) elog(ERROR, "CreateCheckPoint: cannot be called inside transaction block"); - START_CRIT_CODE; + START_CRIT_SECTION(); /* Grab lock, using larger than normal sleep between tries (1 sec) */ while (TAS(&(XLogCtl->chkp_lck))) @@ -2230,7 +2230,7 @@ CreateCheckPoint(bool shutdown) S_UNLOCK(&(XLogCtl->chkp_lck)); MyLastRecPtr.xrecoff = 0; /* to avoid commit record */ - END_CRIT_CODE; + END_CRIT_SECTION(); return; } diff --git a/src/backend/commands/sequence.c b/src/backend/commands/sequence.c index 64fc0102a8..f6e6328312 100644 --- a/src/backend/commands/sequence.c +++ b/src/backend/commands/sequence.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/commands/sequence.c,v 1.47 2000/12/28 13:00:17 vadim Exp $ + * $Header: /cvsroot/pgsql/src/backend/commands/sequence.c,v 1.48 2001/01/12 21:53:56 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -301,7 +301,7 @@ nextval(PG_FUNCTION_ARGS) elm->last = result; /* last returned number */ elm->cached = last; /* last fetched number */ - START_CRIT_CODE; + START_CRIT_SECTION(); if (logit) { xl_seq_rec xlrec; @@ -338,7 +338,7 @@ nextval(PG_FUNCTION_ARGS) seq->is_called = 't'; Assert(log >= 0); seq->log_cnt = log; /* how much is logged */ - END_CRIT_CODE; + END_CRIT_SECTION(); LockBuffer(buf, BUFFER_LOCK_UNLOCK); @@ -398,7 +398,7 @@ do_setval(char *seqname, int32 next, bool iscalled) elm->last = next; /* last returned number */ elm->cached = next; /* last cached number (forget cached values) */ - START_CRIT_CODE; + START_CRIT_SECTION(); { xl_seq_rec xlrec; XLogRecPtr recptr; @@ -429,7 +429,7 @@ do_setval(char *seqname, int32 next, bool iscalled) seq->last_value = next; /* last fetched number */ seq->is_called = iscalled ? 't' : 'f'; seq->log_cnt = (iscalled) ? 0 : 1; - END_CRIT_CODE; + END_CRIT_SECTION(); LockBuffer(buf, BUFFER_LOCK_UNLOCK); diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c index 738d9e7283..76425652b9 100644 --- a/src/backend/commands/vacuum.c +++ b/src/backend/commands/vacuum.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/commands/vacuum.c,v 1.181 2000/12/30 15:19:55 vadim Exp $ + * $Header: /cvsroot/pgsql/src/backend/commands/vacuum.c,v 1.182 2001/01/12 21:53:56 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -1427,7 +1427,7 @@ repair_frag(VRelStats *vacrelstats, Relation onerel, Cpage = BufferGetPage(Cbuf); /* NO ELOG(ERROR) TILL CHANGES ARE LOGGED */ - START_CRIT_CODE; + START_CRIT_SECTION(); Citemid = PageGetItemId(Cpage, ItemPointerGetOffsetNumber(&(tuple.t_self))); @@ -1512,7 +1512,7 @@ repair_frag(VRelStats *vacrelstats, Relation onerel, PageSetLSN(ToPage, recptr); PageSetSUI(ToPage, ThisStartUpID); } - END_CRIT_CODE; + END_CRIT_SECTION(); if (((int) destvacpage->blkno) > last_move_dest_block) last_move_dest_block = destvacpage->blkno; @@ -1637,7 +1637,7 @@ repair_frag(VRelStats *vacrelstats, Relation onerel, newtup.t_data->t_infomask |= HEAP_MOVED_IN; /* NO ELOG(ERROR) TILL CHANGES ARE LOGGED */ - START_CRIT_CODE; + START_CRIT_SECTION(); /* add tuple to the page */ newoff = PageAddItem(ToPage, (Item) newtup.t_data, tuple_len, @@ -1675,7 +1675,7 @@ failed to add item with len = %lu to page %u (free space %lu, nusd %u, noff %u)" PageSetLSN(ToPage, recptr); PageSetSUI(ToPage, ThisStartUpID); } - END_CRIT_CODE; + END_CRIT_SECTION(); cur_page->offsets_used++; num_moved++; @@ -1905,7 +1905,7 @@ failed to add item with len = %lu to page %u (free space %lu, nusd %u, noff %u)" buf = ReadBuffer(onerel, vacpage->blkno); LockBuffer(buf, BUFFER_LOCK_EXCLUSIVE); - START_CRIT_CODE; + START_CRIT_SECTION(); page = BufferGetPage(buf); num_tuples = 0; for (offnum = FirstOffsetNumber; @@ -1941,7 +1941,7 @@ failed to add item with len = %lu to page %u (free space %lu, nusd %u, noff %u)" PageSetLSN(page, recptr); PageSetSUI(page, ThisStartUpID); } - END_CRIT_CODE; + END_CRIT_SECTION(); LockBuffer(buf, BUFFER_LOCK_UNLOCK); WriteBuffer(buf); } @@ -2056,7 +2056,7 @@ vacuum_page(Relation onerel, Buffer buffer, VacPage vacpage) /* There shouldn't be any tuples moved onto the page yet! */ Assert(vacpage->offsets_used == 0); - START_CRIT_CODE; + START_CRIT_SECTION(); for (i = 0; i < vacpage->offsets_free; i++) { itemid = &(((PageHeader) page)->pd_linp[vacpage->offsets[i] - 1]); @@ -2070,7 +2070,7 @@ vacuum_page(Relation onerel, Buffer buffer, VacPage vacpage) PageSetLSN(page, recptr); PageSetSUI(page, ThisStartUpID); } - END_CRIT_CODE; + END_CRIT_SECTION(); } diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c index 6ba74f5e06..2be519193b 100644 --- a/src/backend/storage/buffer/bufmgr.c +++ b/src/backend/storage/buffer/bufmgr.c @@ -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 diff --git a/src/backend/storage/file/fd.c b/src/backend/storage/file/fd.c index 3cff793212..9ad40849eb 100644 --- a/src/backend/storage/file/fd.c +++ b/src/backend/storage/file/fd.c @@ -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; } } diff --git a/src/backend/storage/ipc/spin.c b/src/backend/storage/ipc/spin.c index 554eeba797..ed71d79ad9 100644 --- a/src/backend/storage/ipc/spin.c +++ b/src/backend/storage/ipc/spin.c @@ -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(); } /* diff --git a/src/backend/storage/lmgr/proc.c b/src/backend/storage/lmgr/proc.c index 2730c8811e..baa31413e2 100644 --- a/src/backend/storage/lmgr/proc.c +++ b/src/backend/storage/lmgr/proc.c @@ -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; diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c index a73ba4df4a..1a0aa5d0cd 100644 --- a/src/backend/tcop/postgres.c +++ b/src/backend/tcop/postgres.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/tcop/postgres.c,v 1.199 2001/01/07 04:17:29 tgl Exp $ + * $Header: /cvsroot/pgsql/src/backend/tcop/postgres.c,v 1.200 2001/01/12 21:53:59 tgl Exp $ * * NOTES * this is the "main" module of the postgres backend and @@ -982,7 +982,7 @@ die(SIGNAL_ARGS) /* * This is split out of die() so that it can be invoked later from - * END_CRIT_CODE. + * END_CRIT_SECTION(). */ void ForceProcDie(void) @@ -1683,7 +1683,7 @@ PostgresMain(int argc, char *argv[], int real_argc, char *real_argv[], const cha if (!IsUnderPostmaster) { puts("\nPOSTGRES backend interactive interface "); - puts("$Revision: 1.199 $ $Date: 2001/01/07 04:17:29 $\n"); + puts("$Revision: 1.200 $ $Date: 2001/01/12 21:53:59 $\n"); } /* diff --git a/src/backend/utils/cache/temprel.c b/src/backend/utils/cache/temprel.c index a0066f2eaf..b1580d65b4 100644 --- a/src/backend/utils/cache/temprel.c +++ b/src/backend/utils/cache/temprel.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/utils/cache/Attic/temprel.c,v 1.32 2000/12/22 23:12:07 tgl Exp $ + * $Header: /cvsroot/pgsql/src/backend/utils/cache/Attic/temprel.c,v 1.33 2001/01/12 21:54:00 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -263,8 +263,6 @@ AtEOXact_temp_relations(bool isCommit) temp_rel->created_in_cur_xact) { /* This entry must be removed */ - pfree(temp_rel); - /* remove from linked list */ if (prev != NIL) { lnext(prev) = lnext(l); @@ -277,6 +275,7 @@ AtEOXact_temp_relations(bool isCommit) pfree(l); l = temp_rels; } + pfree(temp_rel); } else { diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.c index c8bd8a0d26..6248c7bea6 100644 --- a/src/backend/utils/init/postinit.c +++ b/src/backend/utils/init/postinit.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/utils/init/postinit.c,v 1.76 2000/12/18 00:44:48 tgl Exp $ + * $Header: /cvsroot/pgsql/src/backend/utils/init/postinit.c,v 1.77 2001/01/12 21:54:00 tgl Exp $ * * *------------------------------------------------------------------------- @@ -395,11 +395,12 @@ ShutdownPostgres(void) * We don't want to do any inessential cleanup, since that just raises * the odds of failure --- but there's some stuff we need to do. * - * Release any spinlocks that we may hold. This is a kluge to improve - * the odds that we won't get into a self-made stuck spinlock scenario - * while trying to shut down. + * Release any spinlocks or buffer context locks we might be holding. + * This is a kluge to improve the odds that we won't get into a self-made + * stuck-spinlock scenario while trying to shut down. */ ProcReleaseSpins(NULL); + UnlockBuffers(); /* * In case a transaction is open, delete any files it created. This * has to happen before bufmgr shutdown, so having smgr register a diff --git a/src/backend/utils/mmgr/aset.c b/src/backend/utils/mmgr/aset.c index cdb2601e38..5c0cf30745 100644 --- a/src/backend/utils/mmgr/aset.c +++ b/src/backend/utils/mmgr/aset.c @@ -11,7 +11,7 @@ * Portions Copyright (c) 1994, Regents of the University of California * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/utils/mmgr/aset.c,v 1.36 2001/01/06 21:59:39 tgl Exp $ + * $Header: /cvsroot/pgsql/src/backend/utils/mmgr/aset.c,v 1.37 2001/01/12 21:54:01 tgl Exp $ * * NOTE: * This is a new (Feb. 05, 1999) implementation of the allocation set @@ -384,6 +384,11 @@ AllocSetReset(MemoryContext context) AllocSetCheck(context); #endif + /* Clear chunk freelists */ + MemSet(set->freelist, 0, sizeof(set->freelist)); + /* New blocks list is either empty or just the keeper block */ + set->blocks = set->keeper; + while (block != NULL) { AllocBlock next = block->next; @@ -411,11 +416,6 @@ AllocSetReset(MemoryContext context) } block = next; } - - /* Now blocks list is either empty or just the keeper block */ - set->blocks = set->keeper; - /* Clear chunk freelists in any case */ - MemSet(set->freelist, 0, sizeof(set->freelist)); } /* @@ -439,6 +439,11 @@ AllocSetDelete(MemoryContext context) AllocSetCheck(context); #endif + /* Make it look empty, just in case... */ + MemSet(set->freelist, 0, sizeof(set->freelist)); + set->blocks = NULL; + set->keeper = NULL; + while (block != NULL) { AllocBlock next = block->next; @@ -450,11 +455,6 @@ AllocSetDelete(MemoryContext context) free(block); block = next; } - - /* Make it look empty, just in case... */ - set->blocks = NULL; - MemSet(set->freelist, 0, sizeof(set->freelist)); - set->keeper = NULL; } /* @@ -605,15 +605,16 @@ AllocSetAlloc(MemoryContext context, Size size) } chunk = (AllocChunk) (block->freeptr); + + block->freeptr += (availchunk + ALLOC_CHUNKHDRSZ); + availspace -= (availchunk + ALLOC_CHUNKHDRSZ); + chunk->size = availchunk; #ifdef MEMORY_CONTEXT_CHECKING chunk->requested_size = 0; /* mark it free */ #endif chunk->aset = (void *) set->freelist[a_fidx]; set->freelist[a_fidx] = chunk; - - block->freeptr += (availchunk + ALLOC_CHUNKHDRSZ); - availspace -= (availchunk + ALLOC_CHUNKHDRSZ); } /* Mark that we need to create a new block */ @@ -696,6 +697,10 @@ AllocSetAlloc(MemoryContext context, Size size) * OK, do the allocation */ chunk = (AllocChunk) (block->freeptr); + + block->freeptr += (chunk_size + ALLOC_CHUNKHDRSZ); + Assert(block->freeptr <= block->endptr); + chunk->aset = (void *) set; chunk->size = chunk_size; #ifdef MEMORY_CONTEXT_CHECKING @@ -705,9 +710,6 @@ AllocSetAlloc(MemoryContext context, Size size) ((char *) AllocChunkGetPointer(chunk))[size] = 0x7E; #endif - block->freeptr += (chunk_size + ALLOC_CHUNKHDRSZ); - Assert(block->freeptr <= block->endptr); - AllocAllocInfo(set, chunk); return AllocChunkGetPointer(chunk); } diff --git a/src/include/access/xlog.h b/src/include/access/xlog.h index 88268b0b0b..7736ec92e8 100644 --- a/src/include/access/xlog.h +++ b/src/include/access/xlog.h @@ -3,7 +3,7 @@ * * PostgreSQL transaction log manager * - * $Header: /cvsroot/pgsql/src/include/access/xlog.h,v 1.15 2000/12/28 13:00:25 vadim Exp $ + * $Header: /cvsroot/pgsql/src/include/access/xlog.h,v 1.16 2001/01/12 21:54:01 tgl Exp $ */ #ifndef XLOG_H #define XLOG_H @@ -101,7 +101,7 @@ typedef XLogPageHeaderData *XLogPageHeader; extern StartUpID ThisStartUpID; /* current SUI */ extern bool InRecovery; extern XLogRecPtr MyLastRecPtr; -extern uint32 CritSectionCount; +extern volatile uint32 CritSectionCount; typedef struct RmgrData { diff --git a/src/include/utils/elog.h b/src/include/utils/elog.h index e7305d67fc..da9178b276 100644 --- a/src/include/utils/elog.h +++ b/src/include/utils/elog.h @@ -7,7 +7,7 @@ * Portions Copyright (c) 1996-2000, PostgreSQL, Inc * Portions Copyright (c) 1994, Regents of the University of California * - * $Id: elog.h,v 1.22 2001/01/07 04:17:28 tgl Exp $ + * $Id: elog.h,v 1.23 2001/01/12 21:54:01 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -34,16 +34,15 @@ extern int Use_syslog; * ProcDiePending will be honored at critical section exit, * but QueryCancel is only checked at specified points. */ -extern uint32 CritSectionCount; /* duplicates access/xlog.h */ +extern volatile uint32 CritSectionCount; /* duplicates access/xlog.h */ extern volatile bool ProcDiePending; extern void ForceProcDie(void); /* in postgres.c */ -#define START_CRIT_CODE (CritSectionCount++) +#define START_CRIT_SECTION() (CritSectionCount++) -#define END_CRIT_CODE \ +#define END_CRIT_SECTION() \ do { \ - if (CritSectionCount == 0) \ - elog(STOP, "Not in critical section"); \ + Assert(CritSectionCount > 0); \ CritSectionCount--; \ if (CritSectionCount == 0 && ProcDiePending) \ ForceProcDie(); \