8000 Change API of ShmemAlloc() so it throws error rather than returning N… · s-monk/postgres@6c03d98 · GitHub
[go: up one dir, main page]
Skip to content
10000

Commit 6c03d98

Browse files
tglsfdctglsfdc
committed
Change API of ShmemAlloc() so it throws error rather than returning NULL.
A majority of callers seem to have believed that this was the API spec already, because they omitted any check for a NULL result, and hence would crash on an out-of-shared-memory failure. The original proposal was to just add such error checks everywhere, but that does nothing to prevent similar omissions in future. Instead, let's make ShmemAlloc() throw the error (so we can remove the caller-side checks that do exist), and introduce a new function ShmemAllocNoError() that has the previous behavior of returning NULL, for the small number of callers that need that and are prepared to do the right thing. This also lets us remove the rather wishy-washy behavior of printing a WARNING for out-of-shmem, which never made much sense: either the caller has a strategy for dealing with that, or it doesn't. It's not ShmemAlloc's business to decide whether a warning is appropriate. The v10 release notes will need to call this out as a significant source-code change. It's likely that it will be a bug fix for extension callers too, but if not, they'll need to change to using ShmemAllocNoError(). This is nominally a bug fix, but the odds that it's fixing any live bug are actually rather small, because in general the requests being made by the unchecked callers were already accounted for in determining the overall shmem size, so really they ought not fail. Between that and the possible impact on extensions, no back-patch. Discussion: <24843.1472563085@sss.pgh.pa.us>
1 parent 6f7c0ea commit 6c03d98

File tree

4 files changed

+26
-34
lines changed

4 files changed

+26
-34
lines changed

‎src/backend/storage/ipc/shmem.c

