Skip to content

Commit a626b78

Browse files
tglsfdctglsfdc
committed
Clean up backend-exit-time cleanup behavior. Use on_shmem_exit callbacks
to ensure that we have released buffer refcounts and so forth, rather than putting ad-hoc operations before (some of the calls to) proc_exit. Add commentary to discourage future hackers from repeating that mistake.
1 parent 8c8ed4f commit a626b78

File tree

14 files changed

+255
-139
lines changed

14 files changed

+255
-139
lines changed

‎src/backend/access/transam/xact.c

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $Header: /cvsroot/pgsql/src/backend/access/transam/xact.c,v 1.88 2000/12/07 10:03:46 inoue Exp $
11+
* $Header: /cvsroot/pgsql/src/backend/access/transam/xact.c,v 1.89 2000/12/18 00:44:45 tgl Exp $
1212
*
1313
* NOTES
1414
* Transaction aborts can now occur two ways:
@@ -1095,6 +1095,15 @@ AbortTransaction(void)
10951095
MyProc->xmin = InvalidTransactionId;
10961096
}
10971097

1098+
/*
1099+
* Release any spinlocks or buffer context locks we might be holding
1100+
* as quickly as possible. (Real locks, however, must be held till
1101+
* we finish aborting.) Releasing spinlocks is critical since we
1102+
* might try to grab them again while cleaning up!
1103+
*/
1104+
ProcReleaseSpins(NULL);
1105+
UnlockBuffers();
1106+
10981107
/* ----------------
10991108
* check the current transaction state
11001109
* ----------------
@@ -1105,31 +1114,24 @@ AbortTransaction(void)
11051114
if (s->state != TRANS_INPROGRESS)
11061115
elog(NOTICE, "AbortTransaction and not in in-progress state");
11071116

1108-
/*
1109-
* Reset user id which might have been changed transiently
1110-
*/
1111-
SetUserId(GetSessionUserId());
1112-
1113-
/* ----------------
1114-
* Tell the trigger manager that this transaction is about to be
1115-
* aborted.
1116-
* ----------------
1117-
*/
1118-
DeferredTriggerAbortXact();
1119-
11201117
/* ----------------
11211118
* set the current transaction state information
11221119
* appropriately during the abort processing
11231120
* ----------------
11241121
*/
11251122
s->state = TRANS_ABORT;
11261123

1124+
/*
1125+
* Reset user id which might have been changed transiently
1126+
*/
1127+
SetUserId(GetSessionUserId());
1128+
11271129
/* ----------------
11281130
* do abort processing
11291131
* ----------------
11301132
*/
1133+
DeferredTriggerAbortXact();
11311134
lo_commit(false); /* 'false' means it's abort */
1132-
UnlockBuffers();
11331135
AtAbort_Notify();
11341136
CloseSequences();
11351137
AtEOXact_portals();

‎src/backend/access/transam/xlog.c

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
* Portions Copyright (c) 1996-2000, PostgreSQL, Inc
77
* Portions Copyright (c) 1994, Regents of the University of California
88
*
9-
* $Header: /cvsroot/pgsql/src/backend/access/transam/xlog.c,v 1.42 2000/12/11 19:27:42 vadim Exp $
9+
* $Header: /cvsroot/pgsql/src/backend/access/transam/xlog.c,v 1.43 2000/12/18 00:44:45 tgl Exp $
1010
*
1111
*-------------------------------------------------------------------------
1212
*/
@@ -40,7 +40,7 @@
4040

4141
int XLOGbuffers = 8;
4242
XLogRecPtr MyLastRecPtr = {0, 0};
43-
uint32 StopIfError = 0;
43+
uint32 CritSectionCount = 0;
4444
bool InRecovery = false;
4545
StartUpID ThisStartUpID = 0;
4646

@@ -1531,7 +1531,7 @@ StartupXLOG()
15311531
char buffer[MAXLOGRECSZ + SizeOfXLogRecord];
15321532

15331533
elog(LOG, "starting up");
1534-
StopIfError++;
1534+
CritSectionCount++;
15351535

15361536
XLogCtl->xlblocks = (XLogRecPtr *) (((char *) XLogCtl) + sizeof(XLogCtlData));
15371537
XLogCtl->pages = ((char *) XLogCtl->xlblocks + sizeof(XLogRecPtr) * XLOGbuffers);
@@ -1748,7 +1748,7 @@ StartupXLOG()
17481748
XLogCtl->ThisStartUpID = ThisStartUpID;
17491749

17501750
elog(LOG, "database system is in production state");
1751-
StopIfError--;
1751+
CritSectionCount--;
17521752

