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>
This commit is contained in:
Tom Lane 2016-09-01 10:13:55 -04:00
parent 6f7c0ea32f
commit 6c03d981a6
4 changed files with 26 additions and 34 deletions

View File

@ -163,14 +163,31 @@ InitShmemAllocation(void)
/*
* ShmemAlloc -- allocate max-aligned chunk from shared memory
*
* Assumes ShmemLock and ShmemSegHdr are initialized.
* Throws error if request cannot be satisfied.
*
* Returns: real pointer to memory or NULL if we are out
* of space. Has to return a real pointer in order
* to be compatible with malloc().
* Assumes ShmemLock and ShmemSegHdr are initialized.
*/
void *
ShmemAlloc(Size size)
{
void *newSpace;
newSpace = ShmemAllocNoError(size);
if (!newSpace)
ereport(ERROR,
(errcode(ERRCODE_OUT_OF_MEMORY),
errmsg("out of shared memory (%zu bytes requested)",
size)));
return newSpace;
}
/*
* ShmemAllocNoError -- allocate max-aligned chunk from shared memory
*
* As ShmemAlloc, but returns NULL if out of space, rather than erroring.
*/
void *
ShmemAllocNoError(Size size)
{
Size newStart;
Size newFree;
@ -206,11 +223,7 @@ ShmemAlloc(Size size)
SpinLockRelease(ShmemLock);
if (!newSpace)
ereport(WARNING,
(errcode(ERRCODE_OUT_OF_MEMORY),
errmsg("out of shared memory")));
/* note this assert is okay with newSpace == NULL */
Assert(newSpace == (void *) CACHELINEALIGN(newSpace));
return newSpace;
@ -293,7 +306,7 @@ ShmemInitHash(const char *name, /* table string name for shmem index */
* The shared memory allocator must be specified too.
*/
infoP->dsize = infoP->max_dsize = hash_select_dirsize(max_size);
infoP->alloc = ShmemAlloc;
infoP->alloc = ShmemAllocNoError;
hash_flags |= HASH_SHARED_MEM | HASH_ALLOC | HASH_DIRSIZE;
/* look it up in the shmem index */
@ -364,12 +377,6 @@ ShmemInitStruct(const char *name, Size size, bool *foundPtr)
*/
Assert(shmemseghdr->index == NULL);
structPtr = ShmemAlloc(size);
if (structPtr == NULL)
ereport(ERROR,
(errcode(ERRCODE_OUT_OF_MEMORY),
errmsg("not enough shared memory for data structure"
" \"%s\" (%zu bytes requested)",
name, size)));
shmemseghdr->index = structPtr;
*foundPtr = FALSE;
}
@ -410,7 +417,7 @@ ShmemInitStruct(const char *name, Size size, bool *foundPtr)
else
{
/* It isn't in the table yet. allocate and initialize it */
structPtr = ShmemAlloc(size);
structPtr = ShmemAllocNoError(size);
if (structPtr == NULL)
{
/* out of memory; remove the failed ShmemIndex entry */

View File

@ -1184,12 +1184,6 @@ InitPredicateLocks(void)
requestSize = mul_size((Size) max_table_size,
PredXactListElementDataSize);
PredXact->element = ShmemAlloc(requestSize);
if (PredXact->element == NULL)
ereport(ERROR,
(errcode(ERRCODE_OUT_OF_MEMORY),
errmsg("not enough shared memory for elements of data structure"
" \"%s\" (%zu bytes requested)",
"PredXactList", requestSize)));
/* Add all elements to available list, clean. */
memset(PredXact->element, 0, requestSize);
for (i = 0; i < max_table_size; i++)
@ -1255,12 +1249,6 @@ InitPredicateLocks(void)
requestSize = mul_size((Size) max_table_size,
RWConflictDataSize);
RWConflictPool->element = ShmemAlloc(requestSize);
if (RWConflictPool->element == NULL)
ereport(ERROR,
(errcode(ERRCODE_OUT_OF_MEMORY),
errmsg("not enough shared memory for elements of data structure"
" \"%s\" (%zu bytes requested)",
"RWConflictPool", requestSize)));
/* Add all elements to available list, clean. */
memset(RWConflictPool->element, 0, requestSize);
for (i = 0; i < max_table_size; i++)

View File

@ -194,14 +194,10 @@ InitProcGlobal(void)
* between groups.
*/
procs = (PGPROC *) ShmemAlloc(TotalProcs * sizeof(PGPROC));
MemSet(procs, 0, TotalProcs * sizeof(PGPROC));
ProcGlobal->allProcs = procs;
/* XXX allProcCount isn't really all of them; it excludes prepared xacts */
ProcGlobal->allProcCount = MaxBackends + NUM_AUXILIARY_PROCS;
if (!procs)
ereport(FATAL,
(errcode(ERRCODE_OUT_OF_MEMORY),
errmsg("out of shared memory")));
MemSet(procs, 0, TotalProcs * sizeof(PGPROC));
/*
* Also allocate a separate array of PGXACT structures. This is separate

View File

@ -35,6 +35,7 @@ typedef struct SHM_QUEUE
extern void InitShmemAccess(void *seghdr);
extern void InitShmemAllocation(void);
extern void *ShmemAlloc(Size size);
extern void *ShmemAllocNoError(Size size);
extern bool ShmemAddrIsValid(const void *addr);
extern void InitShmemIndex(void);
extern HTAB *ShmemInitHash(const char *name, long init_size, long max_size,