Lines changed: 24 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -163,14 +163,31 @@ InitShmemAllocation(void)
163163
/*
164164
* ShmemAlloc -- allocate max-aligned chunk from shared memory
165165
*
166-
* Assumes ShmemLock and ShmemSegHdr are initialized.
166+
* Throws error if request cannot be satisfied.
167167
*
168-
* Returns: real pointer to memory or NULL if we are out
169-
* of space. Has to return a real pointer in order
170-
* to be compatible with malloc().
168+
* Assumes ShmemLock and ShmemSegHdr are initialized.
171169
*/
172170
void *
173171
ShmemAlloc(Size size)
172+
{
173+
void *newSpace;
174+
175+
newSpace = ShmemAllocNoError(size);
176+
if (!newSpace)
177+
ereport(ERROR,
178+
(errcode(ERRCODE_OUT_OF_MEMORY),
179+
errmsg("out of shared memory (%zu bytes requested)",
180+
size)));
181+
return newSpace;
182+
}
183+
184+
/*
185+
* ShmemAllocNoError -- allocate max-aligned chunk from shared memory
186+
*
187+
* As ShmemAlloc, but returns NULL if out of space, rather than erroring.
188+
*/
189+
void *
190+
ShmemAllocNoError(Size size)
174191
{
175192
Size newStart;
176193
Size newFree;
@@ -206,11 +223,7 @@ ShmemAlloc(Size size)
206223

207224
SpinLockRelease(ShmemLock);
208225

209-
if (!newSpace)
210-
ereport(WARNING,
211-
(errcode(ERRCODE_OUT_OF_MEMORY),
212-
errmsg("out of shared memory")));
213-
226+
/* note this assert is okay with newSpace == NULL */
214227
Assert(newSpace == (void *) CACHELINEALIGN(newSpace));
215228

216229
return newSpace;
@@ -293,7 +306,7 @@ ShmemInitHash(const char *name, /* table string name for shmem index */
293306
* The shared memory allocator must be specified too.
294307
*/
295308
infoP->dsize = infoP->max_dsize = hash_select_dirsize(max_size);
296-
infoP->alloc = ShmemAlloc;
309+
infoP->alloc = ShmemAllocNoError;
297310
hash_flags |= HASH_SHARED_MEM | HASH_ALLOC | HASH_DIRSIZE;
298311

299312
/* look it up in the shmem index */
@@ -364,12 +377,6 @@ ShmemInitStruct(const char *name, Size size, bool *foundPtr)
364377
*/
365378
Assert(shmemseghdr->index == NULL);
366379
structPtr = ShmemAlloc(size);
367-
if (structPtr == NULL)
368-
ereport(ERROR,
369-
(errcode(ERRCODE_OUT_OF_MEMORY),
370-
errmsg("not enough shared memory for data structure"
371-
" \"%s\" (%zu bytes requested)",
372-
name, size)));
373380
shmemseghdr->index = structPtr;
374381
*foundPtr = FALSE;
375382
}
@@ -410,7 +417,7 @@ ShmemInitStruct(const char *name, Size size, bool *foundPtr)
410417
else
411418
{
412419
/* It isn't in the table yet. allocate and initialize it */
413-
structPtr = ShmemAlloc(size);
420+
structPtr = ShmemAllocNoError(size);
414421
if (structPtr == NULL)
415422
{
416423
/* out of memory; remove the failed ShmemIndex entry */

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

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1184,12 +1184,6 @@ InitPredicateLocks(void)
11841184
requestSize = mul_size((Size) max_table_size,
11851185
PredXactListElementDataSize);
11861186
PredXact->element = ShmemAlloc(requestSize);
1187-
if (PredXact->element == NULL)
1188-
ereport(ERROR,
1189-
(errcode(ERRCODE_OUT_OF_MEMORY),
1190-
errmsg("not enough shared memory for elements of data structure"
1191-
" \"%s\" (%zu bytes requested)",
1192-
"PredXactList", requestSize)));
11931187
/* Add all elements to available list, clean. */
11941188
memset(PredXact->element, 0, requestSize);
11951189
for (i = 0; i < max_table_size; i++)
@@ -1255,12 +1249,6 @@ InitPredicateLocks(void)
12551249
requestSize = mul_size((Size) max_table_size,
12561250
RWConflictDataSize);
12571251
RWConflictPool->element = ShmemAlloc(requestSize);
1258-
if (RWConflictPool->element == NULL)
1259-
ereport(ERROR,
1260-
(errcode(ERRCODE_OUT_OF_MEMORY),
1261-
errmsg("not enough shared memory for elements of data structure"
1262-
" \"%s\" (%zu bytes requested)",
1263-
"RWConflictPool", requestSize)));
12641252
/* Add all elements to available list, clean. */
12651253
memset(RWConflictPool->element, 0, requestSize);
12661254
for (i = 0; i < max_table_size; i++)

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

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -194,14 +194,10 @@ InitProcGlobal(void)
194194
* between groups.
195195
*/
196196
procs = (PGPROC *) ShmemAlloc(TotalProcs * sizeof(PGPROC));
197+
MemSet(procs, 0, TotalProcs * sizeof(PGPROC));
197198
ProcGlobal->allProcs = procs;
198199
/* XXX allProcCount isn't really all of them; it excludes prepared xacts */
199200
ProcGlobal->allProcCount = MaxBackends + NUM_AUXILIARY_PROCS;
200-
if (!procs)
201-
ereport(FATAL,
202-
(errcode(ERRCODE_OUT_OF_MEMORY),
203-
errmsg("out of shared memory")));
204-
MemSet(procs, 0, TotalProcs * sizeof(PGPROC));
205201

206202
/*
207203
* Also allocate a separate array of PGXACT structures. This is separate

‎src/include/storage/shmem.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ typedef struct SHM_QUEUE
3535
extern void InitShmemAccess(void *seghdr);
3636
extern void InitShmemAllocation(void);
3737
extern void *ShmemAlloc(Size size);
38+
extern void *ShmemAllocNoError(Size size);
3839
extern bool ShmemAddrIsValid(const void *addr);
3940
extern void InitShmemIndex(void);
4041
extern HTAB *ShmemInitHash(const char *name, long init_size, long max_size,

0 commit comments

Comments
 (0)
0