17531753
return;
17541754
}
@@ -1771,10 +1771,10 @@ ShutdownXLOG()
17711771
{
17721772
elog(LOG, "shutting down");
17731773

1774-
StopIfError++;
1774+
CritSectionCount++;
17751775
CreateDummyCaches();
17761776
CreateCheckPoint(true);
1777-
StopIfError--;
1777+
CritSectionCount--;
17781778

17791779
elog(LOG, "database system is shut down");
17801780
}

‎src/backend/commands/trigger.c

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
* Portions Copyright (c) 1994, Regents of the University of California
88
*
99
* IDENTIFICATION
10-
* $Header: /cvsroot/pgsql/src/backend/commands/trigger.c,v 1.81 2000/11/20 20:36:47 tgl Exp $
10+
* $Header: /cvsroot/pgsql/src/backend/commands/trigger.c,v 1.82 2000/12/18 00:44:46 tgl Exp $
1111
*
1212
*-------------------------------------------------------------------------
1313
*/
@@ -1432,16 +1432,14 @@ deferredTriggerInvokeEvents(bool immediate_only)
14321432
* transactions.
14331433
* ----------
14341434
*/
1435-
int
1435+
void
14361436
DeferredTriggerInit(void)
14371437
{
14381438
deftrig_gcxt = AllocSetContextCreate(TopMemoryContext,
14391439
"DeferredTriggerSession",
14401440
ALLOCSET_DEFAULT_MINSIZE,
14411441
ALLOCSET_DEFAULT_INITSIZE,
14421442
ALLOCSET_DEFAULT_MAXSIZE);
1443-
1444-
return 0;
14451443
}
14461444

14471445

‎src/backend/libpq/pqcomm.c

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@
2929
* Portions Copyright (c) 1996-2000, PostgreSQL, Inc
3030
* Portions Copyright (c) 1994, Regents of the University of California
3131
*
32-
* $Id: pqcomm.c,v 1.114 2000/11/29 20:59:51 tgl Exp $
32+
* $Id: pqcomm.c,v 1.115 2000/12/18 00:44:46 tgl Exp $
3333
*
3434
*-------------------------------------------------------------------------
3535
*/
@@ -85,6 +85,9 @@
8585
#endif
8686

8787

88+
static void pq_close(void);
89+
90+
8891
/*
8992
* Configuration options
9093
*/
@@ -122,6 +125,7 @@ pq_init(void)
122125
{
123126
PqSendPointer = PqRecvPointer = PqRecvLength = 0;
124127
DoingCopyOut = false;
128+
on_proc_exit(pq_close, 0);
125129
}
126130

127131

@@ -132,7 +136,7 @@ pq_init(void)
132136
* don't crash during exit...
133137
* --------------------------------
134138
*/
135-
void
139+
static void
136140
pq_close(void)
137141
{
138142
if (MyProcPort != NULL)

‎src/backend/storage/buffer/buf_init.c

Lines changed: 50 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $Header: /cvsroot/pgsql/src/backend/storage/buffer/buf_init.c,v 1.39 2000/11/30 01:39:07 tgl Exp $
11+
* $Header: /cvsroot/pgsql/src/backend/storage/buffer/buf_init.c,v 1.40 2000/12/18 00:44:46 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -36,6 +36,9 @@
3636
#include "utils/hsearch.h"
3737
#include "utils/memutils.h"
3838

39+
40+
static void ShutdownBufferPoolAccess(void);
41+
3942
/*
4043
* if BMTRACE is defined, we trace the last 200 buffer allocations and
4144
* deallocations in a circular buffer in shared memory.
@@ -73,7 +76,7 @@ bool *BufferDirtiedByMe; /* T if buf has been dirtied in cur xact */
7376
* Two important notes. First, the buffer has to be
7477
* available for lookup BEFORE an IO begins. Otherwise
7578
* a second process trying to read the buffer will
76-
* allocate its own copy and the buffeer pool will
79+
* allocate its own copy and the buffer pool will
7780
* become inconsistent.
7881
*
7982
* Buffer Replacement:
@@ -126,10 +129,10 @@ long int LocalBufferFlushCount;
126129

127130

128131
/*
129-
* Initialize module: called once during shared-memory initialization
132+
* Initialize shared buffer pool
130133
*
131-
* should calculate size of pool dynamically based on the
132-
* amount of available memory.
134+
* This is called once during shared-memory initialization (either in the
135+
* postmaster, or in a standalone backend).
133136
*/
134137
void
135138
InitBufferPool(void)
@@ -144,6 +147,10 @@ InitBufferPool(void)
144147
Lookup_List_Descriptor = Data_Descriptors + 1;
145148
Num_Descriptors = Data_Descriptors + 1;
146149

150+
/*
151+
* It's probably not really necessary to grab the lock --- if there's
152+
* anyone else attached to the shmem at this point, we've got problems.
153+
*/
147154
SpinAcquire(BufMgrLock);
148155

149156
#ifdef BMTRACE
@@ -203,12 +210,28 @@ InitBufferPool(void)
203210
BufferDescriptors[Data_Descriptors - 1].freeNext = 0;
204211
}
205212

206-
/* Init the rest of the module */
213+
/* Init other shared buffer-management stuff */
207214
InitBufTable();
208215
InitFreeList(!foundDescs);
209216

210217
SpinRelease(BufMgrLock);
218+
}
219+
220+
/*
221+
* Initialize access to shared buffer pool
222+
*
223+
* This is called during backend startup (whether standalone or under the
224+
* postmaster). It sets up for this backend's access to the already-existing
225+
* buffer pool.
226+
*/
227+
void
228+
InitBufferPoolAccess(void)
229+
{
230+
int i;
211231

232+
/*
233+
* Allocate and zero local arrays of per-buffer info.
234+
*/
212235
BufferBlockPointers = (Block *) calloc(NBuffers, sizeof(Block));
213236
PrivateRefCount = (long *) calloc(NBuffers, sizeof(long));
214237
BufferLocks = (bits8 *) calloc(NBuffers, sizeof(bits8));
@@ -224,6 +247,27 @@ InitBufferPool(void)
224247
{
225248
BufferBlockPointers[i] = (Block) MAKE_PTR(BufferDescriptors[i].data);
226249
}
250+
251+
/*
252+
* Now that buffer access is initialized, set up a callback to shut it
253+
* down again at backend exit.
254+
*/
255+
on_shmem_exit(ShutdownBufferPoolAccess, 0);
256+
}
257+
258+
/*
259+
* Shut down buffer manager at backend exit.
260+
*
261+
* This is needed mainly to ensure that we don't leave any buffer reference
262+
* counts set during an error exit.
263+
*/
264+
static void
265+
ShutdownBufferPoolAccess(void)
266+
{
267+
/* Release any buffer context locks we are holding */
268+
UnlockBuffers();
269+
/* Release any buffer reference counts we are holding */
270+
ResetBufferPool(false);
227271
}
228272

229273
/* -----------------------------------------------------

‎src/backend/storage/lmgr/proc.c

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $Header: /cvsroot/pgsql/src/backend/storage/lmgr/proc.c,v 1.86 2000/12/11 16:35:59 tgl Exp $
11+
* $Header: /cvsroot/pgsql/src/backend/storage/lmgr/proc.c,v 1.87 2000/12/18 00:44:47 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -29,7 +29,8 @@
2929
*
3030
* Interface (b):
3131
*
32-
* ProcReleaseLocks -- frees the locks associated with this process,
32+
* ProcReleaseLocks -- frees the locks associated with current transaction
33+
*
3334
* ProcKill -- destroys the shared memory state (and locks)
3435
* associated with the process.
3536
*
@@ -47,7 +48,7 @@
4748
* This is so that we can support more backends. (system-wide semaphore
4849
* sets run out pretty fast.) -ay 4/95
4950
*
50-
* $Header: /cvsroot/pgsql/src/backend/storage/lmgr/proc.c,v 1.86 2000/12/11 16:35:59 tgl Exp $
51+
* $Header: /cvsroot/pgsql/src/backend/storage/lmgr/proc.c,v 1.87 2000/12/18 00:44:47 tgl Exp $
5152
*/
5253
#include "postgres.h"
5354

@@ -332,15 +333,15 @@ GetOffWaitqueue(PROC *proc)
332333
}
333334

334335
/*
335-
* ProcReleaseLocks() -- release all locks associated with this process
336+
* ProcReleaseLocks() -- release all locks associated with current transaction
336337
*
337338
*/
338339
void
339340
ProcReleaseLocks()
340341
{
341342
if (!MyProc)
342343
return;
343-
LockReleaseAll(1, &MyProc->lockQueue);
344+
LockReleaseAll(DEFAULT_LOCKMETHOD, &MyProc->lockQueue);
344345
GetOffWaitqueue(MyProc);
345346
}
346347

@@ -423,8 +424,6 @@ ProcKill(int exitStatus, Datum pid)
423424
* ----------------
424425
*/
425426
GetOffWaitqueue(proc);
426-
427-
return;
428427
}
429428

430429
/*

0 commit comments

Comments
 (0)